From 9feaf478c6a2d9982fa55086a856ccd2a414ecbe Mon Sep 17 00:00:00 2001 From: Alexandre Garnier Date: Fri, 15 Apr 2022 14:31:56 +0200 Subject: [PATCH 1/2] Do not raise an error proposing to use '--overwrite' when annotating with the same value --- .../src/k8s.io/kubectl/pkg/cmd/annotate/annotate.go | 6 +++--- .../kubectl/pkg/cmd/annotate/annotate_test.go | 13 +++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/annotate/annotate.go b/staging/src/k8s.io/kubectl/pkg/cmd/annotate/annotate.go index 9c410604d78..d4a72bf9b22 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/annotate/annotate.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/annotate/annotate.go @@ -404,16 +404,16 @@ func validateAnnotations(removeAnnotations []string, newAnnotations map[string]s // validateNoAnnotationOverwrites validates that when overwrite is false, to-be-updated annotations don't exist in the object annotation map (yet) func validateNoAnnotationOverwrites(accessor metav1.Object, annotations map[string]string) error { var buf bytes.Buffer - for key := range annotations { + for key, value := range annotations { // change-cause annotation can always be overwritten if key == polymorphichelpers.ChangeCauseAnnotation { continue } - if value, found := accessor.GetAnnotations()[key]; found { + if currValue, found := accessor.GetAnnotations()[key]; found && currValue != value { if buf.Len() > 0 { buf.WriteString("; ") } - buf.WriteString(fmt.Sprintf("'%s' already has a value (%s)", key, value)) + buf.WriteString(fmt.Sprintf("'%s' already has a value (%s)", key, currValue)) } } if buf.Len() > 0 { diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/annotate/annotate_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/annotate/annotate_test.go index 5e5ed0b34d9..02addbcabd6 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/annotate/annotate_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/annotate/annotate_test.go @@ -236,6 +236,19 @@ func TestUpdateAnnotations(t *testing.T) { }, }, annotations: map[string]string{"a": "b"}, + expected: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"a": "b"}, + }, + }, + }, + { + obj: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"a": "b"}, + }, + }, + annotations: map[string]string{"a": "c"}, expectErr: true, }, { From 78cca63817936d03dfc5e7f176f7c3a33c55a80e Mon Sep 17 00:00:00 2001 From: Alexandre Garnier Date: Wed, 11 May 2022 09:17:56 +0200 Subject: [PATCH 2/2] Check expected error value in annotate_test.TestUpdateAnnotations --- .../k8s.io/kubectl/pkg/cmd/annotate/annotate_test.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/annotate/annotate_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/annotate/annotate_test.go index 02addbcabd6..c3ea155afae 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/annotate/annotate_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/annotate/annotate_test.go @@ -227,7 +227,7 @@ func TestUpdateAnnotations(t *testing.T) { annotations map[string]string remove []string expected runtime.Object - expectErr bool + expectedErr string }{ { obj: &v1.Pod{ @@ -249,7 +249,7 @@ func TestUpdateAnnotations(t *testing.T) { }, }, annotations: map[string]string{"a": "c"}, - expectErr: true, + expectedErr: "--overwrite is false but found the following declared annotation(s): 'a' already has a value (b)", }, { obj: &v1.Pod{ @@ -378,13 +378,16 @@ func TestUpdateAnnotations(t *testing.T) { resourceVersion: test.version, } err := options.updateAnnotations(test.obj) - if test.expectErr { + if test.expectedErr != "" { if err == nil { t.Errorf("unexpected non-error: %v", test) } + if err.Error() != test.expectedErr { + t.Errorf("error expected: %v, got: %v", test.expectedErr, err.Error()) + } continue } - if !test.expectErr && err != nil { + if test.expectedErr == "" && err != nil { t.Errorf("unexpected error: %v %v", err, test) } if !reflect.DeepEqual(test.obj, test.expected) {