From 2bba39d723e299f2962d7587cac501c9cc4f94e3 Mon Sep 17 00:00:00 2001
From: Ryan Richard <richardry@vmware.com>
Date: Thu, 15 Jul 2021 13:41:31 -0700
Subject: [PATCH] TestAgentController unit test is flaky, try to add workaround

TestAgentController really runs the controller and evaluates multiple
calls to the controller's Sync with real informers caching updates.
There is a large amount of non-determinism in this unit test, and it
does not always behave the same way. Because it makes assertions about
the specific errors that should be returned by Sync, it was not
accounting for some errors that are only returned by Sync once in a
while depending on the exact (unpredictable) order of operations.

This commit doesn't fix the non-determinism in the test, but rather
tries to work around it by also allowing other (undesired but
inevitable) error messages to appear in the list of actual error
messages returned by the calls to the Sync function.

Signed-off-by: Margo Crawford <margaretc@vmware.com>
---
 .../kubecertagent/kubecertagent_test.go       | 38 +++++++++++++------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/internal/controller/kubecertagent/kubecertagent_test.go b/internal/controller/kubecertagent/kubecertagent_test.go
index f52c3c8e..049f95a6 100644
--- a/internal/controller/kubecertagent/kubecertagent_test.go
+++ b/internal/controller/kubecertagent/kubecertagent_test.go
@@ -207,16 +207,17 @@ func TestAgentController(t *testing.T) {
 	}
 
 	tests := []struct {
-		name                 string
-		discoveryURLOverride *string
-		pinnipedObjects      []runtime.Object
-		kubeObjects          []runtime.Object
-		addKubeReactions     func(*kubefake.Clientset)
-		mocks                func(*testing.T, *mocks.MockPodCommandExecutorMockRecorder, *mocks.MockDynamicCertPrivateMockRecorder, *cache.Expiring)
-		wantDistinctErrors   []string
-		wantDistinctLogs     []string
-		wantAgentDeployment  *appsv1.Deployment
-		wantStrategy         *configv1alpha1.CredentialIssuerStrategy
+		name                             string
+		discoveryURLOverride             *string
+		pinnipedObjects                  []runtime.Object
+		kubeObjects                      []runtime.Object
+		addKubeReactions                 func(*kubefake.Clientset)
+		mocks                            func(*testing.T, *mocks.MockPodCommandExecutorMockRecorder, *mocks.MockDynamicCertPrivateMockRecorder, *cache.Expiring)
+		wantDistinctErrors               []string
+		alsoAllowUndesiredDistinctErrors []string
+		wantDistinctLogs                 []string
+		wantAgentDeployment              *appsv1.Deployment
+		wantStrategy                     *configv1alpha1.CredentialIssuerStrategy
 	}{
 		{
 			name: "no CredentialIssuer found",
@@ -351,6 +352,10 @@ func TestAgentController(t *testing.T) {
 			wantDistinctErrors: []string{
 				"could not find a healthy agent pod (1 candidate)",
 			},
+			alsoAllowUndesiredDistinctErrors: []string{
+				// due to the high amount of nondeterminism in this test, this error will sometimes also happen, but is not required to happen
+				`could not ensure agent deployment: deployments.apps "pinniped-concierge-kube-cert-agent" already exists`,
+			},
 			wantDistinctLogs: []string{
 				`kube-cert-agent-controller "level"=0 "msg"="creating new deployment" "deployment"={"name":"pinniped-concierge-kube-cert-agent","namespace":"concierge"} "templatePod"={"name":"kube-controller-manager-1","namespace":"kube-system"}`,
 			},
@@ -395,6 +400,10 @@ func TestAgentController(t *testing.T) {
 			wantDistinctErrors: []string{
 				"could not find a healthy agent pod (1 candidate)",
 			},
+			alsoAllowUndesiredDistinctErrors: []string{
+				// due to the high amount of nondeterminism in this test, this error will sometimes also happen, but is not required to happen
+				`could not ensure agent deployment: deployments.apps "pinniped-concierge-kube-cert-agent" already exists`,
+			},
 			wantDistinctLogs: []string{
 				`kube-cert-agent-controller "level"=0 "msg"="creating new deployment" "deployment"={"name":"pinniped-concierge-kube-cert-agent","namespace":"concierge"} "templatePod"={"name":"kube-controller-manager-1","namespace":"kube-system"}`,
 			},
@@ -756,7 +765,14 @@ func TestAgentController(t *testing.T) {
 			defer cancel()
 
 			errorMessages := runControllerUntilQuiet(ctx, t, controller, kubeInformers, conciergeInformers)
-			assert.Equal(t, tt.wantDistinctErrors, deduplicate(errorMessages), "unexpected errors")
+
+			actualErrors := deduplicate(errorMessages)
+			require.Subsetf(t, actualErrors, tt.wantDistinctErrors, "required error(s) were not found in the actual errors")
+
+			allAllowedErrors := append([]string{}, tt.wantDistinctErrors...)
+			allAllowedErrors = append(allAllowedErrors, tt.alsoAllowUndesiredDistinctErrors...)
+			require.Subsetf(t, allAllowedErrors, actualErrors, "actual errors contained additional error(s) which is not expected by the test")
+
 			assert.Equal(t, tt.wantDistinctLogs, deduplicate(log.Lines()), "unexpected logs")
 
 			// Assert that the agent deployment is in the expected final state.