diff --git a/pkg/cmd/wait/wait.go b/pkg/cmd/wait/wait.go index dbbbd66a..4dfedf01 100644 --- a/pkg/cmd/wait/wait.go +++ b/pkg/cmd/wait/wait.go @@ -74,6 +74,9 @@ var ( # Wait for the pod "busybox1" to contain the status phase to be "Running" kubectl wait --for=jsonpath='{.status.phase}'=Running pod/busybox1 + # Wait for the service "loadbalancer" to have ingress. + kubectl wait --for=jsonpath='{.status.loadBalancer.ingress}' service/loadbalancer + # Wait for the pod "busybox1" to be deleted, with a timeout of 60s, after having issued the "delete" command kubectl delete pod/busybox1 kubectl wait --for=delete pod/busybox1 --timeout=60s`)) @@ -120,7 +123,7 @@ func NewCmdWait(restClientGetter genericclioptions.RESTClientGetter, streams gen flags := NewWaitFlags(restClientGetter, streams) cmd := &cobra.Command{ - Use: "wait ([-f FILENAME] | resource.group/resource.name | resource.group [(-l label | --all)]) [--for=delete|--for condition=available|--for=jsonpath='{}'=value]", + Use: "wait ([-f FILENAME] | resource.group/resource.name | resource.group [(-l label | --all)]) [--for=delete|--for condition=available|--for=jsonpath='{}'[=value]]", Short: i18n.T("Experimental: Wait for a specific condition on one or many resources"), Long: waitLong, Example: waitExample, @@ -145,7 +148,7 @@ func (flags *WaitFlags) AddFlags(cmd *cobra.Command) { flags.ResourceBuilderFlags.AddFlags(cmd.Flags()) cmd.Flags().DurationVar(&flags.Timeout, "timeout", flags.Timeout, "The length of time to wait before giving up. Zero means check once and don't wait, negative means wait for a week.") - cmd.Flags().StringVar(&flags.ForCondition, "for", flags.ForCondition, "The condition to wait on: [delete|condition=condition-name[=condition-value]|jsonpath='{JSONPath expression}'=JSONPath Condition]. The default condition-value is true. Condition values are compared after Unicode simple case folding, which is a more general form of case-insensitivity.") + cmd.Flags().StringVar(&flags.ForCondition, "for", flags.ForCondition, "The condition to wait on: [delete|condition=condition-name[=condition-value]|jsonpath='{JSONPath expression}'=[JSONPath value]]. The default condition-value is true. Condition values are compared after Unicode simple case folding, which is a more general form of case-insensitivity.") } // ToOptions converts from CLI inputs to runtime inputs @@ -207,10 +210,7 @@ func conditionFuncFor(condition string, errOut io.Writer) (ConditionFunc, error) } if strings.HasPrefix(condition, "jsonpath=") { splitStr := strings.Split(condition, "=") - if len(splitStr) != 3 { - return nil, fmt.Errorf("jsonpath wait format must be --for=jsonpath='{.status.readyReplicas}'=3") - } - jsonPathExp, jsonPathCond, err := processJSONPathInput(splitStr[1], splitStr[2]) + jsonPathExp, jsonPathValue, err := processJSONPathInput(splitStr[1:]) if err != nil { return nil, err } @@ -219,9 +219,10 @@ func conditionFuncFor(condition string, errOut io.Writer) (ConditionFunc, error) return nil, err } return JSONPathWait{ - jsonPathCondition: jsonPathCond, - jsonPathParser: j, - errOut: errOut, + matchAnyValue: jsonPathValue == "", + jsonPathValue: jsonPathValue, + jsonPathParser: j, + errOut: errOut, }.IsJSONPathConditionMet, nil } @@ -240,18 +241,23 @@ func newJSONPathParser(jsonPathExpression string) (*jsonpath.JSONPath, error) { return j, nil } -// processJSONPathInput will parses the user's JSONPath input and process the string -func processJSONPathInput(jsonPathExpression, jsonPathCond string) (string, string, error) { - relaxedJSONPathExp, err := cmdget.RelaxedJSONPathExpression(jsonPathExpression) +// processJSONPathInput will parses the user's JSONPath input containing JSON expression and, optionally, JSON value for matching condition and process it +func processJSONPathInput(jsonPathInput []string) (string, string, error) { + if numOfArgs := len(jsonPathInput); numOfArgs < 1 || numOfArgs > 2 { + return "", "", fmt.Errorf("jsonpath wait format must be --for=jsonpath='{.status.readyReplicas}'=3 or --for=jsonpath='{.status.readyReplicas}'") + } + relaxedJSONPathExp, err := cmdget.RelaxedJSONPathExpression(jsonPathInput[0]) if err != nil { return "", "", err } - if jsonPathCond == "" { - return "", "", errors.New("jsonpath wait condition cannot be empty") + if len(jsonPathInput) == 1 { + return relaxedJSONPathExp, "", nil } - jsonPathCond = strings.Trim(jsonPathCond, `'"`) - - return relaxedJSONPathExp, jsonPathCond, nil + jsonPathValue := strings.Trim(jsonPathInput[1], `'"`) + if jsonPathValue == "" { + return "", "", errors.New("jsonpath wait has to have a value after equal sign, like --for=jsonpath='{.status.readyReplicas}'=3") + } + return relaxedJSONPathExp, jsonPathValue, nil } // ResourceLocation holds the location of a resource @@ -590,8 +596,9 @@ func getObservedGeneration(obj *unstructured.Unstructured, condition map[string] // JSONPathWait holds a JSONPath Parser which has the ability // to check for the JSONPath condition and compare with the API server provided JSON output. type JSONPathWait struct { - jsonPathCondition string - jsonPathParser *jsonpath.JSONPath + matchAnyValue bool + jsonPathValue string + jsonPathParser *jsonpath.JSONPath // errOut is written to if an error occurs errOut io.Writer } @@ -635,7 +642,10 @@ func (j JSONPathWait) checkCondition(obj *unstructured.Unstructured) (bool, erro if err := verifyParsedJSONPath(parseResults); err != nil { return false, err } - isConditionMet, err := compareResults(parseResults[0][0], j.jsonPathCondition) + if j.matchAnyValue { + return true, nil + } + isConditionMet, err := compareResults(parseResults[0][0], j.jsonPathValue) if err != nil { return false, err } diff --git a/pkg/cmd/wait/wait_test.go b/pkg/cmd/wait/wait_test.go index d1ff60fa..2d01cce5 100644 --- a/pkg/cmd/wait/wait_test.go +++ b/pkg/cmd/wait/wait_test.go @@ -1027,10 +1027,11 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) { } tests := []struct { - name string - fakeClient func() *dynamicfakeclient.FakeDynamicClient - jsonPathExp string - jsonPathCond string + name string + fakeClient func() *dynamicfakeclient.FakeDynamicClient + jsonPathExp string + jsonPathValue string + matchAnyValue bool expectedErr string }{ @@ -1041,8 +1042,9 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) { fakeClient.PrependReactor("list", "theresource", listReactionfunc) return fakeClient }, - jsonPathExp: "{.foo.bar}", - jsonPathCond: "baz", + jsonPathExp: "{.foo.bar}", + jsonPathValue: "baz", + matchAnyValue: false, expectedErr: "timed out waiting for the condition on theresource/foo-b6699dcfb-rnv7t", }, @@ -1053,8 +1055,9 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) { fakeClient.PrependReactor("list", "theresource", listReactionfunc) return fakeClient }, - jsonPathExp: "{.status.containerStatuses[0].ready}", - jsonPathCond: "true", + jsonPathExp: "{.status.containerStatuses[0].ready}", + jsonPathValue: "true", + matchAnyValue: false, expectedErr: None, }, @@ -1065,8 +1068,9 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) { fakeClient.PrependReactor("list", "theresource", listReactionfunc) return fakeClient }, - jsonPathExp: "{.status.containerStatuses[0].ready}", - jsonPathCond: "false", + jsonPathExp: "{.status.containerStatuses[0].ready}", + jsonPathValue: "false", + matchAnyValue: false, expectedErr: "timed out waiting for the condition on theresource/foo-b6699dcfb-rnv7t", }, @@ -1077,8 +1081,9 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) { fakeClient.PrependReactor("list", "theresource", listReactionfunc) return fakeClient }, - jsonPathExp: "{.spec.containers[0].ports[0].containerPort}", - jsonPathCond: "80", + jsonPathExp: "{.spec.containers[0].ports[0].containerPort}", + jsonPathValue: "80", + matchAnyValue: false, expectedErr: None, }, @@ -1089,8 +1094,9 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) { fakeClient.PrependReactor("list", "theresource", listReactionfunc) return fakeClient }, - jsonPathExp: "{.spec.containers[0].ports[0].containerPort}", - jsonPathCond: "81", + jsonPathExp: "{.spec.containers[0].ports[0].containerPort}", + jsonPathValue: "81", + matchAnyValue: false, expectedErr: "timed out waiting for the condition on theresource/foo-b6699dcfb-rnv7t", }, @@ -1101,11 +1107,41 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) { fakeClient.PrependReactor("list", "theresource", listReactionfunc) return fakeClient }, - jsonPathExp: "{.spec.nodeName}", - jsonPathCond: "knode0", + jsonPathExp: "{.spec.nodeName}", + jsonPathValue: "knode0", + matchAnyValue: false, expectedErr: None, }, + { + name: "matches literal value of JSONPath entry without value condition", + fakeClient: func() *dynamicfakeclient.FakeDynamicClient { + fakeClient := dynamicfakeclient.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping) + fakeClient.PrependReactor("list", "theresource", listReactionfunc) + return fakeClient + }, + jsonPathExp: "{.spec.nodeName}", + jsonPathValue: "", + matchAnyValue: true, + + expectedErr: None, + }, + { + name: "matches complex types map[string]interface{} without value condition", + 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(createUnstructured(t, podYAML)), nil + }) + return fakeClient + }, + jsonPathExp: "{.spec}", + jsonPathValue: "", + matchAnyValue: true, + + expectedErr: None, + }, + { name: "compare string JSONPath entry wrong value", fakeClient: func() *dynamicfakeclient.FakeDynamicClient { @@ -1113,8 +1149,9 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) { fakeClient.PrependReactor("list", "theresource", listReactionfunc) return fakeClient }, - jsonPathExp: "{.spec.nodeName}", - jsonPathCond: "kmaster", + jsonPathExp: "{.spec.nodeName}", + jsonPathValue: "kmaster", + matchAnyValue: false, expectedErr: "timed out waiting for the condition on theresource/foo-b6699dcfb-rnv7t", }, @@ -1125,8 +1162,22 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) { fakeClient.PrependReactor("list", "theresource", listReactionfunc) return fakeClient }, - jsonPathExp: "{.status.conditions[*]}", - jsonPathCond: "foo", + jsonPathExp: "{.status.conditions[*]}", + jsonPathValue: "foo", + matchAnyValue: false, + + expectedErr: "given jsonpath expression matches more than one value", + }, + { + name: "matches more than one value without value condition", + fakeClient: func() *dynamicfakeclient.FakeDynamicClient { + fakeClient := dynamicfakeclient.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping) + fakeClient.PrependReactor("list", "theresource", listReactionfunc) + return fakeClient + }, + jsonPathExp: "{.status.conditions[*]}", + jsonPathValue: "", + matchAnyValue: true, expectedErr: "given jsonpath expression matches more than one value", }, @@ -1137,8 +1188,22 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) { fakeClient.PrependReactor("list", "theresource", listReactionfunc) return fakeClient }, - jsonPathExp: "{range .status.conditions[*]}[{.status}] {end}", - jsonPathCond: "foo", + jsonPathExp: "{range .status.conditions[*]}[{.status}] {end}", + jsonPathValue: "foo", + matchAnyValue: false, + + expectedErr: "given jsonpath expression matches more than one list", + }, + { + name: "matches more than one list without value condition", + fakeClient: func() *dynamicfakeclient.FakeDynamicClient { + fakeClient := dynamicfakeclient.NewSimpleDynamicClientWithCustomListKinds(scheme, listMapping) + fakeClient.PrependReactor("list", "theresource", listReactionfunc) + return fakeClient + }, + jsonPathExp: "{range .status.conditions[*]}[{.status}] {end}", + jsonPathValue: "", + matchAnyValue: true, expectedErr: "given jsonpath expression matches more than one list", }, @@ -1149,8 +1214,9 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) { fakeClient.PrependReactor("list", "theresource", listReactionfunc) return fakeClient }, - jsonPathExp: "{.status.conditions}", - jsonPathCond: "True", + jsonPathExp: "{.status.conditions}", + jsonPathValue: "True", + matchAnyValue: false, expectedErr: "jsonpath leads to a nested object or list which is not supported", }, @@ -1163,8 +1229,9 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) { }) return fakeClient }, - jsonPathExp: "{.spec}", - jsonPathCond: "foo", + jsonPathExp: "{.spec}", + jsonPathValue: "foo", + matchAnyValue: false, expectedErr: "jsonpath leads to a nested object or list which is not supported", }, @@ -1180,9 +1247,10 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) { Printer: printers.NewDiscardingPrinter(), ConditionFn: JSONPathWait{ - jsonPathCondition: test.jsonPathCond, - jsonPathParser: j, - errOut: io.Discard}.IsJSONPathConditionMet, + matchAnyValue: test.matchAnyValue, + jsonPathValue: test.jsonPathValue, + jsonPathParser: j, + errOut: io.Discard}.IsJSONPathConditionMet, IOStreams: genericiooptions.NewTestIOStreamsDiscard(), } @@ -1212,12 +1280,12 @@ func TestWaitForJSONPathCondition(t *testing.T) { } tests := []struct { - name string - infos []*resource.Info - fakeClient func() *dynamicfakeclient.FakeDynamicClient - timeout time.Duration - jsonPathExp string - jsonPathCond string + name string + infos []*resource.Info + fakeClient func() *dynamicfakeclient.FakeDynamicClient + timeout time.Duration + jsonPathExp string + jsonPathValue string expectedErr string }{ @@ -1240,9 +1308,9 @@ func TestWaitForJSONPathCondition(t *testing.T) { }) return fakeClient }, - timeout: 3 * time.Second, - jsonPathExp: "{.metadata.name}", - jsonPathCond: "foo-b6699dcfb-rnv7t", + timeout: 3 * time.Second, + jsonPathExp: "{.metadata.name}", + jsonPathValue: "foo-b6699dcfb-rnv7t", expectedErr: None, }, @@ -1330,9 +1398,9 @@ func TestWaitForJSONPathCondition(t *testing.T) { }) return fakeClient }, - timeout: 3 * time.Second, - jsonPathExp: "{.metadata.name}", - jsonPathCond: "foo", // use incorrect name so it'll keep waiting + timeout: 3 * time.Second, + jsonPathExp: "{.metadata.name}", + jsonPathValue: "foo", // use incorrect name so it'll keep waiting expectedErr: "timed out waiting for the condition on theresource/foo-b6699dcfb-rnv7t", }, @@ -1360,9 +1428,9 @@ func TestWaitForJSONPathCondition(t *testing.T) { }) return fakeClient }, - timeout: 10 * time.Second, - jsonPathExp: "{.metadata.name}", - jsonPathCond: "foo-b6699dcfb-rnv7t", + timeout: 10 * time.Second, + jsonPathExp: "{.metadata.name}", + jsonPathValue: "foo-b6699dcfb-rnv7t", expectedErr: None, }, @@ -1385,9 +1453,9 @@ func TestWaitForJSONPathCondition(t *testing.T) { }) return fakeClient }, - timeout: 1 * time.Second, - jsonPathExp: "{.spec.containers[0].image}", - jsonPathCond: "nginx", + timeout: 1 * time.Second, + jsonPathExp: "{.spec.containers[0].image}", + jsonPathValue: "nginx", expectedErr: None, }, @@ -1426,9 +1494,9 @@ func TestWaitForJSONPathCondition(t *testing.T) { }) return fakeClient }, - timeout: 10 * time.Second, - jsonPathExp: "{.metadata.name}", - jsonPathCond: "foo-b6699dcfb-rnv7t", + timeout: 10 * time.Second, + jsonPathExp: "{.metadata.name}", + jsonPathValue: "foo-b6699dcfb-rnv7t", expectedErr: None, }, @@ -1444,8 +1512,8 @@ func TestWaitForJSONPathCondition(t *testing.T) { Printer: printers.NewDiscardingPrinter(), ConditionFn: JSONPathWait{ - jsonPathCondition: test.jsonPathCond, - jsonPathParser: j, errOut: io.Discard}.IsJSONPathConditionMet, + jsonPathValue: test.jsonPathValue, + jsonPathParser: j, errOut: io.Discard}.IsJSONPathConditionMet, IOStreams: genericiooptions.NewTestIOStreamsDiscard(), } @@ -1465,3 +1533,41 @@ func TestWaitForJSONPathCondition(t *testing.T) { }) } } + +// TestWaitForJSONPathBadConditionParsing will test errors in parsing JSONPath bad condition expressions +// except for parsing JSONPath expression itself (i.e. call to cmdget.RelaxedJSONPathExpression()) +func TestWaitForJSONPathBadConditionParsing(t *testing.T) { + tests := []struct { + name string + condition string + expectedResult JSONPathWait + expectedErr string + }{ + { + name: "missing JSONPath expression", + condition: "jsonpath=", + expectedErr: "jsonpath expression cannot be empty", + }, + { + name: "value in JSONPath expression has equal sign", + condition: "jsonpath={.metadata.name}='test=wrong'", + expectedErr: "jsonpath wait format must be --for=jsonpath='{.status.readyReplicas}'=3 or --for=jsonpath='{.status.readyReplicas}'", + }, + { + name: "undefined value", + condition: "jsonpath={.metadata.name}=", + expectedErr: "jsonpath wait has to have a value after equal sign, like --for=jsonpath='{.status.readyReplicas}'=3", + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, err := conditionFuncFor(test.condition, io.Discard) + if err == nil && test.expectedErr != "" { + t.Fatalf("expected %q, got empty", test.expectedErr) + } + if !strings.Contains(err.Error(), test.expectedErr) { + t.Fatalf("expected %q, got %q", test.expectedErr, err.Error()) + } + }) + } +}