feat: better error handling for IngressWrapper

argoproj · Sep 30, 2021 · e26b2af · e26b2af
1 parent be512ab
commit e26b2af
Show file tree

Hide file tree

Showing 17 changed files with 605 additions and 245 deletions.
diff --git a/cmd/rollouts-controller/main.go b/cmd/rollouts-controller/main.go
@@ -144,7 +144,8 @@ func newCommand() *cobra.Command {
 
 			k8sRequestProvider := &metrics.K8sRequestsCountProvider{}
 			kubeclientmetrics.AddMetricsTransportWrapper(config, k8sRequestProvider.IncKubernetesRequest)
-			ingressWrapper := ingressutil.NewIngressWrapper(ingressutil.IngressModeExtensions, kubeClient, kubeInformerFactory)
+			ingressWrapper, err := ingressutil.NewIngressWrapper(ingressutil.IngressModeExtensions, kubeClient, kubeInformerFactory)
+			checkError(err)
 
 			cm := controller.NewManager(
 				namespace,

diff --git a/ingress/alb_test.go b/ingress/alb_test.go
@@ -101,7 +101,7 @@ func TestInvalidManagedALBActions(t *testing.T) {
 	ing := newALBIngress("test-ingress", 80, "stable-service", rollout.Name)
 	ing.Annotations[ingressutil.ManagedActionsAnnotation] = "invalid-managed-by"
 
-	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, rollout)
+	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, rollout)
 
 	err := ctrl.syncIngress("default/test-ingress")
 	assert.Nil(t, err)
@@ -113,7 +113,7 @@ func TestInvalidPreviousALBActionAnnotationValue(t *testing.T) {
 	ing := newALBIngress("test-ingress", 80, "stable-service", "not-existing-rollout")
 	ing.Annotations[albActionAnnotation("stable-service")] = "{"
 
-	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, nil)
+	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, nil)
 
 	err := ctrl.syncIngress("default/test-ingress")
 	assert.Nil(t, err)
@@ -124,7 +124,7 @@ func TestInvalidPreviousALBActionAnnotationValue(t *testing.T) {
 func TestInvalidPreviousALBActionAnnotationKey(t *testing.T) {
 	ing := newALBIngress("test-ingress", 80, "stable-service", "not-existing-rollout")
 	ing.Annotations[ingressutil.ManagedActionsAnnotation] = "invalid-action-key"
-	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, nil)
+	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, nil)
 
 	err := ctrl.syncIngress("default/test-ingress")
 	assert.Nil(t, err)
@@ -136,7 +136,7 @@ func TestResetActionFailureFindNoPort(t *testing.T) {
 	ing := newALBIngress("test-ingress", 80, "stable-service", "not-existing-rollout")
 	ing.Annotations[albActionAnnotation("stable-service")] = "{}"
 
-	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, nil)
+	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, nil)
 
 	err := ctrl.syncIngress("default/test-ingress")
 	assert.Nil(t, err)
@@ -148,7 +148,7 @@ func TestALBIngressNoModifications(t *testing.T) {
 	rollout := rollout("rollout", "stable-service", "test-ingress")
 	ing := newALBIngress("test-ingress", 80, "stable-service", rollout.Name)
 
-	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, rollout)
+	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, rollout)
 
 	err := ctrl.syncIngress("default/test-ingress")
 	assert.Nil(t, err)
@@ -159,7 +159,7 @@ func TestALBIngressNoModifications(t *testing.T) {
 func TestALBIngressResetAction(t *testing.T) {
 	ing := newALBIngress("test-ingress", 80, "stable-service", "non-existing-rollout")
 
-	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, nil)
+	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, nil)
 	err := ctrl.syncIngress("default/test-ingress")
 	assert.Nil(t, err)
 	assert.Len(t, enqueuedObjects, 0)

diff --git a/ingress/ingress_test.go b/ingress/ingress_test.go
@@ -54,7 +54,8 @@ func newNginxIngress(name string, port int, serviceName string) *extensionsv1bet
 	}
 }
 
-func newFakeIngressController(ing *extensionsv1beta1.Ingress, rollout *v1alpha1.Rollout) (*Controller, *k8sfake.Clientset, map[string]int) {
+func newFakeIngressController(t *testing.T, ing *extensionsv1beta1.Ingress, rollout *v1alpha1.Rollout) (*Controller, *k8sfake.Clientset, map[string]int) {
+	t.Helper()
 	client := fake.NewSimpleClientset()
 	if rollout != nil {
 		client = fake.NewSimpleClientset(rollout)
@@ -65,7 +66,10 @@ func newFakeIngressController(ing *extensionsv1beta1.Ingress, rollout *v1alpha1.
 	}
 	i := informers.NewSharedInformerFactory(client, 0)
 	k8sI := kubeinformers.NewSharedInformerFactory(kubeclient, 0)
-	ingressWrap := ingressutil.NewIngressWrapper(ingressutil.IngressModeExtensions, kubeclient, k8sI)
+	ingressWrap, err := ingressutil.NewIngressWrapper(ingressutil.IngressModeExtensions, kubeclient, k8sI)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	rolloutWorkqueue := workqueue.NewNamedRateLimitingQueue(queue.DefaultArgoRolloutsRateLimiter(), "Rollouts")
 	ingressWorkqueue := workqueue.NewNamedRateLimitingQueue(queue.DefaultArgoRolloutsRateLimiter(), "Ingresses")
@@ -112,7 +116,7 @@ func newFakeIngressController(ing *extensionsv1beta1.Ingress, rollout *v1alpha1.
 }
 
 func TestSyncMissingIngress(t *testing.T) {
-	ctrl, _, _ := newFakeIngressController(nil, nil)
+	ctrl, _, _ := newFakeIngressController(t, nil, nil)
 
 	err := ctrl.syncIngress("default/test-ingress")
 	assert.NoError(t, err)
@@ -121,7 +125,7 @@ func TestSyncMissingIngress(t *testing.T) {
 func TestSyncIngressNotReferencedByRollout(t *testing.T) {
 	ing := newNginxIngress("test-stable-ingress", 80, "test-stable-service")
 
-	ctrl, kubeclient, _ := newFakeIngressController(ing, nil)
+	ctrl, kubeclient, _ := newFakeIngressController(t, ing, nil)
 
 	err := ctrl.syncIngress("default/test-stable-ingress")
 	assert.NoError(t, err)
@@ -152,7 +156,7 @@ func TestSyncIngressReferencedByRollout(t *testing.T) {
 		},
 	}
 
-	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, rollout)
+	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, rollout)
 
 	err := ctrl.syncIngress("default/test-stable-ingress")
 	assert.NoError(t, err)
@@ -184,7 +188,7 @@ func TestSkipIngressWithNoAnnotations(t *testing.T) {
 		},
 	}
 
-	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, rollout)
+	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, rollout)
 
 	err := ctrl.syncIngress("default/test-stable-ingress")
 	assert.NoError(t, err)