From 740ed760cc9d8dfa0f8accdb1fa80e5f6f65b782 Mon Sep 17 00:00:00 2001 From: Vegoo89 Date: Mon, 23 Oct 2023 12:47:06 +0200 Subject: [PATCH] First version Signed-off-by: Jakub Domanski --- cmd/helm/upgrade.go | 1 + cmd/helm/upgrade_test.go | 6 ++++++ pkg/action/install.go | 2 +- pkg/action/rollback.go | 2 +- pkg/action/upgrade.go | 4 +++- pkg/kube/client.go | 35 ++++++++++++++++++++++++++++++----- pkg/kube/client_test.go | 39 +++++++++++++++++++++++++++++++++------ pkg/kube/fake/fake.go | 4 ++-- pkg/kube/fake/printer.go | 2 +- pkg/kube/interface.go | 2 +- 10 files changed, 79 insertions(+), 18 deletions(-) diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 328497d7e..956fdb19c 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -246,6 +246,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.BoolVar(&client.Recreate, "recreate-pods", false, "performs pods restart for the resource if applicable") f.MarkDeprecated("recreate-pods", "functionality will no longer be updated. Consult the documentation for other methods to recreate pods") f.BoolVar(&client.Force, "force", false, "force resource updates through a replacement strategy") + f.BoolVar(&client.RecreateImmutableResources, "recreate-immutable-resources", false, "On status code 422, try to delete and create the resource again") f.BoolVar(&client.DisableHooks, "no-hooks", false, "disable pre/post upgrade hooks") f.BoolVar(&client.DisableOpenAPIValidation, "disable-openapi-validation", false, "if set, the upgrade process will not validate rendered templates against the Kubernetes OpenAPI Schema") f.BoolVar(&client.SkipCRDs, "skip-crds", false, "if set, no CRDs will be installed when an upgrade is performed with install flag enabled. By default, CRDs are installed if not already present, when an upgrade is performed with install flag enabled") diff --git a/cmd/helm/upgrade_test.go b/cmd/helm/upgrade_test.go index 485267d1d..d3d0f7f5e 100644 --- a/cmd/helm/upgrade_test.go +++ b/cmd/helm/upgrade_test.go @@ -175,6 +175,12 @@ func TestUpgradeCmd(t *testing.T) { wantError: true, rels: []*release.Release{relWithStatusMock("funny-bunny", 2, ch, release.StatusPendingInstall)}, }, + { + name: "upgrade a release with --recreate-immutable-resources flag", + cmd: fmt.Sprintf("upgrade funny-bunny '%s' --recreate-immutable-resources", chartPath), + golden: "output/upgrade.txt", + rels: []*release.Release{relMock("funny-bunny", 2, ch)}, + }, } runTestCmd(t, tests) } diff --git a/pkg/action/install.go b/pkg/action/install.go index e3538a4f5..39280c125 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -439,7 +439,7 @@ func (i *Install) performInstall(rel *release.Release, toBeAdopted kube.Resource if len(toBeAdopted) == 0 && len(resources) > 0 { _, err = i.cfg.KubeClient.Create(resources) } else if len(resources) > 0 { - _, err = i.cfg.KubeClient.Update(toBeAdopted, resources, i.Force) + _, err = i.cfg.KubeClient.Update(toBeAdopted, resources, i.Force, false) } if err != nil { return rel, err diff --git a/pkg/action/rollback.go b/pkg/action/rollback.go index f4ae896e3..bcb208c97 100644 --- a/pkg/action/rollback.go +++ b/pkg/action/rollback.go @@ -187,7 +187,7 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas if err != nil { return targetRelease, errors.Wrap(err, "unable to set metadata visitor from target release") } - results, err := r.cfg.KubeClient.Update(current, target, r.Force) + results, err := r.cfg.KubeClient.Update(current, target, r.Force, false) if err != nil { msg := fmt.Sprintf("Rollback %q failed: %s", targetRelease.Name, err) diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index c74b502e2..eaef1704d 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -78,6 +78,8 @@ type Upgrade struct { // // This should be used with caution. Force bool + // Recreate resources will try to delete and create resources ONLY during upgrade if it receives statusError with code 422 + RecreateImmutableResources bool // ResetValues will reset the values to the chart's built-ins rather than merging with existing. ResetValues bool // ReuseValues will re-use the user's last supplied values. @@ -405,7 +407,7 @@ func (u *Upgrade) releasingUpgrade(c chan<- resultMessage, upgradedRelease *rele u.cfg.Log("upgrade hooks disabled for %s", upgradedRelease.Name) } - results, err := u.cfg.KubeClient.Update(current, target, u.Force) + results, err := u.cfg.KubeClient.Update(current, target, u.Force, u.RecreateImmutableResources) if err != nil { u.cfg.recordRelease(originalRelease) u.reportToPerformUpgrade(c, upgradedRelease, results.Created, err) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 0772678d1..29726a4cc 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -386,7 +386,7 @@ func (c *Client) BuildTable(reader io.Reader, validate bool) (ResourceList, erro // occurs, a Result will still be returned with the error, containing all // resource updates, creations, and deletions that were attempted. These can be // used for cleanup or other logging purposes. -func (c *Client) Update(original, target ResourceList, force bool) (*Result, error) { +func (c *Client) Update(original, target ResourceList, force bool, recreateImmutableResources bool) (*Result, error) { updateErrors := []string{} res := &Result{} @@ -422,8 +422,31 @@ func (c *Client) Update(original, target ResourceList, force bool) (*Result, err } if err := updateResource(c, info, originalInfo.Object, force); err != nil { - c.Log("error updating the resource %q:\n\t %v", info.Name, err) - updateErrors = append(updateErrors, err.Error()) + + var errorMessage error + + if force { + errorMessage = errors.Wrap(err, "failed to replace object") + } else { + errorMessage = errors.Wrapf(err, "cannot patch %q with kind %s", info.Name, info.Mapping.GroupVersionKind.Kind) + } + + if statusError, isStatusError := err.(*apierrors.StatusError); recreateImmutableResources && isStatusError && statusError.ErrStatus.Code == 422 { + if err := deleteResource(info, metav1.DeletePropagationBackground); err != nil { + c.Log("Failed to delete %q, err: %s", info.ObjectName(), err) + } else { + if err := createResource(info); err != nil { + errorMessage = errors.Wrap(err, "failed to create resource") + } else { + errorMessage = nil + } + } + } + + if errorMessage != nil { + c.Log("error updating the resource or replacing object %q:\n\t %v", info.Name, errorMessage) + updateErrors = append(updateErrors, err.Error()) + } } // Because we check for errors later, append the info regardless res.Updated = append(res.Updated, info) @@ -671,7 +694,8 @@ func updateResource(c *Client, target *resource.Info, currentObj runtime.Object, var err error obj, err = helper.Replace(target.Namespace, target.Name, true, target.Object) if err != nil { - return errors.Wrap(err, "failed to replace object") + //errors.Wrap(err, "failed to replace object") + return err } c.Log("Replaced %q with kind %s for kind %s", target.Name, currentObj.GetObjectKind().GroupVersionKind().Kind, kind) } else { @@ -693,7 +717,8 @@ func updateResource(c *Client, target *resource.Info, currentObj runtime.Object, c.Log("Patch %s %q in namespace %s", kind, target.Name, target.Namespace) obj, err = helper.Patch(target.Namespace, target.Name, patchType, patch, nil) if err != nil { - return errors.Wrapf(err, "cannot patch %q with kind %s", target.Name, kind) + //return errors.Wrapf(err, "cannot patch %q with kind %s", target.Name, kind) + return err } } diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go index 55aa5d8ed..48edcdc0a 100644 --- a/pkg/kube/client_test.go +++ b/pkg/kube/client_test.go @@ -101,11 +101,14 @@ func newTestClient(t *testing.T) *Client { } func TestUpdate(t *testing.T) { - listA := newPodList("starfish", "otter", "squid") - listB := newPodList("starfish", "otter", "dolphin") - listC := newPodList("starfish", "otter", "dolphin") + listA := newPodList("starfish", "otter", "squid", "shark") + listB := newPodList("starfish", "otter", "dolphin", "shark") + listC := newPodList("starfish", "otter", "dolphin", "shark") listB.Items[0].Spec.Containers[0].Ports = []v1.ContainerPort{{Name: "https", ContainerPort: 443}} listC.Items[0].Spec.Containers[0].Ports = []v1.ContainerPort{{Name: "https", ContainerPort: 443}} + // Labels change is prohibited as this is immutable field + listB.Items[3].Labels = map[string]string{"changed": "changed"} + listC.Items[3].Labels = map[string]string{"changed": "changed"} var actions []string @@ -146,11 +149,30 @@ func TestUpdate(t *testing.T) { } return newResponse(200, &listB.Items[0]) case p == "/namespaces/default/pods" && m == "POST": - return newResponse(200, &listB.Items[1]) + data, err := io.ReadAll(req.Body) + if err != nil { + t.Fatalf("could not dump request: %s", err) + } + req.Body.Close() + + var returnElement *v1.Pod + if strings.Contains(string(data), "dolphin") { + returnElement = &listB.Items[2] + } else if strings.Contains(string(data), "shark") { + returnElement = &listB.Items[3] + } else { + t.Fatalf("Creation of new pods with unknown name: %s", string(data)) + } + + return newResponse(200, returnElement) case p == "/namespaces/default/pods/squid" && m == "DELETE": return newResponse(200, &listB.Items[1]) case p == "/namespaces/default/pods/squid" && m == "GET": return newResponse(200, &listB.Items[2]) + case p == "/namespaces/default/pods/shark" && (m == "PUT" || m == "PATCH"): + return newResponse(422, &listA.Items[3]) + case p == "/namespaces/default/pods/shark" && (m == "DELETE" || m == "POST" || m == "GET"): + return newResponse(200, &listA.Items[3]) default: t.Fatalf("unexpected request: %s %s", req.Method, req.URL.Path) return nil, nil @@ -166,7 +188,7 @@ func TestUpdate(t *testing.T) { t.Fatal(err) } - result, err := c.Update(first, second, false) + result, err := c.Update(first, second, false, true) if err != nil { t.Fatal(err) } @@ -174,7 +196,7 @@ func TestUpdate(t *testing.T) { if len(result.Created) != 1 { t.Errorf("expected 1 resource created, got %d", len(result.Created)) } - if len(result.Updated) != 2 { + if len(result.Updated) != 3 { t.Errorf("expected 2 resource updated, got %d", len(result.Updated)) } if len(result.Deleted) != 1 { @@ -200,6 +222,11 @@ func TestUpdate(t *testing.T) { "/namespaces/default/pods/otter:GET", "/namespaces/default/pods/dolphin:GET", "/namespaces/default/pods:POST", + "/namespaces/default/pods/shark:GET", + "/namespaces/default/pods/shark:GET", + "/namespaces/default/pods/shark:PATCH", + "/namespaces/default/pods/shark:DELETE", + "/namespaces/default/pods:POST", "/namespaces/default/pods/squid:GET", "/namespaces/default/pods/squid:DELETE", } diff --git a/pkg/kube/fake/fake.go b/pkg/kube/fake/fake.go index 267020d57..f5a5981b9 100644 --- a/pkg/kube/fake/fake.go +++ b/pkg/kube/fake/fake.go @@ -107,11 +107,11 @@ func (f *FailingKubeClient) WatchUntilReady(resources kube.ResourceList, d time. } // Update returns the configured error if set or prints -func (f *FailingKubeClient) Update(r, modified kube.ResourceList, ignoreMe bool) (*kube.Result, error) { +func (f *FailingKubeClient) Update(r, modified kube.ResourceList, ignoreMe bool, alsoIgnoreMe bool) (*kube.Result, error) { if f.UpdateError != nil { return &kube.Result{}, f.UpdateError } - return f.PrintingKubeClient.Update(r, modified, ignoreMe) + return f.PrintingKubeClient.Update(r, modified, ignoreMe, alsoIgnoreMe) } // Build returns the configured error if set or prints diff --git a/pkg/kube/fake/printer.go b/pkg/kube/fake/printer.go index e6c4b6207..b14cd6bf6 100644 --- a/pkg/kube/fake/printer.go +++ b/pkg/kube/fake/printer.go @@ -90,7 +90,7 @@ func (p *PrintingKubeClient) WatchUntilReady(resources kube.ResourceList, _ time } // Update implements KubeClient Update. -func (p *PrintingKubeClient) Update(_, modified kube.ResourceList, _ bool) (*kube.Result, error) { +func (p *PrintingKubeClient) Update(_, modified kube.ResourceList, _ bool, _ bool) (*kube.Result, error) { _, err := io.Copy(p.Out, bufferize(modified)) if err != nil { return nil, err diff --git a/pkg/kube/interface.go b/pkg/kube/interface.go index ce42ed950..3c88c4df3 100644 --- a/pkg/kube/interface.go +++ b/pkg/kube/interface.go @@ -54,7 +54,7 @@ type Interface interface { // Update updates one or more resources or creates the resource // if it doesn't exist. - Update(original, target ResourceList, force bool) (*Result, error) + Update(original, target ResourceList, force bool, recreateImmutableResources bool) (*Result, error) // Build creates a resource list from a Reader. //