feat: Add support for spec.ingressClassName (argoproj#2178)

This change adds support for `spec.ingressClassName` while still supporting `kubernetes.io/ingress.class` annotation for backwards compatibility. Fixes argoproj#1277 Signed-off-by: Siavash Safi <[email protected]> Signed-off-by: Siavash Safi <[email protected]> Signed-off-by: Xijun Dai <[email protected]>
daixijun · Sep 27, 2022 · f59958b · f59958b
1 parent 6b3a907
commit f59958b
Show file tree

Hide file tree

Showing 6 changed files with 161 additions and 19 deletions.
diff --git a/docs/features/traffic-management/alb.md b/docs/features/traffic-management/alb.md
@@ -212,7 +212,7 @@ controller to verify that TargetGroups are accurate before marking newly created
 preventing premature scale down of the older ReplicaSet.
 
 Pod readiness gate injection uses a mutating webhook which decides to inject readiness gates when a
-pod is created based on the following conditions:  
+pod is created based on the following conditions:
 
 * There exists a service matching the pod labels in the same namespace
 * There exists at least one target group binding that refers to the matching service
@@ -343,8 +343,8 @@ The Rollout status object holds the value of who is currently the stable ping or
 And this way allows the rollout to use pod readiness gate injection as the
 services are not changing their labels at the end of the rollout progress.
 
-!!!important 
-     
+!!!important
+
     Ping-Pong feature available since Argo Rollouts v1.2
 
 ## Example
@@ -368,7 +368,7 @@ spec:
         ports:
         - containerPort: 80
   strategy:
-    canary: 
+    canary:
       pingPong: #Indicates that the ping-pong services enabled
         pingService: ping-service
         pongService: pong-service
@@ -401,7 +401,7 @@ spec:
           annotationPrefix: custom.alb.ingress.kubernetes.io
 ```
 
-### Custom kubernetes.io/ingress.class
+### Custom Ingress Class
 
 By default, Argo Rollout will operate on Ingresses with the annotation:
 
@@ -413,14 +413,22 @@ metadata:
     kubernetes.io/ingress.class: alb
 ```
 
-To configure the controller to operate on Ingresses with different `kubernetes.io/ingress.class`
-values, the controller can specify a different value through the `--alb-ingress-classes` flag in
+Or with the `ingressClassName`:
+```yaml
+apiVersion: networking.k8s.io/v1beta1
+kind: Ingress
+spec:
+  ingressClassName: alb
+```
+
+To configure the controller to operate on Ingresses with a different class name,
+you can specify a different value through the `--alb-ingress-classes` flag in
 the controller command line arguments.
 
 
 Note that the `--alb-ingress-classes` flag can be specified multiple times if the Argo Rollouts
 controller should operate on multiple values. This may be desired when a cluster has multiple
-Ingress controllers that operate on different `kubernetes.io/ingress.class` values.
+Ingress controllers that operate on different `kubernetes.io/ingress.class` or `spec.ingressClassName` values.
 
 If the controller needs to operate on any Ingress without the `kubernetes.io/ingress.class`
-annotation, the flag can be specified with an empty string (e.g. `--alb-ingress-classes ''`).
+annotation or `spec.ingressClassName`, the flag can be specified with an empty string (e.g. `--alb-ingress-classes ''`).
diff --git a/docs/features/traffic-management/nginx.md b/docs/features/traffic-management/nginx.md
@@ -39,6 +39,6 @@ Since the Nginx Ingress controller allows users to configure the annotation pref
 
 
 ## Using Argo Rollouts with multiple NGINX ingress controllers
-As a default, the Argo Rollouts controller only operates on ingresses with the `kubernetes.io/ingress.class` annotation set to `nginx`. A user can configure the controller to operate on Ingresses with different `kubernetes.io/ingress.class` values 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 `kubernetes.io/ingress.class` values.
+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, a user should add the following `--nginx-ingress-classes ''`.
+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 ''`.
diff --git a/ingress/ingress.go b/ingress/ingress.go
@@ -140,12 +140,7 @@ func (c *Controller) syncIngress(key string) error {
 	if err != nil {
 		return nil
 	}
-	// An ingress without annotations cannot be a alb or nginx ingress
-	if ingress.GetAnnotations() == nil {
-		return nil
-	}
-	annotations := ingress.GetAnnotations()
-	class := annotations["kubernetes.io/ingress.class"]
+	class := ingress.GetClass()
 	switch {
 	case hasClass(c.albClasses, class):
 		return c.syncALBIngress(ingress, rollouts)

diff --git a/ingress/ingress_test.go b/ingress/ingress_test.go
@@ -23,6 +23,37 @@ import (
 )
 
 func newNginxIngress(name string, port int, serviceName string) *extensionsv1beta1.Ingress {
+	class := "nginx"
+	return &extensionsv1beta1.Ingress{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: metav1.NamespaceDefault,
+		},
+		Spec: extensionsv1beta1.IngressSpec{
+			IngressClassName: &class,
+			Rules: []extensionsv1beta1.IngressRule{
+				{
+					Host: "fakehost.example.com",
+					IngressRuleValue: extensionsv1beta1.IngressRuleValue{
+						HTTP: &extensionsv1beta1.HTTPIngressRuleValue{
+							Paths: []extensionsv1beta1.HTTPIngressPath{
+								{
+									Path: "/foo",
+									Backend: extensionsv1beta1.IngressBackend{
+										ServiceName: serviceName,
+										ServicePort: intstr.FromInt(port),
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+}
+
+func newNginxIngressWithAnnotation(name string, port int, serviceName string) *extensionsv1beta1.Ingress {
 	return &extensionsv1beta1.Ingress{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
@@ -165,8 +196,8 @@ func TestSyncIngressReferencedByRollout(t *testing.T) {
 	assert.Equal(t, 1, enqueuedObjects["default/rollout"])
 }
 
-func TestSkipIngressWithNoAnnotations(t *testing.T) {
-	ing := newNginxIngress("test-stable-ingress", 80, "stable-service")
+func TestSkipIngressWithNoClass(t *testing.T) {
+	ing := newNginxIngressWithAnnotation("test-stable-ingress", 80, "stable-service")
 	ing.Annotations = nil
 	rollout := &v1alpha1.Rollout{
 		ObjectMeta: metav1.ObjectMeta{

diff --git a/utils/ingress/wrapper.go b/utils/ingress/wrapper.go
@@ -95,6 +95,27 @@ func (i *Ingress) GetAnnotations() map[string]string {
 	}
 }
 
+// GetClass returns the ingress class.
+// For backwards compatibility `kubernetes.io/ingress.class` annotation will be used if set,
+// otherwise `spec.ingressClassName` is used.
+func (i *Ingress) GetClass() string {
+	annotations := i.GetAnnotations()
+	class := annotations["kubernetes.io/ingress.class"]
+	if class == "" {
+		switch i.mode {
+		case IngressModeNetworking:
+			if c := i.ingress.Spec.IngressClassName; c != nil {
+				class = *c
+			}
+		case IngressModeExtensions:
+			if c := i.legacyIngress.Spec.IngressClassName; c != nil {
+				class = *c
+			}
+		}
+	}
+	return class
+}
+
 func (i *Ingress) GetLabels() map[string]string {
 	switch i.mode {
 	case IngressModeNetworking:

diff --git a/utils/ingress/wrapper_test.go b/utils/ingress/wrapper_test.go
@@ -124,6 +124,93 @@ func TestGetNetworkingIngress(t *testing.T) {
 	})
 }
 
+func TestGetClass(t *testing.T) {
+	t.Run("will get the class from network Ingress annotation", func(t *testing.T) {
+		// given
+		t.Parallel()
+		i := getNetworkingIngress()
+		annotations := map[string]string{"kubernetes.io/ingress.class": "ingress-name-annotation"}
+		i.SetAnnotations(annotations)
+		emptyClass := ""
+		i.Spec.IngressClassName = &emptyClass
+		w := ingress.NewIngress(i)
+
+		// when
+		class := w.GetClass()
+
+		// then
+		assert.Equal(t, "ingress-name-annotation", class)
+	})
+	t.Run("will get the class from network Ingress annotation with priority", func(t *testing.T) {
+		// given
+		t.Parallel()
+		i := getNetworkingIngress()
+		annotations := map[string]string{"kubernetes.io/ingress.class": "ingress-name-annotation"}
+		i.SetAnnotations(annotations)
+		w := ingress.NewIngress(i)
+
+		// when
+		class := w.GetClass()
+
+		// then
+		assert.Equal(t, "ingress-name-annotation", class)
+	})
+	t.Run("will get the class from network Ingress spec", func(t *testing.T) {
+		// given
+		t.Parallel()
+		i := getNetworkingIngress()
+		w := ingress.NewIngress(i)
+
+		// when
+		class := w.GetClass()
+
+		// then
+		assert.Equal(t, "ingress-name", class)
+	})
+	t.Run("will get the class from extensions Ingress annotation", func(t *testing.T) {
+		// given
+		t.Parallel()
+		i := getExtensionsIngress()
+		annotations := map[string]string{"kubernetes.io/ingress.class": "ingress-name-annotation"}
+		i.SetAnnotations(annotations)
+		emptyClass := ""
+		i.Spec.IngressClassName = &emptyClass
+		w := ingress.NewLegacyIngress(i)
+
+		// when
+		class := w.GetClass()
+
+		// then
+		assert.Equal(t, "ingress-name-annotation", class)
+	})
+	t.Run("will get the class from extensions Ingress annotation with priority", func(t *testing.T) {
+		// given
+		t.Parallel()
+		i := getExtensionsIngress()
+		annotations := map[string]string{"kubernetes.io/ingress.class": "ingress-name-annotation"}
+		i.SetAnnotations(annotations)
+		w := ingress.NewLegacyIngress(i)
+
+		// when
+		class := w.GetClass()
+
+		// then
+		assert.Equal(t, "ingress-name-annotation", class)
+	})
+	t.Run("will get the class from extensions Ingress spec", func(t *testing.T) {
+		// given
+		t.Parallel()
+		i := getExtensionsIngress()
+		w := ingress.NewLegacyIngress(i)
+
+		// when
+		class := w.GetClass()
+
+		// then
+		assert.Equal(t, "ingress-name", class)
+	})
+}
+
 func TestGetLabels(t *testing.T) {
 	t.Run("will get the labels from network Ingress successfully", func(t *testing.T) {
 		// given