-
Notifications
You must be signed in to change notification settings - Fork 915
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 ALB Ingress controller support #444
Merged
Merged
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
474276f
Add ALB ingress structs to types.go
dthomson25 cfb9d11
Add functionality to manipulate ALB Ingresses
dthomson25 8d774f8
Watch alb ingresses in Ingress controller
dthomson25 8558098
Add ALB Ingress documentation
dthomson25 fcb1f77
Fix linting issues
dthomson25 4411015
Allow ALB ingress to change annotation prefix
dthomson25 f0a7f97
Make ingress class customizable
dthomson25 6b7cc73
Add notes on kubernetes.io/ingress.class
dthomson25 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
# AWS Application Load Balancer (ALB) | ||
|
||
The [AWS ALB Ingress Controller](https://kubernetes-sigs.github.io/aws-alb-ingress-controller/) enables traffic management through an Ingress object that configuring an ALB that routes traffic proportionally to different services. | ||
|
||
The ALB consists of a listener and rules with actions. Listeners define how traffic from client comes in, and rules define how to handle those requests with various actions. One action allows users to forward traffic to multiple TargetGroups (with each being defined as a Kubernetes service) You can read more about ALB concepts [here](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/introduction.html). | ||
|
||
An ALB Ingress defines a desired ALB with listener and rules through its annotations and spec. The ALB Ingress controller honors an annotation on an Ingress called `alb.ingress.kubernetes.io/actions.<service-name>` that allows users to define the actions of a service listed in the Ingress with a "use-annotations" value for the ports. Below is an example of an ingress: | ||
|
||
```yaml | ||
apiVersion: extensions/v1beta1 | ||
kind: Ingress | ||
metadata: | ||
annotations: | ||
alb.ingress.kubernetes.io/actions.stable-service: | | ||
{ | ||
"Type":"forward", | ||
"ForwardConfig":{ | ||
"TargetGroups":[ | ||
{ | ||
"Weight":10, | ||
"ServiceName":"canary-service", | ||
"ServicePort":"80" | ||
}, | ||
{ | ||
"Weight":90, | ||
"ServiceName":"stable-service", | ||
"ServicePort":"80" | ||
} | ||
] | ||
} | ||
} | ||
kubernetes.io/ingress.class: aws-alb | ||
name: ingress | ||
spec: | ||
rules: | ||
- http: | ||
paths: | ||
- backend: | ||
serviceName: stable-service | ||
servicePort: use-annotation | ||
path: /* | ||
``` | ||
This Ingress uses the `alb.ingress.kubernetes.io/actions.stable-service` annotation to define how to route traffic to the various services for the rule with the `stable-service` serviceName instead of sending traffic to the canary-service service. You can read more about these annotations on the official [documentation](https://kubernetes-sigs.github.io/aws-alb-ingress-controller/guide/ingress/annotation/#actions). | ||
|
||
## Integration with Argo Rollouts | ||
There are a couple of required fields in a Rollout to send split traffic between versions using ALB ingresses. Below is an example of a Rollout with those fields: | ||
|
||
```yaml | ||
apiVersion: argoproj.io/v1alpha1 | ||
kind: Rollout | ||
spec: | ||
... | ||
strategy: | ||
canary: | ||
canaryService: canary-service # required | ||
stableService: stable-service # required | ||
trafficRouting: | ||
alb: | ||
ingress: ingress # required | ||
``` | ||
|
||
The ingress field is a reference to an Ingress in the same namespace of the Rollout. The Rollout requires this Ingress 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 Rollout adds another annotation called `rollouts.argoproj.io/managed-alb-actions` to the Ingress to help the controller manage the Ingresses. This annotation indicates which actions are being managed by Rollout objects (since multiple Rollouts can reference one Ingress). If a Rollout is deleted, the Argo Rollouts controller uses this annotation to see that this action is no longer managed, and it is reset to only the stable service with 100 weight. | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
package ingress | ||
|
||
import ( | ||
"encoding/json" | ||
"fmt" | ||
"strings" | ||
|
||
"github.com/sirupsen/logrus" | ||
|
||
extensionsv1beta1 "k8s.io/api/extensions/v1beta1" | ||
|
||
"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" | ||
ingressutil "github.com/argoproj/argo-rollouts/utils/ingress" | ||
jsonutil "github.com/argoproj/argo-rollouts/utils/json" | ||
logutil "github.com/argoproj/argo-rollouts/utils/log" | ||
) | ||
|
||
func (c *Controller) syncALBIngress(ingress *extensionsv1beta1.Ingress, rollouts []*v1alpha1.Rollout) error { | ||
|
||
managedActions, err := ingressutil.NewManagedALBActions(ingress.Annotations[ingressutil.ManagedActionsAnnotation]) | ||
if err != nil { | ||
return nil | ||
} | ||
actionHasExistingRollout := map[string]bool{} | ||
for i := range rollouts { | ||
rollout := rollouts[i] | ||
if _, ok := managedActions[rollout.Name]; ok { | ||
actionHasExistingRollout[rollout.Name] = true | ||
c.enqueueRollout(rollout) | ||
} | ||
} | ||
newIngress := ingress.DeepCopy() | ||
modified := false | ||
for roName := range managedActions { | ||
if _, ok := actionHasExistingRollout[roName]; !ok { | ||
modified = true | ||
actionKey := managedActions[roName] | ||
delete(managedActions, roName) | ||
resetALBAction, err := getResetALBActionStr(ingress, actionKey) | ||
if err != nil { | ||
logrus.WithField(logutil.IngressKey, ingress.Name).WithField(logutil.NamespaceKey, ingress.Namespace).Error(err) | ||
return nil | ||
} | ||
newIngress.Annotations[actionKey] = resetALBAction | ||
} | ||
} | ||
if !modified { | ||
return nil | ||
} | ||
newManagedStr := managedActions.String() | ||
newIngress.Annotations[ingressutil.ManagedActionsAnnotation] = newManagedStr | ||
if newManagedStr == "" { | ||
delete(newIngress.Annotations, ingressutil.ManagedActionsAnnotation) | ||
} | ||
_, err = c.client.ExtensionsV1beta1().Ingresses(ingress.Namespace).Update(newIngress) | ||
return err | ||
} | ||
|
||
func getResetALBActionStr(ingress *extensionsv1beta1.Ingress, action string) (string, error) { | ||
previousActionStr := ingress.Annotations[action] | ||
var previousAction ingressutil.ALBAction | ||
err := json.Unmarshal([]byte(previousActionStr), &previousAction) | ||
if err != nil { | ||
return "", fmt.Errorf("unable to unmarshal previous ALB action") | ||
} | ||
|
||
service := strings.TrimPrefix(action, ingressutil.ALBActionAnnotationPrefix) | ||
var port string | ||
for _, tg := range previousAction.ForwardConfig.TargetGroups { | ||
if tg.ServiceName == service { | ||
port = tg.ServicePort | ||
} | ||
} | ||
if port == "" { | ||
return "", fmt.Errorf("unable to reset annotation due to missing port") | ||
} | ||
|
||
albAction := ingressutil.ALBAction{ | ||
Type: "forward", | ||
ForwardConfig: ingressutil.ALBForwardConfig{ | ||
TargetGroups: []ingressutil.ALBTargetGroup{ | ||
{ | ||
ServiceName: service, | ||
ServicePort: port, | ||
Weight: int64(100), | ||
}, | ||
}, | ||
}, | ||
} | ||
bytes := jsonutil.MustMarshal(albAction) | ||
return string(bytes), nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,166 @@ | ||
package ingress | ||
|
||
import ( | ||
"fmt" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/assert" | ||
extensionsv1beta1 "k8s.io/api/extensions/v1beta1" | ||
"k8s.io/apimachinery/pkg/api/meta" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
"k8s.io/apimachinery/pkg/util/intstr" | ||
k8stesting "k8s.io/client-go/testing" | ||
|
||
"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" | ||
ingressutil "github.com/argoproj/argo-rollouts/utils/ingress" | ||
) | ||
|
||
const actionTemplate = `{ | ||
"Type":"forward", | ||
"ForwardConfig":{ | ||
"TargetGroups":[ | ||
{ | ||
"ServiceName":"%s", | ||
"ServicePort":"%d", | ||
"Weight": 85 | ||
},{ | ||
"ServiceName":"%s", | ||
"ServicePort":"%d", | ||
"Weight": 15 | ||
} | ||
] | ||
} | ||
}` | ||
|
||
func newALBIngress(name string, port int, serviceName string, rollout string) *extensionsv1beta1.Ingress { | ||
canaryService := fmt.Sprintf("%s-canary", serviceName) | ||
albActionKey := ingressutil.ALBActionAnnotationKey(serviceName) | ||
managedBy := fmt.Sprintf("%s:%s", rollout, albActionKey) | ||
action := fmt.Sprintf(actionTemplate, serviceName, port, canaryService, port) | ||
return &extensionsv1beta1.Ingress{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: name, | ||
Namespace: metav1.NamespaceDefault, | ||
Annotations: map[string]string{ | ||
"kubernetes.io/ingress.class": "aws-alb", | ||
albActionKey: action, | ||
ingressutil.ManagedActionsAnnotation: managedBy, | ||
}, | ||
}, | ||
Spec: extensionsv1beta1.IngressSpec{ | ||
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.FromString("use-annotations"), | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
} | ||
} | ||
|
||
func rollout(name, service, ingress string) *v1alpha1.Rollout { | ||
return &v1alpha1.Rollout{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: name, | ||
Namespace: metav1.NamespaceDefault, | ||
}, | ||
Spec: v1alpha1.RolloutSpec{ | ||
Strategy: v1alpha1.RolloutStrategy{ | ||
Canary: &v1alpha1.CanaryStrategy{ | ||
StableService: service, | ||
CanaryService: fmt.Sprintf("%s-canary", service), | ||
TrafficRouting: &v1alpha1.RolloutTrafficRouting{ | ||
ALB: &v1alpha1.ALBTrafficRouting{ | ||
Ingress: ingress, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
} | ||
} | ||
|
||
func TestInvalidManagedALBActions(t *testing.T) { | ||
rollout := rollout("rollout", "stable-service", "test-ingress") | ||
ing := newALBIngress("test-ingress", 80, "stable-service", rollout.Name) | ||
ing.Annotations[ingressutil.ManagedActionsAnnotation] = "invalid-managed-by" | ||
|
||
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, rollout) | ||
|
||
err := ctrl.syncIngress("default/test-ingress") | ||
assert.Nil(t, err) | ||
assert.Len(t, kubeclient.Actions(), 0) | ||
assert.Len(t, enqueuedObjects, 0) | ||
} | ||
|
||
func TestInvalidPreviousALBActionAnnotation(t *testing.T) { | ||
ing := newALBIngress("test-ingress", 80, "stable-service", "not-existing-rollout") | ||
ing.Annotations[ingressutil.ALBActionAnnotationKey("stable-service")] = "{" | ||
|
||
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, nil) | ||
|
||
err := ctrl.syncIngress("default/test-ingress") | ||
assert.Nil(t, err) | ||
assert.Len(t, kubeclient.Actions(), 0) | ||
assert.Len(t, enqueuedObjects, 0) | ||
} | ||
|
||
func TestResetActionFailureFindNoPort(t *testing.T) { | ||
ing := newALBIngress("test-ingress", 80, "stable-service", "not-existing-rollout") | ||
ing.Annotations[ingressutil.ALBActionAnnotationKey("stable-service")] = "{}" | ||
|
||
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, nil) | ||
|
||
err := ctrl.syncIngress("default/test-ingress") | ||
assert.Nil(t, err) | ||
assert.Len(t, kubeclient.Actions(), 0) | ||
assert.Len(t, enqueuedObjects, 0) | ||
} | ||
|
||
func TestALBIngressNoModifications(t *testing.T) { | ||
rollout := rollout("rollout", "stable-service", "test-ingress") | ||
ing := newALBIngress("test-ingress", 80, "stable-service", rollout.Name) | ||
|
||
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, rollout) | ||
|
||
err := ctrl.syncIngress("default/test-ingress") | ||
assert.Nil(t, err) | ||
assert.Len(t, kubeclient.Actions(), 0) | ||
assert.Len(t, enqueuedObjects, 1) | ||
} | ||
|
||
func TestALBIngressResetAction(t *testing.T) { | ||
ing := newALBIngress("test-ingress", 80, "stable-service", "non-existing-rollout") | ||
|
||
ctrl, kubeclient, enqueuedObjects := newFakeIngressController(ing, nil) | ||
err := ctrl.syncIngress("default/test-ingress") | ||
assert.Nil(t, err) | ||
assert.Len(t, enqueuedObjects, 0) | ||
actions := kubeclient.Actions() | ||
assert.Len(t, actions, 1) | ||
updateAction, ok := actions[0].(k8stesting.UpdateAction) | ||
if !ok { | ||
assert.Fail(t, "Client call was not an update") | ||
updateAction.GetObject() | ||
} | ||
acc, err := meta.Accessor(updateAction.GetObject()) | ||
if err != nil { | ||
panic(err) | ||
} | ||
annotations := acc.GetAnnotations() | ||
assert.NotContains(t, annotations, ingressutil.ManagedActionsAnnotation) | ||
expectedAction := `{"Type":"forward","ForwardConfig":{"TargetGroups":[{"ServiceName":"stable-service","ServicePort":"80","Weight":100}]}}` | ||
assert.Equal(t, expectedAction, annotations[ingressutil.ALBActionAnnotationKey("stable-service")]) | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to call out that this behavior is such that we would be leaving around our annotation when the original ingress object did not have it in the beginning.
I think this is the correct behavior, since it is safer than deleting the annotation, which could cause downtime, but just wanted to point out that we could be leaving around leftover cruft.