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.

Sign up for GitHub

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

Adding Stickiness option for Target Groups #711

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Adding Stickiness option for Target Groups #711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1c9d86d
Adding Stickiness option for Target Groups
drewmudgett Aug 15, 2024
b44847f
Fixed typos in stickiness config
drewmudgett Aug 16, 2024
363686d
Updating tests to include stickiness setting
Aug 19, 2024
a5e76c6
Updating testing to only apply stickiness if enabled
Aug 20, 2024
f241a81
Removed unused variable
Aug 21, 2024
d313295
Removing redundant nil check
Aug 21, 2024
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
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ Overview of configuration which can be set via Ingress annotations.
|`zalando.org/aws-load-balancer-http2`| `true` \| `false`|`true`|
|`zalando.org/aws-waf-web-acl-id` | `string` | N/A |
|`kubernetes.io/ingress.class`|`string`|N/A|
|`zalando.org/aws-load-balancer-stickiness`| `true` \| `false`|`false`|

The defaults can also be configured globally via a flag on the controller.

Expand Down
11 changes: 11 additions & 0 deletions aws/cf.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type Stack struct {
WAFWebACLID string
CertificateARNs map[string]time.Time
tags map[string]string
Stickiness bool
}

// IsComplete returns true if the stack status is a complete state.
Expand Down Expand Up @@ -148,6 +149,7 @@ const (
parameterLoadBalancerTypeParameter = "Type"
parameterLoadBalancerWAFWebACLIDParameter = "LoadBalancerWAFWebACLIDParameter"
parameterHTTP2Parameter = "HTTP2"
parameterStickinessParameter = "Stickiness"
)

type stackSpec struct {
Expand Down Expand Up @@ -188,6 +190,7 @@ type stackSpec struct {
denyInternalDomainsResponse denyResp
internalDomains []string
tags map[string]string
stickiness bool
}

type healthCheck struct {
Expand Down Expand Up @@ -229,6 +232,7 @@ func createStack(svc cloudformationiface.CloudFormationAPI, spec *stackSpec) (st
cfParam(parameterIpAddressTypeParameter, spec.ipAddressType),
cfParam(parameterLoadBalancerTypeParameter, spec.loadbalancerType),
cfParam(parameterHTTP2Parameter, fmt.Sprintf("%t", spec.http2)),
cfParam(parameterStickinessParameter, fmt.Sprintf("%t", spec.stickiness)),
},
Tags: tagMapToCloudformationTags(tags),
TemplateBody: aws.String(template),
Expand Down Expand Up @@ -304,6 +308,7 @@ func updateStack(svc cloudformationiface.CloudFormationAPI, spec *stackSpec) (st
cfParam(parameterIpAddressTypeParameter, spec.ipAddressType),
cfParam(parameterLoadBalancerTypeParameter, spec.loadbalancerType),
cfParam(parameterHTTP2Parameter, fmt.Sprintf("%t", spec.http2)),
cfParam(parameterStickinessParameter, fmt.Sprintf("%t", spec.stickiness)),
},
Tags: tagMapToCloudformationTags(tags),
TemplateBody: aws.String(template),
Expand Down Expand Up @@ -481,6 +486,11 @@ func mapToManagedStack(stack *cloudformation.Stack) *Stack {
http2 = false
}

stickiness := false
if parameters[parameterStickinessParameter] == "true" {
stickiness = true
}

return &Stack{
Name: aws.StringValue(stack.StackName),
DNSName: outputs.dnsName(),
Expand All @@ -498,6 +508,7 @@ func mapToManagedStack(stack *cloudformation.Stack) *Stack {
statusReason: aws.StringValue(stack.StackStatusReason),
CWAlarmConfigHash: tags[cwAlarmConfigHashTag],
WAFWebACLID: parameters[parameterLoadBalancerWAFWebACLIDParameter],
Stickiness: stickiness,
}
}

Expand Down
48 changes: 38 additions & 10 deletions aws/cf_template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,14 @@ func generateTemplate(spec *stackSpec) (string, error) {
},
}

if spec.stickiness {
template.Parameters[parameterStickinessParameter] = &cloudformation.Parameter{
Type: "String",
Description: "Enable Target Group Stickiness",
Default: "false",
}
}

if spec.wafWebAclId != "" {
template.Parameters[parameterLoadBalancerWAFWebACLIDParameter] = &cloudformation.Parameter{
Type: "String",
Expand Down Expand Up @@ -165,17 +173,37 @@ func generateTemplate(spec *stackSpec) (string, error) {
Protocol: cloudformation.String("HTTP"),
})
} else {
template.AddResource("HTTPListener", &cloudformation.ElasticLoadBalancingV2Listener{
DefaultActions: &cloudformation.ElasticLoadBalancingV2ListenerActionList{
{
Type: cloudformation.String("forward"),
TargetGroupArn: cloudformation.Ref(httpTargetGroupName).String(),
if spec.stickiness {
template.AddResource("HTTPListener", &cloudformation.ElasticLoadBalancingV2Listener{
DefaultActions: &cloudformation.ElasticLoadBalancingV2ListenerActionList{
{
Type: cloudformation.String("forward"),
TargetGroupArn: cloudformation.Ref(httpTargetGroupName).String(),
ForwardConfig: &cloudformation.ElasticLoadBalancingV2ListenerForwardConfig{
TargetGroupStickinessConfig: &cloudformation.ElasticLoadBalancingV2ListenerTargetGroupStickinessConfig{
Enabled: cloudformation.Ref(parameterStickinessParameter).Bool(),
DurationSeconds: cloudformation.Integer(3600),
},
},
},
},
},
LoadBalancerArn: cloudformation.Ref("LB").String(),
Port: cloudformation.Integer(80),
Protocol: cloudformation.String("HTTP"),
})
LoadBalancerArn: cloudformation.Ref("LB").String(),
Port: cloudformation.Integer(80),
Protocol: cloudformation.String("HTTP"),
})
} else {
template.AddResource("HTTPListener", &cloudformation.ElasticLoadBalancingV2Listener{
DefaultActions: &cloudformation.ElasticLoadBalancingV2ListenerActionList{
{
Type: cloudformation.String("forward"),
TargetGroupArn: cloudformation.Ref(httpTargetGroupName).String(),
},
},
LoadBalancerArn: cloudformation.Ref("LB").String(),
Port: cloudformation.Integer(80),
Protocol: cloudformation.String("HTTP"),
})
}
if spec.denyInternalDomains {
template.AddResource(
"HTTPRuleBlockInternalTraffic",
Expand Down
7 changes: 7 additions & 0 deletions kubernetes/adapter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ type Ingress struct {
LoadBalancerType string
WAFWebACLID string
Hostnames []string
Stickiness bool
}

// String returns a string representation of the Ingress instance containing the type, namespace and the resource name.
Expand Down Expand Up @@ -234,6 +235,11 @@ func (a *Adapter) newIngress(typ IngressType, metadata kubeItemMetadata, host st
if getAnnotationsString(annotations, ingressHTTP2Annotation, "") == "false" {
http2 = false
}

stickiness := false
if getAnnotationsString(annotations, ingressLoadBalancerStickiness, "") == "true" {
stickiness = true
}

return &Ingress{
ResourceType: typ,
Expand All @@ -251,6 +257,7 @@ func (a *Adapter) newIngress(typ IngressType, metadata kubeItemMetadata, host st
LoadBalancerType: loadBalancerType,
WAFWebACLID: wafWebAclId,
HTTP2: http2,
Stickiness: stickiness,
}, nil
}

Expand Down
1 change: 1 addition & 0 deletions kubernetes/ingress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ const (
ingressHTTP2Annotation = "zalando.org/aws-load-balancer-http2"
ingressWAFWebACLIDAnnotation = "zalando.org/aws-waf-web-acl-id"
ingressClassAnnotation = "kubernetes.io/ingress.class"
ingressLoadBalancerStickiness = "zalando.org/aws-load-balancer-stickiness"
)

func getAnnotationsString(annotations map[string]string, key string, defaultValue string) string {
Expand Down
3 changes: 1 addition & 2 deletions kubernetes/pods.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,7 @@ func isPodTerminating(p *corev1.Pod) bool {
}

func isPodRunning(p *corev1.Pod) bool {
return p.Status.ContainerStatuses != nil &&
len(p.Status.ContainerStatuses) > 0 &&
return len(p.Status.ContainerStatuses) > 0 &&
p.Status.ContainerStatuses[0].State.Running != nil &&
p.Status.PodIP != ""
}
4 changes: 4 additions & 0 deletions testdata/ing_shared_rg_notshared_alb/output/params/01-ing.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,9 @@
{
"parameterKey": "TargetGroupHealthCheckTimeoutParameter",
"parameterValue": "0"
},
{
"parameterKey": "Stickiness",
"parameterValue": "false"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess these fields should not be there if we have not sticky configuration.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the line that the linter was complaining about, but I still see this error locally:

Error: kubernetes/pods.go:103:9: S1009: should omit nil check; len() for []k8s.io/api/core/v1.ContainerStatus is defined as zero (gosimple)
	return p.Status.ContainerStatuses != nil &&

But it looks like that error was there before my commits (i locally tested against main).

Copy link
Member

@szuecs szuecs Aug 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a new staticcheck version, so would be great if you just fix it :)
https://staticcheck.dev/docs/checks#S1009

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no problem, this is fixed in the latest commit

Copy link
Member

@szuecs szuecs Aug 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but the result params should not change and as far as I read the code.
This should not be there in the PR:

},
  {
    "parameterKey": "Stickiness",
    "parameterValue": "false"

Ah and please add a test that actually tests the feature with "true" and "false" set :)

}
]
4 changes: 4 additions & 0 deletions testdata/ing_shared_rg_notshared_alb/output/params/02-rg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,9 @@
{
"parameterKey": "TargetGroupHealthCheckTimeoutParameter",
"parameterValue": "0"
},
{
"parameterKey": "Stickiness",
"parameterValue": "false"
}
]
4 changes: 4 additions & 0 deletions testdata/ingress_alb/output/params/ing.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,9 @@
{
"parameterKey": "TargetGroupHealthCheckTimeoutParameter",
"parameterValue": "0"
},
{
"parameterKey": "Stickiness",
"parameterValue": "false"
}
]
4 changes: 4 additions & 0 deletions testdata/ingress_nlb/output/params/ing.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,9 @@
{
"parameterKey": "TargetGroupHealthCheckTimeoutParameter",
"parameterValue": "0"
},
{
"parameterKey": "Stickiness",
"parameterValue": "false"
}
]
4 changes: 4 additions & 0 deletions testdata/ingress_rg_shared_alb/output/params/shared.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,9 @@
{
"parameterKey": "TargetGroupHealthCheckTimeoutParameter",
"parameterValue": "0"
},
{
"parameterKey": "Stickiness",
"parameterValue": "false"
}
]
4 changes: 4 additions & 0 deletions testdata/ingress_rg_shared_nlb/output/params/shared.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,9 @@
{
"parameterKey": "TargetGroupHealthCheckTimeoutParameter",
"parameterValue": "0"
},
{
"parameterKey": "Stickiness",
"parameterValue": "false"
}
]
4 changes: 4 additions & 0 deletions testdata/rg_alb/output/params/rg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,9 @@
{
"parameterKey": "TargetGroupHealthCheckTimeoutParameter",
"parameterValue": "0"
},
{
"parameterKey": "Stickiness",
"parameterValue": "false"
}
]
4 changes: 4 additions & 0 deletions testdata/rg_nlb/output/params/rg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,9 @@
{
"parameterKey": "TargetGroupHealthCheckTimeoutParameter",
"parameterValue": "0"
},
{
"parameterKey": "Stickiness",
"parameterValue": "false"
}
]
Loading