fix set cmd bug caused by MergePatchType (#49)

Signed-off-by: hantmac <hantmac@outlook.com>
openkruise · Mar 9, 2022 · 1d835fb · 1d835fb
1 parent fb8c560
commit 1d835fb
Show file tree

Hide file tree

Showing 6 changed files with 11 additions and 10 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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