Merge pull request #125630 from liggitt/rollback-wait

Revert kubectl wait regression

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

@@ -82,10 +82,7 @@ var (
 
 		# 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
+		kubectl wait --for=delete pod/busybox1 --timeout=60s`))
-
-		# Wait for the creation of the service "loadbalancer" in addition to wait to have ingress
-		kubectl wait --for=jsonpath='{.status.loadBalancer.ingress}' service/loadbalancer --wait-for-creation`))
 )
 
 // errNoMatchingResources is returned when there is no resources matching a query.
@@ -99,9 +96,8 @@ type WaitFlags struct {
 	PrintFlags           *genericclioptions.PrintFlags
 	ResourceBuilderFlags *genericclioptions.ResourceBuilderFlags
 
-	Timeout         time.Duration
+	Timeout      time.Duration
-	ForCondition    string
+	ForCondition string
-	WaitForCreation bool
 
 	genericiooptions.IOStreams
 }
@@ -119,8 +115,7 @@ func NewWaitFlags(restClientGetter genericclioptions.RESTClientGetter, streams g
 			WithLocal(false).
 			WithLatest(),
 
-		Timeout:         30 * time.Second,
+		Timeout: 30 * time.Second,
-		WaitForCreation: true,
 
 		IOStreams: streams,
 	}
@@ -157,7 +152,6 @@ func (flags *WaitFlags) AddFlags(cmd *cobra.Command) {
 
 	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 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.")
-	cmd.Flags().BoolVar(&flags.WaitForCreation, "wait-for-creation", flags.WaitForCreation, "The default value is true. If set to true, also wait for creation of objects if they do not already exist. This flag is ignored in --for=delete")
 }
 
 // ToOptions converts from CLI inputs to runtime inputs
@@ -186,11 +180,10 @@ func (flags *WaitFlags) ToOptions(args []string) (*WaitOptions, error) {
 	}
 
 	o := &WaitOptions{
-		ResourceFinder:  builder,
+		ResourceFinder: builder,
-		DynamicClient:   dynamicClient,
+		DynamicClient:  dynamicClient,
-		Timeout:         effectiveTimeout,
+		Timeout:        effectiveTimeout,
-		ForCondition:    flags.ForCondition,
+		ForCondition:   flags.ForCondition,
-		WaitForCreation: flags.WaitForCreation,
 
 		Printer:     printer,
 		ConditionFn: conditionFn,
@@ -309,11 +302,10 @@ type WaitOptions struct {
 	ResourceFinder genericclioptions.ResourceFinder
 	// UIDMap maps a resource location to a UID.  It is optional, but ConditionFuncs may choose to use it to make the result
 	// more reliable.  For instance, delete can look for UID consistency during delegated calls.
-	UIDMap          UIDMap
+	UIDMap        UIDMap
-	DynamicClient   dynamic.Interface
+	DynamicClient dynamic.Interface
-	Timeout         time.Duration
+	Timeout       time.Duration
-	ForCondition    string
+	ForCondition  string
-	WaitForCreation bool
 
 	Printer     printers.ResourcePrinter
 	ConditionFn ConditionFunc
@@ -328,40 +320,6 @@ func (o *WaitOptions) RunWait() error {
 	ctx, cancel := watchtools.ContextWithOptionalTimeout(context.Background(), o.Timeout)
 	defer cancel()
 
-	isForDelete := strings.ToLower(o.ForCondition) == "delete"
-	if o.WaitForCreation && o.Timeout == 0 {
-		return fmt.Errorf("--wait-for-creation requires a timeout value greater than 0")
-	}
-
-	if o.WaitForCreation && !isForDelete {
-		err := func() error {
-			for {
-				select {
-				case <-ctx.Done():
-					return fmt.Errorf("context deadline is exceeded while waiting for the creation of the resources")
-				default:
-					err := o.ResourceFinder.Do().Visit(func(info *resource.Info, err error) error {
-						// We don't need to do anything after we assure that the resources exist. Because
-						// actual logic will be incorporated after we wait all the resources' existence.
-						return nil
-					})
-					// It is verified that all the resources exist.
-					if err == nil {
-						return nil
-					}
-					// We specifically wait for the creation of resources and all the errors
-					// other than not found means that this is something we cannot handle.
-					if !apierrors.IsNotFound(err) {
-						return err
-					}
-				}
-			}
-		}()
-		if err != nil {
-			return err
-		}
-	}
-
 	visitCount := 0
 	visitFunc := func(info *resource.Info, err error) error {
 		if err != nil {
@@ -380,6 +338,7 @@ func (o *WaitOptions) RunWait() error {
 		return err
 	}
 	visitor := o.ResourceFinder.Do()
+	isForDelete := strings.ToLower(o.ForCondition) == "delete"
 	if visitor, ok := visitor.(*resource.Result); ok && isForDelete {
 		visitor.IgnoreErrors(apierrors.IsNotFound)
 	}

test/cmd/legacy-script.sh

@@ -1036,7 +1036,6 @@ runTests() {
   ####################
 
   record_command run_wait_tests
-  record_command run_wait_with_non_existence_check_tests
 
   ####################
   # kubectl debug    #

test/cmd/wait.sh

@@ -105,11 +105,14 @@ EOF
     output_message_1=$(kubectl wait \
         --for='jsonpath=spec.template.spec.containers[?(@.name=="busybox")].image=busybox' \
         deploy/test-3)
+    # Command: Wait with jsonpath without value with check-once behavior
+    output_message_2=$(kubectl wait --for=jsonpath='{.status.replicas}' deploy/test-3 --timeout=0 2>&1)
     set -o errexit
 
     # Post-Condition: Wait succeed
     kube::test::if_has_string "${output_message_0}" 'deployment.apps/test-3 condition met'
     kube::test::if_has_string "${output_message_1}" 'deployment.apps/test-3 condition met'
+    kube::test::if_has_string "${output_message_2}" 'deployment.apps/test-3 condition met'
 
     # Clean deployment
     kubectl delete deployment test-3
@@ -117,106 +120,3 @@ EOF
     set +o nounset
     set +o errexit
 }
-
-run_wait_with_non_existence_check_tests() {
-    set -o nounset
-    set -o errexit
-
-    kube::log::status "Testing kubectl wait"
-
-    create_and_use_new_namespace
-
-    ### Wait for deletion using --all flag
-
-    # create test data
-    kubectl create deployment test-1 --image=busybox
-    kubectl create deployment test-2 --image=busybox
-
-    # Post-Condition: deployments exists
-    kube::test::get_object_assert "deployments" "{{range .items}}{{.metadata.name}},{{end}}" 'test-1,test-2,'
-
-    # wait with jsonpath will timout for busybox deployment
-    set +o errexit
-    # Command: Wait with jsonpath support fields not exist in the first place
-    output_message=$(kubectl wait --wait-for-creation --for=jsonpath=.status.readyReplicas=1 deploy/test-1 2>&1)
-    set -o errexit
-
-    # Post-Condition: Wait failed
-    kube::test::if_has_string "${output_message}" 'timed out'
-
-    # Delete all deployments async to kubectl wait
-    ( sleep 2 && kubectl delete deployment --all ) &
-
-    # Command: Wait for all deployments to be deleted
-    output_message=$(kubectl wait deployment --for=delete --all)
-
-    # Post-Condition: Wait was successful
-    kube::test::if_has_string "${output_message}" 'test-1 condition met'
-    kube::test::if_has_string "${output_message}" 'test-2 condition met'
-
-    # create test data to test timeout error is occurred in correct time
-    kubectl apply -f - <<EOF
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  labels:
-    app: dtest
-  name: dtest
-spec:
-  replicas: 3
-  selector:
-    matchLabels:
-      app: dtest
-  template:
-    metadata:
-      labels:
-        app: dtest
-    spec:
-      containers:
-      - name: bb
-        image: busybox
-        command: ["/bin/sh", "-c", "sleep infinity"]
-EOF
-
-    set +o errexit
-    # wait timeout error because condition is invalid
-    start_sec=$(date +"%s")
-    output_message=$(time kubectl wait pod --wait-for-creation --selector=app=dtest --for=condition=InvalidCondition --timeout=1s 2>&1)
-    end_sec=$(date +"%s")
-    len_sec=$((end_sec-start_sec))
-    set -o errexit
-    kube::test::if_has_string "${output_message}" 'timed out waiting for the condition '
-    test $len_sec -ge 1 && test $len_sec -le 2
-
-    # 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_0=$(kubectl wait --wait-for-creation --for=jsonpath='{.status.replicas}' deploy/test-3 2>&1)
-    # Command: Wait with relaxed jsonpath and filter expression
-    output_message_1=$(kubectl wait \
-        --for='jsonpath=spec.template.spec.containers[?(@.name=="busybox")].image=busybox' \
-        deploy/test-3)
-    set -o errexit
-
-    # Post-Condition: Wait succeed
-    kube::test::if_has_string "${output_message_0}" 'deployment.apps/test-3 condition met'
-    kube::test::if_has_string "${output_message_1}" 'deployment.apps/test-3 condition met'
-
-    # Clean deployment
-    kubectl delete deployment test-3
-
-    ( sleep 3 && kubectl create deployment test-4 --image=busybox ) &
-    output_message=$(kubectl wait --wait-for-creation --for=jsonpath=.status.replicas=1 deploy/test-4 2>&1)
-    kube::test::if_has_string "${output_message}" 'test-4 condition met'
-
-    kubectl delete deployment test-4
-
-    set +o nounset
-    set +o errexit
-}