From 45ec841fbc02533e869f950a6e42266440f88403 Mon Sep 17 00:00:00 2001 From: Feruzjon Muyassarov Date: Thu, 27 Mar 2025 18:56:14 +0200 Subject: [PATCH] Handle missing resources during upgrade and rollback Check if a resource exists in the cluster before returning an error. If the resource exists, the continue to updating thr resource if not, return error. Signed-off-by: Feruzjon Muyassarov --- pkg/kube/client.go | 47 ++++++++----- pkg/kube/client_test.go | 144 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 176 insertions(+), 15 deletions(-) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index ff0f05e88..034ccc2ad 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -427,18 +427,27 @@ func (c *Client) update(original, target ResourceList, force, threeWayMerge bool } originalInfo := original.Get(info) - if originalInfo == nil { - kind := info.Mapping.GroupVersionKind.Kind - return errors.Errorf("no %s with the name %q found", kind, info.Name) - } - - if err := updateResource(c, info, originalInfo.Object, force, threeWayMerge); err != nil { - slog.Debug("error updating the resource", "namespace", info.Namespace, "name", info.Name, "kind", info.Mapping.GroupVersionKind.Kind, slog.Any("error", err)) - updateErrors = append(updateErrors, err.Error()) + if originalInfo != nil { + // Proceed to update the resource + if err := updateResource(c, info, originalInfo.Object, force, threeWayMerge); err != nil { + slog.Debug("Error updating the resource %q: %v", info.Name, err) + updateErrors = append(updateErrors, fmt.Sprintf("failed to update resource %q: %v", info.Name, err)) + } + } else { + // If the original resource is not found, force update the resource + if err := updateResource(c, info, nil, true, threeWayMerge); err != nil { + slog.Debug("Error force updating the resource %q: %v", info.Name, err) + updateErrors = append(updateErrors, fmt.Sprintf("failed to force update resource %q: %v", info.Name, err)) + } } // Because we check for errors later, append the info regardless res.Updated = append(res.Updated, info) + kind := info.Mapping.GroupVersionKind.Kind + if _, err := helper.Get(info.Namespace, info.Name); err != nil && !apierrors.IsNotFound(err) { + return errors.Errorf("no %s with the name %q found in namespace %q", kind, info.Name, info.Namespace) + } + return nil }) @@ -672,23 +681,31 @@ func createPatch(target *resource.Info, current runtime.Object, threeWayMergeFor func updateResource(_ *Client, target *resource.Info, currentObj runtime.Object, force, threeWayMergeForUnstructured bool) error { var ( - obj runtime.Object - helper = resource.NewHelper(target.Client, target.Mapping).WithFieldManager(getManagedFieldsManager()) - kind = target.Mapping.GroupVersionKind.Kind + obj runtime.Object + helper = resource.NewHelper(target.Client, target.Mapping).WithFieldManager(getManagedFieldsManager()) + kind = target.Mapping.GroupVersionKind.Kind + patch []byte + patchType types.PatchType + err error ) // if --force is applied, attempt to replace the existing resource with the new object. if force { - 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") } slog.Debug("replace succeeded", "name", target.Name, "initialKind", currentObj.GetObjectKind().GroupVersionKind().Kind, "kind", kind) } else { - patch, patchType, err := createPatch(target, currentObj, threeWayMergeForUnstructured) - if err != nil { - return errors.Wrap(err, "failed to create patch") + if currentObj != nil { + patch, patchType, err = createPatch(target, currentObj, threeWayMergeForUnstructured) + if err != nil { + return errors.Wrap(err, "failed to create patch") + } + } else { + // If currentObj is nil, treat it as a full replacement + patch = nil + patchType = types.MergePatchType } if patch == nil || string(patch) == "{}" { diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go index 56c7eebc9..8fb2899ba 100644 --- a/pkg/kube/client_test.go +++ b/pkg/kube/client_test.go @@ -317,6 +317,8 @@ func testUpdate(t *testing.T, threeWayMerge bool) { "/namespaces/default/pods/starfish:GET", "/namespaces/default/pods/starfish:GET", "/namespaces/default/pods/starfish:PATCH", + "/namespaces/default/pods/starfish:GET", + "/namespaces/default/pods/otter:GET", "/namespaces/default/pods/otter:GET", "/namespaces/default/pods/otter:GET", "/namespaces/default/pods/otter:GET", @@ -735,7 +737,149 @@ func TestGetPodList(t *testing.T) { clientAssertions.Equal(&responsePodList, podList) } +func TestUpdateWithPatch(t *testing.T) { + listA := newPodList("test-pod") + listB := newPodList("test-pod") + listB.Items[0].Spec.Containers[0].Image = "nginx:latest" + listB.Items[0].Spec.Containers[0].Name = "app:v4" // Update container name to match actual behavior + + c := newTestClient(t) + c.Factory.(*cmdtesting.TestFactory).UnstructuredClient = &fake.RESTClient{ + NegotiatedSerializer: unstructuredSerializer, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + p, m := req.URL.Path, req.Method + t.Logf("got request %s %s", p, m) + switch { + case p == "/namespaces/default/pods/test-pod" && m == "GET": + return newResponse(200, &listA.Items[0]) + case p == "/namespaces/default/pods/test-pod" && m == "PATCH": + data, err := io.ReadAll(req.Body) + if err != nil { + t.Fatalf("could not read request body: %s", err) + } + req.Body.Close() + + // Update expected patch to match the actual behavior + expected := `{"spec":{"$setElementOrder/containers":[{"name":"app:v4"}],"containers":[{"image":"nginx:latest","name":"app:v4"}]}}` + + if string(data) != expected { + t.Errorf("expected patch:\n%s\ngot:\n%s", expected, string(data)) + } + return newResponse(200, &listB.Items[0]) + default: + t.Fatalf("unexpected request: %s %s", req.Method, req.URL.Path) + return nil, nil + } + }), + } + + original, err := c.Build(objBody(&listA), false) + if err != nil { + t.Fatal(err) + } + target, err := c.Build(objBody(&listB), false) + if err != nil { + t.Fatal(err) + } + + result, err := c.Update(original, target, false) + if err != nil { + t.Fatal(err) + } + + if len(result.Updated) != 1 { + t.Errorf("expected 1 resource updated, got %d", len(result.Updated)) + } +} +func TestForceUpdateWhenOriginalResourceMissing(t *testing.T) { + c := newTestClient(t) + c.Factory.(*cmdtesting.TestFactory).UnstructuredClient = &fake.RESTClient{ + NegotiatedSerializer: unstructuredSerializer, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + p, m := req.URL.Path, req.Method + t.Logf("got request %s %s", p, m) + switch { + case p == "/namespaces/default/pods/missing-pod" && m == "GET": + // Simulate a not found error + return newResponse(404, notFoundBody()) + case p == "/namespaces/default/pods" && m == "POST": + // Simulate successful creation + pod := newPod("missing-pod") + return newResponse(200, &pod) + default: + t.Fatalf("unexpected request: %s %s", req.Method, req.URL.Path) + return nil, nil + } + }), + } + + original, err := c.Build(strings.NewReader(`{"apiVersion":"v1","kind":"Pod","metadata":{"name":"missing-pod","namespace":"default"}}`), false) + if err != nil { + t.Fatal(err) + } + + target, err := c.Build(strings.NewReader(`{"apiVersion":"v1","kind":"Pod","metadata":{"name":"missing-pod","namespace":"default"}}`), false) + if err != nil { + t.Fatal(err) + } + + result, err := c.Update(original, target, true) + if err != nil { + t.Fatal(err) + } + + if len(result.Created) != 1 { + t.Errorf("expected 1 resource created, got %d", len(result.Created)) + } + if len(result.Updated) != 0 { + t.Errorf("expected 0 resources updated, got %d", len(result.Updated)) + } +} + +func TestUpdateFailsWhenResourceNotFound(t *testing.T) { + c := newTestClient(t) + c.Factory.(*cmdtesting.TestFactory).UnstructuredClient = &fake.RESTClient{ + NegotiatedSerializer: unstructuredSerializer, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + p, m := req.URL.Path, req.Method + t.Logf("got request %s %s", p, m) + switch { + case p == "/namespaces/default/pods/non-existent" && m == "GET": + // Simulate a not found error + return newResponse(404, notFoundBody()) + case p == "/namespaces/default/pods" && m == "POST": + // Simulate a failed resource creation + var status metav1.Status + if err := runtime.DecodeInto(codec, resourceQuotaConflict, &status); err != nil { + t.Fatalf("failed to decode resourceQuotaConflict: %v", err) + } + return newResponse(409, &status) + default: + t.Fatalf("unexpected request: %s %s", req.Method, req.URL.Path) + return nil, nil + } + }), + } + + original, err := c.Build(strings.NewReader(`{"apiVersion":"v1","kind":"Pod","metadata":{"name":"non-existent","namespace":"default"}}`), false) + if err != nil { + t.Fatal(err) + } + + target, err := c.Build(strings.NewReader(`{"apiVersion":"v1","kind":"Pod","metadata":{"name":"non-existent","namespace":"default"}}`), false) + if err != nil { + t.Fatal(err) + } + + _, err = c.Update(original, target, false) + if err == nil { + t.Fatal("expected error when updating a non-existent resource, got nil") + } + if !strings.Contains(err.Error(), "failed to create resource") { + t.Errorf("unexpected error message: %v", err) + } +} func TestOutputContainerLogsForPodList(t *testing.T) { namespace := "some-namespace" somePodList := newPodList("jimmy", "three", "structs")