From 10eb7092f854c71122c03752465e868bce23c0b6 Mon Sep 17 00:00:00 2001 From: Alexandre Garnier Date: Wed, 4 May 2022 02:17:49 +0200 Subject: [PATCH] Do not raise an error proposing to use '--overwrite' when labeling with the same value (#105936) * Do not propose to use '--overwrite' when labeling with the same value * Check expected error value in label_test.TestLabelFunc --- .../src/k8s.io/kubectl/pkg/cmd/label/label.go | 6 ++--- .../kubectl/pkg/cmd/label/label_test.go | 27 ++++++++++++++----- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/label/label.go b/staging/src/k8s.io/kubectl/pkg/cmd/label/label.go index bf9c12c38af..a5e2ce04136 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/label/label.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/label/label.go @@ -408,9 +408,9 @@ func updateDataChangeMsg(oldObj []byte, newObj []byte, overwrite bool) string { func validateNoOverwrites(accessor metav1.Object, labels map[string]string) error { allErrs := []error{} - for key := range labels { - if value, found := accessor.GetLabels()[key]; found { - allErrs = append(allErrs, fmt.Errorf("'%s' already has a value (%s), and --overwrite is false", key, value)) + for key, value := range labels { + if currValue, found := accessor.GetLabels()[key]; found && currValue != value { + allErrs = append(allErrs, fmt.Errorf("'%s' already has a value (%s), and --overwrite is false", key, currValue)) } } return utilerrors.NewAggregate(allErrs) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/label/label_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/label/label_test.go index cf7526fa44b..ea796e70bd7 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/label/label_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/label/label_test.go @@ -166,7 +166,7 @@ func TestLabelFunc(t *testing.T) { labels map[string]string remove []string expected runtime.Object - expectErr bool + expectErr string }{ { obj: &v1.Pod{ @@ -174,8 +174,21 @@ func TestLabelFunc(t *testing.T) { Labels: map[string]string{"a": "b"}, }, }, - labels: map[string]string{"a": "b"}, - expectErr: true, + labels: map[string]string{"a": "b"}, + expected: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"a": "b"}, + }, + }, + }, + { + obj: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"a": "b"}, + }, + }, + labels: map[string]string{"a": "c"}, + expectErr: "'a' already has a value (b), and --overwrite is false", }, { obj: &v1.Pod{ @@ -264,13 +277,16 @@ func TestLabelFunc(t *testing.T) { } for _, test := range tests { err := labelFunc(test.obj, test.overwrite, test.version, test.labels, test.remove) - if test.expectErr { + if test.expectErr != "" { if err == nil { t.Errorf("unexpected non-error: %v", test) } + if err.Error() != test.expectErr { + t.Errorf("error expected: %v, got: %v", test.expectErr, err.Error()) + } continue } - if !test.expectErr && err != nil { + if test.expectErr == "" && err != nil { t.Errorf("unexpected error: %v %v", err, test) } if !reflect.DeepEqual(test.obj, test.expected) { @@ -586,7 +602,6 @@ func TestLabelMsg(t *testing.T) { }, labels: map[string]string{"a": "b"}, expectMsg: MsgNotLabeled, - expectErr: true, }, { obj: &v1.Pod{