OCPBUGS-6850: [release-4.12] upgrade/adminack: guarantee one admin ack check post-upgrade

test/extended/util/openshift/clusterversionoperator/adminack.go

@@ -14,6 +14,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/kubernetes/test/e2e/framework"
@@ -40,35 +41,43 @@ var adminAckGateRegexp = regexp.MustCompile(adminAckGateFmt)
 // and verifies that the Upgradeable condition no longer complains about the ack.
 func (t *AdminAckTest) Test(ctx context.Context) {
 	if t.Poll == 0 {
-		t.test(ctx, nil)
+		if err := t.test(ctx, nil, nil); err != nil {
+			framework.Fail(err.Error())
+		}
 		return
 	}
 
-	exercisedGates := map[string]struct{}{}
+	exercisedGates := sets.NewString()
+	exercisedVersions := sets.NewString()
 	success := false
 	var lastError error
-	if err := wait.PollImmediateUntilWithContext(ctx, t.Poll, func(ctx context.Context) (bool, error) {
-		if err := t.test(ctx, exercisedGates); err != nil {
+	wait.UntilWithContext(ctx, func(ctx context.Context) {
+		if err := t.test(ctx, exercisedGates, exercisedVersions); err != nil {
 			framework.Logf("Retriable failure to evaluate admin acks: %v", err)
 			lastError = err
 		} else {
 			success = true
 		}
-		return false, nil
-	}); err == nil || err == wait.ErrWaitTimeout {
-		return
-	} else {
-		framework.Fail(err.Error())
-	}
+	}, t.Poll)
 
 	if !success {
 		framework.Failf("Never able to evaluate admin acks.  Most recent failure: %v", lastError)
 	}
-}
 
-func (t *AdminAckTest) test(ctx context.Context, exercisedGates map[string]struct{}) error {
-	exists := struct{}{}
+	// Perform one guaranteed check after the upgrade is complete. The polled check above is cancelled
+	// on done signal (so we never know whether the poll was lucky to run at least once since
+	// the version was bumped).
+	postUpdateCtx, postUpdateCtxCancel := context.WithTimeout(context.Background(), 15*time.Minute)
+	defer postUpdateCtxCancel()
+	if current := getCurrentVersion(postUpdateCtx, t.Config); current != "" && !exercisedVersions.Has(current) {
+		// We never saw the current version while polling, so lets check it now
+		if err := t.test(postUpdateCtx, exercisedGates, exercisedVersions); err != nil {
+			framework.Fail(err.Error())
+		}
+	}
+}
 
+func (t *AdminAckTest) test(ctx context.Context, exercisedGates, exercisedVersions sets.String) error {
 	gateCm, err := getAdminGatesConfigMap(ctx, t.Oc)
 	if err != nil {
 		return err
@@ -83,11 +92,12 @@ func (t *AdminAckTest) test(ctx context.Context, exercisedGates map[string]struc
 		return err
 	}
 	currentVersion := getCurrentVersion(ctx, t.Config)
+	if exercisedVersions != nil {
+		exercisedVersions.Insert(currentVersion)
+	}
 	for k, v := range gateCm.Data {
-		if exercisedGates != nil {
-			if _, ok := exercisedGates[k]; ok {
-				continue
-			}
+		if exercisedGates != nil && exercisedGates.Has(k) {
+			continue
 		}
 		ackVersion := adminAckGateRegexp.FindString(k)
 		if ackVersion == "" {
@@ -97,6 +107,7 @@ func (t *AdminAckTest) test(ctx context.Context, exercisedGates map[string]struc
 			framework.Failf("Configmap openshift-config-managed/admin-gates gate %s does not contain description.", k)
 		}
 		if !gateApplicableToCurrentVersion(ackVersion, currentVersion) {
+			framework.Logf("Gate %s not applicable to current version %s", ackVersion, currentVersion)
 			continue
 		}
 		if ackCm.Data[k] == "true" {
@@ -124,7 +135,7 @@ func (t *AdminAckTest) test(ctx context.Context, exercisedGates map[string]struc
 			framework.Fail(err.Error())
 		}
 		if exercisedGates != nil {
-			exercisedGates[k] = exists
+			exercisedGates.Insert(k)
 		}
 	}
 	framework.Logf("Admin Ack verified")
@@ -239,9 +250,14 @@ func setAdminGate(ctx context.Context, gateName string, gateValue string, oc *ex
 	return nil
 }
 
+// adminAckDeadline is the upper bound of time for CVO to notice a new adminack
+// gate. CVO sync loop duration is nondeterministic 2-4m interval so we set this
+// slightly above the worst case.
+const adminAckDeadline = 4*time.Minute + 5*time.Second
+
 func waitForAdminAckRequired(ctx context.Context, config *restclient.Config, message string) error {
 	framework.Logf("Waiting for Upgradeable to be AdminAckRequired for %q ...", message)
-	if err := wait.PollImmediate(10*time.Second, 3*time.Minute, func() (bool, error) {
+	if err := wait.PollImmediate(10*time.Second, adminAckDeadline, func() (bool, error) {
 		if adminAckRequiredWithMessage(ctx, config, message) {
 			return true, nil
 		}