feat: support N nginx ingresses

docs/features/specification.md

@@ -387,7 +387,12 @@ spec:
 
         # NGINX Ingress Controller routing configuration
         nginx:
-          stableIngress: primary-ingress  # required
+          # Either stableIngress or stableIngresses must be configured, but not both.
+          stableIngress: primary-ingress
+          stableIngresses:
+            - primary-ingress
+            - secondary-ingress
+            - tertiary-ingress
           annotationPrefix: customingress.nginx.ingress.kubernetes.io # optional
           additionalIngressAnnotations:   # optional
             canary-by-header: X-Canary

docs/features/traffic-management/nginx.md

@@ -24,7 +24,12 @@ spec:
       stableService: stable-service  # required
       trafficRouting:
         nginx:
-          stableIngress: primary-ingress  # required
+          # Either stableIngress or stableIngresses must be configured, but not both.
+          stableIngress: primary-ingress
+          stableIngresses:
+            - primary-ingress
+            - secondary-ingress
+            - tertiary-ingress
           annotationPrefix: customingress.nginx.ingress.kubernetes.io # optional
           additionalIngressAnnotations:   # optional
             canary-by-header: X-Canary
@@ -38,7 +43,11 @@ The controller routes traffic to the canary Service by creating a second Ingress
 Since the Nginx Ingress controller allows users to configure the annotation prefix used by the Ingress controller, Rollouts can specify the optional `annotationPrefix` field. The canary Ingress uses that prefix instead of the default `nginx.ingress.kubernetes.io` if the field set.
 
 
-## Using Argo Rollouts with multiple NGINX ingress controllers
+## Using Argo Rollouts with multiple NGINX ingress controllers per service
+Starting with v1.5, argo rollouts supports multiple Nginx ingress controllers pointing at one service with canary deployments. If only one ingress controller is needed, utilize the existing key `stableIngress`. If multiple ingress controllers are needed (e.g., separating internal vs external traffic), use the key `stableIngresses` instead. It takes an array of string values that are the names of the ingress controllers. Canary steps are applied identically across all ingress controllers.
+
+
+## Using Argo Rollouts with custom NGINX ingress controller names
 As a default, the Argo Rollouts controller only operates on ingresses with the `kubernetes.io/ingress.class` annotation or `spec.ingressClassName` set to `nginx`. A user can configure the controller to operate on Ingresses with different class name by specifying the `--nginx-ingress-classes` flag. A user can list the `--nginx-ingress-classes` flag multiple times if the Argo Rollouts controller should operate on multiple values. This solves the case where a cluster has multiple Ingress controllers operating on different class values.
 
 If the user would like the controller to operate on any Ingress without the `kubernetes.io/ingress.class` annotation or `spec.ingressClassName`, a user should add the following `--nginx-ingress-classes ''`.

ingress/ingress_test.go

@@ -2,6 +2,7 @@ package ingress
 
 import (
 	"context"
+	"fmt"
 	"sync"
 	"testing"
 	"time"
@@ -11,6 +12,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	kubeinformers "k8s.io/client-go/informers"
 	k8sfake "k8s.io/client-go/kubernetes/fake"
@@ -24,6 +26,15 @@ import (
 	"k8s.io/client-go/tools/cache"
 )
 
+const stableService string = "test-stable-service"
+const additionalIngress string = "test-stable-ingress-additional"
+const stableIngress string = "test-stable-ingress"
+
+func testString(val string) string {
+	return fmt.Sprintf("default/%s", val)
+
+}
+
 func newNginxIngress(name string, port int, serviceName string) *extensionsv1beta1.Ingress {
 	class := "nginx"
 	return &extensionsv1beta1.Ingress{
@@ -87,15 +98,29 @@ func newNginxIngressWithAnnotation(name string, port int, serviceName string) *e
 	}
 }
 
+func newFakeIngressControllerMultiIngress(t *testing.T, ing []*extensionsv1beta1.Ingress, rollout *v1alpha1.Rollout) (*Controller, *k8sfake.Clientset, map[string]int) {
+	return underlyingControllerBuilder(t, ing, rollout)
+}
+
 func newFakeIngressController(t *testing.T, ing *extensionsv1beta1.Ingress, rollout *v1alpha1.Rollout) (*Controller, *k8sfake.Clientset, map[string]int) {
+	return underlyingControllerBuilder(t, []*extensionsv1beta1.Ingress{ing}, rollout)
+}
+
+func underlyingControllerBuilder(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)
 	}
 	kubeclient := k8sfake.NewSimpleClientset()
 	if ing != nil {
-		kubeclient = k8sfake.NewSimpleClientset(ing)
+		var x []runtime.Object
+		for _, i := range ing {
+			if i != nil {
+				x = append(x, i)
+			}
+		}
+		kubeclient = k8sfake.NewSimpleClientset(x...)
 	}
 	i := informers.NewSharedInformerFactory(client, 0)
 	k8sI := kubeinformers.NewSharedInformerFactory(kubeclient, 0)
@@ -140,7 +165,11 @@ func newFakeIngressController(t *testing.T, ing *extensionsv1beta1.Ingress, roll
 	}
 
 	if ing != nil {
-		k8sI.Extensions().V1beta1().Ingresses().Informer().GetIndexer().Add(ing)
+		for _, i := range ing {
+			if i != nil {
+				k8sI.Extensions().V1beta1().Ingresses().Informer().GetIndexer().Add(i)
+			}
+		}
 	}
 	if rollout != nil {
 		i.Argoproj().V1alpha1().Rollouts().Informer().GetIndexer().Add(rollout)
@@ -156,78 +185,184 @@ func TestSyncMissingIngress(t *testing.T) {
 }
 
 func TestSyncIngressNotReferencedByRollout(t *testing.T) {
-	ing := newNginxIngress("test-stable-ingress", 80, "test-stable-service")
 
-	ctrl, kubeclient, _ := newFakeIngressController(t, ing, nil)
+	tests := []struct {
+		ings []*extensionsv1beta1.Ingress
+		name string
+		keys []string
+	}{
+		{
+			[]*extensionsv1beta1.Ingress{
+				newNginxIngress(stableIngress, 80, stableService),
+			},
+			"Single Ingress",
+			[]string{
+				testString(stableIngress),
+			},
+		},
+		{
+			[]*extensionsv1beta1.Ingress{
+				newNginxIngress(stableIngress, 80, stableService),
+				newNginxIngress(additionalIngress, 80, stableService),
+			},
+			"Multi Ingress",
+			[]string{
+				testString(stableIngress),
+				testString(additionalIngress),
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
 
-	err := ctrl.syncIngress(context.Background(), "default/test-stable-ingress")
-	assert.NoError(t, err)
-	actions := kubeclient.Actions()
-	assert.Len(t, actions, 0)
+			ctrl, kubeclient, _ := newFakeIngressControllerMultiIngress(t, test.ings, nil)
+			for _, key := range test.keys {
+				err := ctrl.syncIngress(context.Background(), key)
+				assert.NoError(t, err)
+				actions := kubeclient.Actions()
+				assert.Len(t, actions, 0)
+			}
+		})
+	}
 }
 
 func TestSyncIngressReferencedByRollout(t *testing.T) {
-	ing := newNginxIngress("test-stable-ingress", 80, "stable-service")
-
-	rollout := &v1alpha1.Rollout{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "rollout",
-			Namespace: metav1.NamespaceDefault,
+	tests := []struct {
+		ings                   []*extensionsv1beta1.Ingress
+		name                   string
+		keys                   []string
+		additionalIngressNames []string
+	}{
+		{
+			[]*extensionsv1beta1.Ingress{
+				newNginxIngress(stableIngress, 80, stableService),
+			},
+			"Single Ingress",
+			[]string{
+				testString(stableIngress),
+			},
+			[]string{},
 		},
-		Spec: v1alpha1.RolloutSpec{
-			Strategy: v1alpha1.RolloutStrategy{
-				Canary: &v1alpha1.CanaryStrategy{
-					StableService: "stable-service",
-					CanaryService: "canary-service",
-					TrafficRouting: &v1alpha1.RolloutTrafficRouting{
-						Nginx: &v1alpha1.NginxTrafficRouting{
-							StableIngress: "test-stable-ingress",
-						},
-					},
-				},
+		{
+			[]*extensionsv1beta1.Ingress{
+				newNginxIngress(stableIngress, 80, stableService),
+				newNginxIngress(additionalIngress, 80, stableService),
+			},
+			"Multi Ingress",
+			[]string{
+				testString(stableIngress),
+				testString(additionalIngress),
+				testString(additionalIngress),
 			},
+			[]string{additionalIngress},
 		},
 	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
 
-	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, rollout)
+			rollout := &v1alpha1.Rollout{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "rollout",
+					Namespace: metav1.NamespaceDefault,
+				},
+				Spec: v1alpha1.RolloutSpec{
+					Strategy: v1alpha1.RolloutStrategy{
+						Canary: &v1alpha1.CanaryStrategy{
+							StableService: stableService,
+							CanaryService: "canary-service",
+							TrafficRouting: &v1alpha1.RolloutTrafficRouting{
+								Nginx: &v1alpha1.NginxTrafficRouting{
+									StableIngress:   stableIngress,
+									StableIngresses: test.additionalIngressNames,
+								},
+							},
+						},
+					},
+				},
+			}
 
-	err := ctrl.syncIngress(context.Background(), "default/test-stable-ingress")
-	assert.NoError(t, err)
-	actions := kubeclient.Actions()
-	assert.Len(t, actions, 0)
-	assert.Equal(t, 1, enqueuedObjects["default/rollout"])
+			ctrl, kubeclient, enqueuedObjects := newFakeIngressControllerMultiIngress(t, test.ings, rollout)
+
+			for i, key := range test.keys {
+				err := ctrl.syncIngress(context.Background(), key)
+				assert.NoError(t, err)
+				actions := kubeclient.Actions()
+				assert.Len(t, actions, 0)
+				assert.Equal(t, i+1, enqueuedObjects["default/rollout"])
+			}
+		})
+	}
 }
 
 func TestSkipIngressWithNoClass(t *testing.T) {
-	ing := newNginxIngressWithAnnotation("test-stable-ingress", 80, "stable-service")
-	ing.Annotations = nil
-	rollout := &v1alpha1.Rollout{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "rollout",
-			Namespace: metav1.NamespaceDefault,
+	tests := []struct {
+		ings                   []*extensionsv1beta1.Ingress
+		name                   string
+		keys                   []string
+		additionalIngressNames []string
+	}{
+		{
+			[]*extensionsv1beta1.Ingress{
+				newNginxIngressWithAnnotation(stableIngress, 80, stableService),
+			},
+			"Single Ingress",
+			[]string{
+				testString(stableIngress),
+			},
+			[]string{},
 		},
-		Spec: v1alpha1.RolloutSpec{
-			Strategy: v1alpha1.RolloutStrategy{
-				Canary: &v1alpha1.CanaryStrategy{
-					StableService: "stable-service",
-					CanaryService: "canary-service",
-					TrafficRouting: &v1alpha1.RolloutTrafficRouting{
-						Nginx: &v1alpha1.NginxTrafficRouting{
-							StableIngress: "test-stable-ingress",
-						},
-					},
-				},
+		{
+			[]*extensionsv1beta1.Ingress{
+				newNginxIngressWithAnnotation(stableIngress, 80, stableService),
+				newNginxIngressWithAnnotation(additionalIngress, 80, stableService),
 			},
+			"Multi Ingress",
+			[]string{
+				testString(stableIngress),
+				testString(additionalIngress),
+			},
+			[]string{stableIngress},
 		},
 	}
 
-	ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, rollout)
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			for _, i := range test.ings {
+				i.Annotations = nil
+			}
 
-	err := ctrl.syncIngress(context.Background(), "default/test-stable-ingress")
-	assert.NoError(t, err)
-	actions := kubeclient.Actions()
-	assert.Len(t, actions, 0)
-	assert.Len(t, enqueuedObjects, 0)
+			rollout := &v1alpha1.Rollout{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "rollout",
+					Namespace: metav1.NamespaceDefault,
+				},
+				Spec: v1alpha1.RolloutSpec{
+					Strategy: v1alpha1.RolloutStrategy{
+						Canary: &v1alpha1.CanaryStrategy{
+							StableService: stableService,
+							CanaryService: "canary-service",
+							TrafficRouting: &v1alpha1.RolloutTrafficRouting{
+								Nginx: &v1alpha1.NginxTrafficRouting{
+									StableIngress:   stableIngress,
+									StableIngresses: test.additionalIngressNames,
+								},
+							},
+						},
+					},
+				},
+			}
+
+			ctrl, kubeclient, enqueuedObjects := newFakeIngressControllerMultiIngress(t, test.ings, rollout)
+
+			for _, key := range test.keys {
+				err := ctrl.syncIngress(context.Background(), key)
+				assert.NoError(t, err)
+				actions := kubeclient.Actions()
+				assert.Len(t, actions, 0)
+				assert.Len(t, enqueuedObjects, 0)
+			}
+		})
+	}
 }
 
 func TestRun(t *testing.T) {

manifests/crds/rollout-crd.yaml

@@ -864,8 +864,10 @@ spec:
                                 type: string
                               stableIngress:
                                 type: string
-                            required:
-                            - stableIngress
+                              stableIngresses:
+                                items:
+                                  type: string
+                                type: array
                             type: object
                           plugins:
                             type: object

manifests/install.yaml

@@ -11919,8 +11919,10 @@ spec:
                                 type: string
                               stableIngress:
                                 type: string
-                            required:
-                            - stableIngress
+                              stableIngresses:
+                                items:
+                                  type: string
+                                type: array
                             type: object
                           plugins:
                             type: object

pkg/apiclient/rollout/rollout.swagger.json

@@ -1081,6 +1081,13 @@
             "type": "string"
           },
           "title": "+optional"
+        },
+        "stableIngresses": {
+          "type": "array",
+          "items": {
+            "type": "string"
+          },
+          "title": "StableIngresses refers to the names of `Ingress` resources in the same namespace as the `Rollout` in a multi ingress scenario\n+optional"
         }
       },
       "title": "NginxTrafficRouting configuration for Nginx ingress controller to control traffic routing"