From 84b1ec1cd4b5cbe99d85dd968c243128c2363b4c Mon Sep 17 00:00:00 2001 From: MrJack <36191829+biagiopietro@users.noreply.github.com> Date: Tue, 18 Aug 2026 10:55:19 +0200 Subject: [PATCH] honor dry-run when pruning removed resources Signed-off-by: MrJack <36191829+biagiopietro@users.noreply.github.com> --- pkg/kube/client.go | 15 +++++--- pkg/kube/client_test.go | 82 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index ba881c560..da3016fc9 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -572,7 +572,7 @@ func (c *Client) BuildTable(reader io.Reader, validate bool) (ResourceList, erro transformRequests) } -func (c *Client) update(originals, targets ResourceList, createApplyFunc CreateApplyFunc, updateApplyFunc UpdateApplyFunc) (*Result, error) { +func (c *Client) update(originals, targets ResourceList, createApplyFunc CreateApplyFunc, updateApplyFunc UpdateApplyFunc, dryRun bool) (*Result, error) { updateErrors := []error{} res := &Result{} @@ -655,7 +655,7 @@ func (c *Client) update(originals, targets ResourceList, createApplyFunc CreateA } for _, info := range originals.Difference(targets) { - c.Logger().Debug("deleting resource", "namespace", info.Namespace, "name", info.Name, "kind", info.Mapping.GroupVersionKind.Kind) + c.Logger().Debug("deleting resource", "namespace", info.Namespace, "name", info.Name, "kind", info.Mapping.GroupVersionKind.Kind, "dryRun", dryRun) if err := info.Get(); err != nil { c.Logger().Debug( @@ -681,7 +681,7 @@ func (c *Client) update(originals, targets ResourceList, createApplyFunc CreateA c.Logger().Debug("skipping delete due to annotation", "namespace", info.Namespace, "name", info.Name, "kind", info.Mapping.GroupVersionKind.Kind, "annotation", ResourcePolicyAnno, "value", KeepPolicy) continue } - if err := deleteResource(info, metav1.DeletePropagationBackground); err != nil { + if err := deleteResource(info, metav1.DeletePropagationBackground, dryRun); err != nil { c.Logger().Debug( "failed to delete resource", slog.String("namespace", info.Namespace), @@ -902,7 +902,7 @@ func (c *Client) Update(originals, targets ResourceList, options ...ClientUpdate } } - return c.update(originals, targets, createApplyFunc, makeUpdateApplyFunc()) + return c.update(originals, targets, createApplyFunc, makeUpdateApplyFunc(), updateOptions.dryRun) } // Delete deletes Kubernetes resources specified in the resources list with @@ -915,7 +915,7 @@ func (c *Client) Delete(resources ResourceList, policy metav1.DeletionPropagatio mtx := sync.Mutex{} err := perform(resources, func(target *resource.Info) error { c.Logger().Debug("starting delete resource", "namespace", target.Namespace, "name", target.Name, "kind", target.Mapping.GroupVersionKind.Kind) - err := deleteResource(target, policy) + err := deleteResource(target, policy, false) if err == nil || apierrors.IsNotFound(err) { if err != nil { c.Logger().Debug( @@ -1062,11 +1062,14 @@ func createResource(info *resource.Info) error { }) } -func deleteResource(info *resource.Info, policy metav1.DeletionPropagation) error { +func deleteResource(info *resource.Info, policy metav1.DeletionPropagation, dryRun bool) error { return retry.RetryOnConflict( retry.DefaultRetry, func() error { opts := &metav1.DeleteOptions{PropagationPolicy: &policy} + if dryRun { + opts.DryRun = []string{metav1.DryRunAll} + } _, err := resource.NewHelper(info.Client, info.Mapping).WithFieldManager(getManagedFieldsManager()).DeleteWithOptions(info.Namespace, info.Name, opts) return err }) diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go index 86574a3e5..269b7b205 100644 --- a/pkg/kube/client_test.go +++ b/pkg/kube/client_test.go @@ -19,6 +19,7 @@ package kube import ( "bytes" "context" + "encoding/json" "errors" "io" "net/http" @@ -593,6 +594,87 @@ func TestUpdate(t *testing.T) { } } +// TestUpdateDryRunDelete verifies that the dry-run option is propagated to the +// deletions Update performs when a resource present in the original release is +// absent from the target. Without this, a server-side dry-run upgrade would +// really remove pruned resources from the cluster. +func TestUpdateDryRunDelete(t *testing.T) { + for name, dryRun := range map[string]bool{"dry run": true, "normal": false} { + t.Run(name, func(t *testing.T) { + listOriginal := newPodList("starfish", "squid") + listTarget := newPodList("starfish") + + expectedDryRun := "" + var expectedDeleteDryRun []string + if dryRun { + expectedDryRun = "All" + expectedDeleteDryRun = []string{metav1.DryRunAll} + } + + c := newTestClient(t) + cb := func(_ []RequestResponseAction, req *http.Request) (*http.Response, error) { + p, m := req.URL.Path, req.Method + + switch { + case p == "/namespaces/default/pods/starfish" && m == http.MethodGet: + return newResponse(http.StatusOK, &listOriginal.Items[0]) + case p == "/namespaces/default/pods/starfish" && m == http.MethodPatch: + assert.Equal(t, expectedDryRun, req.URL.Query().Get("dryRun")) + return newResponse(http.StatusOK, &listTarget.Items[0]) + case p == "/namespaces/default/pods/squid" && m == http.MethodGet: + return newResponse(http.StatusOK, &listOriginal.Items[1]) + case p == "/namespaces/default/pods/squid" && m == http.MethodDelete: + // DeleteOptions are sent in the request body, not the query string + defer req.Body.Close() + data, err := io.ReadAll(req.Body) + require.NoError(t, err) + + opts := metav1.DeleteOptions{} + require.NoError(t, json.Unmarshal(data, &opts)) + assert.Equal(t, expectedDeleteDryRun, opts.DryRun) + + return newResponse(http.StatusOK, &listOriginal.Items[1]) + } + + t.FailNow() + return nil, nil + } + + client := NewRequestResponseLogClient(t, cb) + c.Factory.(*cmdtesting.TestFactory).UnstructuredClient = &fake.RESTClient{ + NegotiatedSerializer: unstructuredSerializer, + Client: fake.CreateHTTPClient(client.Do), + } + + first, err := c.Build(objBody(&listOriginal), false) + require.NoError(t, err) + + second, err := c.Build(objBody(&listTarget), false) + require.NoError(t, err) + + result, err := c.Update( + first, + second, + ClientUpdateOptionServerSideApply(true, false), + ClientUpdateOptionDryRun(dryRun)) + require.NoError(t, err) + + assert.Len(t, result.Deleted, 1, "expected 1 resource deleted") + actions := []string{} + for _, action := range client.Actions { + actions = append(actions, action.Request.URL.Path+":"+action.Request.Method) + } + + assert.Equal(t, []string{ + "/namespaces/default/pods/starfish:GET", + "/namespaces/default/pods/starfish:PATCH", + "/namespaces/default/pods/squid:GET", + "/namespaces/default/pods/squid:DELETE", + }, actions) + }) + } +} + func TestBuild(t *testing.T) { tests := []struct { name string