From 1d835fb07f12bf9f9a5102b4572d585ac80bfe05 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Wed, 9 Mar 2022 11:08:23 +0800 Subject: [PATCH] fix set cmd bug caused by MergePatchType (#49) Signed-off-by: hantmac --- pkg/cmd/set/helper.go | 9 +++++---- pkg/cmd/set/set_image.go | 2 +- pkg/cmd/set/set_image_test.go | 4 ++-- pkg/cmd/set/set_selector.go | 2 +- pkg/cmd/set/set_serviceaccount.go | 2 +- pkg/cmd/set/set_subject.go | 2 +- 6 files changed, 11 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/set/helper.go b/pkg/cmd/set/helper.go index 809c149..05a4565 100644 --- a/pkg/cmd/set/helper.go +++ b/pkg/cmd/set/helper.go @@ -28,8 +28,8 @@ import ( // selectContainers allows one or more containers to be matched against a string or wildcard func selectContainers(containers []v1.Container, spec string) ([]*v1.Container, []*v1.Container) { - var out []*v1.Container - var skipped []*v1.Container + out := []*v1.Container{} + skipped := []*v1.Container{} for i, c := range containers { if selectString(c.Name, spec) { out = append(out, &containers[i]) @@ -54,6 +54,7 @@ func selectString(s, spec string) bool { pos := 0 match := true parts := strings.Split(spec, "*") +Loop: for i, part := range parts { if len(part) == 0 { continue @@ -69,7 +70,7 @@ func selectString(s, spec string) bool { // last part does not exactly match remaining part of string case i == (len(parts)-1) && len(s) != (len(part)+next): match = false - break + break Loop default: pos = next } @@ -131,7 +132,7 @@ func findEnv(env []v1.EnvVar, name string) (v1.EnvVar, bool) { } func updateEnv(existing []v1.EnvVar, env []v1.EnvVar, remove []string) []v1.EnvVar { - var out []v1.EnvVar + out := []v1.EnvVar{} covered := sets.NewString(remove...) for _, e := range existing { if covered.Has(e.Name) { diff --git a/pkg/cmd/set/set_image.go b/pkg/cmd/set/set_image.go index e13a72c..da61cba 100644 --- a/pkg/cmd/set/set_image.go +++ b/pkg/cmd/set/set_image.go @@ -290,7 +290,7 @@ func (o *SetImageOptions) Run() error { actual, err := resource. NewHelper(info.Client, info.Mapping). DryRun(o.DryRunStrategy == cmdutil.DryRunServer). - Patch(info.Namespace, info.Name, types.MergePatchType, patch.Patch, nil) + Patch(info.Namespace, info.Name, types.MergePatchType, patch.After, nil) if err != nil { allErrs = append(allErrs, fmt.Errorf("failed to patch image update to pod template: %v", err)) continue diff --git a/pkg/cmd/set/set_image_test.go b/pkg/cmd/set/set_image_test.go index fd7eeef..9bea234 100644 --- a/pkg/cmd/set/set_image_test.go +++ b/pkg/cmd/set/set_image_test.go @@ -746,8 +746,8 @@ func TestSetImageRemoteWithSpecificContainers(t *testing.T) { if err != nil { return nil, err } - assert.Contains(t, string(bytes), `"image":"`+"thingy"+`","name":`+`"nginx"`, fmt.Sprintf("image not updated for %#v", input.object)) - assert.NotContains(t, string(bytes), `"image":"`+"thingy"+`","name":`+`"busybox"`, fmt.Sprintf("image updated for %#v", input.object)) + assert.Contains(t, string(bytes), `"name":"`+"nginx"+`","image":`+`"thingy"`, fmt.Sprintf("image not updated for %#v", input.object)) + assert.NotContains(t, string(bytes), `"name":"`+"busybox"+`","image":`+`"thingy"`, fmt.Sprintf("image updated for %#v", input.object)) return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: objBody(input.object)}, nil default: t.Errorf("%s: unexpected request: %s %#v\n%#v", "image", req.Method, req.URL, req) diff --git a/pkg/cmd/set/set_selector.go b/pkg/cmd/set/set_selector.go index 7663a91..68b1c94 100644 --- a/pkg/cmd/set/set_selector.go +++ b/pkg/cmd/set/set_selector.go @@ -224,7 +224,7 @@ func (o *SetSelectorOptions) RunSelector() error { actual, err := resource. NewHelper(info.Client, info.Mapping). DryRun(o.dryRunStrategy == cmdutil.DryRunServer). - Patch(info.Namespace, info.Name, types.MergePatchType, patch.Patch, nil) + Patch(info.Namespace, info.Name, types.MergePatchType, patch.After, nil) if err != nil { return err } diff --git a/pkg/cmd/set/set_serviceaccount.go b/pkg/cmd/set/set_serviceaccount.go index aeeae10..f8862e4 100644 --- a/pkg/cmd/set/set_serviceaccount.go +++ b/pkg/cmd/set/set_serviceaccount.go @@ -224,7 +224,7 @@ func (o *SetServiceAccountOptions) Run() error { actual, err := resource. NewHelper(info.Client, info.Mapping). DryRun(o.dryRunStrategy == cmdutil.DryRunServer). - Patch(info.Namespace, info.Name, types.MergePatchType, patch.Patch, nil) + Patch(info.Namespace, info.Name, types.MergePatchType, patch.After, nil) if err != nil { patchErrs = append(patchErrs, fmt.Errorf("failed to patch ServiceAccountName %v", err)) continue diff --git a/pkg/cmd/set/set_subject.go b/pkg/cmd/set/set_subject.go index 6424c36..95548fc 100644 --- a/pkg/cmd/set/set_subject.go +++ b/pkg/cmd/set/set_subject.go @@ -281,7 +281,7 @@ func (o *SubjectOptions) Run(fn updateSubjects) error { actual, err := resource. NewHelper(info.Client, info.Mapping). DryRun(o.DryRunStrategy == cmdutil.DryRunServer). - Patch(info.Namespace, info.Name, types.MergePatchType, patch.Patch, nil) + Patch(info.Namespace, info.Name, types.MergePatchType, patch.After, nil) if err != nil { allErrs = append(allErrs, fmt.Errorf("failed to patch subjects to rolebinding: %v", err)) continue