Fix comments and alb_test ARNs

Signed-off-by: n888 <[email protected]>
argoproj · Apr 9, 2023 · 108b888 · 108b888
1 parent 32b0b6c
commit 108b888
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 155 deletions.
diff --git a/pkg/apis/rollouts/validation/validation_references.go b/pkg/apis/rollouts/validation/validation_references.go
@@ -249,15 +249,6 @@ func validateNginxIngress(canary *v1alpha1.CanaryStrategy, ingress *ingressutil.
 }
 
 func validateAlbIngress(canary *v1alpha1.CanaryStrategy, ingress *ingressutil.Ingress, fldPath *field.Path) field.ErrorList {
-	//fldPath = fldPath.Child("alb").Child("ingress")
-	//ingressName := canary.TrafficRouting.ALB.Ingress
-	//serviceName := canary.StableService
-	//if canary.TrafficRouting.ALB.RootService != "" {
-	//	serviceName = canary.TrafficRouting.ALB.RootService
-	//}
-
-	//return reportErrors(ingress, serviceName, ingressName, fldPath, field.ErrorList{})
-
 	ingresses := canary.TrafficRouting.ALB.Ingresses
 	allErrs := field.ErrorList{}
 	// If there are multiple ALB ingresses, and one of them is being validated,

diff --git a/pkg/apis/rollouts/validation/validation_references_test.go b/pkg/apis/rollouts/validation/validation_references_test.go
@@ -259,7 +259,6 @@ func getAlbRollout(ingress string) *v1alpha1.Rollout {
 					TrafficRouting: &v1alpha1.RolloutTrafficRouting{
 						ALB: &v1alpha1.ALBTrafficRouting{
 							Ingress: ingress,
-							//Ingress: "alb-ingress",
 						},
 					},
 				},
@@ -636,7 +635,6 @@ func TestValidateRolloutReferencedResourcesAlbIngress(t *testing.T) {
 				allErrs = ValidateRolloutReferencedResources(getAlbRolloutMultiIngress([]string{StableIngress, AddStableIngress1, AddStableIngress2}), refResources)
 			} else {
 				allErrs = ValidateRolloutReferencedResources(getAlbRollout(StableIngress), refResources)
-				//allErrs = ValidateRolloutReferencedResources(getRolloutSingleIngress(StableIngress), refResources)
 			}
 
 			if len(test.expectedErrors) > 0 {

diff --git a/rollout/controller.go b/rollout/controller.go
@@ -841,17 +841,9 @@ func (c *rolloutContext) getReferencedNginxIngresses(canary *v1alpha1.CanaryStra
 	return &ingresses, nil
 }
 
-// return nil, field.Invalid(fldPath.Child("alb", "ingress"), canary.TrafficRouting.ALB.Ingress, err.Error())
 func (c *rolloutContext) getReferencedALBIngresses(canary *v1alpha1.CanaryStrategy) (*[]ingressutil.Ingress, error) {
 	ingresses := []ingressutil.Ingress{}
 
-	//	ingress, err := c.ingressWrapper.GetCached(c.rollout.Namespace, canary.TrafficRouting.ALB.Ingress)
-	//	if err != nil {
-	//		return handleCacheError("alb", []string{"ingress"}, canary.TrafficRouting.ALB.Ingress, err)
-	//	}
-	//	ingresses = append(ingresses, *ingress)
-	//	return &ingresses, nil
-
 	// The rollout resource manages more than 1 ingress.
 	if canary.TrafficRouting.ALB.Ingresses != nil {
 		for _, ing := range canary.TrafficRouting.ALB.Ingresses {

diff --git a/rollout/trafficrouting/alb/alb.go b/rollout/trafficrouting/alb/alb.go
@@ -132,19 +132,6 @@ func (r *Reconciler) SetHeaderRoute(headerRoute *v1alpha1.SetHeaderRoute) error
 		return nil
 	}
 
-	//TODO
-	// Set header route for additional ingresses if present
-	//if ingresses := r.cfg.Rollout.Spec.Strategy.Canary.TrafficRouting.ALB.Ingresses; ingresses != nil {
-	//	// Fail out if there is an issue setting header route on additional ingresses.
-	//	// Fundamental assumption is that each additional Ingress is equal in importance
-	//	// as primary Ingress resource.
-	//	if err := r.SetHeaderRoutePerIngress(headerRoute, ingresses); err != nil {
-	//		return err
-	//	}
-	//}
-
-	//return r.SetHeaderRoutePerIngress(headerRoute, []string{r.cfg.Rollout.Spec.Strategy.Canary.TrafficRouting.ALB.Ingress})
-
 	if ingresses := r.cfg.Rollout.Spec.Strategy.Canary.TrafficRouting.ALB.Ingresses; ingresses != nil {
 		return r.SetHeaderRoutePerIngress(headerRoute, ingresses)
 	} else {
@@ -232,18 +219,6 @@ func (r *Reconciler) VerifyWeight(desiredWeight int32, additionalDestinations ..
 	} else {
 		return r.VerifyWeightPerIngress(desiredWeight, []string{r.cfg.Rollout.Spec.Strategy.Canary.TrafficRouting.ALB.Ingress}, additionalDestinations...)
 	}
-
-	//TODO
-	//if ingresses := r.cfg.Rollout.Spec.Strategy.Canary.TrafficRouting.ALB.Ingresses; ingresses != nil {
-	//	// Fail out if there is an issue verifying weight on additional ingresses.
-	//	// Fundamental assumption is that each additional Ingress is equal in importance
-	//	// as primary Ingress resource.
-	//	if weightVerified, err := r.VerifyWeightPerIngress(desiredWeight, ingresses, additionalDestinations...); err != nil {
-	//		return weightVerified, err
-	//	}
-	//}
-
-	//return r.VerifyWeightPerIngress(desiredWeight, []string{r.cfg.Rollout.Spec.Strategy.Canary.TrafficRouting.ALB.Ingress}, additionalDestinations...)
 }
 
 func (r *Reconciler) VerifyWeightPerIngress(desiredWeight int32, ingresses []string, additionalDestinations ...v1alpha1.WeightDestination) (*bool, error) {
@@ -348,7 +323,6 @@ func (r *Reconciler) VerifyWeightPerIngress(desiredWeight int32, ingresses []str
 			}
 		}
 	}
-	//return pointer.BoolPtr(numVerifiedWeights == 1+len(additionalDestinations)), nil
 	return pointer.BoolPtr(numVerifiedWeights == len(ingresses)+len(additionalDestinations)), nil
 }
 
@@ -551,23 +525,11 @@ func (r *Reconciler) RemoveManagedRoutes() error {
 		return nil
 	}
 
-	//TODO
-	// Remove managed routes for additional ingresses if present
 	if ingresses := r.cfg.Rollout.Spec.Strategy.Canary.TrafficRouting.ALB.Ingresses; ingresses != nil {
 		return r.RemoveManagedRoutesPerIngress(ingresses)
 	} else {
 		return r.RemoveManagedRoutesPerIngress([]string{r.cfg.Rollout.Spec.Strategy.Canary.TrafficRouting.ALB.Ingress})
 	}
-	//if ingresses := r.cfg.Rollout.Spec.Strategy.Canary.TrafficRouting.ALB.Ingresses; ingresses != nil {
-	//	// Fail out if there is an issue remove routes on additional ingresses.
-	//	// Fundamental assumption is that each additional Ingress is equal in importance
-	//	// as primary Ingress resource.
-	//	if err := r.RemoveManagedRoutesPerIngress(ingresses); err != nil {
-	//		return err
-	//	}
-	//}
-
-	// return r.RemoveManagedRoutesPerIngress([]string{r.cfg.Rollout.Spec.Strategy.Canary.TrafficRouting.ALB.Ingress})
 }
 
 func (r *Reconciler) RemoveManagedRoutesPerIngress(ingresses []string) error {