Skip to content

Commit 5544187

Browse files
author
Kubernetes Submit Queue
authored
Merge pull request kubernetes#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 kubernetes#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 ```
2 parents b759b00 + d3445d7 commit 5544187

File tree

2 files changed

+121
-58
lines changed

2 files changed

+121
-58
lines changed

‎pkg/kubectl/cmd/wait/wait.go‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,14 @@ limitations under the License.
1717
package wait
1818

1919
import (
20+
"errors"
2021
"fmt"
2122
"strings"
2223
"time"
2324

2425
"github.com/spf13/cobra"
2526

26-
"k8s.io/apimachinery/pkg/api/errors"
27+
apierrors "k8s.io/apimachinery/pkg/api/errors"
2728
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2829
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2930
"k8s.io/apimachinery/pkg/runtime"
@@ -61,6 +62,9 @@ var (
6162
kubectl wait --for=delete pod/busybox1 --timeout=60s`)
6263
)
6364

65+
// errNoMatchingResources is returned when there is no resources matching a query.
66+
var errNoMatchingResources = errors.New("no matching resources found")
67+
6468
// WaitFlags directly reflect the information that CLI is gathering via flags. They will be converted to Options, which
6569
// reflect the runtime requirements for the command. This structure reduces the transformation to wiring and makes
6670
// the logic itself easy to unit test
@@ -206,11 +210,13 @@ type ConditionFunc func(info *resource.Info, o *WaitOptions) (finalObject runtim
206210

207211
// RunWait runs the waiting logic
208212
func (o *WaitOptions) RunWait() error {
209-
return o.ResourceFinder.Do().Visit(func(info *resource.Info, err error) error {
213+
visitCount := 0
214+
err := o.ResourceFinder.Do().Visit(func(info *resource.Info, err error) error {
210215
if err != nil {
211216
return err
212217
}
213218

219+
visitCount++
214220
finalObject, success, err := o.ConditionFn(info, o)
215221
if success {
216222
o.Printer.PrintObj(finalObject, o.Out)
@@ -221,14 +227,21 @@ func (o *WaitOptions) RunWait() error {
221227
}
222228
return err
223229
})
230+
if err != nil {
231+
return err
232+
}
233+
if visitCount == 0 {
234+
return errNoMatchingResources
235+
}
236+
return err
224237
}
225238

226239
// IsDeleted is a condition func for waiting for something to be deleted
227240
func IsDeleted(info *resource.Info, o *WaitOptions) (runtime.Object, bool, error) {
228241
endTime := time.Now().Add(o.Timeout)
229242
for {
230243
gottenObj, err := o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).Get(info.Name, metav1.GetOptions{})
231-
if errors.IsNotFound(err) {
244+
if apierrors.IsNotFound(err) {
232245
return info.Object, true, nil
233246
}
234247
if err != nil {
@@ -293,7 +306,7 @@ func (w ConditionalWait) IsConditionMet(info *resource.Info, o *WaitOptions) (ru
293306
resourceVersion := ""
294307
gottenObj, err := o.DynamicClient.Resource(info.Mapping.Resource).Namespace(info.Namespace).Get(info.Name, metav1.GetOptions{})
295308
switch {
296-
case errors.IsNotFound(err):
309+
case apierrors.IsNotFound(err):
297310
resourceVersion = "0"
298311
case err != nil:
299312
return info.Object, false, err

‎pkg/kubectl/cmd/wait/wait_test.go‎

Lines changed: 104 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func TestWaitForDeletion(t *testing.T) {
6868

6969
tests := []struct {
7070
name string
71-
info *resource.Info
71+
infos []*resource.Info
7272
fakeClient func() *dynamicfakeclient.FakeDynamicClient
7373
timeout time.Duration
7474
uidMap UIDMap
@@ -78,12 +78,14 @@ func TestWaitForDeletion(t *testing.T) {
7878
}{
7979
{
8080
name: "missing on get",
81-
info: &resource.Info{
82-
Mapping: &meta.RESTMapping{
83-
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
81+
infos: []*resource.Info{
82+
{
83+
Mapping: &meta.RESTMapping{
84+
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
85+
},
86+
Name: "name-foo",
87+
Namespace: "ns-foo",
8488
},
85-
Name: "name-foo",
86-
Namespace: "ns-foo",
8789
},
8890
fakeClient: func() *dynamicfakeclient.FakeDynamicClient {
8991
return dynamicfakeclient.NewSimpleDynamicClient(scheme)
@@ -99,14 +101,31 @@ func TestWaitForDeletion(t *testing.T) {
99101
}
100102
},
101103
},
104+
{
105+
name: "handles no infos",
106+
infos: []*resource.Info{},
107+
fakeClient: func() *dynamicfakeclient.FakeDynamicClient {
108+
return dynamicfakeclient.NewSimpleDynamicClient(scheme)
109+
},
110+
timeout: 10 * time.Second,
111+
expectedErr: errNoMatchingResources.Error(),
112+
113+
validateActions: func(t *testing.T, actions []clienttesting.Action) {
114+
if len(actions) != 0 {
115+
t.Fatal(spew.Sdump(actions))
116+
}
117+
},
118+
},
102119
{
103120
name: "uid conflict on get",
104-
info: &resource.Info{
105-
Mapping: &meta.RESTMapping{
106-
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
121+
infos: []*resource.Info{
122+
{
123+
Mapping: &meta.RESTMapping{
124+
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
125+
},
126+
Name: "name-foo",
127+
Namespace: "ns-foo",
107128
},
108-
Name: "name-foo",
109-
Namespace: "ns-foo",
110129
},
111130
fakeClient: func() *dynamicfakeclient.FakeDynamicClient {
112131
fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme)
@@ -146,12 +165,14 @@ func TestWaitForDeletion(t *testing.T) {
146165
},
147166
{
148167
name: "times out",
149-
info: &resource.Info{
150-
Mapping: &meta.RESTMapping{
151-
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
168+
infos: []*resource.Info{
169+
{
170+
Mapping: &meta.RESTMapping{
171+
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
172+
},
173+
Name: "name-foo",
174+
Namespace: "ns-foo",
152175
},
153-
Name: "name-foo",
154-
Namespace: "ns-foo",
155176
},
156177
fakeClient: func() *dynamicfakeclient.FakeDynamicClient {
157178
fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme)
@@ -177,12 +198,14 @@ func TestWaitForDeletion(t *testing.T) {
177198
},
178199
{
179200
name: "handles watch close out",
180-
info: &resource.Info{
181-
Mapping: &meta.RESTMapping{
182-
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
201+
infos: []*resource.Info{
202+
{
203+
Mapping: &meta.RESTMapping{
204+
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
205+
},
206+
Name: "name-foo",
207+
Namespace: "ns-foo",
183208
},
184-
Name: "name-foo",
185-
Namespace: "ns-foo",
186209
},
187210
fakeClient: func() *dynamicfakeclient.FakeDynamicClient {
188211
fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme)
@@ -228,12 +251,14 @@ func TestWaitForDeletion(t *testing.T) {
228251
},
229252
{
230253
name: "handles watch delete",
231-
info: &resource.Info{
232-
Mapping: &meta.RESTMapping{
233-
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
254+
infos: []*resource.Info{
255+
{
256+
Mapping: &meta.RESTMapping{
257+
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
258+
},
259+
Name: "name-foo",
260+
Namespace: "ns-foo",
234261
},
235-
Name: "name-foo",
236-
Namespace: "ns-foo",
237262
},
238263
fakeClient: func() *dynamicfakeclient.FakeDynamicClient {
239264
fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme)
@@ -267,7 +292,7 @@ func TestWaitForDeletion(t *testing.T) {
267292
t.Run(test.name, func(t *testing.T) {
268293
fakeClient := test.fakeClient()
269294
o := &WaitOptions{
270-
ResourceFinder: genericclioptions.NewSimpleFakeResourceFinder(test.info),
295+
ResourceFinder: genericclioptions.NewSimpleFakeResourceFinder(test.infos...),
271296
UIDMap: test.uidMap,
272297
DynamicClient: fakeClient,
273298
Timeout: test.timeout,
@@ -299,7 +324,7 @@ func TestWaitForCondition(t *testing.T) {
299324

300325
tests := []struct {
301326
name string
302-
info *resource.Info
327+
infos []*resource.Info
303328
fakeClient func() *dynamicfakeclient.FakeDynamicClient
304329
timeout time.Duration
305330

@@ -308,12 +333,14 @@ func TestWaitForCondition(t *testing.T) {
308333
}{
309334
{
310335
name: "present on get",
311-
info: &resource.Info{
312-
Mapping: &meta.RESTMapping{
313-
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
336+
infos: []*resource.Info{
337+
{
338+
Mapping: &meta.RESTMapping{
339+
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
340+
},
341+
Name: "name-foo",
342+
Namespace: "ns-foo",
314343
},
315-
Name: "name-foo",
316-
Namespace: "ns-foo",
317344
},
318345
fakeClient: func() *dynamicfakeclient.FakeDynamicClient {
319346
fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme)
@@ -336,14 +363,31 @@ func TestWaitForCondition(t *testing.T) {
336363
}
337364
},
338365
},
366+
{
367+
name: "handles no infos",
368+
infos: []*resource.Info{},
369+
fakeClient: func() *dynamicfakeclient.FakeDynamicClient {
370+
return dynamicfakeclient.NewSimpleDynamicClient(scheme)
371+
},
372+
timeout: 10 * time.Second,
373+
expectedErr: errNoMatchingResources.Error(),
374+
375+
validateActions: func(t *testing.T, actions []clienttesting.Action) {
376+
if len(actions) != 0 {
377+
t.Fatal(spew.Sdump(actions))
378+
}
379+
},
380+
},
339381
{
340382
name: "times out",
341-
info: &resource.Info{
342-
Mapping: &meta.RESTMapping{
343-
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
383+
infos: []*resource.Info{
384+
{
385+
Mapping: &meta.RESTMapping{
386+
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
387+
},
388+
Name: "name-foo",
389+
Namespace: "ns-foo",
344390
},
345-
Name: "name-foo",
346-
Namespace: "ns-foo",
347391
},
348392
fakeClient: func() *dynamicfakeclient.FakeDynamicClient {
349393
fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme)
@@ -372,12 +416,14 @@ func TestWaitForCondition(t *testing.T) {
372416
},
373417
{
374418
name: "handles watch close out",
375-
info: &resource.Info{
376-
Mapping: &meta.RESTMapping{
377-
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
419+
infos: []*resource.Info{
420+
{
421+
Mapping: &meta.RESTMapping{
422+
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
423+
},
424+
Name: "name-foo",
425+
Namespace: "ns-foo",
378426
},
379-
Name: "name-foo",
380-
Namespace: "ns-foo",
381427
},
382428
fakeClient: func() *dynamicfakeclient.FakeDynamicClient {
383429
fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme)
@@ -423,12 +469,14 @@ func TestWaitForCondition(t *testing.T) {
423469
},
424470
{
425471
name: "handles watch condition change",
426-
info: &resource.Info{
427-
Mapping: &meta.RESTMapping{
428-
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
472+
infos: []*resource.Info{
473+
{
474+
Mapping: &meta.RESTMapping{
475+
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
476+
},
477+
Name: "name-foo",
478+
Namespace: "ns-foo",
429479
},
430-
Name: "name-foo",
431-
Namespace: "ns-foo",
432480
},
433481
fakeClient: func() *dynamicfakeclient.FakeDynamicClient {
434482
fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme)
@@ -461,12 +509,14 @@ func TestWaitForCondition(t *testing.T) {
461509
},
462510
{
463511
name: "handles watch created",
464-
info: &resource.Info{
465-
Mapping: &meta.RESTMapping{
466-
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
512+
infos: []*resource.Info{
513+
{
514+
Mapping: &meta.RESTMapping{
515+
Resource: schema.GroupVersionResource{Group: "group", Version: "version", Resource: "theresource"},
516+
},
517+
Name: "name-foo",
518+
Namespace: "ns-foo",
467519
},
468-
Name: "name-foo",
469-
Namespace: "ns-foo",
470520
},
471521
fakeClient: func() *dynamicfakeclient.FakeDynamicClient {
472522
fakeClient := dynamicfakeclient.NewSimpleDynamicClient(scheme)
@@ -500,7 +550,7 @@ func TestWaitForCondition(t *testing.T) {
500550
t.Run(test.name, func(t *testing.T) {
501551
fakeClient := test.fakeClient()
502552
o := &WaitOptions{
503-
ResourceFinder: genericclioptions.NewSimpleFakeResourceFinder(test.info),
553+
ResourceFinder: genericclioptions.NewSimpleFakeResourceFinder(test.infos...),
504554
DynamicClient: fakeClient,
505555
Timeout: test.timeout,
506556

0 commit comments

Comments
 (0)