From c12d8a56f8397b6d1dfcb853eb36de0fc06cbd9f Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Fri, 11 Jan 2019 10:38:18 -0500 Subject: [PATCH] Find current resourceVersion for waiting for deletion/conditions --- pkg/kubectl/cmd/wait/BUILD | 1 + pkg/kubectl/cmd/wait/wait.go | 32 +++++-- pkg/kubectl/cmd/wait/wait_test.go | 96 +++++++++++-------- .../k8s.io/client-go/dynamic/fake/simple.go | 1 + 4 files changed, 82 insertions(+), 48 deletions(-) diff --git a/pkg/kubectl/cmd/wait/BUILD b/pkg/kubectl/cmd/wait/BUILD index 919e4bd95b..08ddcfc6fe 100644 --- a/pkg/kubectl/cmd/wait/BUILD +++ b/pkg/kubectl/cmd/wait/BUILD @@ -11,6 +11,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", diff --git a/pkg/kubectl/cmd/wait/wait.go b/pkg/kubectl/cmd/wait/wait.go index 7e85fbcf7b..1fd91f2ca9 100644 --- a/pkg/kubectl/cmd/wait/wait.go +++ b/pkg/kubectl/cmd/wait/wait.go @@ -29,6 +29,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -255,7 +256,11 @@ func IsDeleted(info *resource.Info, o *WaitOptions) (runtime.Object, bool, error if len(info.Name) == 0 { return info.Object, false, fmt.Errorf("resource name must be provided") } - gottenObj, err := o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).Get(info.Name, metav1.GetOptions{}) + + nameSelector := fields.OneTermEqualSelector("metadata.name", info.Name).String() + + // List with a name field selector to get the current resourceVersion to watch from (not the object's resourceVersion) + gottenObjList, err := o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).List(metav1.ListOptions{FieldSelector: nameSelector}) if apierrors.IsNotFound(err) { return info.Object, true, nil } @@ -263,6 +268,10 @@ func IsDeleted(info *resource.Info, o *WaitOptions) (runtime.Object, bool, error // TODO this could do something slightly fancier if we wish return info.Object, false, err } + if len(gottenObjList.Items) != 1 { + return info.Object, true, nil + } + gottenObj := &gottenObjList.Items[0] resourceLocation := ResourceLocation{ GroupResource: info.Mapping.Resource.GroupResource(), Namespace: gottenObj.GetNamespace(), @@ -275,8 +284,8 @@ func IsDeleted(info *resource.Info, o *WaitOptions) (runtime.Object, bool, error } watchOptions := metav1.ListOptions{} - watchOptions.FieldSelector = "metadata.name=" + info.Name - watchOptions.ResourceVersion = gottenObj.GetResourceVersion() + watchOptions.FieldSelector = nameSelector + watchOptions.ResourceVersion = gottenObjList.GetResourceVersion() objWatch, err := o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).Watch(watchOptions) if err != nil { return gottenObj, false, err @@ -343,14 +352,21 @@ func (w ConditionalWait) IsConditionMet(info *resource.Info, o *WaitOptions) (ru if len(info.Name) == 0 { return info.Object, false, fmt.Errorf("resource name must be provided") } + + nameSelector := fields.OneTermEqualSelector("metadata.name", info.Name).String() + + var gottenObj *unstructured.Unstructured + // List with a name field selector to get the current resourceVersion to watch from (not the object's resourceVersion) + gottenObjList, err := o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).List(metav1.ListOptions{FieldSelector: nameSelector}) + resourceVersion := "" - gottenObj, err := o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).Get(info.Name, metav1.GetOptions{}) switch { - case apierrors.IsNotFound(err): - resourceVersion = "0" case err != nil: return info.Object, false, err + case len(gottenObjList.Items) != 1: + resourceVersion = gottenObjList.GetResourceVersion() default: + gottenObj = &gottenObjList.Items[0] conditionMet, err := w.checkCondition(gottenObj) if conditionMet { return gottenObj, true, nil @@ -358,11 +374,11 @@ func (w ConditionalWait) IsConditionMet(info *resource.Info, o *WaitOptions) (ru if err != nil { return gottenObj, false, err } - resourceVersion = gottenObj.GetResourceVersion() + resourceVersion = gottenObjList.GetResourceVersion() } watchOptions := metav1.ListOptions{} - watchOptions.FieldSelector = "metadata.name=" + info.Name + watchOptions.FieldSelector = nameSelector watchOptions.ResourceVersion = resourceVersion objWatch, err := o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).Watch(watchOptions) if err != nil { diff --git a/pkg/kubectl/cmd/wait/wait_test.go b/pkg/kubectl/cmd/wait/wait_test.go index cea5e64f32..3f14f9d537 100644 --- a/pkg/kubectl/cmd/wait/wait_test.go +++ b/pkg/kubectl/cmd/wait/wait_test.go @@ -41,6 +41,14 @@ import ( clienttesting "k8s.io/client-go/testing" ) +func newUnstructuredList(items ...*unstructured.Unstructured) *unstructured.UnstructuredList { + list := &unstructured.UnstructuredList{} + for i := range items { + list.Items = append(list.Items, *items[i]) + } + return list +} + func newUnstructured(apiVersion, kind, namespace, name string) *unstructured.Unstructured { return &unstructured.Unstructured{ Object: map[string]interface{}{ @@ -108,7 +116,7 @@ func TestWaitForDeletion(t *testing.T) { if len(actions) != 1 { t.Fatal(spew.Sdump(actions)) } - if !actions[0].Matches("get", "theresource") || actions[0].(clienttesting.GetAction).GetName() != "name-foo" { + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { t.Error(spew.Sdump(actions)) } }, @@ -141,8 +149,8 @@ func TestWaitForDeletion(t *testing.T) { }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) - fakeClient.PrependReactor("get", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { - return true, newUnstructured("group/version", "TheKind", "ns-foo", "name-foo"), nil + fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + return true, newUnstructuredList(newUnstructured("group/version", "TheKind", "ns-foo", "name-foo")), nil }) count := 0 fakeClient.PrependWatchReactor("theresource", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { @@ -170,7 +178,7 @@ func TestWaitForDeletion(t *testing.T) { if len(actions) != 1 { t.Fatal(spew.Sdump(actions)) } - if !actions[0].Matches("get", "theresource") || actions[0].(clienttesting.GetAction).GetName() != "name-foo" { + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { t.Error(spew.Sdump(actions)) } }, @@ -188,8 +196,8 @@ func TestWaitForDeletion(t *testing.T) { }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) - fakeClient.PrependReactor("get", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { - return true, newUnstructured("group/version", "TheKind", "ns-foo", "name-foo"), nil + fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + return true, newUnstructuredList(newUnstructured("group/version", "TheKind", "ns-foo", "name-foo")), nil }) return fakeClient }, @@ -200,7 +208,7 @@ func TestWaitForDeletion(t *testing.T) { if len(actions) != 2 { t.Fatal(spew.Sdump(actions)) } - if !actions[0].Matches("get", "theresource") || actions[0].(clienttesting.GetAction).GetName() != "name-foo" { + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { t.Error(spew.Sdump(actions)) } if !actions[1].Matches("watch", "theresource") { @@ -221,8 +229,12 @@ func TestWaitForDeletion(t *testing.T) { }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) - fakeClient.PrependReactor("get", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { - return true, newUnstructured("group/version", "TheKind", "ns-foo", "name-foo"), nil + fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + unstructuredObj := newUnstructured("group/version", "TheKind", "ns-foo", "name-foo") + unstructuredObj.SetResourceVersion("123") + unstructuredList := newUnstructuredList(unstructuredObj) + unstructuredList.SetResourceVersion("234") + return true, unstructuredList, nil }) count := 0 fakeClient.PrependWatchReactor("theresource", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { @@ -247,16 +259,16 @@ func TestWaitForDeletion(t *testing.T) { if len(actions) != 4 { t.Fatal(spew.Sdump(actions)) } - if !actions[0].Matches("get", "theresource") || actions[0].(clienttesting.GetAction).GetName() != "name-foo" { + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { t.Error(spew.Sdump(actions)) } - if !actions[1].Matches("watch", "theresource") { + if !actions[1].Matches("watch", "theresource") || actions[1].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { t.Error(spew.Sdump(actions)) } - if !actions[2].Matches("get", "theresource") || actions[2].(clienttesting.GetAction).GetName() != "name-foo" { + if !actions[2].Matches("list", "theresource") || actions[2].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { t.Error(spew.Sdump(actions)) } - if !actions[3].Matches("watch", "theresource") { + if !actions[3].Matches("watch", "theresource") || actions[3].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { t.Error(spew.Sdump(actions)) } }, @@ -274,8 +286,8 @@ func TestWaitForDeletion(t *testing.T) { }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) - fakeClient.PrependReactor("get", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { - return true, newUnstructured("group/version", "TheKind", "ns-foo", "name-foo"), nil + fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + return true, newUnstructuredList(newUnstructured("group/version", "TheKind", "ns-foo", "name-foo")), nil }) fakeClient.PrependWatchReactor("theresource", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { fakeWatch := watch.NewRaceFreeFake() @@ -290,7 +302,7 @@ func TestWaitForDeletion(t *testing.T) { if len(actions) != 2 { t.Fatal(spew.Sdump(actions)) } - if !actions[0].Matches("get", "theresource") || actions[0].(clienttesting.GetAction).GetName() != "name-foo" { + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { t.Error(spew.Sdump(actions)) } if !actions[1].Matches("watch", "theresource") { @@ -311,8 +323,8 @@ func TestWaitForDeletion(t *testing.T) { }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) - fakeClient.PrependReactor("get", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { - return true, newUnstructured("group/version", "TheKind", "ns-foo", "name-foo"), nil + fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + return true, newUnstructuredList(newUnstructured("group/version", "TheKind", "ns-foo", "name-foo")), nil }) count := 0 fakeClient.PrependWatchReactor("theresource", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { @@ -339,13 +351,13 @@ func TestWaitForDeletion(t *testing.T) { if len(actions) != 4 { t.Fatal(spew.Sdump(actions)) } - if !actions[0].Matches("get", "theresource") || actions[0].(clienttesting.GetAction).GetName() != "name-foo" { + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { t.Error(spew.Sdump(actions)) } if !actions[1].Matches("watch", "theresource") { t.Error(spew.Sdump(actions)) } - if !actions[2].Matches("get", "theresource") || actions[2].(clienttesting.GetAction).GetName() != "name-foo" { + if !actions[2].Matches("list", "theresource") || actions[2].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { t.Error(spew.Sdump(actions)) } if !actions[3].Matches("watch", "theresource") { @@ -411,11 +423,11 @@ func TestWaitForCondition(t *testing.T) { }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) - fakeClient.PrependReactor("get", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { - return true, addCondition( + fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + return true, newUnstructuredList(addCondition( newUnstructured("group/version", "TheKind", "ns-foo", "name-foo"), "the-condition", "status-value", - ), nil + )), nil }) return fakeClient }, @@ -425,7 +437,7 @@ func TestWaitForCondition(t *testing.T) { if len(actions) != 1 { t.Fatal(spew.Sdump(actions)) } - if !actions[0].Matches("get", "theresource") || actions[0].(clienttesting.GetAction).GetName() != "name-foo" { + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { t.Error(spew.Sdump(actions)) } }, @@ -480,7 +492,7 @@ func TestWaitForCondition(t *testing.T) { }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) - fakeClient.PrependReactor("get", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { return true, addCondition( newUnstructured("group/version", "TheKind", "ns-foo", "name-foo"), "some-other-condition", "status-value", @@ -495,7 +507,7 @@ func TestWaitForCondition(t *testing.T) { if len(actions) != 2 { t.Fatal(spew.Sdump(actions)) } - if !actions[0].Matches("get", "theresource") || actions[0].(clienttesting.GetAction).GetName() != "name-foo" { + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { t.Error(spew.Sdump(actions)) } if !actions[1].Matches("watch", "theresource") { @@ -516,8 +528,12 @@ func TestWaitForCondition(t *testing.T) { }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) - fakeClient.PrependReactor("get", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { - return true, newUnstructured("group/version", "TheKind", "ns-foo", "name-foo"), nil + fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + unstructuredObj := newUnstructured("group/version", "TheKind", "ns-foo", "name-foo") + unstructuredObj.SetResourceVersion("123") + unstructuredList := newUnstructuredList(unstructuredObj) + unstructuredList.SetResourceVersion("234") + return true, unstructuredList, nil }) count := 0 fakeClient.PrependWatchReactor("theresource", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { @@ -542,16 +558,16 @@ func TestWaitForCondition(t *testing.T) { if len(actions) != 4 { t.Fatal(spew.Sdump(actions)) } - if !actions[0].Matches("get", "theresource") || actions[0].(clienttesting.GetAction).GetName() != "name-foo" { + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { t.Error(spew.Sdump(actions)) } - if !actions[1].Matches("watch", "theresource") { + if !actions[1].Matches("watch", "theresource") || actions[1].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { t.Error(spew.Sdump(actions)) } - if !actions[2].Matches("get", "theresource") || actions[2].(clienttesting.GetAction).GetName() != "name-foo" { + if !actions[2].Matches("list", "theresource") || actions[2].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { t.Error(spew.Sdump(actions)) } - if !actions[3].Matches("watch", "theresource") { + if !actions[3].Matches("watch", "theresource") || actions[3].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { t.Error(spew.Sdump(actions)) } }, @@ -569,8 +585,8 @@ func TestWaitForCondition(t *testing.T) { }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) - fakeClient.PrependReactor("get", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { - return true, newUnstructured("group/version", "TheKind", "ns-foo", "name-foo"), nil + fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + return true, newUnstructuredList(newUnstructured("group/version", "TheKind", "ns-foo", "name-foo")), nil }) fakeClient.PrependWatchReactor("theresource", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { fakeWatch := watch.NewRaceFreeFake() @@ -588,7 +604,7 @@ func TestWaitForCondition(t *testing.T) { if len(actions) != 2 { t.Fatal(spew.Sdump(actions)) } - if !actions[0].Matches("get", "theresource") || actions[0].(clienttesting.GetAction).GetName() != "name-foo" { + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { t.Error(spew.Sdump(actions)) } if !actions[1].Matches("watch", "theresource") { @@ -625,7 +641,7 @@ func TestWaitForCondition(t *testing.T) { if len(actions) != 2 { t.Fatal(spew.Sdump(actions)) } - if !actions[0].Matches("get", "theresource") || actions[0].(clienttesting.GetAction).GetName() != "name-foo" { + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { t.Error(spew.Sdump(actions)) } if !actions[1].Matches("watch", "theresource") { @@ -646,8 +662,8 @@ func TestWaitForCondition(t *testing.T) { }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) - fakeClient.PrependReactor("get", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { - return true, newUnstructured("group/version", "TheKind", "ns-foo", "name-foo"), nil + fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + return true, newUnstructuredList(newUnstructured("group/version", "TheKind", "ns-foo", "name-foo")), nil }) count := 0 fakeClient.PrependWatchReactor("theresource", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { @@ -677,13 +693,13 @@ func TestWaitForCondition(t *testing.T) { if len(actions) != 4 { t.Fatal(spew.Sdump(actions)) } - if !actions[0].Matches("get", "theresource") || actions[0].(clienttesting.GetAction).GetName() != "name-foo" { + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { t.Error(spew.Sdump(actions)) } if !actions[1].Matches("watch", "theresource") { t.Error(spew.Sdump(actions)) } - if !actions[2].Matches("get", "theresource") || actions[2].(clienttesting.GetAction).GetName() != "name-foo" { + if !actions[2].Matches("list", "theresource") || actions[2].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { t.Error(spew.Sdump(actions)) } if !actions[3].Matches("watch", "theresource") { diff --git a/staging/src/k8s.io/client-go/dynamic/fake/simple.go b/staging/src/k8s.io/client-go/dynamic/fake/simple.go index eec3cfbc92..6ce3b9b415 100644 --- a/staging/src/k8s.io/client-go/dynamic/fake/simple.go +++ b/staging/src/k8s.io/client-go/dynamic/fake/simple.go @@ -303,6 +303,7 @@ func (c *dynamicResourceClient) List(opts metav1.ListOptions) (*unstructured.Uns } list := &unstructured.UnstructuredList{} + list.SetResourceVersion(entireList.GetResourceVersion()) for i := range entireList.Items { item := &entireList.Items[i] metadata, err := meta.Accessor(item)