chore: Cleanup additional/redundant kube client Interfaces

Signed-off-by: George Jenkins <gvjenkins@gmail.com>
pull/30980/head
George Jenkins 3 months ago
parent 5b43b744b8
commit b5de5b1591

@ -154,7 +154,7 @@ func (cfg *Configuration) deleteHookByPolicy(h *release.Hook, policy release.Hoo
if err != nil {
return fmt.Errorf("unable to build kubernetes object for deleting hook %s: %w", h.Path, err)
}
_, errs := cfg.KubeClient.Delete(resources)
_, errs := cfg.KubeClient.Delete(resources, metav1.DeletePropagationBackground)
if len(errs) > 0 {
return joinErrors(errs, "; ")
}
@ -223,16 +223,12 @@ func (cfg *Configuration) outputLogsByPolicy(h *release.Hook, releaseNamespace s
}
func (cfg *Configuration) outputContainerLogsForListOptions(namespace string, listOptions metav1.ListOptions) error {
// TODO Helm 4: Remove this check when GetPodList and OutputContainerLogsForPodList are moved from InterfaceLogs to Interface
if kubeClient, ok := cfg.KubeClient.(kube.InterfaceLogs); ok {
podList, err := kubeClient.GetPodList(namespace, listOptions)
if err != nil {
return err
}
err = kubeClient.OutputContainerLogsForPodList(podList, namespace, cfg.HookOutputFunc)
podList, err := cfg.KubeClient.GetPodList(namespace, listOptions)
if err != nil {
return err
}
return nil
return cfg.KubeClient.OutputContainerLogsForPodList(podList, namespace, cfg.HookOutputFunc)
}
func (cfg *Configuration) deriveNamespace(h *release.Hook, namespace string) (string, error) {

@ -27,6 +27,7 @@ import (
"github.com/stretchr/testify/assert"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/yaml"
"k8s.io/cli-runtime/pkg/resource"
@ -259,7 +260,7 @@ func (h *HookFailingKubeWaiter) WatchUntilReady(resources kube.ResourceList, _ t
return nil
}
func (h *HookFailingKubeClient) Delete(resources kube.ResourceList) (*kube.Result, []error) {
func (h *HookFailingKubeClient) Delete(resources kube.ResourceList, deletionPropagation metav1.DeletionPropagation) (*kube.Result, []error) {
for _, res := range resources {
h.deleteRecord = append(h.deleteRecord, resource.Info{
Name: res.Name,
@ -267,7 +268,7 @@ func (h *HookFailingKubeClient) Delete(resources kube.ResourceList) (*kube.Resul
})
}
return h.PrintingKubeClient.Delete(resources)
return h.PrintingKubeClient.Delete(resources, deletionPropagation)
}
func (h *HookFailingKubeClient) GetWaiter(strategy kube.WaitStrategy) (kube.Waiter, error) {

@ -23,6 +23,8 @@ import (
"strings"
"time"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
chartutil "helm.sh/helm/v4/pkg/chart/v2/util"
"helm.sh/helm/v4/pkg/kube"
release "helm.sh/helm/v4/pkg/release/v1"
@ -222,7 +224,7 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas
r.cfg.recordRelease(targetRelease)
if r.CleanupOnFail {
slog.Debug("cleanup on fail set, cleaning up resources", "count", len(results.Created))
_, errs := r.cfg.KubeClient.Delete(results.Created)
_, errs := r.cfg.KubeClient.Delete(results.Created, metav1.DeletePropagationBackground)
if errs != nil {
return targetRelease, fmt.Errorf(
"an error occurred while cleaning up resources. original rollback error: %w",

@ -18,7 +18,6 @@ package action
import (
"bytes"
"errors"
"helm.sh/helm/v4/pkg/kube"
release "helm.sh/helm/v4/pkg/release/v1"
@ -55,28 +54,25 @@ func (s *Status) Run(name string) (*release.Release, error) {
return nil, err
}
if kubeClient, ok := s.cfg.KubeClient.(kube.InterfaceResources); ok {
var resources kube.ResourceList
if s.ShowResourcesTable {
resources, err = kubeClient.BuildTable(bytes.NewBufferString(rel.Manifest), false)
if err != nil {
return nil, err
}
} else {
resources, err = s.cfg.KubeClient.Build(bytes.NewBufferString(rel.Manifest), false)
if err != nil {
return nil, err
}
var resources kube.ResourceList
if s.ShowResourcesTable {
resources, err = s.cfg.KubeClient.BuildTable(bytes.NewBufferString(rel.Manifest), false)
if err != nil {
return nil, err
}
resp, err := kubeClient.Get(resources, true)
} else {
resources, err = s.cfg.KubeClient.Build(bytes.NewBufferString(rel.Manifest), false)
if err != nil {
return nil, err
}
}
rel.Info.Resources = resp
return rel, nil
resp, err := s.cfg.KubeClient.Get(resources, true)
if err != nil {
return nil, err
}
return nil, errors.New("unable to get kubeClient with interface InterfaceResources")
rel.Info.Resources = resp
return rel, nil
}

@ -245,11 +245,7 @@ func (u *Uninstall) deleteRelease(rel *release.Release) (kube.ResourceList, stri
return nil, "", []error{fmt.Errorf("unable to build kubernetes objects for delete: %w", err)}
}
if len(resources) > 0 {
if kubeClient, ok := u.cfg.KubeClient.(kube.InterfaceDeletionPropagation); ok {
_, errs = kubeClient.DeleteWithPropagationPolicy(resources, parseCascadingFlag(u.DeletionPropagation))
return resources, kept, errs
}
_, errs = u.cfg.KubeClient.Delete(resources)
_, errs = u.cfg.KubeClient.Delete(resources, parseCascadingFlag(u.DeletionPropagation))
}
return resources, kept, errs
}

@ -146,7 +146,7 @@ func TestUninstallRelease_Cascade(t *testing.T) {
}`
unAction.cfg.Releases.Create(rel)
failer := unAction.cfg.KubeClient.(*kubefake.FailingKubeClient)
failer.DeleteWithPropagationError = fmt.Errorf("Uninstall with cascade failed")
failer.DeleteError = fmt.Errorf("Uninstall with cascade failed")
failer.BuildDummy = true
unAction.cfg.KubeClient = failer
_, err := unAction.Run(rel.Name)

@ -26,6 +26,7 @@ import (
"sync"
"time"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/cli-runtime/pkg/resource"
"helm.sh/helm/v4/pkg/chart"
@ -524,7 +525,7 @@ func (u *Upgrade) failRelease(rel *release.Release, created kube.ResourceList, e
u.cfg.recordRelease(rel)
if u.CleanupOnFail && len(created) > 0 {
slog.Debug("cleanup on fail set", "cleaning_resources", len(created))
_, errs := u.cfg.KubeClient.Delete(created)
_, errs := u.cfg.KubeClient.Delete(created, metav1.DeletePropagationBackground)
if errs != nil {
return rel, fmt.Errorf(
"an error occurred while cleaning up resources. original upgrade error: %w: %w",

@ -86,6 +86,8 @@ type Client struct {
kubeClient kubernetes.Interface
}
var _ Interface = (*Client)(nil)
type WaitStrategy string
const (
@ -773,25 +775,13 @@ func (c *Client) Update(originals, targets ResourceList, options ...ClientUpdate
// background cascade deletion. It will attempt to delete all resources even
// if one or more fail and collect any errors. All successfully deleted items
// will be returned in the `Deleted` ResourceList that is part of the result.
func (c *Client) Delete(resources ResourceList) (*Result, []error) {
return deleteResources(resources, metav1.DeletePropagationBackground)
}
// Delete deletes Kubernetes resources specified in the resources list with
// given deletion propagation policy. It will attempt to delete all resources even
// if one or more fail and collect any errors. All successfully deleted items
// will be returned in the `Deleted` ResourceList that is part of the result.
func (c *Client) DeleteWithPropagationPolicy(resources ResourceList, policy metav1.DeletionPropagation) (*Result, []error) {
return deleteResources(resources, policy)
}
func deleteResources(resources ResourceList, propagation metav1.DeletionPropagation) (*Result, []error) {
func (c *Client) Delete(resources ResourceList, policy metav1.DeletionPropagation) (*Result, []error) {
var errs []error
res := &Result{}
mtx := sync.Mutex{}
err := perform(resources, func(target *resource.Info) error {
slog.Debug("starting delete resource", "namespace", target.Namespace, "name", target.Name, "kind", target.Mapping.GroupVersionKind.Kind)
err := deleteResource(target, propagation)
err := deleteResource(target, policy)
if err == nil || apierrors.IsNotFound(err) {
if err != nil {
slog.Debug("ignoring delete failure", "namespace", target.Namespace, "name", target.Name, "kind", target.Mapping.GroupVersionKind.Kind, slog.Any("error", err))

@ -816,7 +816,7 @@ func TestWaitDelete(t *testing.T) {
if len(result.Created) != 1 {
t.Errorf("expected 1 resource created, got %d", len(result.Created))
}
if _, err := c.Delete(resources); err != nil {
if _, err := c.Delete(resources, metav1.DeletePropagationBackground); err != nil {
t.Fatal(err)
}
@ -855,7 +855,7 @@ func TestReal(t *testing.T) {
t.Fatal(err)
}
if _, errs := c.Delete(resources); errs != nil {
if _, errs := c.Delete(resources, metav1.DeletePropagationBackground); errs != nil {
t.Fatal(errs)
}
@ -864,7 +864,7 @@ func TestReal(t *testing.T) {
t.Fatal(err)
}
// ensures that delete does not fail if a resource is not found
if _, errs := c.Delete(resources); errs != nil {
if _, errs := c.Delete(resources, metav1.DeletePropagationBackground); errs != nil {
t.Fatal(errs)
}
}

@ -33,22 +33,23 @@ import (
// delegates all its calls to `PrintingKubeClient`
type FailingKubeClient struct {
PrintingKubeClient
CreateError error
GetError error
DeleteError error
DeleteWithPropagationError error
UpdateError error
BuildError error
BuildTableError error
ConnectionError error
BuildDummy bool
DummyResources kube.ResourceList
BuildUnstructuredError error
WaitError error
WaitForDeleteError error
WatchUntilReadyError error
WaitDuration time.Duration
}
CreateError error
GetError error
DeleteError error
UpdateError error
BuildError error
BuildTableError error
ConnectionError error
BuildDummy bool
DummyResources kube.ResourceList
BuildUnstructuredError error
WaitError error
WaitForDeleteError error
WatchUntilReadyError error
WaitDuration time.Duration
}
var _ kube.Interface = &FailingKubeClient{}
// FailingKubeWaiter implements kube.Waiter for testing purposes.
// It also has additional errors you can set to fail different functions, otherwise it delegates all its calls to `PrintingKubeWaiter`
@ -102,11 +103,12 @@ func (f *FailingKubeWaiter) WaitForDelete(resources kube.ResourceList, d time.Du
}
// Delete returns the configured error if set or prints
func (f *FailingKubeClient) Delete(resources kube.ResourceList) (*kube.Result, []error) {
func (f *FailingKubeClient) Delete(resources kube.ResourceList, deletionPropagation metav1.DeletionPropagation) (*kube.Result, []error) {
if f.DeleteError != nil {
return nil, []error{f.DeleteError}
}
return f.PrintingKubeClient.Delete(resources)
return f.PrintingKubeClient.Delete(resources, deletionPropagation)
}
// WatchUntilReady returns the configured error if set or prints
@ -147,14 +149,6 @@ func (f *FailingKubeClient) BuildTable(r io.Reader, _ bool) (kube.ResourceList,
return f.PrintingKubeClient.BuildTable(r, false)
}
// DeleteWithPropagationPolicy returns the configured error if set or prints
func (f *FailingKubeClient) DeleteWithPropagationPolicy(resources kube.ResourceList, policy metav1.DeletionPropagation) (*kube.Result, []error) {
if f.DeleteWithPropagationError != nil {
return nil, []error{f.DeleteWithPropagationError}
}
return f.PrintingKubeClient.DeleteWithPropagationPolicy(resources, policy)
}
func (f *FailingKubeClient) GetWaiter(ws kube.WaitStrategy) (kube.Waiter, error) {
waiter, _ := f.PrintingKubeClient.GetWaiter(ws)
printingKubeWaiter, _ := waiter.(*PrintingKubeWaiter)

@ -43,6 +43,8 @@ type PrintingKubeWaiter struct {
LogOutput io.Writer
}
var _ kube.Interface = &PrintingKubeClient{}
// IsReachable checks if the cluster is reachable
func (p *PrintingKubeClient) IsReachable() error {
return nil
@ -89,7 +91,7 @@ func (p *PrintingKubeWaiter) WatchUntilReady(resources kube.ResourceList, _ time
// Delete implements KubeClient delete.
//
// It only prints out the content to be deleted.
func (p *PrintingKubeClient) Delete(resources kube.ResourceList) (*kube.Result, []error) {
func (p *PrintingKubeClient) Delete(resources kube.ResourceList, _ metav1.DeletionPropagation) (*kube.Result, []error) {
_, err := io.Copy(p.Out, bufferize(resources))
if err != nil {
return nil, []error{err}

@ -29,11 +29,17 @@ import (
//
// A KubernetesClient must be concurrency safe.
type Interface interface {
// Get details of deployed resources.
// The first argument is a list of resources to get. The second argument
// specifies if related pods should be fetched. For example, the pods being
// managed by a deployment.
Get(resources ResourceList, related bool) (map[string][]runtime.Object, error)
// Create creates one or more resources.
Create(resources ResourceList, options ...ClientCreateOption) (*Result, error)
// Delete destroys one or more resources.
Delete(resources ResourceList) (*Result, []error)
Delete(resources ResourceList, policy metav1.DeletionPropagation) (*Result, []error)
// Update updates one or more resources or creates the resource
// if it doesn't exist.
@ -51,6 +57,23 @@ type Interface interface {
// Get Waiter gets the Kube.Waiter
GetWaiter(ws WaitStrategy) (Waiter, error)
// GetPodList list all pods that match the specified listOptions
GetPodList(namespace string, listOptions metav1.ListOptions) (*v1.PodList, error)
// OutputContainerLogsForPodList output the logs for a pod list
OutputContainerLogsForPodList(podList *v1.PodList, namespace string, writerFunc func(namespace, pod, container string) io.Writer) error
// BuildTable creates a resource list from a Reader. This differs from
// Interface.Build() in that a table kind is returned. A table is useful
// if you want to use a printer to display the information.
//
// Reader must contain a YAML stream (one or more YAML documents separated
// by "\n---\n")
//
// Validates against OpenAPI schema if validate is true.
// TODO Helm 4: Integrate into Build with an argument
BuildTable(reader io.Reader, validate bool) (ResourceList, error)
}
// Waiter defines methods related to waiting for resource states.
@ -75,49 +98,3 @@ type Waiter interface {
// error.
WatchUntilReady(resources ResourceList, timeout time.Duration) error
}
// InterfaceLogs was introduced to avoid breaking backwards compatibility for Interface implementers.
//
// TODO Helm 4: Remove InterfaceLogs and integrate its method(s) into the Interface.
type InterfaceLogs interface {
// GetPodList list all pods that match the specified listOptions
GetPodList(namespace string, listOptions metav1.ListOptions) (*v1.PodList, error)
// OutputContainerLogsForPodList output the logs for a pod list
OutputContainerLogsForPodList(podList *v1.PodList, namespace string, writerFunc func(namespace, pod, container string) io.Writer) error
}
// InterfaceDeletionPropagation is introduced to avoid breaking backwards compatibility for Interface implementers.
//
// TODO Helm 4: Remove InterfaceDeletionPropagation and integrate its method(s) into the Interface.
type InterfaceDeletionPropagation interface {
// DeleteWithPropagationPolicy destroys one or more resources. The deletion propagation is handled as per the given deletion propagation value.
DeleteWithPropagationPolicy(resources ResourceList, policy metav1.DeletionPropagation) (*Result, []error)
}
// InterfaceResources is introduced to avoid breaking backwards compatibility for Interface implementers.
//
// TODO Helm 4: Remove InterfaceResources and integrate its method(s) into the Interface.
type InterfaceResources interface {
// Get details of deployed resources.
// The first argument is a list of resources to get. The second argument
// specifies if related pods should be fetched. For example, the pods being
// managed by a deployment.
Get(resources ResourceList, related bool) (map[string][]runtime.Object, error)
// BuildTable creates a resource list from a Reader. This differs from
// Interface.Build() in that a table kind is returned. A table is useful
// if you want to use a printer to display the information.
//
// Reader must contain a YAML stream (one or more YAML documents separated
// by "\n---\n")
//
// Validates against OpenAPI schema if validate is true.
// TODO Helm 4: Integrate into Build with an argument
BuildTable(reader io.Reader, validate bool) (ResourceList, error)
}
var _ Interface = (*Client)(nil)
var _ InterfaceLogs = (*Client)(nil)
var _ InterfaceDeletionPropagation = (*Client)(nil)
var _ InterfaceResources = (*Client)(nil)

Loading…
Cancel
Save