Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
8 changes: 7 additions & 1 deletion pkg/app/piped/executor/ecs/ecs.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,13 @@ func routing(ctx context.Context, in *executor.Input, platformProviderName strin
return false
}

if err := client.ModifyListeners(ctx, currListenerArns, routingTrafficCfg); err != nil {
currListenerRuleArns, err := client.GetListenerRuleArns(ctx, currListenerArns)
if err != nil {
in.LogPersister.Errorf("Failed to get current active listener rule: %v", err)
return false
}

if err := client.ModifyListenerOrRule(ctx, currListenerArns, currListenerRuleArns, routingTrafficCfg); err != nil {
in.LogPersister.Errorf("Failed to routing traffic to PRIMARY/CANARY variants: %v", err)
return false
}
Expand Down
10 changes: 8 additions & 2 deletions pkg/app/piped/executor/ecs/rollback.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,14 @@ func rollback(ctx context.Context, in *executor.Input, platformProviderName stri
return false
}

if err := client.ModifyListeners(ctx, currListenerArns, routingTrafficCfg); err != nil {
in.LogPersister.Errorf("Failed to routing traffic to PRIMARY variant: %v", err)
currListenerRuleArns, err := client.GetListenerRuleArns(ctx, currListenerArns)
if err != nil {
in.LogPersister.Errorf("Failed to get current active listener rule: %v", err)
return false
}

if err := client.ModifyListenerOrRule(ctx, currListenerArns, currListenerRuleArns, routingTrafficCfg); err != nil {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of passing currListenerRuleArns to the function ModifyListenerOrRule (yes, the name is confusing), we should check currListenerRuleArns > 0 in this place, and call to ModifyRules if the condition check passed or call to ModifyListeners (the previous one) if the condition return false. wdyt? 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@khanhtc1202
Thanks for the review. I've implemented the check for currListenerRuleArns and used ModifyRules or ModifyListeners accordingly to enhance rollback precision.
965ba41

in.LogPersister.Errorf("Failed to routing traffic to PRIMARY/CANARY variants: %v", err)
return false
}
}
Expand Down
154 changes: 132 additions & 22 deletions pkg/app/piped/platformprovider/ecs/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,30 @@ func (c *client) GetListenerArns(ctx context.Context, targetGroup types.LoadBala
return arns, nil
}

func (c *client) GetListenerRuleArns(ctx context.Context, listenerArns []string) ([]string, error) {
var ruleArns []string

// Fetch all rules by listeners
for _, listenerArn := range listenerArns {
input := &elasticloadbalancingv2.DescribeRulesInput{
ListenerArn: aws.String(listenerArn),
}
output, err := c.elbClient.DescribeRules(ctx, input)
if err != nil {
return nil, err
}
for _, rule := range output.Rules {
ruleArns = append(ruleArns, *rule.RuleArn)
}
}

if len(ruleArns) == 0 {
return nil, platformprovider.ErrNotFound
}

return ruleArns, nil
}

func (c *client) getLoadBalancerArn(ctx context.Context, targetGroupArn string) (string, error) {
input := &elasticloadbalancingv2.DescribeTargetGroupsInput{
TargetGroupArns: []string{targetGroupArn},
Expand All @@ -432,41 +456,127 @@ func (c *client) getLoadBalancerArn(ctx context.Context, targetGroupArn string)
return output.TargetGroups[0].LoadBalancerArns[0], nil
}

func (c *client) ModifyListeners(ctx context.Context, listenerArns []string, routingTrafficCfg RoutingTrafficConfig) error {
func (c *client) ModifyListenerOrRule(ctx context.Context, listenerArns []string, listenerRuleArns []string, routingTrafficCfg RoutingTrafficConfig) error {
if len(routingTrafficCfg) != 2 {
return fmt.Errorf("invalid listener configuration: requires 2 target groups")
}

modifyListener := func(ctx context.Context, listenerArn string) error {
input := &elasticloadbalancingv2.ModifyListenerInput{
ListenerArn: aws.String(listenerArn),
DefaultActions: []elbtypes.Action{
{
Type: elbtypes.ActionTypeEnumForward,
ForwardConfig: &elbtypes.ForwardActionConfig{
TargetGroups: []elbtypes.TargetGroupTuple{
{
TargetGroupArn: aws.String(routingTrafficCfg[0].TargetGroupArn),
Weight: aws.Int32(int32(routingTrafficCfg[0].Weight)),
},
{
TargetGroupArn: aws.String(routingTrafficCfg[1].TargetGroupArn),
Weight: aws.Int32(int32(routingTrafficCfg[1].Weight)),
if len(listenerRuleArns) > 0 {
modifyListenerRule := func(ctx context.Context, listenerRuleArn string) error {
input := &elasticloadbalancingv2.ModifyRuleInput{
RuleArn: aws.String(listenerRuleArn),
Actions: []elbtypes.Action{
{
Type: elbtypes.ActionTypeEnumForward,
ForwardConfig: &elbtypes.ForwardActionConfig{
TargetGroups: []elbtypes.TargetGroupTuple{
{
TargetGroupArn: aws.String(routingTrafficCfg[0].TargetGroupArn),
Weight: aws.Int32(int32(routingTrafficCfg[0].Weight)),
},
{
TargetGroupArn: aws.String(routingTrafficCfg[1].TargetGroupArn),
Weight: aws.Int32(int32(routingTrafficCfg[1].Weight)),
},
},
},
},
},
},
}
_, err := c.elbClient.ModifyRule(ctx, input)
return err
}
_, err := c.elbClient.ModifyListener(ctx, input)
return err
}

for _, listener := range listenerArns {
if err := modifyListener(ctx, listener); err != nil {
for _, ruleArn := range listenerRuleArns {
// Describe the rule to get current actions
describeRulesOutput, err := c.elbClient.DescribeRules(ctx, &elasticloadbalancingv2.DescribeRulesInput{
RuleArns: []string{ruleArn},
})
if err != nil {
return fmt.Errorf("error describing listener rule %v: %w", ruleArn, err)
}

// No rules found, nothing to modify
if len(describeRulesOutput.Rules) == 0 {
return fmt.Errorf("no rules found for ARN: %s", ruleArn)
}

// Check if the current action type is forward
for _, rule := range describeRulesOutput.Rules {
for _, action := range rule.Actions {
if action.Type == elbtypes.ActionTypeEnumForward {
// Call modifyListenerRule only if action type is forward
if err := modifyListenerRule(ctx, ruleArn); err != nil {
return err
}
// Break the loop once the modify function is called for the rule
break
}
}
}
}

return nil
}

if len(listenerRuleArns) == 0 {
modifyListener := func(ctx context.Context, listenerArn string) error {
input := &elasticloadbalancingv2.ModifyListenerInput{
ListenerArn: aws.String(listenerArn),
DefaultActions: []elbtypes.Action{
{
Type: elbtypes.ActionTypeEnumForward,
ForwardConfig: &elbtypes.ForwardActionConfig{
TargetGroups: []elbtypes.TargetGroupTuple{
{
TargetGroupArn: aws.String(routingTrafficCfg[0].TargetGroupArn),
Weight: aws.Int32(int32(routingTrafficCfg[0].Weight)),
},
{
TargetGroupArn: aws.String(routingTrafficCfg[1].TargetGroupArn),
Weight: aws.Int32(int32(routingTrafficCfg[1].Weight)),
},
},
},
},
},
}
_, err := c.elbClient.ModifyListener(ctx, input)
return err
}

for _, listenerArn := range listenerArns {
// Describe the listener to get current default actions
describeListenerOutput, err := c.elbClient.DescribeListeners(ctx, &elasticloadbalancingv2.DescribeListenersInput{
ListenerArns: []string{listenerArn},
})
if err != nil {
return fmt.Errorf("error describing listener %v: %w", listenerArn, err)
}

// No listeners found, nothing to modify
if len(describeListenerOutput.Listeners) == 0 {
return fmt.Errorf("no listeners found for ARN: %s", listenerArn)
}

// Check if the current default action type is forward
for _, listener := range describeListenerOutput.Listeners {
for _, action := range listener.DefaultActions {
if action.Type == elbtypes.ActionTypeEnumForward {
// Call modifyListener only if default action type is forward
if err := modifyListener(ctx, listenerArn); err != nil {
return err
}
// Break the loop once the modify function is called for the listener
break
}
}
}
}

return nil
}

return nil
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/app/piped/platformprovider/ecs/ecs.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ type ECS interface {

type ELB interface {
GetListenerArns(ctx context.Context, targetGroup types.LoadBalancer) ([]string, error)
ModifyListeners(ctx context.Context, listenerArns []string, routingTrafficCfg RoutingTrafficConfig) error
GetListenerRuleArns(ctx context.Context, listenerArns []string) ([]string, error)
ModifyListenerOrRule(ctx context.Context, listenerArns []string, listenerRuleArns []string, routingTrafficCfg RoutingTrafficConfig) error
}

// Registry holds a pool of aws client wrappers.
Expand Down