Merge pull request #118160 from minherz/master

Support JSONPath condition without value

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,7 +219,8 @@ func conditionFuncFor(condition string, errOut io.Writer) (ConditionFunc, error)
 			return nil, err
 		}
 		return JSONPathWait{
-			jsonPathCondition: jsonPathCond,
+			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,7 +596,8 @@ 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
+	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
 	}

staging/src/k8s.io/kubectl/pkg/cmd/wait/wait_test.go

@@ -1030,7 +1030,8 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) {
 		name          string
 		fakeClient    func() *dynamicfakeclient.FakeDynamicClient
 		jsonPathExp   string
-		jsonPathCond string
+		jsonPathValue string
+		matchAnyValue bool
 
 		expectedErr string
 	}{
@@ -1042,7 +1043,8 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) {
 				return fakeClient
 			},
 			jsonPathExp:   "{.foo.bar}",
-			jsonPathCond: "baz",
+			jsonPathValue: "baz",
+			matchAnyValue: false,
 
 			expectedErr: "timed out waiting for the condition on theresource/foo-b6699dcfb-rnv7t",
 		},
@@ -1054,7 +1056,8 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) {
 				return fakeClient
 			},
 			jsonPathExp:   "{.status.containerStatuses[0].ready}",
-			jsonPathCond: "true",
+			jsonPathValue: "true",
+			matchAnyValue: false,
 
 			expectedErr: None,
 		},
@@ -1066,7 +1069,8 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) {
 				return fakeClient
 			},
 			jsonPathExp:   "{.status.containerStatuses[0].ready}",
-			jsonPathCond: "false",
+			jsonPathValue: "false",
+			matchAnyValue: false,
 
 			expectedErr: "timed out waiting for the condition on theresource/foo-b6699dcfb-rnv7t",
 		},
@@ -1078,7 +1082,8 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) {
 				return fakeClient
 			},
 			jsonPathExp:   "{.spec.containers[0].ports[0].containerPort}",
-			jsonPathCond: "80",
+			jsonPathValue: "80",
+			matchAnyValue: false,
 
 			expectedErr: None,
 		},
@@ -1090,7 +1095,8 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) {
 				return fakeClient
 			},
 			jsonPathExp:   "{.spec.containers[0].ports[0].containerPort}",
-			jsonPathCond: "81",
+			jsonPathValue: "81",
+			matchAnyValue: false,
 
 			expectedErr: "timed out waiting for the condition on theresource/foo-b6699dcfb-rnv7t",
 		},
@@ -1102,10 +1108,40 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) {
 				return fakeClient
 			},
 			jsonPathExp:   "{.spec.nodeName}",
-			jsonPathCond: "knode0",
+			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 {
@@ -1114,7 +1150,8 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) {
 				return fakeClient
 			},
 			jsonPathExp:   "{.spec.nodeName}",
-			jsonPathCond: "kmaster",
+			jsonPathValue: "kmaster",
+			matchAnyValue: false,
 
 			expectedErr: "timed out waiting for the condition on theresource/foo-b6699dcfb-rnv7t",
 		},
@@ -1126,7 +1163,21 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) {
 				return fakeClient
 			},
 			jsonPathExp:   "{.status.conditions[*]}",
-			jsonPathCond: "foo",
+			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",
 		},
@@ -1138,7 +1189,21 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) {
 				return fakeClient
 			},
 			jsonPathExp:   "{range .status.conditions[*]}[{.status}] {end}",
-			jsonPathCond: "foo",
+			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",
 		},
@@ -1150,7 +1215,8 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) {
 				return fakeClient
 			},
 			jsonPathExp:   "{.status.conditions}",
-			jsonPathCond: "True",
+			jsonPathValue: "True",
+			matchAnyValue: false,
 
 			expectedErr: "jsonpath leads to a nested object or list which is not supported",
 		},
@@ -1164,7 +1230,8 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) {
 				return fakeClient
 			},
 			jsonPathExp:   "{.spec}",
-			jsonPathCond: "foo",
+			jsonPathValue: "foo",
+			matchAnyValue: false,
 
 			expectedErr: "jsonpath leads to a nested object or list which is not supported",
 		},
@@ -1180,7 +1247,8 @@ func TestWaitForDifferentJSONPathExpression(t *testing.T) {
 
 				Printer: printers.NewDiscardingPrinter(),
 				ConditionFn: JSONPathWait{
-					jsonPathCondition: test.jsonPathCond,
+					matchAnyValue:  test.matchAnyValue,
+					jsonPathValue:  test.jsonPathValue,
 					jsonPathParser: j,
 					errOut:         io.Discard}.IsJSONPathConditionMet,
 				IOStreams: genericiooptions.NewTestIOStreamsDiscard(),
@@ -1217,7 +1285,7 @@ func TestWaitForJSONPathCondition(t *testing.T) {
 		fakeClient    func() *dynamicfakeclient.FakeDynamicClient
 		timeout       time.Duration
 		jsonPathExp   string
-		jsonPathCond string
+		jsonPathValue string
 
 		expectedErr string
 	}{
@@ -1242,7 +1310,7 @@ func TestWaitForJSONPathCondition(t *testing.T) {
 			},
 			timeout:       3 * time.Second,
 			jsonPathExp:   "{.metadata.name}",
-			jsonPathCond: "foo-b6699dcfb-rnv7t",
+			jsonPathValue: "foo-b6699dcfb-rnv7t",
 
 			expectedErr: None,
 		},
@@ -1332,7 +1400,7 @@ func TestWaitForJSONPathCondition(t *testing.T) {
 			},
 			timeout:       3 * time.Second,
 			jsonPathExp:   "{.metadata.name}",
-			jsonPathCond: "foo", // use incorrect name so it'll keep waiting
+			jsonPathValue: "foo", // use incorrect name so it'll keep waiting
 
 			expectedErr: "timed out waiting for the condition on theresource/foo-b6699dcfb-rnv7t",
 		},
@@ -1362,7 +1430,7 @@ func TestWaitForJSONPathCondition(t *testing.T) {
 			},
 			timeout:       10 * time.Second,
 			jsonPathExp:   "{.metadata.name}",
-			jsonPathCond: "foo-b6699dcfb-rnv7t",
+			jsonPathValue: "foo-b6699dcfb-rnv7t",
 
 			expectedErr: None,
 		},
@@ -1387,7 +1455,7 @@ func TestWaitForJSONPathCondition(t *testing.T) {
 			},
 			timeout:       1 * time.Second,
 			jsonPathExp:   "{.spec.containers[0].image}",
-			jsonPathCond: "nginx",
+			jsonPathValue: "nginx",
 
 			expectedErr: None,
 		},
@@ -1428,7 +1496,7 @@ func TestWaitForJSONPathCondition(t *testing.T) {
 			},
 			timeout:       10 * time.Second,
 			jsonPathExp:   "{.metadata.name}",
-			jsonPathCond: "foo-b6699dcfb-rnv7t",
+			jsonPathValue: "foo-b6699dcfb-rnv7t",
 
 			expectedErr: None,
 		},
@@ -1444,7 +1512,7 @@ func TestWaitForJSONPathCondition(t *testing.T) {
 
 				Printer: printers.NewDiscardingPrinter(),
 				ConditionFn: JSONPathWait{
-					jsonPathCondition: test.jsonPathCond,
+					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())
+			}
+		})
+	}
+}

test/cmd/wait.sh

@@ -94,6 +94,21 @@ EOF
     # Clean deployment
     kubectl delete deployment dtest
 
+    # create test data
+    kubectl create deployment test-3 --image=busybox
+
+    # wait with jsonpath without value to succeed
+    set +o errexit
+    # Command: Wait with jsonpath without value
+    output_message=$(kubectl wait --for=jsonpath='{.status.replicas}' deploy/test-3 2>&1)
+    set -o errexit
+
+    # Post-Condition: Wait succeed
+    kube::test::if_has_string "${output_message}" 'deployment.apps/test-3 condition met'
+
+    # Clean deployment
+    kubectl delete deployment test-3
+
     set +o nounset
     set +o errexit
 }