From 9d3e55ec431f3f595a7739fcc592602f7cc1d69b Mon Sep 17 00:00:00 2001 From: minherz Date: Sun, 21 May 2023 19:03:24 -0700 Subject: [PATCH] Support JSONPath condition without value Extend current JSONPath condition logic to return from wait on "any" value. Change parsing JSONPath input to support the syntax without value. Match any simple or complex (object or array) values. --- .../src/k8s.io/kubectl/pkg/cmd/wait/wait.go | 50 +++-- .../k8s.io/kubectl/pkg/cmd/wait/wait_test.go | 172 ++++++++++++------ 2 files changed, 150 insertions(+), 72 deletions(-) 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 66fdabfc2ba..561007a4263 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait.go +++ b/staging/src/k8s.io/kubectl/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); 1 < numOfArgs || 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 { + jsonPathValue := jsonPathInput[1] + if jsonPathValue == "" { + return "", "", errors.New("jsonpath wait value cannot be empty") + } + return relaxedJSONPathExp, strings.Trim(jsonPathValue, `'"`), nil } - jsonPathCond = strings.Trim(jsonPathCond, `'"`) - - return relaxedJSONPathExp, jsonPathCond, nil + return relaxedJSONPathExp, "", 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/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait_test.go index d1ff60fab9b..50d2b0a0989 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 @@ -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(), }