From 5badb731a895dfa0a188a3f4aec2d3edfe709b12 Mon Sep 17 00:00:00 2001 From: Marcus Puckett <13648427+mpuckett159@users.noreply.github.com> Date: Mon, 18 Jul 2022 08:48:29 -0700 Subject: [PATCH] Revert "Move kubectl wait to informers with a cache to avoid hanging due to objects disappearing from the cluster" --- .../src/k8s.io/kubectl/pkg/cmd/wait/wait.go | 247 ++++++++--------- .../k8s.io/kubectl/pkg/cmd/wait/wait_test.go | 256 +++++------------- 2 files changed, 176 insertions(+), 327 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait.go b/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait.go index 175cf442487..83ca3fc910e 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait.go @@ -40,13 +40,11 @@ import ( "k8s.io/cli-runtime/pkg/printers" "k8s.io/cli-runtime/pkg/resource" "k8s.io/client-go/dynamic" - "k8s.io/client-go/tools/cache" watchtools "k8s.io/client-go/tools/watch" "k8s.io/client-go/util/jsonpath" cmdget "k8s.io/kubectl/pkg/cmd/get" cmdutil "k8s.io/kubectl/pkg/cmd/util" "k8s.io/kubectl/pkg/util/i18n" - "k8s.io/kubectl/pkg/util/interrupt" "k8s.io/kubectl/pkg/util/templates" ) @@ -319,86 +317,70 @@ func (o *WaitOptions) RunWait() error { // IsDeleted is a condition func for waiting for something to be deleted 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, initObjGetErr := o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).Get(context.Background(), info.Name, metav1.GetOptions{}) - if apierrors.IsNotFound(initObjGetErr) { - return info.Object, true, nil - } - if initObjGetErr != nil { - // TODO this could do something slightly fancier if we wish - return info.Object, false, initObjGetErr - } - resourceLocation := ResourceLocation{ - GroupResource: info.Mapping.Resource.GroupResource(), - Namespace: gottenObj.GetNamespace(), - Name: gottenObj.GetName(), - } - if uid, ok := o.UIDMap[resourceLocation]; ok { - if gottenObj.GetUID() != uid { - return gottenObj, true, nil - } - } - endTime := time.Now().Add(o.Timeout) - timeout := time.Until(endTime) - errWaitTimeoutWithName := extendErrWaitTimeout(wait.ErrWaitTimeout, info) - if o.Timeout == 0 { - // If timeout is zero check if the object exists once only - if gottenObj == nil { - return nil, true, nil + for { + if len(info.Name) == 0 { + return info.Object, false, fmt.Errorf("resource name must be provided") } - return gottenObj, false, fmt.Errorf("condition not met for %s", info.ObjectName()) - } - if timeout < 0 { - // we're out of time - return info.Object, false, errWaitTimeoutWithName - } - ctx, cancel := watchtools.ContextWithOptionalTimeout(context.Background(), o.Timeout) - defer cancel() + nameSelector := fields.OneTermEqualSelector("metadata.name", info.Name).String() - fieldSelector := fields.OneTermEqualSelector("metadata.name", info.Name).String() - lw := &cache.ListWatch{ - ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { - options.FieldSelector = fieldSelector - return o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).List(context.TODO(), options) - }, - WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { - options.FieldSelector = fieldSelector - return o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).Watch(context.TODO(), options) - }, - } - - // this function is used to refresh the cache to prevent timeout waits on resources that have disappeared - preconditionFunc := func(store cache.Store) (bool, error) { - _, exists, err := store.Get(&metav1.ObjectMeta{Namespace: info.Namespace, Name: info.Name}) + // 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(context.TODO(), metav1.ListOptions{FieldSelector: nameSelector}) + if apierrors.IsNotFound(err) { + return info.Object, true, nil + } if err != nil { - return true, err + // TODO this could do something slightly fancier if we wish + return info.Object, false, err } - if !exists { - // since we're looking for it to disappear we just return here if it no longer exists - return true, nil + if len(gottenObjList.Items) != 1 { + return info.Object, true, nil + } + gottenObj := &gottenObjList.Items[0] + resourceLocation := ResourceLocation{ + GroupResource: info.Mapping.Resource.GroupResource(), + Namespace: gottenObj.GetNamespace(), + Name: gottenObj.GetName(), + } + if uid, ok := o.UIDMap[resourceLocation]; ok { + if gottenObj.GetUID() != uid { + return gottenObj, true, nil + } } - return false, nil - } + watchOptions := metav1.ListOptions{} + watchOptions.FieldSelector = nameSelector + watchOptions.ResourceVersion = gottenObjList.GetResourceVersion() + objWatch, err := o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).Watch(context.TODO(), watchOptions) + if err != nil { + return gottenObj, false, err + } - intr := interrupt.New(nil, cancel) - err := intr.Run(func() error { - _, err := watchtools.UntilWithSync(ctx, lw, &unstructured.Unstructured{}, preconditionFunc, Wait{errOut: o.ErrOut}.IsDeleted) - return err - }) - if err != nil { - if err == wait.ErrWaitTimeout { + timeout := endTime.Sub(time.Now()) + errWaitTimeoutWithName := extendErrWaitTimeout(wait.ErrWaitTimeout, info) + if timeout < 0 { + // we're out of time return gottenObj, false, errWaitTimeoutWithName } - return gottenObj, false, err - } - return gottenObj, true, nil + ctx, cancel := watchtools.ContextWithOptionalTimeout(context.Background(), o.Timeout) + watchEvent, err := watchtools.UntilWithoutRetry(ctx, objWatch, Wait{errOut: o.ErrOut}.IsDeleted) + cancel() + switch { + case err == nil: + return watchEvent.Object, true, nil + case err == watchtools.ErrWatchClosed: + continue + case err == wait.ErrWaitTimeout: + if watchEvent != nil { + return watchEvent.Object, false, errWaitTimeoutWithName + } + return gottenObj, false, errWaitTimeoutWithName + default: + return gottenObj, false, err + } + } } // Wait has helper methods for handling watches, including error handling. @@ -428,85 +410,68 @@ type checkCondFunc func(obj *unstructured.Unstructured) (bool, error) // getObjAndCheckCondition will make a List query to the API server to get the object and check if the condition is met using check function. // If the condition is not met, it will make a Watch query to the server and pass in the condMet function func getObjAndCheckCondition(info *resource.Info, o *WaitOptions, condMet isCondMetFunc, check checkCondFunc) (runtime.Object, bool, error) { - if len(info.Name) == 0 { - return info.Object, false, fmt.Errorf("resource name must be provided") - } - endTime := time.Now().Add(o.Timeout) - timeout := time.Until(endTime) - errWaitTimeoutWithName := extendErrWaitTimeout(wait.ErrWaitTimeout, info) - if o.Timeout == 0 { - // If timeout is zero we will fetch the object(s) once only and check - gottenObj, initObjGetErr := o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).Get(context.Background(), info.Name, metav1.GetOptions{}) - if initObjGetErr != nil { - return nil, false, initObjGetErr + for { + if len(info.Name) == 0 { + return info.Object, false, fmt.Errorf("resource name must be provided") } - if gottenObj == nil { - return nil, false, fmt.Errorf("condition not met for %s", info.ObjectName()) + + 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(context.TODO(), metav1.ListOptions{FieldSelector: nameSelector}) + + resourceVersion := "" + switch { + case err != nil: + return info.Object, false, err + case len(gottenObjList.Items) != 1: + resourceVersion = gottenObjList.GetResourceVersion() + default: + gottenObj = &gottenObjList.Items[0] + conditionMet, err := check(gottenObj) + if conditionMet { + return gottenObj, true, nil + } + if err != nil { + return gottenObj, false, err + } + resourceVersion = gottenObjList.GetResourceVersion() } - conditionCheck, err := check(gottenObj) + + watchOptions := metav1.ListOptions{} + watchOptions.FieldSelector = nameSelector + watchOptions.ResourceVersion = resourceVersion + objWatch, err := o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).Watch(context.TODO(), watchOptions) if err != nil { return gottenObj, false, err } - if conditionCheck == false { - return gottenObj, false, fmt.Errorf("condition not met for %s", info.ObjectName()) + + timeout := endTime.Sub(time.Now()) + errWaitTimeoutWithName := extendErrWaitTimeout(wait.ErrWaitTimeout, info) + if timeout < 0 { + // we're out of time + return gottenObj, false, errWaitTimeoutWithName + } + + ctx, cancel := watchtools.ContextWithOptionalTimeout(context.Background(), o.Timeout) + watchEvent, err := watchtools.UntilWithoutRetry(ctx, objWatch, watchtools.ConditionFunc(condMet)) + cancel() + switch { + case err == nil: + return watchEvent.Object, true, nil + case err == watchtools.ErrWatchClosed: + continue + case err == wait.ErrWaitTimeout: + if watchEvent != nil { + return watchEvent.Object, false, errWaitTimeoutWithName + } + return gottenObj, false, errWaitTimeoutWithName + default: + return gottenObj, false, err } - return gottenObj, true, nil } - if timeout < 0 { - // we're out of time - return info.Object, false, errWaitTimeoutWithName - } - - ctx, cancel := watchtools.ContextWithOptionalTimeout(context.Background(), o.Timeout) - defer cancel() - - mapping := info.ResourceMapping() // used to pass back meaningful errors if object disappears - fieldSelector := fields.OneTermEqualSelector("metadata.name", info.Name).String() - lw := &cache.ListWatch{ - ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { - options.FieldSelector = fieldSelector - return o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).List(context.TODO(), options) - }, - WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { - options.FieldSelector = fieldSelector - return o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).Watch(context.TODO(), options) - }, - } - - // this function is used to refresh the cache to prevent timeout waits on resources that have disappeared - preconditionFunc := func(store cache.Store) (bool, error) { - _, exists, err := store.Get(&metav1.ObjectMeta{Namespace: info.Namespace, Name: info.Name}) - if err != nil { - return true, err - } - if !exists { - return true, apierrors.NewNotFound(mapping.Resource.GroupResource(), info.Name) - } - - return false, nil - } - - var result runtime.Object - intr := interrupt.New(nil, cancel) - err := intr.Run(func() error { - ev, err := watchtools.UntilWithSync(ctx, lw, &unstructured.Unstructured{}, preconditionFunc, watchtools.ConditionFunc(condMet)) - if ev != nil { - result = ev.Object - } - if err == context.DeadlineExceeded { - return errWaitTimeoutWithName - } - return err - }) - if err != nil { - if err == wait.ErrWaitTimeout { - return result, false, errWaitTimeoutWithName - } - return result, false, err - } - - return result, true, nil } // ConditionalWait hold information to check an API status condition diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait_test.go index 3bb28e09683..94e4a24fe80 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait_test.go @@ -276,9 +276,8 @@ 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" { - t.Error(spew.Sdump(actions[0])) + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { + t.Error(spew.Sdump(actions)) } }, }, @@ -319,7 +318,7 @@ func TestWaitForDeletion(t *testing.T) { count++ fakeWatch := watch.NewRaceFreeFake() go func() { - time.Sleep(1 * time.Second) + time.Sleep(100 * time.Millisecond) fakeWatch.Stop() }() return true, fakeWatch, nil @@ -339,7 +338,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)) } }, @@ -357,9 +356,6 @@ func TestWaitForDeletion(t *testing.T) { }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping) - fakeClient.PrependReactor("get", "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.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 }) @@ -369,72 +365,17 @@ func TestWaitForDeletion(t *testing.T) { expectedErr: "timed out waiting for the condition on theresource/name-foo", validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 3 { + 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("list", "theresource") || actions[1].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { - t.Error(spew.Sdump(actions)) - } - if !actions[2].Matches("watch", "theresource") || actions[2].(clienttesting.WatchAction).GetWatchRestrictions().Fields.String() != "metadata.name=name-foo" { + if !actions[1].Matches("watch", "theresource") { t.Error(spew.Sdump(actions)) } }, }, - { - name: "delete for existing resource with no timeout", - infos: []*resource.Info{ - { - Mapping: &meta.RESTMapping{ - Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, - }, - Name: "name-foo", - Namespace: "ns-foo", - }, - }, - fakeClient: func() *dynamicfakeclient.FakeDynamicClient { - fakeClient := dynamicfakeclient.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping) - fakeClient.PrependReactor("get", "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.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 - }, - timeout: 0 * time.Second, - - expectedErr: "condition not met for theresource/name-foo", - validateActions: nil, - }, - { - name: "delete for nonexisting resource with no timeout", - infos: []*resource.Info{ - { - Mapping: &meta.RESTMapping{ - Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "thenonexistentresource"}, - }, - Name: "name-foo", - Namespace: "ns-foo", - }, - }, - fakeClient: func() *dynamicfakeclient.FakeDynamicClient { - fakeClient := dynamicfakeclient.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping) - fakeClient.PrependReactor("get", "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.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 - }, - timeout: 0 * time.Second, - - expectedErr: "", - validateActions: nil, - }, { name: "handles watch close out", infos: []*resource.Info{ @@ -448,13 +389,6 @@ func TestWaitForDeletion(t *testing.T) { }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping) - fakeClient.PrependReactor("get", "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 - }) 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") @@ -468,7 +402,7 @@ func TestWaitForDeletion(t *testing.T) { count++ fakeWatch := watch.NewRaceFreeFake() go func() { - time.Sleep(1 * time.Second) + time.Sleep(100 * time.Millisecond) fakeWatch.Stop() }() return true, fakeWatch, nil @@ -485,17 +419,17 @@ 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" { - t.Error(spew.Sdump(actions[0])) + 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("list", "theresource") || actions[1].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { - t.Error(spew.Sdump(actions[1])) + if !actions[1].Matches("watch", "theresource") || actions[1].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { + t.Error(spew.Sdump(actions)) } - if !actions[2].Matches("watch", "theresource") || actions[2].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { - t.Error(spew.Sdump(actions[2])) + 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") || actions[3].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { - t.Error(spew.Sdump(actions[3])) + t.Error(spew.Sdump(actions)) } }, }, @@ -525,10 +459,13 @@ func TestWaitForDeletion(t *testing.T) { timeout: 10 * time.Second, validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 1 { + 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") { t.Error(spew.Sdump(actions)) } }, @@ -574,26 +511,14 @@ func TestWaitForDeletion(t *testing.T) { timeout: 10 * time.Second, validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 6 { + if len(actions) != 2 { t.Fatal(spew.Sdump(actions)) } - if !actions[0].Matches("get", "theresource-1") || actions[0].(clienttesting.GetAction).GetName() != "name-foo-1" { - t.Error(spew.Sdump(actions[0])) + if !actions[0].Matches("list", "theresource-1") { + t.Error(spew.Sdump(actions)) } - if !actions[1].Matches("list", "theresource-1") || actions[1].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo-1" { - t.Error(spew.Sdump(actions[1])) - } - if !actions[2].Matches("watch", "theresource-1") || actions[2].(clienttesting.WatchAction).GetWatchRestrictions().Fields.String() != "metadata.name=name-foo-1" { - t.Error(spew.Sdump(actions[2])) - } - if !actions[3].Matches("get", "theresource-2") || actions[3].(clienttesting.GetAction).GetName() != "name-foo-2" { - t.Error(spew.Sdump(actions[3])) - } - if !actions[4].Matches("list", "theresource-2") || actions[4].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo-2" { - t.Error(spew.Sdump(actions[4])) - } - if !actions[5].Matches("watch", "theresource-2") || actions[5].(clienttesting.WatchAction).GetWatchRestrictions().Fields.String() != "metadata.name=name-foo-2" { - t.Error(spew.Sdump(actions[5])) + if !actions[1].Matches("list", "theresource-2") { + t.Error(spew.Sdump(actions)) } }, }, @@ -635,10 +560,19 @@ func TestWaitForDeletion(t *testing.T) { timeout: 10 * time.Second, validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 1 { + 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("list", "theresource") || actions[2].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { + t.Error(spew.Sdump(actions)) + } + if !actions[3].Matches("watch", "theresource") { t.Error(spew.Sdump(actions)) } }, @@ -671,9 +605,7 @@ func TestWaitForDeletion(t *testing.T) { } } - if test.validateActions != nil { - test.validateActions(t, fakeClient.Actions()) - } + test.validateActions(t, fakeClient.Actions()) }) } } @@ -717,14 +649,10 @@ func TestWaitForCondition(t *testing.T) { timeout: 10 * time.Second, validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 2 { + if len(actions) != 1 { t.Fatal(spew.Sdump(actions)) } - if !actions[0].Matches("list", "theresource") { - t.Error(spew.Sdump(actions)) - } else if actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { - t.Error(spew.Sdump(actions)) - } else if actions[1].(clienttesting.WatchAction).GetWatchRestrictions().Fields.String() != "metadata.name=name-foo" { + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { t.Error(spew.Sdump(actions)) } }, @@ -789,7 +717,7 @@ func TestWaitForCondition(t *testing.T) { }, timeout: 1 * time.Second, - expectedErr: `theresource.group "name-foo" not found`, + expectedErr: "timed out waiting for the condition on theresource/name-foo", validateActions: func(t *testing.T, actions []clienttesting.Action) { if len(actions) != 2 { t.Fatal(spew.Sdump(actions)) @@ -802,58 +730,6 @@ func TestWaitForCondition(t *testing.T) { } }, }, - { - name: "for nonexisting resource with no timeout", - infos: []*resource.Info{ - { - Mapping: &meta.RESTMapping{ - Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "thenonexistingresource"}, - }, - Name: "name-foo", - Namespace: "ns-foo", - }, - }, - fakeClient: func() *dynamicfakeclient.FakeDynamicClient { - fakeClient := dynamicfakeclient.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping) - fakeClient.PrependReactor("get", "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.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 - }, - timeout: 0 * time.Second, - - expectedErr: "thenonexistingresource.group \"name-foo\" not found", - validateActions: nil, - }, - { - name: "for existing resource with no timeout", - infos: []*resource.Info{ - { - Mapping: &meta.RESTMapping{ - Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, - }, - Name: "name-foo", - Namespace: "ns-foo", - }, - }, - fakeClient: func() *dynamicfakeclient.FakeDynamicClient { - fakeClient := dynamicfakeclient.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping) - fakeClient.PrependReactor("get", "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.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 - }, - timeout: 0 * time.Second, - - expectedErr: "condition not met for theresource/name-foo", - validateActions: nil, - }, { name: "handles watch close out", infos: []*resource.Info{ @@ -880,7 +756,7 @@ func TestWaitForCondition(t *testing.T) { count++ fakeWatch := watch.NewRaceFreeFake() go func() { - time.Sleep(1 * time.Second) + time.Sleep(100 * time.Millisecond) fakeWatch.Stop() }() return true, fakeWatch, nil @@ -894,7 +770,7 @@ func TestWaitForCondition(t *testing.T) { expectedErr: "timed out waiting for the condition on theresource/name-foo", validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 3 { + if len(actions) != 4 { t.Fatal(spew.Sdump(actions)) } if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { @@ -903,7 +779,10 @@ func TestWaitForCondition(t *testing.T) { if !actions[1].Matches("watch", "theresource") || actions[1].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { t.Error(spew.Sdump(actions)) } - if !actions[2].Matches("watch", "theresource") || actions[2].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { + 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") || actions[3].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { t.Error(spew.Sdump(actions)) } }, @@ -1066,7 +945,7 @@ func TestWaitForCondition(t *testing.T) { }, timeout: 1 * time.Second, - expectedErr: `theresource.group "name-foo" not found`, + expectedErr: "timed out waiting for the condition on theresource/name-foo", validateActions: func(t *testing.T, actions []clienttesting.Action) { if len(actions) != 2 { t.Fatal(spew.Sdump(actions)) @@ -1143,7 +1022,7 @@ func TestWaitForCondition(t *testing.T) { }, timeout: 1 * time.Second, - expectedErr: `theresource.group "name-foo" not found`, + expectedErr: "timed out waiting for the condition on theresource/name-foo", validateActions: func(t *testing.T, actions []clienttesting.Action) { if len(actions) != 2 { t.Fatal(spew.Sdump(actions)) @@ -1228,9 +1107,7 @@ func TestWaitForCondition(t *testing.T) { } } - if test.validateActions != nil { - test.validateActions(t, fakeClient.Actions()) - } + test.validateActions(t, fakeClient.Actions()) }) } } @@ -1428,7 +1305,7 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) { o := &WaitOptions{ ResourceFinder: genericclioptions.NewSimpleFakeResourceFinder(infos...), DynamicClient: fakeClient, - Timeout: 1 * time.Second, + Timeout: 1 * time.Millisecond, Printer: printers.NewDiscardingPrinter(), ConditionFn: JSONPathWait{ @@ -1499,12 +1376,10 @@ func TestWaitForJSONPathCondition(t *testing.T) { expectedErr: None, validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 2 { + if len(actions) != 1 { t.Fatal(spew.Sdump(actions)) } - if !actions[0].Matches("list", "theresource") || - actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=foo-b6699dcfb-rnv7t" && - actions[1].(clienttesting.WatchAction).GetWatchRestrictions().Fields.String() != "metadata.name=foo-b6699dcfb-rnv7t" { + if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=foo-b6699dcfb-rnv7t" { t.Error(spew.Sdump(actions)) } }, @@ -1566,7 +1441,7 @@ func TestWaitForJSONPathCondition(t *testing.T) { }, timeout: 1 * time.Second, - expectedErr: `theresource.group "foo-b6699dcfb-rnv7t" not found`, + expectedErr: "timed out waiting for the condition on theresource/foo-b6699dcfb-rnv7t", validateActions: func(t *testing.T, actions []clienttesting.Action) { if len(actions) != 2 { t.Fatal(spew.Sdump(actions)) @@ -1605,7 +1480,7 @@ func TestWaitForJSONPathCondition(t *testing.T) { count++ fakeWatch := watch.NewRaceFreeFake() go func() { - time.Sleep(1 * time.Second) + time.Sleep(100 * time.Millisecond) fakeWatch.Stop() }() return true, fakeWatch, nil @@ -1621,7 +1496,7 @@ func TestWaitForJSONPathCondition(t *testing.T) { expectedErr: "timed out waiting for the condition on theresource/foo-b6699dcfb-rnv7t", validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 3 { + if len(actions) != 4 { t.Fatal(spew.Sdump(actions)) } if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=foo-b6699dcfb-rnv7t" { @@ -1630,7 +1505,10 @@ func TestWaitForJSONPathCondition(t *testing.T) { if !actions[1].Matches("watch", "theresource") || actions[1].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { t.Error(spew.Sdump(actions)) } - if !actions[2].Matches("watch", "theresource") || actions[2].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { + if !actions[2].Matches("list", "theresource") || actions[2].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=foo-b6699dcfb-rnv7t" { + t.Error(spew.Sdump(actions)) + } + if !actions[3].Matches("watch", "theresource") || actions[3].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { t.Error(spew.Sdump(actions)) } }, @@ -1728,7 +1606,7 @@ func TestWaitForJSONPathCondition(t *testing.T) { fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping) fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { - return true, newUnstructuredList(newUnstructured("group/version", "TheKind", "default", "foo-b6699dcfb-rnv7t")), nil + 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) { @@ -1755,14 +1633,20 @@ func TestWaitForJSONPathCondition(t *testing.T) { expectedErr: None, validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 2 { + if len(actions) != 4 { t.Fatal(spew.Sdump(actions)) } if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=foo-b6699dcfb-rnv7t" { - t.Error(spew.Sdump(actions[0])) + t.Error(spew.Sdump(actions)) } - if !actions[1].Matches("watch", "theresource") || actions[1].(clienttesting.WatchAction).GetWatchRestrictions().Fields.String() != "metadata.name=foo-b6699dcfb-rnv7t" { - t.Error(spew.Sdump(actions[1])) + if !actions[1].Matches("watch", "theresource") { + t.Error(spew.Sdump(actions)) + } + if !actions[2].Matches("list", "theresource") || actions[2].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=foo-b6699dcfb-rnv7t" { + t.Error(spew.Sdump(actions)) + } + if !actions[3].Matches("watch", "theresource") { + t.Error(spew.Sdump(actions)) } }, },