Skip to content

Commit 141329f

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#85285 from liggitt/kubectl-resource-version
Fix --resource-version handling in kubectl
2 parents 8dffc8d + 0ac8345 commit 141329f

File tree

7 files changed

+191
-5
lines changed

7 files changed

+191
-5
lines changed

‎staging/src/k8s.io/kubectl/pkg/cmd/annotate/annotate.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,16 @@ func (o AnnotateOptions) RunAnnotate() error {
264264
outputObj = obj
265265
} else {
266266
name, namespace := info.Name, info.Namespace
267+
268+
if len(o.resourceVersion) != 0 {
269+
// ensure resourceVersion is always sent in the patch by clearing it from the starting JSON
270+
accessor, err := meta.Accessor(obj)
271+
if err != nil {
272+
return err
273+
}
274+
accessor.SetResourceVersion("")
275+
}
276+
267277
oldData, err := json.Marshal(obj)
268278
if err != nil {
269279
return err

‎staging/src/k8s.io/kubectl/pkg/cmd/annotate/annotate_test.go‎

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,14 @@ limitations under the License.
1717
package annotate
1818

1919
import (
20+
"bytes"
21+
"io/ioutil"
2022
"net/http"
2123
"reflect"
2224
"strings"
2325
"testing"
2426

25-
"k8s.io/api/core/v1"
27+
v1 "k8s.io/api/core/v1"
2628
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2729
"k8s.io/apimachinery/pkg/runtime"
2830
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -502,6 +504,73 @@ func TestAnnotateObject(t *testing.T) {
502504
}
503505
}
504506

507+
func TestAnnotateResourceVersion(t *testing.T) {
508+
tf := cmdtesting.NewTestFactory().WithNamespace("test")
509+
defer tf.Cleanup()
510+
511+
tf.UnstructuredClient = &fake.RESTClient{
512+
GroupVersion: schema.GroupVersion{Group: "testgroup", Version: "v1"},
513+
NegotiatedSerializer: resource.UnstructuredPlusDefaultContentConfig().NegotiatedSerializer,
514+
Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
515+
switch req.Method {
516+
case "GET":
517+
switch req.URL.Path {
518+
case "/namespaces/test/pods/foo":
519+
return &http.Response{
520+
StatusCode: http.StatusOK,
521+
Header: cmdtesting.DefaultHeader(),
522+
Body: ioutil.NopCloser(bytes.NewBufferString(
523+
`{"kind":"Pod","apiVersion":"v1","metadata":{"name":"foo","namespace":"test","resourceVersion":"10"}}`,
524+
))}, nil
525+
default:
526+
t.Fatalf("unexpected request: %#v\n%#v", req.URL, req)
527+
return nil, nil
528+
}
529+
case "PATCH":
530+
switch req.URL.Path {
531+
case "/namespaces/test/pods/foo":
532+
body, err := ioutil.ReadAll(req.Body)
533+
if err != nil {
534+
t.Fatal(err)
535+
}
536+
if !bytes.Equal(body, []byte(`{"metadata":{"annotations":{"a":"b"},"resourceVersion":"10"}}`)) {
537+
t.Fatalf("expected patch with resourceVersion set, got %s", string(body))
538+
}
539+
return &http.Response{
540+
StatusCode: http.StatusOK,
541+
Header: cmdtesting.DefaultHeader(),
542+
Body: ioutil.NopCloser(bytes.NewBufferString(
543+
`{"kind":"Pod","apiVersion":"v1","metadata":{"name":"foo","namespace":"test","resourceVersion":"11"}}`,
544+
))}, nil
545+
default:
546+
t.Fatalf("unexpected request: %#v\n%#v", req.URL, req)
547+
return nil, nil
548+
}
549+
default:
550+
t.Fatalf("unexpected request: %s %#v\n%#v", req.Method, req.URL, req)
551+
return nil, nil
552+
}
553+
}),
554+
}
555+
tf.ClientConfigVal = cmdtesting.DefaultClientConfig()
556+
557+
iostreams, _, bufOut, _ := genericclioptions.NewTestIOStreams()
558+
cmd := NewCmdAnnotate("kubectl", tf, iostreams)
559+
cmd.SetOutput(bufOut)
560+
options := NewAnnotateOptions(iostreams)
561+
options.resourceVersion = "10"
562+
args := []string{"pods/foo", "a=b"}
563+
if err := options.Complete(tf, cmd, args); err != nil {
564+
t.Fatalf("unexpected error: %v", err)
565+
}
566+
if err := options.Validate(); err != nil {
567+
t.Fatalf("unexpected error: %v", err)
568+
}
569+
if err := options.RunAnnotate(); err != nil {
570+
t.Fatalf("unexpected error: %v", err)
571+
}
572+
}
573+
505574
func TestAnnotateObjectFromFile(t *testing.T) {
506575
pods, _, _ := cmdtesting.TestData()
507576

‎staging/src/k8s.io/kubectl/pkg/cmd/label/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ go_test(
3737
"//staging/src/k8s.io/api/core/v1:go_default_library",
3838
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
3939
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
40+
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
4041
"//staging/src/k8s.io/cli-runtime/pkg/genericclioptions:go_default_library",
4142
"//staging/src/k8s.io/cli-runtime/pkg/resource:go_default_library",
4243
"//staging/src/k8s.io/client-go/rest/fake:go_default_library",

‎staging/src/k8s.io/kubectl/pkg/cmd/label/label.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,16 @@ func (o *LabelOptions) RunLabel() error {
253253
var outputObj runtime.Object
254254
var dataChangeMsg string
255255
obj := info.Object
256+
257+
if len(o.resourceVersion) != 0 {
258+
// ensure resourceVersion is always sent in the patch by clearing it from the starting JSON
259+
accessor, err := meta.Accessor(obj)
260+
if err != nil {
261+
return err
262+
}
263+
accessor.SetResourceVersion("")
264+
}
265+
256266
oldData, err := json.Marshal(obj)
257267
if err != nil {
258268
return err

‎staging/src/k8s.io/kubectl/pkg/cmd/label/label_test.go‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package label
1818

1919
import (
2020
"bytes"
21+
"io/ioutil"
2122
"net/http"
2223
"reflect"
2324
"strings"
@@ -26,6 +27,7 @@ import (
2627
"k8s.io/api/core/v1"
2728
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2829
"k8s.io/apimachinery/pkg/runtime"
30+
"k8s.io/apimachinery/pkg/runtime/schema"
2931
"k8s.io/cli-runtime/pkg/genericclioptions"
3032
"k8s.io/cli-runtime/pkg/resource"
3133
"k8s.io/client-go/rest/fake"
@@ -496,3 +498,70 @@ func TestLabelMultipleObjects(t *testing.T) {
496498
t.Errorf("not all labels are set: %s", buf.String())
497499
}
498500
}
501+
502+
func TestLabelResourceVersion(t *testing.T) {
503+
tf := cmdtesting.NewTestFactory().WithNamespace("test")
504+
defer tf.Cleanup()
505+
506+
tf.UnstructuredClient = &fake.RESTClient{
507+
GroupVersion: schema.GroupVersion{Group: "testgroup", Version: "v1"},
508+
NegotiatedSerializer: resource.UnstructuredPlusDefaultContentConfig().NegotiatedSerializer,
509+
Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
510+
switch req.Method {
511+
case "GET":
512+
switch req.URL.Path {
513+
case "/namespaces/test/pods/foo":
514+
return &http.Response{
515+
StatusCode: http.StatusOK,
516+
Header: cmdtesting.DefaultHeader(),
517+
Body: ioutil.NopCloser(bytes.NewBufferString(
518+
`{"kind":"Pod","apiVersion":"v1","metadata":{"name":"foo","namespace":"test","resourceVersion":"10"}}`,
519+
))}, nil
520+
default:
521+
t.Fatalf("unexpected request: %#v\n%#v", req.URL, req)
522+
return nil, nil
523+
}
524+
case "PATCH":
525+
switch req.URL.Path {
526+
case "/namespaces/test/pods/foo":
527+
body, err := ioutil.ReadAll(req.Body)
528+
if err != nil {
529+
t.Fatal(err)
530+
}
531+
if !bytes.Equal(body, []byte(`{"metadata":{"labels":{"a":"b"},"resourceVersion":"10"}}`)) {
532+
t.Fatalf("expected patch with resourceVersion set, got %s", string(body))
533+
}
534+
return &http.Response{
535+
StatusCode: http.StatusOK,
536+
Header: cmdtesting.DefaultHeader(),
537+
Body: ioutil.NopCloser(bytes.NewBufferString(
538+
`{"kind":"Pod","apiVersion":"v1","metadata":{"name":"foo","namespace":"test","resourceVersion":"11"}}`,
539+
))}, nil
540+
default:
541+
t.Fatalf("unexpected request: %#v\n%#v", req.URL, req)
542+
return nil, nil
543+
}
544+
default:
545+
t.Fatalf("unexpected request: %s %#v\n%#v", req.Method, req.URL, req)
546+
return nil, nil
547+
}
548+
}),
549+
}
550+
tf.ClientConfigVal = cmdtesting.DefaultClientConfig()
551+
552+
iostreams, _, bufOut, _ := genericclioptions.NewTestIOStreams()
553+
cmd := NewCmdLabel(tf, iostreams)
554+
cmd.SetOutput(bufOut)
555+
options := NewLabelOptions(iostreams)
556+
options.resourceVersion = "10"
557+
args := []string{"pods/foo", "a=b"}
558+
if err := options.Complete(tf, cmd, args); err != nil {
559+
t.Fatalf("unexpected error: %v", err)
560+
}
561+
if err := options.Validate(); err != nil {
562+
t.Fatalf("unexpected error: %v", err)
563+
}
564+
if err := options.RunLabel(); err != nil {
565+
t.Fatalf("unexpected error: %v", err)
566+
}
567+
}

‎staging/src/k8s.io/kubectl/pkg/cmd/set/set_selector.go‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ import (
2222
"github.com/spf13/cobra"
2323
"k8s.io/klog"
2424

25-
"k8s.io/api/core/v1"
25+
v1 "k8s.io/api/core/v1"
26+
"k8s.io/apimachinery/pkg/api/meta"
2627
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2728
"k8s.io/apimachinery/pkg/runtime"
2829
"k8s.io/apimachinery/pkg/types"
@@ -46,8 +47,9 @@ type SetSelectorOptions struct {
4647
dryrun bool
4748

4849
// set by args
49-
resources []string
50-
selector *metav1.LabelSelector
50+
resources []string
51+
selector *metav1.LabelSelector
52+
resourceVersion string
5153

5254
// computed
5355
WriteToServer bool
@@ -111,7 +113,7 @@ func NewCmdSelector(f cmdutil.Factory, streams genericclioptions.IOStreams) *cob
111113
o.PrintFlags.AddFlags(cmd)
112114
o.RecordFlags.AddFlags(cmd)
113115

114-
cmd.Flags().String("resource-version", "", "If non-empty, the selectors update will only succeed if this is the current resource-version for the object. Only valid when specifying a single resource.")
116+
cmd.Flags().StringVarP(&o.resourceVersion, "resource-version", "", o.resourceVersion, "If non-empty, the selectors update will only succeed if this is the current resource-version for the object. Only valid when specifying a single resource.")
115117
cmdutil.AddDryRunFlag(cmd)
116118

117119
return cmd
@@ -163,7 +165,26 @@ func (o *SetSelectorOptions) RunSelector() error {
163165

164166
return r.Visit(func(info *resource.Info, err error) error {
165167
patch := &Patch{Info: info}
168+
169+
if len(o.resourceVersion) != 0 {
170+
// ensure resourceVersion is always sent in the patch by clearing it from the starting JSON
171+
accessor, err := meta.Accessor(info.Object)
172+
if err != nil {
173+
return err
174+
}
175+
accessor.SetResourceVersion("")
176+
}
177+
166178
CalculatePatch(patch, scheme.DefaultJSONEncoder(), func(obj runtime.Object) ([]byte, error) {
179+
180+
if len(o.resourceVersion) != 0 {
181+
accessor, err := meta.Accessor(info.Object)
182+
if err != nil {
183+
return nil, err
184+
}
185+
accessor.SetResourceVersion(o.resourceVersion)
186+
}
187+
167188
selectErr := updateSelectorForObject(info.Object, *o.selector)
168189
if selectErr != nil {
169190
return nil, selectErr

‎test/cmd/core.sh‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -896,6 +896,12 @@ run_service_tests() {
896896
kubectl set selector services redis-master role=padawan --dry-run -o yaml "${kube_flags[@]}"
897897
! kubectl set selector services redis-master role=padawan --local -o yaml "${kube_flags[@]}" || exit 1
898898
kube::test::get_object_assert 'services redis-master' "{{range$service_selector_field}}{{.}}:{{end}}" "redis:master:backend:"
899+
# --resource-version=<current-resource-version> succeeds
900+
rv=$(kubectl get services redis-master -o jsonpath='{.metadata.resourceVersion}' "${kube_flags[@]}")
901+
kubectl set selector services redis-master rvtest1=true "--resource-version=${rv}" "${kube_flags[@]}"
902+
# --resource-version=<non-current-resource-version> fails
903+
output_message=$(! kubectl set selector services redis-master rvtest1=true --resource-version=1 2>&1 "${kube_flags[@]}")
904+
kube::test::if_has_string "${output_message}" 'Conflict'
899905

900906
### Dump current redis-master service
901907
output_service=$(kubectl get service redis-master -o json "${kube_flags[@]}")

0 commit comments

Comments
 (0)