From 369534c6ec625ed9b251b453215c9578cdeb59bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Mon, 19 Dec 2022 20:21:56 +0300 Subject: [PATCH] kubectl wait: wire generic context (#114574) * Wire generic context to better handle timeout * Add integration test for wait timeout * kubectl wait: Fix integration test always passing issue Currently, `kubectl wait` integration test always passes even if it gets an error. Problem is object check is done after errexit is turned off. This PR redirects error to output and correctly assures that object is expected status and if it is not, test should fail. --- .../src/k8s.io/kubectl/pkg/cmd/wait/wait.go | 38 +++++++++------- test/cmd/wait.sh | 45 ++++++++++++++++--- 2 files changed, 61 insertions(+), 22 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 e9632332bb2..056bfef923d 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/wait/wait.go @@ -280,10 +280,13 @@ type WaitOptions struct { } // ConditionFunc is the interface for providing condition checks -type ConditionFunc func(info *resource.Info, o *WaitOptions) (finalObject runtime.Object, done bool, err error) +type ConditionFunc func(ctx context.Context, info *resource.Info, o *WaitOptions) (finalObject runtime.Object, done bool, err error) // RunWait runs the waiting logic func (o *WaitOptions) RunWait() error { + ctx, cancel := watchtools.ContextWithOptionalTimeout(context.Background(), o.Timeout) + defer cancel() + visitCount := 0 visitFunc := func(info *resource.Info, err error) error { if err != nil { @@ -291,7 +294,7 @@ func (o *WaitOptions) RunWait() error { } visitCount++ - finalObject, success, err := o.ConditionFn(info, o) + finalObject, success, err := o.ConditionFn(ctx, info, o) if success { o.Printer.PrintObj(finalObject, o.Out) return nil @@ -318,7 +321,7 @@ func (o *WaitOptions) RunWait() error { } // IsDeleted is a condition func for waiting for something to be deleted -func IsDeleted(info *resource.Info, o *WaitOptions) (runtime.Object, bool, error) { +func IsDeleted(ctx context.Context, info *resource.Info, o *WaitOptions) (runtime.Object, bool, error) { if len(info.Name) == 0 { return info.Object, false, fmt.Errorf("resource name must be provided") } @@ -357,9 +360,6 @@ func IsDeleted(info *resource.Info, o *WaitOptions) (runtime.Object, bool, error return info.Object, false, errWaitTimeoutWithName } - ctx, cancel := watchtools.ContextWithOptionalTimeout(context.Background(), o.Timeout) - defer cancel() - fieldSelector := fields.OneTermEqualSelector("metadata.name", info.Name).String() lw := &cache.ListWatch{ ListFunc: func(options metav1.ListOptions) (runtime.Object, error) { @@ -386,9 +386,14 @@ func IsDeleted(info *resource.Info, o *WaitOptions) (runtime.Object, bool, error return false, nil } + intrCtx, cancel := context.WithCancel(ctx) + defer cancel() intr := interrupt.New(nil, cancel) err := intr.Run(func() error { - _, err := watchtools.UntilWithSync(ctx, lw, &unstructured.Unstructured{}, preconditionFunc, Wait{errOut: o.ErrOut}.IsDeleted) + _, err := watchtools.UntilWithSync(intrCtx, lw, &unstructured.Unstructured{}, preconditionFunc, Wait{errOut: o.ErrOut}.IsDeleted) + if errors.Is(err, context.DeadlineExceeded) { + return errWaitTimeoutWithName + } return err }) if err != nil { @@ -427,7 +432,7 @@ type checkCondFunc func(obj *unstructured.Unstructured) (bool, error) // getObjAndCheckCondition will make a List query to the API server to get the object and check if the condition is met using check function. // If the condition is not met, it will make a Watch query to the server and pass in the condMet function -func getObjAndCheckCondition(info *resource.Info, o *WaitOptions, condMet isCondMetFunc, check checkCondFunc) (runtime.Object, bool, error) { +func getObjAndCheckCondition(ctx context.Context, info *resource.Info, o *WaitOptions, condMet isCondMetFunc, check checkCondFunc) (runtime.Object, bool, error) { if len(info.Name) == 0 { return info.Object, false, fmt.Errorf("resource name must be provided") } @@ -458,9 +463,6 @@ func getObjAndCheckCondition(info *resource.Info, o *WaitOptions, condMet isCond return info.Object, false, errWaitTimeoutWithName } - ctx, cancel := watchtools.ContextWithOptionalTimeout(context.Background(), o.Timeout) - defer cancel() - mapping := info.ResourceMapping() // used to pass back meaningful errors if object disappears fieldSelector := fields.OneTermEqualSelector("metadata.name", info.Name).String() lw := &cache.ListWatch{ @@ -487,14 +489,16 @@ func getObjAndCheckCondition(info *resource.Info, o *WaitOptions, condMet isCond return false, nil } + intrCtx, cancel := context.WithCancel(ctx) + defer cancel() var result runtime.Object intr := interrupt.New(nil, cancel) err := intr.Run(func() error { - ev, err := watchtools.UntilWithSync(ctx, lw, &unstructured.Unstructured{}, preconditionFunc, watchtools.ConditionFunc(condMet)) + ev, err := watchtools.UntilWithSync(intrCtx, lw, &unstructured.Unstructured{}, preconditionFunc, watchtools.ConditionFunc(condMet)) if ev != nil { result = ev.Object } - if err == context.DeadlineExceeded { + if errors.Is(err, context.DeadlineExceeded) { return errWaitTimeoutWithName } return err @@ -518,8 +522,8 @@ type ConditionalWait struct { } // IsConditionMet is a conditionfunc for waiting on an API condition to be met -func (w ConditionalWait) IsConditionMet(info *resource.Info, o *WaitOptions) (runtime.Object, bool, error) { - return getObjAndCheckCondition(info, o, w.isConditionMet, w.checkCondition) +func (w ConditionalWait) IsConditionMet(ctx context.Context, info *resource.Info, o *WaitOptions) (runtime.Object, bool, error) { + return getObjAndCheckCondition(ctx, info, o, w.isConditionMet, w.checkCondition) } func (w ConditionalWait) checkCondition(obj *unstructured.Unstructured) (bool, error) { @@ -592,8 +596,8 @@ type JSONPathWait struct { } // IsJSONPathConditionMet fulfills the requirements of the interface ConditionFunc which provides condition check -func (j JSONPathWait) IsJSONPathConditionMet(info *resource.Info, o *WaitOptions) (runtime.Object, bool, error) { - return getObjAndCheckCondition(info, o, j.isJSONPathConditionMet, j.checkCondition) +func (j JSONPathWait) IsJSONPathConditionMet(ctx context.Context, info *resource.Info, o *WaitOptions) (runtime.Object, bool, error) { + return getObjAndCheckCondition(ctx, info, o, j.isJSONPathConditionMet, j.checkCondition) } // isJSONPathConditionMet is a helper function of IsJSONPathConditionMet diff --git a/test/cmd/wait.sh b/test/cmd/wait.sh index 9144ac3a02d..474cbb5ba76 100644 --- a/test/cmd/wait.sh +++ b/test/cmd/wait.sh @@ -37,15 +37,13 @@ run_wait_tests() { # 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 --for=jsonpath=.status.readyReplicas=1 deploy/test-1) - + output_message=$(kubectl wait --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' - set -o errexit - # Delete all deployments async to kubectl wait ( sleep 2 && kubectl delete deployment --all ) & @@ -56,6 +54,43 @@ run_wait_tests() { 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 - <&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 ' + kube::test::if_has_string "${len_sec}" '1' + + # Clean deployment + kubectl delete deployment dtest + set +o nounset set +o errexit } \ No newline at end of file