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: Add support for rootService within ALB traffic routing #634

Merged
merged 4 commits into from
Aug 15, 2020
Merged
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
3 changes: 2 additions & 1 deletion docs/features/traffic-management/alb.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,11 @@ spec:
alb:
ingress: ingress # required
servicePort: 443 # required
rootService: # optional
annotationPrefix: custom.alb.ingress.kubernetes.io # optional
```

The ingress field is a reference to an Ingress in the same namespace of the Rollout, and the servicePort field refers a port of a service. The Rollout requires the Ingress and servicePort to modify the ALB to route traffic to the stable and canary Services. Within the Ingress, looks for the stableService within the Ingress's rules and adds an action annotation for that the action. As the Rollout progresses through the Canary steps, the controller updates the Ingress's action annotation to reflect the desired state of the Rollout enabling traffic splitting between two different versions.
The ingress field is a reference to an Ingress in the same namespace of the Rollout, and the servicePort field refers a port of a service. The Rollout requires the Ingress and servicePort to modify the ALB to route traffic to the stable and canary Services. Within the Ingress, looks for the stableService (or the optional rootService if specified) within the Ingress's rules and adds an action annotation for that the action. As the Rollout progresses through the Canary steps, the controller updates the Ingress's action annotation to reflect the desired state of the Rollout enabling traffic splitting between two different versions.

Since the ALB Ingress controller allows users to configure the annotation prefix used by the Ingress controller, Rollouts can specify the optional `annotationPrefix` field. The Ingress uses that prefix instead of the default `alb.ingress.kubernetes.io` if the field set.

Expand Down
5 changes: 4 additions & 1 deletion manifests/crds/rollout-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,8 @@ spec:
type: string
ingress:
type: string
rootService:
type: string
servicePort:
format: int32
type: integer
Expand Down Expand Up @@ -2910,7 +2912,8 @@ spec:
abort:
type: boolean
abortedAt:
type: boolean
format: date-time
type: string
availableReplicas:
format: int32
type: integer
Expand Down
5 changes: 4 additions & 1 deletion manifests/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11346,6 +11346,8 @@ spec:
type: string
ingress:
type: string
rootService:
type: string
servicePort:
format: int32
type: integer
Expand Down Expand Up @@ -13859,7 +13861,8 @@ spec:
abort:
type: boolean
abortedAt:
type: boolean
format: date-time
type: string
availableReplicas:
format: int32
type: integer
Expand Down
5 changes: 4 additions & 1 deletion manifests/namespace-install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11346,6 +11346,8 @@ spec:
type: string
ingress:
type: string
rootService:
type: string
servicePort:
format: int32
type: integer
Expand Down Expand Up @@ -13859,7 +13861,8 @@ spec:
abort:
type: boolean
abortedAt:
type: boolean
format: date-time
type: string
availableReplicas:
format: int32
type: integer
Expand Down
12 changes: 9 additions & 3 deletions pkg/apis/rollouts/v1alpha1/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions pkg/apis/rollouts/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ type ALBTrafficRouting struct {
Ingress string `json:"ingress"`
// ServicePort refers to the port that the Ingress action should route traffic to
ServicePort int32 `json:"servicePort"`
// RootService references the service in the ingress to the controller should add the action to
RootService string `json:"rootService,omitempty"`
// AnnotationPrefix has to match the configured annotation prefix on the alb ingress controller
// +optional
AnnotationPrefix string `json:"annotationPrefix,omitempty"`
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions pkg/apis/rollouts/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ const (
ScaleDownLimitLargerThanRevisionLimit = "This rollout's revision history limit can not be smaller than the rollout's scale down limit"
// InvalidTrafficRoutingMessage indicates that both canary and stable service must be set to use Traffic Routing
InvalidTrafficRoutingMessage = "Canary service and Stable service must to be set to use Traffic Routing"
// InvalidIstioRoutesMessage indicates that rollout does not have a route specified for the istio Traffic Routing
InvalidIstioRoutesMessage = "Istio virtual service must have at least 1 route specified"
)

func ValidateRollout(rollout *v1alpha1.Rollout) field.ErrorList {
Expand Down Expand Up @@ -160,6 +162,10 @@ func ValidateRolloutStrategyCanary(rollout *v1alpha1.Rollout, fldPath *field.Pat
if canary.TrafficRouting != nil && (canary.StableService == "" || canary.CanaryService == "") {
allErrs = append(allErrs, field.Invalid(fldPath.Child("trafficRouting"), canary.TrafficRouting, InvalidTrafficRoutingMessage))
}
if canary.TrafficRouting != nil && canary.TrafficRouting.Istio != nil && len(canary.TrafficRouting.Istio.VirtualService.Routes) == 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("trafficRouting").Child("istio").Child("virtualService").Child("routes"), canary.TrafficRouting, InvalidIstioRoutesMessage))

}
for i, step := range canary.Steps {
stepFldPath := fldPath.Child("steps").Index(i)
allErrs = append(allErrs, hasMultipleStepsType(step, stepFldPath)...)
Expand Down
11 changes: 9 additions & 2 deletions pkg/apis/rollouts/validation/validation_references.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,17 +118,24 @@ func ValidateIngress(rollout *v1alpha1.Rollout, ingress v1beta1.Ingress) field.E
allErrs := field.ErrorList{}
fldPath := field.NewPath("spec", "strategy", "canary", "trafficRouting")
var ingressName string
var serviceName string
if rollout.Spec.Strategy.Canary.TrafficRouting.Nginx != nil {
fldPath = fldPath.Child("nginx").Child("stableIngress")
serviceName = rollout.Spec.Strategy.Canary.StableService
ingressName = rollout.Spec.Strategy.Canary.TrafficRouting.Nginx.StableIngress
} else if rollout.Spec.Strategy.Canary.TrafficRouting.ALB != nil {
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
}
if !ingressutil.HasRuleWithService(&ingress, rollout.Spec.Strategy.Canary.StableService) {
msg := fmt.Sprintf("ingress `%s` has no rules using service %s backend", ingress.Name, rollout.Spec.Strategy.Canary.StableService)
if !ingressutil.HasRuleWithService(&ingress, serviceName) {
msg := fmt.Sprintf("ingress `%s` has no rules using service %s backend", ingress.Name, serviceName)
allErrs = append(allErrs, field.Invalid(fldPath, ingressName, msg))
}
return allErrs
Expand Down
10 changes: 7 additions & 3 deletions rollout/trafficrouting/alb/alb.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,14 @@ func (r *Reconciler) Reconcile(desiredWeight int32) error {
if err != nil {
return err
}
stableService := r.cfg.Rollout.Spec.Strategy.Canary.StableService
actionService := r.cfg.Rollout.Spec.Strategy.Canary.StableService
if r.cfg.Rollout.Spec.Strategy.Canary.TrafficRouting.ALB.RootService != "" {
actionService = r.cfg.Rollout.Spec.Strategy.Canary.TrafficRouting.ALB.RootService

}
port := r.cfg.Rollout.Spec.Strategy.Canary.TrafficRouting.ALB.ServicePort
if !ingressutil.HasRuleWithService(ingress, stableService) {
return fmt.Errorf("ingress does not use the stable service")
if !ingressutil.HasRuleWithService(ingress, actionService) {
return fmt.Errorf("ingress does not have service `%s` in rules", actionService)
}

desired, err := getDesiredAnnotations(ingress, rollout, port, desiredWeight)
Expand Down
7 changes: 4 additions & 3 deletions rollout/trafficrouting/alb/alb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,10 @@ func TestIngressNotFound(t *testing.T) {
assert.True(t, k8serrors.IsNotFound(err))
}

func TestStableServiceNotFoundInIngress(t *testing.T) {
ro := fakeRollout("different-stable", "canary-service", "ingress", 443)
i := ingress("ingress", "stable-service", "preview-svc", 443, 50, ro.Name)
func TestServiceNotFoundInIngress(t *testing.T) {
ro := fakeRollout("stable-stable", "canary-service", "ingress", 443)
ro.Spec.Strategy.Canary.TrafficRouting.ALB.RootService = "invalid-svc"
i := ingress("ingress", "stable-service", "canary-svc", 443, 50, ro.Name)
client := fake.NewSimpleClientset()
k8sI := kubeinformers.NewSharedInformerFactory(client, 0)
k8sI.Extensions().V1beta1().Ingresses().Informer().GetIndexer().Add(i)
Expand Down
6 changes: 5 additions & 1 deletion utils/ingress/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,5 +140,9 @@ func ALBActionAnnotationKey(r *v1alpha1.Rollout) string {
if r.Spec.Strategy.Canary.TrafficRouting.ALB.AnnotationPrefix != "" {
prefix = r.Spec.Strategy.Canary.TrafficRouting.ALB.AnnotationPrefix
}
return fmt.Sprintf("%s%s%s", prefix, ALBActionPrefix, r.Spec.Strategy.Canary.StableService)
actionService := r.Spec.Strategy.Canary.StableService
if r.Spec.Strategy.Canary.TrafficRouting.ALB.RootService != "" {
actionService = r.Spec.Strategy.Canary.TrafficRouting.ALB.RootService
}
return fmt.Sprintf("%s%s%s", prefix, ALBActionPrefix, actionService)
}
4 changes: 3 additions & 1 deletion utils/ingress/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,9 @@ func TestALBActionAnnotationKey(t *testing.T) {
Status: v1alpha1.RolloutStatus{},
}
assert.Equal(t, "test.annotation/actions.svc", ALBActionAnnotationKey(r))
r.Spec.Strategy.Canary.TrafficRouting.ALB.RootService = "root-svc"
assert.Equal(t, "test.annotation/actions.root-svc", ALBActionAnnotationKey(r))
r.Spec.Strategy.Canary.TrafficRouting.ALB.AnnotationPrefix = ""
assert.Equal(t, "alb.ingress.kubernetes.io/actions.svc", ALBActionAnnotationKey(r))
assert.Equal(t, "alb.ingress.kubernetes.io/actions.root-svc", ALBActionAnnotationKey(r))

}