Skip to content

Commit 86a5009

Browse files
bigkraigbigkraig
committed
Improved listener/rule reconciliation but still need to address the usage of the default rule's target group for the listeners default action. it should be using the default backend service
1 parent 7500f98 commit 86a5009

File tree

8 files changed

+100
-88
lines changed

8 files changed

+100
-88
lines changed

‎pkg/alb/lb/loadbalancer.go

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -176,15 +176,6 @@ type NewCurrentLoadBalancerOptions struct {
176176

177177
// NewCurrentLoadBalancer returns a new loadbalancer.LoadBalancer based on an elbv2.LoadBalancer.
178178
func NewCurrentLoadBalancer(o *NewCurrentLoadBalancerOptions) (newLoadBalancer *LoadBalancer, err error) {
179-
lbTags := o.ResourceTags.LoadBalancers[*o.LoadBalancer.LoadBalancerArn]
180-
181-
namespace, ingressName, err := tagsFromLB(lbTags)
182-
if err != nil {
183-
return nil, fmt.Errorf("The LoadBalancer %s does not have the proper tags, can't import: %s", *o.LoadBalancer.LoadBalancerName, err.Error())
184-
}
185-
186-
name := createLBName(namespace, ingressName, o.ALBNamePrefix)
187-
188179
attrs, err := albelbv2.ELBV2svc.DescribeLoadBalancerAttributesFiltered(o.LoadBalancer.LoadBalancerArn)
189180
if err != nil {
190181
return newLoadBalancer, fmt.Errorf("Failed to retrieve attributes from ELBV2 in AWS. Error: %s", err.Error())
@@ -239,8 +230,8 @@ func NewCurrentLoadBalancer(o *NewCurrentLoadBalancerOptions) (newLoadBalancer *
239230
}
240231

241232
newLoadBalancer = &LoadBalancer{
242-
id: name,
243-
tags: tags{current: lbTags},
233+
id: *o.LoadBalancer.LoadBalancerName,
234+
tags: tags{current: o.ResourceTags.LoadBalancers[*o.LoadBalancer.LoadBalancerArn]},
244235
lb: lb{current: o.LoadBalancer},
245236
logger: o.Logger,
246237
attributes: attributes{current: attrs},
@@ -355,6 +346,7 @@ func (l *LoadBalancer) Reconcile(rOpts *ReconcileOptions) []error {
355346

356347
// Decide: Is this still needed?
357348
for _, listener := range l.listeners {
349+
// Does not consider TG used for listener default action
358350
unusedTGs := listener.GetRules().FindUnusedTGs(l.targetgroups)
359351
unusedTGs.StripDesiredState()
360352
}

‎pkg/alb/ls/listener.go

Lines changed: 19 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,20 @@ func NewCurrentListener(o *NewCurrentListenerOptions) (*Listener, error) {
110110
// results in no action, the creation, the deletion, or the modification of an AWS listener to
111111
// satisfy the ingress's current state.
112112
func (l *Listener) Reconcile(rOpts *ReconcileOptions) error {
113+
// If there is a desired listener, set some of the ARNs which are not available when we assemble the desired state
114+
if l.ls.desired != nil {
115+
l.ls.desired.LoadBalancerArn = rOpts.LoadBalancerArn
116+
117+
// Set the listener default action to the targetgroup from the default rule.
118+
// Not good
119+
if rOpts != nil {
120+
defaultRule := l.rules.DefaultRule()
121+
if defaultRule != nil {
122+
l.ls.desired.DefaultActions[0].TargetGroupArn = defaultRule.TargetGroupArn(rOpts.TargetGroups)
123+
}
124+
}
125+
}
126+
113127
switch {
114128
case l.ls.desired == nil: // listener should be deleted
115129
if l.ls.current == nil {
@@ -156,14 +170,6 @@ func (l *Listener) Reconcile(rOpts *ReconcileOptions) error {
156170

157171
// Adds a Listener to an existing ALB in AWS. This Listener maps the ALB to an existing TargetGroup.
158172
func (l *Listener) create(rOpts *ReconcileOptions) error {
159-
l.ls.desired.LoadBalancerArn = rOpts.LoadBalancerArn
160-
161-
// Set the listener default action to the targetgroup from the default rule.
162-
defaultRule := l.rules.DefaultRule()
163-
if defaultRule != nil {
164-
l.ls.desired.DefaultActions[0].TargetGroupArn = defaultRule.TargetGroupArn(rOpts.TargetGroups)
165-
}
166-
167173
// Attempt listener creation.
168174
desired := l.ls.desired
169175
in := &elbv2.CreateListenerInput{
@@ -172,12 +178,7 @@ func (l *Listener) create(rOpts *ReconcileOptions) error {
172178
Protocol: desired.Protocol,
173179
Port: desired.Port,
174180
SslPolicy: desired.SslPolicy,
175-
DefaultActions: []*elbv2.Action{
176-
{
177-
Type: desired.DefaultActions[0].Type,
178-
TargetGroupArn: desired.DefaultActions[0].TargetGroupArn,
179-
},
180-
},
181+
DefaultActions: desired.DefaultActions,
181182
}
182183
o, err := albelbv2.ELBV2svc.CreateListener(in)
183184
if err != nil {
@@ -191,11 +192,6 @@ func (l *Listener) create(rOpts *ReconcileOptions) error {
191192

192193
// Modifies a listener
193194
func (l *Listener) modify(rOpts *ReconcileOptions) error {
194-
if l.ls.current == nil {
195-
// not a modify, a create
196-
return l.create(rOpts)
197-
}
198-
199195
desired := l.ls.desired
200196
in := &elbv2.ModifyListenerInput{
201197
ListenerArn: l.ls.current.ListenerArn,
@@ -233,14 +229,6 @@ func (l *Listener) needsModification(rOpts *ReconcileOptions) bool {
233229
lsc := l.ls.current
234230
lsd := l.ls.desired
235231

236-
// Set the listener default action to the targetgroup from the default rule.
237-
if rOpts != nil {
238-
defaultRule := l.rules.DefaultRule()
239-
if defaultRule != nil {
240-
lsd.DefaultActions[0].TargetGroupArn = defaultRule.TargetGroupArn(rOpts.TargetGroups)
241-
}
242-
}
243-
244232
switch {
245233
case lsc == nil && lsd == nil:
246234
return false
@@ -281,3 +269,7 @@ func (l *Listener) stripCurrentState() {
281269
func (l *Listener) GetRules() rs.Rules {
282270
return l.rules
283271
}
272+
273+
func (l *Listener) DefaultActionArn() *string {
274+
return l.ls.current.DefaultActions[0].TargetGroupArn
275+
}

‎pkg/alb/rs/rule.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,13 @@ func NewCurrentRule(o *NewCurrentRuleOptions) *Rule {
8585
// results in no action, the creation, the deletion, or the modification of an AWS Rule to
8686
// satisfy the ingress's current state.
8787
func (r *Rule) Reconcile(rOpts *ReconcileOptions) error {
88+
// If there is a desired rule, set some of the ARNs which are not available when we assemble the desired state
89+
if r.rs.desired != nil {
90+
for i := range r.rs.desired.Actions {
91+
r.rs.desired.Actions[i].TargetGroupArn = r.TargetGroupArn(rOpts.TargetGroups)
92+
}
93+
}
94+
8895
switch {
8996
case r.rs.desired == nil: // rule should be deleted
9097
if r.rs.current == nil {
@@ -102,9 +109,9 @@ func (r *Rule) Reconcile(rOpts *ReconcileOptions) error {
102109
log.Prettify(r.rs.current.Priority),
103110
log.Prettify(r.rs.current.Conditions))
104111

105-
case *r.rs.desired.IsDefault: // rule is default (attached to listener), do nothing
106-
// r.logger.Debugf("Found desired rule that is a default and is already created with its respective listener. Rule: %s", log.Prettify(r.rs.desired))
107-
r.rs.current = r.rs.desired
112+
// case *r.rs.desired.IsDefault: // rule is default (attached to listener), do nothing
113+
// r.logger.Debugf("Found desired rule that is a default and is already created with its respective listener. Rule: %s", log.Prettify(r.rs.desired))
114+
// r.rs.current = r.rs.desired
108115

109116
case r.rs.current == nil: // rule doesn't exist and should be created
110117
r.logger.Infof("Start Rule creation.")
@@ -121,9 +128,6 @@ func (r *Rule) Reconcile(rOpts *ReconcileOptions) error {
121128
return err
122129
}
123130
rOpts.Eventf(api.EventTypeNormal, "MODIFY", "%s rule modified", *r.rs.current.Priority)
124-
125-
default:
126-
// r.logger.Debugf("No rule modification required.")
127131
}
128132

129133
return nil
@@ -150,8 +154,6 @@ func (r *Rule) create(rOpts *ReconcileOptions) error {
150154
Priority: priority(r.rs.desired.Priority),
151155
}
152156

153-
in.Actions[0].TargetGroupArn = r.TargetGroupArn(rOpts.TargetGroups)
154-
155157
o, err := albelbv2.ELBV2svc.CreateRule(in)
156158
if err != nil {
157159
rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error creating %v rule: %s", *in.Priority, err.Error())
@@ -169,7 +171,6 @@ func (r *Rule) modify(rOpts *ReconcileOptions) error {
169171
Conditions: r.rs.desired.Conditions,
170172
RuleArn: r.rs.current.RuleArn,
171173
}
172-
in.Actions[0].TargetGroupArn = r.TargetGroupArn(rOpts.TargetGroups)
173174

174175
o, err := albelbv2.ELBV2svc.ModifyRule(in)
175176
if err != nil {
@@ -217,7 +218,6 @@ func (r *Rule) needsModification() bool {
217218
case crs == nil:
218219
r.logger.Debugf("Current is nil")
219220
return true
220-
// TODO: We need to sort these because they're causing false positives
221221
case !conditionsEqual(crs.Conditions, drs.Conditions):
222222
r.logger.Debugf("Conditions needs to be changed (%v != %v)", log.Prettify(crs.Conditions), log.Prettify(drs.Conditions))
223223
return true

‎pkg/alb/rs/rules.go

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ func NewCurrentRules(o *NewCurrentRulesOptions) (Rules, error) {
4444

4545
rs = append(rs, newRule)
4646
}
47+
4748
return rs, nil
4849
}
4950

@@ -130,23 +131,30 @@ func (r Rules) FindByPriority(priority *string) (int, *Rule) {
130131
// FindUnusedTGs returns a list of TargetGroups that are no longer referncd by any of
131132
// the rules passed into this method.
132133
func (r Rules) FindUnusedTGs(tgs tg.TargetGroups) tg.TargetGroups {
133-
unused := tg.TargetGroups{}
134+
var unused tg.TargetGroups
134135

136+
TG:
135137
for _, t := range tgs {
136138
used := false
139+
140+
arn := t.CurrentARN()
141+
if arn == nil {
142+
continue
143+
}
144+
137145
for _, rule := range r {
138-
if rule.rs.current != nil && rule.rs.current.Actions[0] != nil && rule.rs.current.Actions[0].TargetGroupArn == nil {
139-
continue
140-
}
141-
arn := t.CurrentARN()
142-
if arn == nil {
143-
continue
146+
if rule.rs.current == nil {
147+
continue TG
144148
}
145-
if rule.rs.current != nil && rule.rs.current.Actions[0] != nil && *rule.rs.current.Actions[0].TargetGroupArn == *arn {
146-
used = true
147-
break
149+
150+
for _, action := range rule.rs.current.Actions {
151+
if *action.TargetGroupArn == *arn {
152+
used = true
153+
continue TG
154+
}
148155
}
149156
}
157+
150158
if !used {
151159
unused = append(unused, t)
152160
}

‎pkg/alb/tg/targetgroup.go

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,14 @@ type NewDesiredTargetGroupOptions struct {
3636
func NewDesiredTargetGroup(o *NewDesiredTargetGroupOptions) *TargetGroup {
3737
hasher := md5.New()
3838
hasher.Write([]byte(o.LoadBalancerID))
39-
output := hex.EncodeToString(hasher.Sum(nil))
4039

41-
id := fmt.Sprintf("%.12s-%.5d-%.5s-%.7s", o.ALBNamePrefix, o.Port, *o.Annotations.BackendProtocol, output)
40+
targetType := aws.String("instance")
41+
if *o.Annotations.RoutingTarget == "pod" {
42+
targetType = aws.String("ip")
43+
hasher.Write([]byte(*targetType))
44+
}
45+
46+
id := fmt.Sprintf("%.12s-%.5d-%.5s-%.7s", o.ALBNamePrefix, o.Port, *o.Annotations.BackendProtocol, hex.EncodeToString(hasher.Sum(nil)))
4247

4348
// TODO: Quick fix as we can't have the loadbalancer and target groups share pointers to the same
4449
// tags. Each modify tags individually and can cause bad side-effects.
@@ -73,6 +78,7 @@ func NewDesiredTargetGroup(o *NewDesiredTargetGroupOptions) *TargetGroup {
7378
Port: aws.Int64(o.Port),
7479
Protocol: o.Annotations.BackendProtocol,
7580
TargetGroupName: aws.String(id),
81+
TargetType: targetType,
7682
UnhealthyThresholdCount: o.Annotations.UnhealthyThresholdCount,
7783
// VpcId:
7884
},
@@ -109,12 +115,6 @@ type NewCurrentTargetGroupOptions struct {
109115

110116
// NewCurrentTargetGroup returns a new targetgroup.TargetGroup from an elbv2.TargetGroup.
111117
func NewCurrentTargetGroup(o *NewCurrentTargetGroupOptions) (*TargetGroup, error) {
112-
hasher := md5.New()
113-
hasher.Write([]byte(o.LoadBalancerID))
114-
output := hex.EncodeToString(hasher.Sum(nil))
115-
116-
id := fmt.Sprintf("%.12s-%.5d-%.5s-%.7s", o.ALBNamePrefix, *o.TargetGroup.Port, *o.TargetGroup.Protocol, output)
117-
118118
tgTags := o.ResourceTags.TargetGroups[*o.TargetGroup.TargetGroupArn]
119119

120120
svcName, err := tagsFromTG(tgTags)
@@ -128,7 +128,7 @@ func NewCurrentTargetGroup(o *NewCurrentTargetGroupOptions) (*TargetGroup, error
128128
}
129129

130130
return &TargetGroup{
131-
ID: id,
131+
ID: *o.TargetGroup.TargetGroupName,
132132
SvcName: svcName,
133133
logger: o.Logger,
134134
attributes: attributes{current: attrs},
@@ -146,12 +146,16 @@ func (t *TargetGroup) Reconcile(rOpts *ReconcileOptions) error {
146146
// However, target groups aren't deleted until after rules are created
147147
// Ensuring we know what target groups are truly no longer in use.
148148
case t.tg.desired == nil && !rOpts.IgnoreDeletes:
149-
t.logger.Infof("Start TargetGroup deletion.")
149+
t.logger.Infof("Start TargetGroup deletion. ARN: %s | Name: %s.",
150+
*t.tg.current.TargetGroupArn,
151+
*t.tg.current.TargetGroupName)
150152
if err := t.delete(rOpts); err != nil {
151153
return err
152154
}
153155
rOpts.Eventf(api.EventTypeNormal, "DELETE", "%v target group deleted", t.ID)
154-
t.logger.Infof("Completed TargetGroup deletion.")
156+
t.logger.Infof("Completed TargetGroup deletion. ARN: %s | Name: %s.",
157+
*t.tg.current.TargetGroupArn,
158+
*t.tg.current.TargetGroupName)
155159

156160
// No CurrentState means target group doesn't exist in AWS and should be created.
157161
case t.tg.current == nil:
@@ -169,7 +173,7 @@ func (t *TargetGroup) Reconcile(rOpts *ReconcileOptions) error {
169173
if err := t.modify(mods, rOpts); err != nil {
170174
return err
171175
}
172-
rOpts.Eventf(api.EventTypeNormal, "CREATE", "%s target group modified", t.ID)
176+
rOpts.Eventf(api.EventTypeNormal, "MODIFY", "%s target group modified", t.ID)
173177
}
174178
}
175179

@@ -191,7 +195,8 @@ func (t *TargetGroup) create(rOpts *ReconcileOptions) error {
191195
Port: desired.Port,
192196
Protocol: desired.Protocol,
193197
Name: desired.TargetGroupName,
194-
UnhealthyThresholdCount: desired.UnhealthyThresholdCount,
198+
TargetType: desired.TargetType,
199+
UnhealthyThresholdCount: desired.UnhealthyThresholdCount,
195200
VpcId: rOpts.VpcID,
196201
}
197202

@@ -329,6 +334,11 @@ func (t *TargetGroup) needsModification() tgChange {
329334
return changes
330335
}
331336

337+
if dtg == nil {
338+
// t.logger.Debugf("Desired Target Group is undefined")
339+
return changes
340+
}
341+
332342
if !util.DeepEqual(ctg.HealthCheckIntervalSeconds, dtg.HealthCheckIntervalSeconds) {
333343
t.logger.Debugf("HealthCheckIntervalSeconds needs to be changed (%v != %v)", log.Prettify(ctg.HealthCheckIntervalSeconds), log.Prettify(dtg.HealthCheckIntervalSeconds))
334344
changes |= paramsModified
@@ -446,3 +456,17 @@ func (t *TargetGroup) CurrentARN() *string {
446456
func (t *TargetGroup) CurrentTargets() albelbv2.TargetDescriptions {
447457
return t.targets.current
448458
}
459+
460+
func (t *TargetGroup) StripDesiredState() {
461+
t.tags.desired = nil
462+
t.tg.desired = nil
463+
t.targets.desired = nil
464+
t.attributes.desired = nil
465+
}
466+
467+
func (t *TargetGroup) copyDesiredState(s *TargetGroup) {
468+
t.tags.desired = s.tags.desired
469+
t.attributes.desired = s.attributes.desired
470+
t.targets.desired = s.targets.desired
471+
t.tg.desired = s.tg.desired
472+
}

0 commit comments

Comments
 (0)