From d3445d71d097f4f9fe8619af65269b6de735b74a Mon Sep 17 00:00:00 2001 From: Mikalai Radchuk Date: Thu, 26 Jul 2018 22:59:42 +0100 Subject: [PATCH] Return an error if there is no resources matching This makes `kubectl wait` print useful message when there is no resources matching a query. Also it will now exit with the exit status 1. --- pkg/kubectl/cmd/wait/wait.go | 21 +++- pkg/kubectl/cmd/wait/wait_test.go | 158 ++++++++++++++++++++---------- 2 files changed, 121 insertions(+), 58 deletions(-) diff --git a/pkg/kubectl/cmd/wait/wait.go b/pkg/kubectl/cmd/wait/wait.go index 9590c07e18..69cdb6c5e5 100644 --- a/pkg/kubectl/cmd/wait/wait.go +++ b/pkg/kubectl/cmd/wait/wait.go @@ -17,13 +17,14 @@ limitations under the License. package wait import ( + "errors" "fmt" "strings" "time" "github.com/spf13/cobra" - "k8s.io/apimachinery/pkg/api/errors" + 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/runtime" @@ -61,6 +62,9 @@ var ( kubectl wait --for=delete pod/busybox1 --timeout=60s`) ) +// errNoMatchingResources is returned when there is no resources matching a query. +var errNoMatchingResources = errors.New("no matching resources found") + // WaitFlags directly reflect the information that CLI is gathering via flags. They will be converted to Options, which // reflect the runtime requirements for the command. This structure reduces the transformation to wiring and makes // the logic itself easy to unit test @@ -206,11 +210,13 @@ type ConditionFunc func(info *resource.Info, o *WaitOptions) (finalObject runtim // RunWait runs the waiting logic func (o *WaitOptions) RunWait() error { - return o.ResourceFinder.Do().Visit(func(info *resource.Info, err error) error { + visitCount := 0 + err := o.ResourceFinder.Do().Visit(func(info *resource.Info, err error) error { if err != nil { return err } + visitCount++ finalObject, success, err := o.ConditionFn(info, o) if success { o.Printer.PrintObj(finalObject, o.Out) @@ -221,6 +227,13 @@ func (o *WaitOptions) RunWait() error { } return err }) + if err != nil { + return err + } + if visitCount == 0 { + return errNoMatchingResources + } + return err } // IsDeleted is a condition func for waiting for something to be deleted @@ -228,7 +241,7 @@ func IsDeleted(info *resource.Info, o *WaitOptions) (runtime.Object, bool, error endTime := time.Now().Add(o.Timeout) for { gottenObj, err := o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).Get(info.Name, metav1.GetOptions{}) - if errors.IsNotFound(err) { + if apierrors.IsNotFound(err) { return info.Object, true, nil } if err != nil { @@ -293,7 +306,7 @@ func (w ConditionalWait) IsConditionMet(info *resource.Info, o *WaitOptions) (ru resourceVersion := "" gottenObj, err := o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).Get(info.Name, metav1.GetOptions{}) switch { - case errors.IsNotFound(err): + case apierrors.IsNotFound(err): resourceVersion = "0" case err != nil: return info.Object, false, err diff --git a/pkg/kubectl/cmd/wait/wait_test.go b/pkg/kubectl/cmd/wait/wait_test.go index 06fbc859b1..db69e19590 100644 --- a/pkg/kubectl/cmd/wait/wait_test.go +++ b/pkg/kubectl/cmd/wait/wait_test.go @@ -68,7 +68,7 @@ func TestWaitForDeletion(t *testing.T) { tests := []struct { name string - info *resource.Info + infos []*resource.Info fakeClient func() *dynamicfakeclient.FakeDynamicClient timeout time.Duration uidMap UIDMap @@ -78,12 +78,14 @@ func TestWaitForDeletion(t *testing.T) { }{ { name: "missing on get", - info: &resource.Info{ - Mapping: &meta.RESTMapping{ - Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + infos: []*resource.Info{ + { + Mapping: &meta.RESTMapping{ + Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + }, + Name: "name-foo", + Namespace: "ns-foo", }, - Name: "name-foo", - Namespace: "ns-foo", }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { return dynamicfakeclient.NewSimpleDynamicClient(scheme) @@ -99,14 +101,31 @@ func TestWaitForDeletion(t *testing.T) { } }, }, + { + name: "handles no infos", + infos: []*resource.Info{}, + fakeClient: func() *dynamicfakeclient.FakeDynamicClient { + return dynamicfakeclient.NewSimpleDynamicClient(scheme) + }, + 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", - info: &resource.Info{ - Mapping: &meta.RESTMapping{ - Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + infos: []*resource.Info{ + { + Mapping: &meta.RESTMapping{ + Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + }, + Name: "name-foo", + Namespace: "ns-foo", }, - Name: "name-foo", - Namespace: "ns-foo", }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) @@ -146,12 +165,14 @@ func TestWaitForDeletion(t *testing.T) { }, { name: "times out", - info: &resource.Info{ - Mapping: &meta.RESTMapping{ - Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + infos: []*resource.Info{ + { + Mapping: &meta.RESTMapping{ + Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + }, + Name: "name-foo", + Namespace: "ns-foo", }, - Name: "name-foo", - Namespace: "ns-foo", }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) @@ -177,12 +198,14 @@ func TestWaitForDeletion(t *testing.T) { }, { name: "handles watch close out", - info: &resource.Info{ - Mapping: &meta.RESTMapping{ - Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + infos: []*resource.Info{ + { + Mapping: &meta.RESTMapping{ + Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + }, + Name: "name-foo", + Namespace: "ns-foo", }, - Name: "name-foo", - Namespace: "ns-foo", }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) @@ -228,12 +251,14 @@ func TestWaitForDeletion(t *testing.T) { }, { name: "handles watch delete", - info: &resource.Info{ - Mapping: &meta.RESTMapping{ - Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + infos: []*resource.Info{ + { + Mapping: &meta.RESTMapping{ + Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + }, + Name: "name-foo", + Namespace: "ns-foo", }, - Name: "name-foo", - Namespace: "ns-foo", }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) @@ -267,7 +292,7 @@ func TestWaitForDeletion(t *testing.T) { t.Run(test.name, func(t *testing.T) { fakeClient := test.fakeClient() o := &WaitOptions{ - ResourceFinder: genericclioptions.NewSimpleFakeResourceFinder(test.info), + ResourceFinder: genericclioptions.NewSimpleFakeResourceFinder(test.infos...), UIDMap: test.uidMap, DynamicClient: fakeClient, Timeout: test.timeout, @@ -299,7 +324,7 @@ func TestWaitForCondition(t *testing.T) { tests := []struct { name string - info *resource.Info + infos []*resource.Info fakeClient func() *dynamicfakeclient.FakeDynamicClient timeout time.Duration @@ -308,12 +333,14 @@ func TestWaitForCondition(t *testing.T) { }{ { name: "present on get", - info: &resource.Info{ - Mapping: &meta.RESTMapping{ - Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + infos: []*resource.Info{ + { + Mapping: &meta.RESTMapping{ + Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + }, + Name: "name-foo", + Namespace: "ns-foo", }, - Name: "name-foo", - Namespace: "ns-foo", }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) @@ -336,14 +363,31 @@ func TestWaitForCondition(t *testing.T) { } }, }, + { + name: "handles no infos", + infos: []*resource.Info{}, + fakeClient: func() *dynamicfakeclient.FakeDynamicClient { + return dynamicfakeclient.NewSimpleDynamicClient(scheme) + }, + 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: "times out", - info: &resource.Info{ - Mapping: &meta.RESTMapping{ - Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + infos: []*resource.Info{ + { + Mapping: &meta.RESTMapping{ + Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + }, + Name: "name-foo", + Namespace: "ns-foo", }, - Name: "name-foo", - Namespace: "ns-foo", }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) @@ -372,12 +416,14 @@ func TestWaitForCondition(t *testing.T) { }, { name: "handles watch close out", - info: &resource.Info{ - Mapping: &meta.RESTMapping{ - Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + infos: []*resource.Info{ + { + Mapping: &meta.RESTMapping{ + Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + }, + Name: "name-foo", + Namespace: "ns-foo", }, - Name: "name-foo", - Namespace: "ns-foo", }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) @@ -423,12 +469,14 @@ func TestWaitForCondition(t *testing.T) { }, { name: "handles watch condition change", - info: &resource.Info{ - Mapping: &meta.RESTMapping{ - Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + infos: []*resource.Info{ + { + Mapping: &meta.RESTMapping{ + Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + }, + Name: "name-foo", + Namespace: "ns-foo", }, - Name: "name-foo", - Namespace: "ns-foo", }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) @@ -461,12 +509,14 @@ func TestWaitForCondition(t *testing.T) { }, { name: "handles watch created", - info: &resource.Info{ - Mapping: &meta.RESTMapping{ - Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + infos: []*resource.Info{ + { + Mapping: &meta.RESTMapping{ + Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"}, + }, + Name: "name-foo", + Namespace: "ns-foo", }, - Name: "name-foo", - Namespace: "ns-foo", }, fakeClient: func() *dynamicfakeclient.FakeDynamicClient { fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme) @@ -500,7 +550,7 @@ func TestWaitForCondition(t *testing.T) { t.Run(test.name, func(t *testing.T) { fakeClient := test.fakeClient() o := &WaitOptions{ - ResourceFinder: genericclioptions.NewSimpleFakeResourceFinder(test.info), + ResourceFinder: genericclioptions.NewSimpleFakeResourceFinder(test.infos...), DynamicClient: fakeClient, Timeout: test.timeout,