CMP-3895: Improve rescan wait logic for e2e tests (#835)

The e2e tests have been failing with what appears to be a transient race
condition when a test rescans the environment, then waits for the suite
or scan to get to a terminal state, like DONE and NON-COMPLIANT.

The problem is that the test does something to initiate a rescan
(deleting a suite, creating a scan setting binding, etc), and then it
immediately starts looking for the ComplianceSuite status to see if it's
done.

In the case of rescans, the data from the last scan might still be
lingering around and if the test is faster than the reconcile loops of
the ComplianceScan controller, it could breeze through the wait logic
and think the scan is already complete.

This commit attempts to address that by using the rescan annotation. By
using the annotation to rescan (instead of deleting a scan suite
entirely), we can update the function that polls the suite status to
also check for the annotations. If the wait function sees the
annotation, it should continue polling since that's a clear signal that
the ComplianceScan controller has not updated the scan data or reset the
states back to PENDING. If none of the scans in the suite have the
annotation, then it's probably safe to move forward and assume the
status for each scan is updated and accurate, since the ComplianceScan
controller will remove the rescan annotation when it restarts the scan.

Author: rhmdnd

tests/e2e/framework/common.go

@@ -1176,6 +1176,40 @@ func (f *Framework) WaitForSuiteScansStatus(namespace, name string, targetStatus
 			return false, nil
 		}
 
+		// Loop through each scan to make sure it doesn't have the
+		// rescan annotation. This makes it safer to poll for suite
+		// status because we're making sure the ComplianceScan
+		// controller has the opportunity to update the scan status for
+		// each scan and remove the annotation before we look at the
+		// overall suite status. Without this, it's possible to rerun a
+		// scan and immediately pass through the
+		// WaitForSuiteScansStatus function because the ComplianceScan
+		// objects are still referencing the old scan data and haven't
+		// been reset to "PENDING".
+		for _, scanStatus := range suite.Status.ScanStatuses {
+			key := types.NamespacedName{Name: scanStatus.Name, Namespace: namespace}
+			scan := &compv1alpha1.ComplianceScan{}
+			err := f.Client.Get(context.TODO(), key, scan)
+			if err != nil {
+				if apierrors.IsNotFound(err) {
+					log.Printf("Waiting for scan %s to appear in suite: %s", scanStatus.Name, name)
+					return false, nil
+				}
+				return false, fmt.Errorf("failed to get scan %s: %s", scanStatus.Name, err)
+			}
+			if _, exists := scan.Annotations[compv1alpha1.ComplianceScanRescanAnnotation]; !exists {
+				// Move along, the rescan annotation has been
+				// removed (if the caller did use it to perform
+				// a rescan) by the ComplianceScan controller
+				// so we know we're not looking at stale
+				// results from a previous run.
+				continue
+			}
+
+			log.Printf("Waiting for ComplianceScan controller to remove rescan annotation for %s", scan.Name)
+			return false, nil
+		}
+
 		if suite.Status.Phase != targetStatus {
 			log.Printf("waiting until suite %s reaches target status '%s'. Current status: %s", suite.Name, targetStatus, suite.Status.Phase)
 			return false, nil
@@ -1228,6 +1262,62 @@ func (f *Framework) WaitForSuiteScansStatus(namespace, name string, targetStatus
 	return nil
 }
 
+func (f *Framework) RescanSuite(suiteName, namespace string) error {
+	scanList := &compv1alpha1.ComplianceScanList{}
+	labelSelector, err := labels.Parse(compv1alpha1.SuiteLabel + "=" + suiteName)
+	if err != nil {
+		return fmt.Errorf("Failed to parse label selector: %s", err)
+	}
+
+	opts := &client.ListOptions{
+		LabelSelector: labelSelector,
+	}
+	err = f.Client.List(context.TODO(), scanList, opts)
+	if err != nil {
+		return fmt.Errorf("Failed to get scans for suite %s", suiteName)
+	}
+
+	// Add rescan annotation to each scan
+	for i := range scanList.Items {
+		scan := &scanList.Items[i]
+		err := f.RescanScan(scan.Name, scan.Namespace)
+		if err != nil {
+			return fmt.Errorf("Failed to apply rescan annotation to scan %s: %s", scan.Name, err)
+		}
+	}
+	return nil
+}
+
+func (f *Framework) RescanScan(scanName, namespace string) error {
+	key := types.NamespacedName{Name: scanName, Namespace: namespace}
+
+	interval := 5 * time.Second
+	retries := uint64(30)
+	bo := backoff.WithMaxRetries(backoff.NewConstantBackOff(interval), retries)
+	err := backoff.RetryNotify(func() error {
+		s := &compv1alpha1.ComplianceScan{}
+		err := f.Client.Get(context.TODO(), key, s)
+		if err != nil {
+			return fmt.Errorf("failed to get scan %s: %s", scanName, err)
+		}
+
+		// Add rescan annotation
+		if s.Annotations == nil {
+			s.Annotations = make(map[string]string)
+		}
+		s.Annotations[compv1alpha1.ComplianceScanRescanAnnotation] = ""
+
+		return f.Client.Update(context.TODO(), s)
+	}, bo, func(err error, d time.Duration) {
+		log.Printf("Failed to add rescan annotation to scan %s after %s: %s", scanName, d.String(), err)
+	})
+	if err != nil {
+		return fmt.Errorf("Failed to trigger rescan for scan %s: %s", scanName, err)
+	}
+	log.Printf("Triggered rescan for scan %s", scanName)
+	return nil
+}
+
 func (f *Framework) logContainerOutput(namespace, name string) {
 	logContainerOutputEnv := os.Getenv("LOG_CONTAINER_OUTPUT")
 	if logContainerOutputEnv == "" {

tests/e2e/parallel/main_test.go

@@ -2546,10 +2546,10 @@ func TestCustomRuleTailoredProfile(t *testing.T) {
 	if err := f.Client.Get(context.TODO(), key, suite); err != nil {
 		t.Fatal(err)
 	}
-	// let's rescans and expect the check to be non compliant by deleting the suite
-	err = f.Client.Delete(context.TODO(), suite)
+	// Annotate all ComplianceScans in the suite to generate fresh results
+	err = f.RescanSuite(suiteName, testNamespace)
 	if err != nil {
-		t.Fatalf("Failed to delete suite: %v", err)
+		t.Fatalf("Failed to rescan suite: %s", err)
 	}
 	err = f.WaitForSuiteScansStatus(testNamespace, suiteName, compv1alpha1.PhaseDone, compv1alpha1.ResultNonCompliant)
 	if err != nil {
@@ -3459,10 +3459,10 @@ func TestCustomRuleFailureReasonInCheckResult(t *testing.T) {
 		t.Fatalf("Failed to get ComplianceSuite: %v", err)
 	}
 
-	// Delete and recreate the suite to trigger a new scan
-	err = f.Client.Delete(context.TODO(), suite)
+	// Annotate all ComplianceScans in the suite to generate fresh results
+	err = f.RescanSuite(suiteName, testNamespace)
 	if err != nil {
-		t.Fatalf("Failed to delete ComplianceSuite: %v", err)
+		t.Fatalf("Failed to rescan ComplianceSuite: %v", err)
 	}
 
 	// Wait for the new scan to complete