Merge pull request #66692 from m1kola/66456_waitcmd__error_for_selectors

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Makes kubectl wait exit with status 1 and print an error message, if there is no resources matching selectors

**What this PR does / why we need it**:

It makes the `kubectl wait` command print an error message and exit with exit code 1, if there is no resource matching users's query. This can happen when user specifies selectors. Example:

```
kubectl wait deployment -l app=something-that-does-not-exist --for condition=available --timeout=5s
```

**Which issue(s) this PR fixes**:
Fixes #66456

**Special notes for your reviewer**:

This is my first contribution into the project (except one line change in docs) and don't have much experience with Go. I learned a lot while working on this (about resource finders and the `Visitor` interface and it's implementations), but it is very likely that I'm doing something wrong :)

I'm keen to continue contributing into the project (into the cli part for now), so I will really appreciate detailed feedback, if you have a chance to provide it (point me into a right direction and/or explain why it's not a good idea to do something in a certain way).

Thanks!

**Release note**:
```release-note
kubectl: the wait command now prints an error message and exits with the code 1, if there is no resources matching selectors
```

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

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{
+			infos: []*resource.Info{
-				Mapping: &meta.RESTMapping{
+				{
-					Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
+					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{
+			infos: []*resource.Info{
-				Mapping: &meta.RESTMapping{
+				{
-					Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
+					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{
+			infos: []*resource.Info{
-				Mapping: &meta.RESTMapping{
+				{
-					Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
+					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{
+			infos: []*resource.Info{
-				Mapping: &meta.RESTMapping{
+				{
-					Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
+					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{
+			infos: []*resource.Info{
-				Mapping: &meta.RESTMapping{
+				{
-					Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
+					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{
+			infos: []*resource.Info{
-				Mapping: &meta.RESTMapping{
+				{
-					Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
+					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{
+			infos: []*resource.Info{
-				Mapping: &meta.RESTMapping{
+				{
-					Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
+					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{
+			infos: []*resource.Info{
-				Mapping: &meta.RESTMapping{
+				{
-					Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
+					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{
+			infos: []*resource.Info{
-				Mapping: &meta.RESTMapping{
+				{
-					Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
+					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{
+			infos: []*resource.Info{
-				Mapping: &meta.RESTMapping{
+				{
-					Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
+					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,