diff --git a/staging/src/k8s.io/kubectl/go.mod b/staging/src/k8s.io/kubectl/go.mod index 2dc3da6a8d7..643a28d9256 100644 --- a/staging/src/k8s.io/kubectl/go.mod +++ b/staging/src/k8s.io/kubectl/go.mod @@ -7,7 +7,6 @@ go 1.18 require ( github.com/MakeNowJust/heredoc v1.0.0 github.com/chai2010/gettext-go v1.0.2 - github.com/davecgh/go-spew v1.1.1 github.com/daviddengcn/go-colortext v1.0.0 github.com/docker/distribution v2.8.1+incompatible github.com/evanphx/json-patch v4.12.0+incompatible @@ -50,6 +49,7 @@ require ( github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect github.com/PuerkitoBio/purell v1.1.1 // indirect github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect github.com/emicklei/go-restful/v3 v3.8.0 // indirect github.com/go-errors/errors v1.0.1 // indirect github.com/go-logr/logr v1.2.3 // indirect 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 83ca3fc910e..175cf442487 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait.go @@ -40,11 +40,13 @@ 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" ) @@ -317,70 +319,86 @@ 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) { - endTime := time.Now().Add(o.Timeout) - for { - if len(info.Name) == 0 { - return info.Object, false, fmt.Errorf("resource name must be provided") - } + if len(info.Name) == 0 { + return info.Object, false, fmt.Errorf("resource name must be provided") + } - 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(context.TODO(), metav1.ListOptions{FieldSelector: nameSelector}) - if apierrors.IsNotFound(err) { - return info.Object, true, nil - } - if err != nil { - // 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(), - Name: gottenObj.GetName(), - } - if uid, ok := o.UIDMap[resourceLocation]; ok { - if gottenObj.GetUID() != uid { - return gottenObj, true, 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 - } - - 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, 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 + 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 + } + 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() + + 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 { + // since we're looking for it to disappear we just return here if it no longer exists + return true, nil + } + + return false, nil + } + + 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 { + return gottenObj, false, errWaitTimeoutWithName + } + return gottenObj, false, err + } + + return gottenObj, true, nil } // Wait has helper methods for handling watches, including error handling. @@ -410,68 +428,85 @@ 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) - for { - if len(info.Name) == 0 { - return info.Object, false, fmt.Errorf("resource name must be provided") + 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 } - - 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() + if gottenObj == nil { + return nil, false, fmt.Errorf("condition not met for %s", info.ObjectName()) } - - watchOptions := metav1.ListOptions{} - watchOptions.FieldSelector = nameSelector - watchOptions.ResourceVersion = resourceVersion - objWatch, err := o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).Watch(context.TODO(), watchOptions) + conditionCheck, err := check(gottenObj) if err != nil { return gottenObj, false, err } - - 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 + if conditionCheck == false { + return gottenObj, false, fmt.Errorf("condition not met for %s", info.ObjectName()) } + 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 94e4a24fe80..1f3fdeedb2a 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 @@ -22,7 +22,6 @@ import ( "testing" "time" - "github.com/davecgh/go-spew/spew" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/api/meta" @@ -253,8 +252,7 @@ func TestWaitForDeletion(t *testing.T) { timeout time.Duration uidMap UIDMap - expectedErr string - validateActions func(t *testing.T, actions []clienttesting.Action) + expectedErr string }{ { name: "missing on get", @@ -271,15 +269,6 @@ func TestWaitForDeletion(t *testing.T) { return dynamicfakeclient.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping) }, timeout: 10 * time.Second, - - validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 1 { - t.Fatal(spew.Sdump(actions)) - } - if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { - t.Error(spew.Sdump(actions)) - } - }, }, { name: "handles no infos", @@ -289,12 +278,6 @@ func TestWaitForDeletion(t *testing.T) { }, timeout: 10 * time.Second, expectedErr: errNoMatchingResources.Error(), - - validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 0 { - t.Fatal(spew.Sdump(actions)) - } - }, }, { name: "uid conflict on get", @@ -318,7 +301,7 @@ func TestWaitForDeletion(t *testing.T) { count++ fakeWatch := watch.NewRaceFreeFake() go func() { - time.Sleep(100 * time.Millisecond) + time.Sleep(1 * time.Second) fakeWatch.Stop() }() return true, fakeWatch, nil @@ -333,15 +316,6 @@ func TestWaitForDeletion(t *testing.T) { ResourceLocation{Namespace: "ns-foo", Name: "name-foo"}: types.UID("some-UID-value"), ResourceLocation{GroupResource: schema.GroupResource{Group: "group", Resource: "theresource"}, Namespace: "ns-foo", Name: "name-foo"}: types.UID("some-nonmatching-UID-value"), }, - - validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 1 { - t.Fatal(spew.Sdump(actions)) - } - if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { - t.Error(spew.Sdump(actions)) - } - }, }, { name: "times out", @@ -356,6 +330,9 @@ 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 }) @@ -364,17 +341,56 @@ func TestWaitForDeletion(t *testing.T) { timeout: 1 * time.Second, 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)) - } - 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)) - } + }, + { + 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", + }, + { + 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: "", }, { name: "handles watch close out", @@ -389,6 +405,13 @@ 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") @@ -402,7 +425,7 @@ func TestWaitForDeletion(t *testing.T) { count++ fakeWatch := watch.NewRaceFreeFake() go func() { - time.Sleep(100 * time.Millisecond) + time.Sleep(1 * time.Second) fakeWatch.Stop() }() return true, fakeWatch, nil @@ -415,23 +438,6 @@ func TestWaitForDeletion(t *testing.T) { timeout: 3 * time.Second, expectedErr: "timed out waiting for the condition on theresource/name-foo", - validateActions: func(t *testing.T, actions []clienttesting.Action) { - 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" { - t.Error(spew.Sdump(actions)) - } - if !actions[1].Matches("watch", "theresource") || actions[1].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { - 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") || actions[3].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { - t.Error(spew.Sdump(actions)) - } - }, }, { name: "handles watch delete", @@ -457,18 +463,6 @@ func TestWaitForDeletion(t *testing.T) { return fakeClient }, timeout: 10 * time.Second, - - validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 2 { - t.Fatal(spew.Sdump(actions)) - } - 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)) - } - }, }, { name: "handles watch delete multiple", @@ -509,18 +503,6 @@ func TestWaitForDeletion(t *testing.T) { return fakeClient }, timeout: 10 * time.Second, - - validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 2 { - t.Fatal(spew.Sdump(actions)) - } - if !actions[0].Matches("list", "theresource-1") { - t.Error(spew.Sdump(actions)) - } - if !actions[1].Matches("list", "theresource-2") { - t.Error(spew.Sdump(actions)) - } - }, }, { name: "ignores watch error", @@ -558,24 +540,6 @@ func TestWaitForDeletion(t *testing.T) { return fakeClient }, timeout: 10 * time.Second, - - validateActions: func(t *testing.T, actions []clienttesting.Action) { - 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" { - 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)) - } - }, }, } @@ -604,8 +568,6 @@ func TestWaitForDeletion(t *testing.T) { t.Fatalf("expected %q, got %q", test.expectedErr, err.Error()) } } - - test.validateActions(t, fakeClient.Actions()) }) } } @@ -622,8 +584,7 @@ func TestWaitForCondition(t *testing.T) { fakeClient func() *dynamicfakeclient.FakeDynamicClient timeout time.Duration - expectedErr string - validateActions func(t *testing.T, actions []clienttesting.Action) + expectedErr string }{ { name: "present on get", @@ -647,15 +608,6 @@ func TestWaitForCondition(t *testing.T) { return fakeClient }, timeout: 10 * time.Second, - - validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 1 { - t.Fatal(spew.Sdump(actions)) - } - if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=name-foo" { - t.Error(spew.Sdump(actions)) - } - }, }, { name: "handles no infos", @@ -665,12 +617,6 @@ func TestWaitForCondition(t *testing.T) { }, timeout: 10 * time.Second, expectedErr: errNoMatchingResources.Error(), - - validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 0 { - t.Fatal(spew.Sdump(actions)) - } - }, }, { name: "handles empty object name", @@ -687,12 +633,6 @@ func TestWaitForCondition(t *testing.T) { }, timeout: 10 * time.Second, expectedErr: "resource name must be provided", - - validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 0 { - t.Fatal(spew.Sdump(actions)) - } - }, }, { name: "times out", @@ -717,18 +657,57 @@ func TestWaitForCondition(t *testing.T) { }, timeout: 1 * time.Second, - 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)) - } - 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)) - } + expectedErr: `theresource.group "name-foo" not found`, + }, + { + 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", + }, + { + 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", }, { name: "handles watch close out", @@ -756,7 +735,7 @@ func TestWaitForCondition(t *testing.T) { count++ fakeWatch := watch.NewRaceFreeFake() go func() { - time.Sleep(100 * time.Millisecond) + time.Sleep(1 * time.Second) fakeWatch.Stop() }() return true, fakeWatch, nil @@ -769,23 +748,6 @@ func TestWaitForCondition(t *testing.T) { timeout: 3 * time.Second, expectedErr: "timed out waiting for the condition on theresource/name-foo", - validateActions: func(t *testing.T, actions []clienttesting.Action) { - 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" { - t.Error(spew.Sdump(actions)) - } - if !actions[1].Matches("watch", "theresource") || actions[1].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { - 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") || actions[3].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { - t.Error(spew.Sdump(actions)) - } - }, }, { name: "handles watch condition change", @@ -814,18 +776,6 @@ func TestWaitForCondition(t *testing.T) { return fakeClient }, timeout: 10 * time.Second, - - validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 2 { - t.Fatal(spew.Sdump(actions)) - } - 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)) - } - }, }, { name: "handles watch created", @@ -840,29 +790,15 @@ func TestWaitForCondition(t *testing.T) { }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping) - fakeClient.PrependWatchReactor("theresource", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { - fakeWatch := watch.NewRaceFreeFake() - fakeWatch.Action(watch.Added, 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", - )) - return true, fakeWatch, nil + )), nil }) return fakeClient }, - timeout: 10 * time.Second, - - validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 2 { - t.Fatal(spew.Sdump(actions)) - } - 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)) - } - }, + timeout: 1 * time.Second, }, { name: "ignores watch error", @@ -903,24 +839,6 @@ func TestWaitForCondition(t *testing.T) { return fakeClient }, timeout: 10 * time.Second, - - validateActions: func(t *testing.T, actions []clienttesting.Action) { - 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" { - 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)) - } - }, }, { name: "times out due to stale .status.conditions[0].observedGeneration", @@ -945,18 +863,7 @@ func TestWaitForCondition(t *testing.T) { }, timeout: 1 * time.Second, - 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)) - } - 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)) - } - }, + expectedErr: `theresource.group "name-foo" not found`, }, { name: "handles watch .status.conditions[0].observedGeneration change", @@ -985,18 +892,6 @@ func TestWaitForCondition(t *testing.T) { return fakeClient }, timeout: 10 * time.Second, - - validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 2 { - t.Fatal(spew.Sdump(actions)) - } - 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)) - } - }, }, { name: "times out due to stale .status.observedGeneration", @@ -1022,18 +917,7 @@ func TestWaitForCondition(t *testing.T) { }, timeout: 1 * time.Second, - 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)) - } - 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)) - } - }, + expectedErr: `theresource.group "name-foo" not found`, }, { name: "handles watch .status.observedGeneration change", @@ -1067,18 +951,6 @@ func TestWaitForCondition(t *testing.T) { return fakeClient }, timeout: 10 * time.Second, - - validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 2 { - t.Fatal(spew.Sdump(actions)) - } - 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)) - } - }, }, } @@ -1106,8 +978,6 @@ func TestWaitForCondition(t *testing.T) { t.Fatalf("expected %q, got %q", test.expectedErr, err.Error()) } } - - test.validateActions(t, fakeClient.Actions()) }) } } @@ -1305,7 +1175,7 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) { o := &WaitOptions{ ResourceFinder: genericclioptions.NewSimpleFakeResourceFinder(infos...), DynamicClient: fakeClient, - Timeout: 1 * time.Millisecond, + Timeout: 1 * time.Second, Printer: printers.NewDiscardingPrinter(), ConditionFn: JSONPathWait{ @@ -1348,8 +1218,7 @@ func TestWaitForJSONPathCondition(t *testing.T) { jsonPathExp string jsonPathCond string - expectedErr string - validateActions func(t *testing.T, actions []clienttesting.Action) + expectedErr string }{ { name: "present on get", @@ -1375,14 +1244,6 @@ func TestWaitForJSONPathCondition(t *testing.T) { jsonPathCond: "foo-b6699dcfb-rnv7t", expectedErr: None, - validateActions: func(t *testing.T, actions []clienttesting.Action) { - 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" { - t.Error(spew.Sdump(actions)) - } - }, }, { name: "handles no infos", @@ -1392,12 +1253,6 @@ func TestWaitForJSONPathCondition(t *testing.T) { }, timeout: 10 * time.Second, expectedErr: errNoMatchingResources.Error(), - - validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 0 { - t.Fatal(spew.Sdump(actions)) - } - }, }, { name: "handles empty object name", @@ -1415,11 +1270,6 @@ func TestWaitForJSONPathCondition(t *testing.T) { timeout: 10 * time.Second, expectedErr: "resource name must be provided", - validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 0 { - t.Fatal(spew.Sdump(actions)) - } - }, }, { name: "times out", @@ -1441,18 +1291,7 @@ func TestWaitForJSONPathCondition(t *testing.T) { }, timeout: 1 * time.Second, - 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)) - } - if !actions[0].Matches("list", "theresource") || actions[0].(clienttesting.ListAction).GetListRestrictions().Fields.String() != "metadata.name=foo-b6699dcfb-rnv7t" { - t.Error(spew.Sdump(actions)) - } - if !actions[1].Matches("watch", "theresource") { - t.Error(spew.Sdump(actions)) - } - }, + expectedErr: `theresource.group "foo-b6699dcfb-rnv7t" not found`, }, { name: "handles watch close out", @@ -1480,7 +1319,7 @@ func TestWaitForJSONPathCondition(t *testing.T) { count++ fakeWatch := watch.NewRaceFreeFake() go func() { - time.Sleep(100 * time.Millisecond) + time.Sleep(1 * time.Second) fakeWatch.Stop() }() return true, fakeWatch, nil @@ -1495,23 +1334,6 @@ func TestWaitForJSONPathCondition(t *testing.T) { jsonPathCond: "foo", // use incorrect name so it'll keep waiting expectedErr: "timed out waiting for the condition on theresource/foo-b6699dcfb-rnv7t", - validateActions: func(t *testing.T, actions []clienttesting.Action) { - 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)) - } - if !actions[1].Matches("watch", "theresource") || actions[1].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { - 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") || actions[3].(clienttesting.WatchAction).GetWatchRestrictions().ResourceVersion != "234" { - t.Error(spew.Sdump(actions)) - } - }, }, { name: "handles watch condition change", @@ -1529,12 +1351,11 @@ func TestWaitForJSONPathCondition(t *testing.T) { fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { unstructuredObj := createUnstructured(t, podYAML) unstructuredObj.SetName("foo") - return true, newUnstructuredList(), nil + return true, newUnstructuredList(unstructuredObj), nil }) - fakeClient.PrependWatchReactor("theresource", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { - fakeWatch := watch.NewRaceFreeFake() - fakeWatch.Action(watch.Modified, createUnstructured(t, podYAML)) - return true, fakeWatch, nil + fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + unstructuredObj := createUnstructured(t, podYAML) + return true, newUnstructuredList(unstructuredObj), nil }) return fakeClient }, @@ -1543,17 +1364,6 @@ func TestWaitForJSONPathCondition(t *testing.T) { jsonPathCond: "foo-b6699dcfb-rnv7t", expectedErr: None, - validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 2 { - 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)) - } - if !actions[1].Matches("watch", "theresource") { - t.Error(spew.Sdump(actions)) - } - }, }, { name: "handles watch created", @@ -1568,29 +1378,17 @@ func TestWaitForJSONPathCondition(t *testing.T) { }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping) - fakeClient.PrependWatchReactor("theresource", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { - fakeWatch := watch.NewRaceFreeFake() - fakeWatch.Action(watch.Added, createUnstructured(t, podYAML)) - return true, fakeWatch, nil + fakeClient.PrependReactor("list", "theresource", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + return true, newUnstructuredList( + createUnstructured(t, podYAML)), nil }) return fakeClient }, - timeout: 10 * time.Second, + timeout: 1 * time.Second, jsonPathExp: "{.spec.containers[0].image}", jsonPathCond: "nginx", expectedErr: None, - validateActions: func(t *testing.T, actions []clienttesting.Action) { - if len(actions) != 2 { - 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)) - } - if !actions[1].Matches("watch", "theresource") { - t.Error(spew.Sdump(actions)) - } - }, }, { name: "ignores watch error", @@ -1606,7 +1404,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", "ns-foo", "name-foo")), nil + return true, newUnstructuredList(newUnstructured("group/version", "TheKind", "default", "foo-b6699dcfb-rnv7t")), nil }) count := 0 fakeClient.PrependWatchReactor("theresource", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { @@ -1632,23 +1430,6 @@ func TestWaitForJSONPathCondition(t *testing.T) { jsonPathCond: "foo-b6699dcfb-rnv7t", expectedErr: None, - validateActions: func(t *testing.T, actions []clienttesting.Action) { - 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)) - } - 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)) - } - }, }, } for _, test := range tests { @@ -1680,8 +1461,6 @@ func TestWaitForJSONPathCondition(t *testing.T) { t.Fatalf("expected %q, got %q", test.expectedErr, err.Error()) } } - - test.validateActions(t, fakeClient.Actions()) }) } }