Wrap Timeout errors within Condition steps (#3005)

pkg/util/steps/condition.go

@@ -5,6 +5,7 @@ package steps
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
@@ -54,12 +55,20 @@ func (c conditionStep) run(ctx context.Context, log *logrus.Entry) error {
 
 	// Run the condition function immediately, and then every
 	// runner.pollInterval, until the condition returns true or timeoutCtx's
-	// timeout fires. Errors from `f` are returned directly.
+	// timeout fires. Errors from `f` are returned directly unless the error
+	// is ErrWaitTimeout. Internal ErrWaitTimeout errors are wrapped to avoid
+	// confusion with wait.PollImmediateUntil's own behavior of returning
+	// ErrWaitTimeout when the condition is not met.
 	err := wait.PollImmediateUntil(pollInterval, func() (bool, error) {
 		// We use the outer context, not the timeout context, as we do not want
 		// to time out the condition function itself, only stop retrying once
 		// timeoutCtx's timeout has fired.
-		return c.f(ctx)
+		cnd, cndErr := c.f(ctx)
+		if errors.Is(cndErr, wait.ErrWaitTimeout) {
+			return cnd, fmt.Errorf("condition encountered internal timeout: %w", cndErr)
+		}
+
+		return cnd, cndErr
 	}, timeoutCtx.Done())
 
 	if err != nil && !c.fail {

pkg/util/steps/runner_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/onsi/gomega"
 	"github.com/onsi/gomega/types"
 	"github.com/sirupsen/logrus"
+	"k8s.io/apimachinery/pkg/util/wait"
 
 	utilerror "github.com/Azure/ARO-RP/test/util/error"
 	testlog "github.com/Azure/ARO-RP/test/util/log"
@@ -26,6 +27,9 @@ func timingOutCondition(ctx context.Context) (bool, error) {
 	time.Sleep(60 * time.Millisecond)
 	return false, nil
 }
+func internalTimeoutCondition(ctx context.Context) (bool, error) {
+	return false, wait.ErrWaitTimeout
+}
 
 func currentTimeFunc() time.Time {
 	return time.Now()
@@ -169,6 +173,36 @@ func TestStepRunner(t *testing.T) {
 			},
 			wantErr: "timed out waiting for the condition",
 		},
+		{
+			name: "A Condition that returns a timeout error causes a different failure from a timed out Condition",
+			steps: func(controller *gomock.Controller) []Step {
+				return []Step{
+					Action(successfulFunc),
+					&conditionStep{
+						f:            internalTimeoutCondition,
+						fail:         true,
+						pollInterval: 20 * time.Millisecond,
+						timeout:      50 * time.Millisecond,
+					},
+					Action(successfulFunc),
+				}
+			},
+			wantEntries: []map[string]types.GomegaMatcher{
+				{
+					"msg":   gomega.Equal("running step [Action github.com/Azure/ARO-RP/pkg/util/steps.successfulFunc]"),
+					"level": gomega.Equal(logrus.InfoLevel),
+				},
+				{
+					"msg":   gomega.Equal("running step [Condition github.com/Azure/ARO-RP/pkg/util/steps.internalTimeoutCondition, timeout 50ms]"),
+					"level": gomega.Equal(logrus.InfoLevel),
+				},
+				{
+					"msg":   gomega.Equal("step [Condition github.com/Azure/ARO-RP/pkg/util/steps.internalTimeoutCondition, timeout 50ms] encountered error: condition encountered internal timeout: timed out waiting for the condition"),
+					"level": gomega.Equal(logrus.ErrorLevel),
+				},
+			},
+			wantErr: "condition encountered internal timeout: timed out waiting for the condition",
+		},
 		{
 			name: "A Condition that does not return true in the timeout time causes a failure",
 			steps: func(controller *gomock.Controller) []Step {