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.
This commit is contained in:
Arda Güçlü 2022-12-19 20:21:56 +03:00 committed by GitHub
parent 0783cf49e8
commit 369534c6ec
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 61 additions and 22 deletions

View File

@ -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

View File

@ -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 - <<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 --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 '
kube::test::if_has_string "${len_sec}" '1'
# Clean deployment
kubectl delete deployment dtest
set +o nounset
set +o errexit
}