Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Allow Traffic shaping through header based routing ALB #2061

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 25 additions & 17 deletions ingress/alb.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import (
func (c *Controller) syncALBIngress(ingress *ingressutil.Ingress, rollouts []*v1alpha1.Rollout) error {
ctx := context.TODO()
annotations := ingress.GetAnnotations()
managedActions, err := ingressutil.NewManagedALBActions(annotations[ingressutil.ManagedActionsAnnotation])
managedAnnotationsKey := ingressutil.ManagedAnnotations
managedActions, err := ingressutil.NewManagedALBAnnotations(annotations[managedAnnotationsKey])
if err != nil {
return nil
}
Expand All @@ -35,31 +36,38 @@ func (c *Controller) syncALBIngress(ingress *ingressutil.Ingress, rollouts []*v1
for roName := range managedActions {
if _, ok := actionHasExistingRollout[roName]; !ok {
modified = true
actionKey := managedActions[roName]
actionKeys := managedActions[roName]
delete(managedActions, roName)
resetALBAction, err := getResetALBActionStr(ingress, actionKey)
if err != nil {
log.WithField(logutil.RolloutKey, roName).
WithField(logutil.IngressKey, ingress.GetName()).
WithField(logutil.NamespaceKey, ingress.GetNamespace()).
Error(err)
return nil
for _, actionKey := range actionKeys {
if !strings.Contains(actionKey, ingressutil.ALBActionPrefix) {
continue
}
resetALBAction, err := getResetALBActionStr(ingress, actionKey)
if err != nil {
log.WithField(logutil.RolloutKey, roName).
WithField(logutil.IngressKey, ingress.GetName()).
WithField(logutil.NamespaceKey, ingress.GetNamespace()).
Error(err)
return nil
}
annotations := newIngress.GetAnnotations()
annotations[actionKey] = resetALBAction
newIngress.SetAnnotations(annotations)
}
annotations := newIngress.GetAnnotations()
annotations[actionKey] = resetALBAction
newIngress.SetAnnotations(annotations)
}
}
if !modified {
return nil
}
newManagedStr := managedActions.String()
newAnnotations := newIngress.GetAnnotations()
newAnnotations[ingressutil.ManagedActionsAnnotation] = newManagedStr
newIngress.SetAnnotations(newAnnotations)
if newManagedStr == "" {
delete(newIngress.GetAnnotations(), ingressutil.ManagedActionsAnnotation)
if len(managedActions) == 0 {
delete(newAnnotations, managedAnnotationsKey)
} else {
newAnnotations[managedAnnotationsKey] = managedActions.String()
}
// delete leftovers from old implementation ManagedActionsAnnotation
delete(newAnnotations, ingressutil.ManagedActionsAnnotation)
newIngress.SetAnnotations(newAnnotations)
_, err = c.ingressWrapper.Update(ctx, ingress.GetNamespace(), newIngress)
return err
}
Expand Down
19 changes: 11 additions & 8 deletions ingress/alb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ func albActionAnnotation(stable string) string {
func newALBIngress(name string, port int, serviceName string, rollout string, includeStickyConfig bool) *extensionsv1beta1.Ingress {
canaryService := fmt.Sprintf("%s-canary", serviceName)
albActionKey := albActionAnnotation(serviceName)
managedBy := fmt.Sprintf("%s:%s", rollout, albActionKey)
albConditionKey := fmt.Sprintf("%s%s%s", ingressutil.ALBIngressAnnotation, ingressutil.ALBConditionPrefix, serviceName)
managedBy := ingressutil.ManagedALBAnnotations{
rollout: ingressutil.ManagedALBAnnotation{albActionKey, albConditionKey},
}
action := fmt.Sprintf(actionTemplate, serviceName, port, canaryService, port)
if includeStickyConfig {
action = fmt.Sprintf(actionTemplateWithStickyConfig, serviceName, port, canaryService, port)
Expand All @@ -70,9 +73,9 @@ func newALBIngress(name string, port int, serviceName string, rollout string, in
Name: name,
Namespace: metav1.NamespaceDefault,
Annotations: map[string]string{
"kubernetes.io/ingress.class": "alb",
albActionKey: action,
ingressutil.ManagedActionsAnnotation: managedBy,
"kubernetes.io/ingress.class": "alb",
albActionKey: action,
ingressutil.ManagedAnnotations: managedBy.String(),
},
},
Spec: extensionsv1beta1.IngressSpec{
Expand Down Expand Up @@ -123,7 +126,7 @@ func rollout(name, service, ingress string) *v1alpha1.Rollout {
func TestInvalidManagedALBActions(t *testing.T) {
rollout := rollout("rollout", "stable-service", "test-ingress")
ing := newALBIngress("test-ingress", 80, "stable-service", rollout.Name, false)
ing.Annotations[ingressutil.ManagedActionsAnnotation] = "invalid-managed-by"
ing.Annotations[ingressutil.ManagedAnnotations] = "invalid-managed-by"

ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, rollout)

Expand All @@ -147,7 +150,7 @@ func TestInvalidPreviousALBActionAnnotationValue(t *testing.T) {

func TestInvalidPreviousALBActionAnnotationKey(t *testing.T) {
ing := newALBIngress("test-ingress", 80, "stable-service", "also-not-existing-rollout", false)
ing.Annotations[ingressutil.ManagedActionsAnnotation] = "invalid-action-key"
ing.Annotations[ingressutil.ManagedAnnotations] = "invalid-action-key"
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(t, ing, nil)

err := ctrl.syncIngress("default/test-ingress")
Expand Down Expand Up @@ -199,7 +202,7 @@ func TestALBIngressResetAction(t *testing.T) {
panic(err)
}
annotations := acc.GetAnnotations()
assert.NotContains(t, annotations, ingressutil.ManagedActionsAnnotation)
assert.NotContains(t, annotations, ingressutil.ManagedAnnotations)
expectedAction := `{"Type":"forward","ForwardConfig":{"TargetGroups":[{"ServiceName":"stable-service","ServicePort":"80","Weight":100}]}}`
assert.Equal(t, expectedAction, annotations[albActionAnnotation("stable-service")])
}
Expand All @@ -223,7 +226,7 @@ func TestALBIngressResetActionWithStickyConfig(t *testing.T) {
panic(err)
}
annotations := acc.GetAnnotations()
assert.NotContains(t, annotations, ingressutil.ManagedActionsAnnotation)
assert.NotContains(t, annotations, ingressutil.ManagedAnnotations)
expectedAction := `{"Type":"forward","ForwardConfig":{"TargetGroups":[{"ServiceName":"stable-service","ServicePort":"80","Weight":100}],"TargetGroupStickinessConfig":{"Enabled":true,"DurationSeconds":300}}}`
assert.Equal(t, expectedAction, annotations[albActionAnnotation("stable-service")])
}
44 changes: 34 additions & 10 deletions pkg/apis/rollouts/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,18 @@ const (
InvalidCanaryExperimentTemplateWeightWithoutTrafficRouting = "Experiment template weight cannot be set unless TrafficRouting is enabled"
// InvalidSetCanaryScaleTrafficPolicy indicates that TrafficRouting, required for SetCanaryScale, is missing
InvalidSetCanaryScaleTrafficPolicy = "SetCanaryScale requires TrafficRouting to be set"
// InvalidSetHeaderRoutingTrafficPolicy indicates that TrafficRouting, required for SetCanaryScale, is missing
InvalidSetHeaderRoutingTrafficPolicy = "SetHeaderRoute requires TrafficRouting, supports Istio only"
// InvalidSetHeaderRoutingTrafficPolicy indicates that TrafficRouting required for SetHeaderRouting, is missing
InvalidSetHeaderRoutingTrafficPolicy = "SetHeaderRoute requires TrafficRouting, supports Istio and ALB"
// InvalidSetMirrorRouteTrafficPolicy indicates that TrafficRouting, required for SetCanaryScale, is missing
InvalidSetMirrorRouteTrafficPolicy = "SetMirrorRoute requires TrafficRouting, supports Istio only"
// InvalidStringMatchMultipleValuePolicy indicates that SetCanaryScale, has multiple values set
InvalidStringMatchMultipleValuePolicy = "StringMatch match value must have exactly one of the following: exact, regex, prefix"
// InvalidStringMatchMissedValuePolicy indicates that SetCanaryScale, has multiple values set
// InvalidStringMatchMultipleValuePolicy indicates that SetHeaderRouting match, has multiple values set
InvalidStringMatchMultipleValuePolicy = "StringMatch match has multiple values, but it must have exactly one of the following: exact, regex, prefix"
// InvalidStringMatchMissedValuePolicy indicates that SetHeaderRouting, has multiple values set
InvalidStringMatchMissedValuePolicy = "StringMatch value missed, match value must have one of the following: exact, regex, prefix"
// InvalidSetHeaderRoutingALBValueMissing indicates that SetHeaderRouting using with ALB missed the 'exact' value
InvalidSetHeaderRoutingALBValueMissing = "SetHeaderRouting match value missed. ALB supports 'exact' value only"
// InvalidSetHeaderRoutingALBValuePolicy indicates that SetHeaderRouting using with ALB missed the 'exact' value
InvalidSetHeaderRoutingALBValuePolicy = "SetHeaderRouting match value invalid. ALB supports 'exact' value only"
// InvalidDurationMessage indicates the Duration value needs to be greater than 0
InvalidDurationMessage = "Duration needs to be greater than 0"
// InvalidMaxSurgeMaxUnavailable indicates both maxSurge and MaxUnavailable can not be set to zero
Expand Down Expand Up @@ -305,13 +309,17 @@ func ValidateRolloutStrategyCanary(rollout *v1alpha1.Rollout, fldPath *field.Pat

if step.SetHeaderRoute != nil {
trafficRouting := rollout.Spec.Strategy.Canary.TrafficRouting
if trafficRouting == nil || trafficRouting.Istio == nil {
if trafficRouting == nil || (trafficRouting.Istio == nil && trafficRouting.ALB == nil) {
allErrs = append(allErrs, field.Invalid(stepFldPath.Child("setHeaderRoute"), step.SetHeaderRoute, InvalidSetHeaderRoutingTrafficPolicy))
}
if step.SetHeaderRoute.Match != nil && len(step.SetHeaderRoute.Match) > 0 {
} else if step.SetHeaderRoute.Match != nil && len(step.SetHeaderRoute.Match) > 0 {
for j, match := range step.SetHeaderRoute.Match {
matchFld := stepFldPath.Child("setHeaderRoute").Child("match").Index(j)
allErrs = append(allErrs, hasMultipleMatchValues(match.HeaderValue, matchFld)...)
if trafficRouting.ALB != nil {
matchFld := stepFldPath.Child("setHeaderRoute").Child("match").Index(j)
allErrs = append(allErrs, hasALBInvalidValues(match.HeaderValue, matchFld)...)
} else {
matchFld := stepFldPath.Child("setHeaderRoute").Child("match").Index(j)
allErrs = append(allErrs, hasMultipleMatchValues(match.HeaderValue, matchFld)...)
}
}
}
}
Expand Down Expand Up @@ -473,6 +481,22 @@ func hasMultipleStepsType(s v1alpha1.CanaryStep, fldPath *field.Path) field.Erro
return allErrs
}

func hasALBInvalidValues(match *v1alpha1.StringMatch, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if match == nil {
e := field.Invalid(fldPath, match, InvalidStringMatchMissedValuePolicy)
allErrs = append(allErrs, e)
return allErrs
}
if match.Exact == "" {
return append(allErrs, field.Invalid(fldPath, match, InvalidSetHeaderRoutingALBValueMissing))
}
if match.Regex != "" || match.Prefix != "" {
return append(allErrs, field.Invalid(fldPath, match, InvalidSetHeaderRoutingALBValuePolicy))
}
return allErrs
}

func hasMultipleMatchValues(match *v1alpha1.StringMatch, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

Expand Down
68 changes: 61 additions & 7 deletions pkg/apis/rollouts/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,21 +281,15 @@ func TestValidateRolloutStrategyAntiAffinity(t *testing.T) {
assert.Equal(t, InvalidAntiAffinityWeightMessage, allErrs[0].Detail)
}

func TestValidateRolloutStrategyCanarySetHeaderRouteIstio(t *testing.T) {
func TestValidateRolloutStrategyCanarySetHeaderRoute(t *testing.T) {
ro := &v1alpha1.Rollout{}
ro.Spec.Strategy.Canary = &v1alpha1.CanaryStrategy{
CanaryService: "canary",
StableService: "stable",
TrafficRouting: &v1alpha1.RolloutTrafficRouting{
Istio: &v1alpha1.IstioTrafficRouting{
VirtualService: &v1alpha1.IstioVirtualService{Name: "virtual-service"},
},
},
}

t.Run("using SetHeaderRoute step without the traffic routing", func(t *testing.T) {
invalidRo := ro.DeepCopy()
invalidRo.Spec.Strategy.Canary.TrafficRouting = nil
invalidRo.Spec.Strategy.Canary.Steps = []v1alpha1.CanaryStep{{
SetHeaderRoute: &v1alpha1.SetHeaderRoute{
Match: []v1alpha1.HeaderRoutingMatch{
Expand All @@ -309,6 +303,19 @@ func TestValidateRolloutStrategyCanarySetHeaderRouteIstio(t *testing.T) {
allErrs := ValidateRolloutStrategyCanary(invalidRo, field.NewPath(""))
assert.Equal(t, InvalidSetHeaderRoutingTrafficPolicy, allErrs[0].Detail)
})
}

func TestValidateRolloutStrategyCanarySetHeaderRouteIstio(t *testing.T) {
ro := &v1alpha1.Rollout{}
ro.Spec.Strategy.Canary = &v1alpha1.CanaryStrategy{
CanaryService: "canary",
StableService: "stable",
TrafficRouting: &v1alpha1.RolloutTrafficRouting{
Istio: &v1alpha1.IstioTrafficRouting{
VirtualService: &v1alpha1.IstioVirtualService{Name: "virtual-service"},
},
},
}

t.Run("using SetHeaderRoute step with multiple values", func(t *testing.T) {
invalidRo := ro.DeepCopy()
Expand Down Expand Up @@ -364,6 +371,53 @@ func TestValidateRolloutStrategyCanarySetHeaderRouteIstio(t *testing.T) {
})
}

func TestValidateRolloutStrategyCanarySetHeaderRoutingALB(t *testing.T) {
ro := &v1alpha1.Rollout{}
ro.Spec.Strategy.Canary = &v1alpha1.CanaryStrategy{
CanaryService: "canary",
StableService: "stable",
TrafficRouting: &v1alpha1.RolloutTrafficRouting{
ALB: &v1alpha1.ALBTrafficRouting{
RootService: "action_name",
},
},
}

t.Run("using SetHeaderRouting step with multiple values", func(t *testing.T) {
invalidRo := ro.DeepCopy()
invalidRo.Spec.Strategy.Canary.Steps = []v1alpha1.CanaryStep{{
SetHeaderRoute: &v1alpha1.SetHeaderRoute{
Match: []v1alpha1.HeaderRoutingMatch{
{
HeaderName: "agent",
HeaderValue: &v1alpha1.StringMatch{
Exact: "chrome",
Regex: "chrome(.*)",
},
},
},
},
}}
allErrs := ValidateRolloutStrategyCanary(invalidRo, field.NewPath(""))
assert.Equal(t, InvalidSetHeaderRoutingALBValuePolicy, allErrs[0].Detail)
})

t.Run("using SetHeaderRouting step with missed values", func(t *testing.T) {
invalidRo := ro.DeepCopy()
invalidRo.Spec.Strategy.Canary.Steps = []v1alpha1.CanaryStep{{
SetHeaderRoute: &v1alpha1.SetHeaderRoute{
Match: []v1alpha1.HeaderRoutingMatch{
{
HeaderName: "agent",
},
},
},
}}
allErrs := ValidateRolloutStrategyCanary(invalidRo, field.NewPath(""))
assert.Equal(t, InvalidSetHeaderRoutingALBValueMissing, allErrs[0].Detail)
})
}

func TestValidateRolloutStrategyCanarySetMirrorRouteIstio(t *testing.T) {
ro := &v1alpha1.Rollout{}
ro.Spec.Strategy.Canary = &v1alpha1.CanaryStrategy{
Expand Down
Loading