Merge pull request argoproj#1 from n888/master-multi-alb

integrate multi-alb
huynhsontung · Dec 7, 2022 · ad8232a · ad8232a
2 parents 086a1e0 + 72ca604
commit ad8232a
Show file tree

Hide file tree

Showing 15 changed files with 1,201 additions and 122 deletions.
diff --git a/manifests/crds/rollout-crd.yaml b/manifests/crds/rollout-crd.yaml
@@ -680,6 +680,10 @@ spec:
                                 type: string
                               ingress:
                                 type: string
+                              additionalIngresses:
+                                items:
+                                  type: string
+                                type: array
                               rootService:
                                 type: string
                               servicePort:

diff --git a/manifests/install.yaml b/manifests/install.yaml
@@ -11690,6 +11690,10 @@ spec:
                                 type: string
                               ingress:
                                 type: string
+                              additionalIngresses:
+                                items:
+                                  type: string
+                                type: array
                               rootService:
                                 type: string
                               servicePort:

diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml
@@ -11690,6 +11690,10 @@ spec:
                                 type: string
                               ingress:
                                 type: string
+                              additionalIngresses:
+                                items:
+                                  type: string
+                                type: array
                               rootService:
                                 type: string
                               servicePort:

diff --git a/pkg/apiclient/rollout/rollout.swagger.json b/pkg/apiclient/rollout/rollout.swagger.json
@@ -517,6 +517,10 @@
           "type": "string",
           "title": "Ingress refers to the name of an `Ingress` resource in the same namespace as the `Rollout`"
         },
+        "additionalIngresses": {
+          "type": "array",
+          "title": "AdditionalIngresses refers to the names of `Ingress` resources in the same namespace as the `Rollout` in a multi ingress scenario\n+optional"
+        },
         "servicePort": {
           "type": "integer",
           "format": "int32",

diff --git a/pkg/apis/api-rules/violation_exceptions.list b/pkg/apis/api-rules/violation_exceptions.list
@@ -1,3 +1,4 @@
+API rule violation: list_type_missing,github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1,ALBTrafficRouting,AdditionalIngresses
 API rule violation: list_type_missing,github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1,AmbassadorTrafficRouting,Mappings
 API rule violation: list_type_missing,github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1,AnalysisRunSpec,Args
 API rule violation: list_type_missing,github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1,AnalysisRunSpec,DryRun

diff --git a/pkg/apis/rollouts/v1alpha1/types.go b/pkg/apis/rollouts/v1alpha1/types.go
@@ -333,6 +333,9 @@ type AnalysisRunStrategy struct {
 type ALBTrafficRouting struct {
 	// Ingress refers to the name of an `Ingress` resource in the same namespace as the `Rollout`
 	Ingress string `json:"ingress" protobuf:"bytes,1,opt,name=ingress"`
+	// AdditionalIngresses refers to the names of `Ingress` resources in the same namespace as the `Rollout` in a multi ingress scenario
+	// +optional
+	AdditionalIngresses []string `json:"additionalIngresses,omitempty" protobuf:"bytes,4,rep,name=additionalIngresses"`
 	// ServicePort refers to the port that the Ingress action should route traffic to
 	ServicePort int32 `json:"servicePort" protobuf:"varint,2,opt,name=servicePort"`
 	// RootService references the service in the ingress to the controller should add the action to

diff --git a/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/apis/rollouts/validation/validation_references.go b/pkg/apis/rollouts/validation/validation_references.go
@@ -224,17 +224,35 @@ func ValidateIngress(rollout *v1alpha1.Rollout, ingress *ingressutil.Ingress) fi
 		fldPath = fldPath.Child("nginx").Child("stableIngress")
 		serviceName = rollout.Spec.Strategy.Canary.StableService
 		ingressName = rollout.Spec.Strategy.Canary.TrafficRouting.Nginx.StableIngress
+
+		allErrs = reportErrors(ingress, serviceName, ingressName, fldPath, allErrs)
 	} else if rollout.Spec.Strategy.Canary.TrafficRouting.ALB != nil {
+		// If the rollout resource manages more than 1 ingress
+		if len(rollout.Spec.Strategy.Canary.TrafficRouting.ALB.AdditionalIngresses) > 0 {
+			// Validate each ingress as valid
+			serviceName = rollout.Spec.Strategy.Canary.StableService
+			for _, ing := range rollout.Spec.Strategy.Canary.TrafficRouting.ALB.AdditionalIngresses {
+				ingressName = ing
+				serviceName = rollout.Spec.Strategy.Canary.StableService
+				if rollout.Spec.Strategy.Canary.TrafficRouting.ALB.RootService != "" {
+					serviceName = rollout.Spec.Strategy.Canary.TrafficRouting.ALB.RootService
+				}
+				allErrs = reportErrors(ingress, serviceName, ingressName, fldPath, allErrs)
+			}
+		}
 		fldPath = fldPath.Child("alb").Child("ingress")
 		ingressName = rollout.Spec.Strategy.Canary.TrafficRouting.ALB.Ingress
 		serviceName = rollout.Spec.Strategy.Canary.StableService
 		if rollout.Spec.Strategy.Canary.TrafficRouting.ALB.RootService != "" {
 			serviceName = rollout.Spec.Strategy.Canary.TrafficRouting.ALB.RootService
 		}
-
-	} else {
-		return allErrs
+		allErrs = reportErrors(ingress, serviceName, ingressName, fldPath, allErrs)
 	}
+
+	return allErrs
+}
+
+func reportErrors(ingress *ingressutil.Ingress, serviceName, ingressName string, fldPath *field.Path, allErrs field.ErrorList) field.ErrorList {
 	if !ingressutil.HasRuleWithService(ingress, serviceName) {
 		msg := fmt.Sprintf("ingress `%s` has no rules using service %s backend", ingress.GetName(), serviceName)
 		allErrs = append(allErrs, field.Invalid(fldPath, ingressName, msg))

diff --git a/rollout/controller.go b/rollout/controller.go
@@ -798,6 +798,19 @@ func (c *rolloutContext) getReferencedIngresses() (*[]ingressutil.Ingress, error
 	fldPath := field.NewPath("spec", "strategy", "canary", "trafficRouting")
 	if canary != nil && canary.TrafficRouting != nil {
 		if canary.TrafficRouting.ALB != nil {
+			// If the rollout resource manages more than 1 ingress
+			if len(canary.TrafficRouting.ALB.AdditionalIngresses) > 0 {
+				for _, ing := range canary.TrafficRouting.ALB.AdditionalIngresses {
+					ingress, err := c.ingressWrapper.GetCached(c.rollout.Namespace, ing)
+					if k8serrors.IsNotFound(err) {
+						return nil, field.Invalid(fldPath.Child("alb", "AdditionalIngresses"), ing, err.Error())
+					}
+					if err != nil {
+						return nil, err
+					}
+					ingresses = append(ingresses, *ingress)
+				}
+			}
 			ingress, err := c.ingressWrapper.GetCached(c.rollout.Namespace, canary.TrafficRouting.ALB.Ingress)
 			if k8serrors.IsNotFound(err) {
 				return nil, field.Invalid(fldPath.Child("alb", "ingress"), canary.TrafficRouting.ALB.Ingress, err.Error())

diff --git a/rollout/controller_test.go b/rollout/controller_test.go
@@ -1705,6 +1705,51 @@ func TestGetReferencedIngressesALB(t *testing.T) {
 	})
 }
 
+func TestGetReferencedIngressesALBMultiIngress(t *testing.T) {
+	f := newFixture(t)
+	defer f.Close()
+	r := newCanaryRollout("rollout", 1, nil, nil, nil, intstr.FromInt(0), intstr.FromInt(1))
+	r.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{
+		ALB: &v1alpha1.ALBTrafficRouting{
+			Ingress:             "alb-ingress-name",
+			AdditionalIngresses: []string{"multi-ingress-name"},
+		},
+	}
+	r.Namespace = metav1.NamespaceDefault
+
+	t.Run("get referenced ALB ingress - fail", func(t *testing.T) {
+		c, _, _ := f.newController(noResyncPeriodFunc)
+		roCtx, err := c.newRolloutContext(r)
+		assert.NoError(t, err)
+		_, err = roCtx.getReferencedIngresses()
+		expectedErr := field.Invalid(field.NewPath("spec", "strategy", "canary", "trafficRouting", "alb", "AdditionalIngresses"), "multi-ingress-name", "ingress.extensions \"multi-ingress-name\" not found")
+		assert.Equal(t, expectedErr.Error(), err.Error())
+	})
+
+	t.Run("get referenced ALB ingress - success", func(t *testing.T) {
+		ingress := &extensionsv1beta1.Ingress{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "alb-ingress-name",
+				Namespace: metav1.NamespaceDefault,
+			},
+		}
+		multiIngress := &extensionsv1beta1.Ingress{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "multi-ingress-name",
+				Namespace: metav1.NamespaceDefault,
+			},
+		}
+		f.ingressLister = append(f.ingressLister, ingressutil.NewLegacyIngress(ingress))
+		f.ingressLister = append(f.ingressLister, ingressutil.NewLegacyIngress(multiIngress))
+		c, _, _ := f.newController(noResyncPeriodFunc)
+		roCtx, err := c.newRolloutContext(r)
+		assert.NoError(t, err)
+		i, err := roCtx.getReferencedIngresses()
+		assert.NoError(t, err)
+		assert.NotNil(t, i)
+	})
+}
+
 func TestGetReferencedIngressesNginx(t *testing.T) {
 	f := newFixture(t)
 	defer f.Close()