ECS: Add UI Logs for Listener Rules in TRAFFIC_ROUTING and ROLLBACK #5842

niladrix719 · 2025-05-16T17:32:19Z

What this PR does:

Adds UI Logs for Listener Rules in TRAFFIC_ROUTING and ROLLBACK

Why we need it:

Users will now see additional logs or UI messages specifying which listener rules were controlled during deployment transitions

Which issue(s) this PR fixes:

Fixes #5835

Does this PR introduce a user-facing change?: No

How are users affected by this change: No
Is this breaking change: No
How to migrate (if breaking change): No

Signed-off-by: “niladrix719” <[email protected]>

t-kikuc

Thank you!
Almost LGTM

t-kikuc · 2025-05-16T17:39:44Z

pkg/app/piped/platformprovider/ecs/ecs.go

@@ -64,7 +64,7 @@ type ELB interface {
 	GetListenerArns(ctx context.Context, targetGroup types.LoadBalancer) ([]string, error)
 	// ModifyListeners modifies the actions of type ActionTypeEnumForward to perform routing traffic
 	// to the given target groups. Other actions won't be modified.
-	ModifyListeners(ctx context.Context, listenerArns []string, routingTrafficCfg RoutingTrafficConfig) error
+	ModifyListeners(ctx context.Context, listenerArns []string, routingTrafficCfg RoutingTrafficConfig) ([]string, error)


Please add the name of []string to clarify

t-kikuc · 2025-05-16T17:42:44Z

pkg/app/piped/executor/ecs/rollback.go

 			in.LogPersister.Errorf("Failed to routing traffic to PRIMARY/CANARY variants: %v", err)
 			return false
 		}
+
+		if len(modifiedRules) > 0 {
+			in.LogPersister.Infof("Successfully modified ELB listener rules: %s", strings.Join(modifiedRules, ", "))


[nit] readable when n of rules >1 (ecs.go too)

Suggested change

in.LogPersister.Infof("Successfully modified ELB listener rules: %s", strings.Join(modifiedRules, ", "))

in.LogPersister.Infof("Successfully modified %d ELB listener rules: %s", len(modifiedRules), strings.Join(modifiedRules, ", "))

t-kikuc · 2025-05-16T17:48:20Z

pkg/app/piped/platformprovider/ecs/client.go

 				}
+				modifiedRuleArns = append(modifiedRuleArns, *rule.RuleArn)


In error handlings, return the current modifiedRuleArns and show UI log in order to tell which rules are modified

Signed-off-by: Niladri Adhikary <[email protected]>

Signed-off-by: “niladrix719” <[email protected]>

niladrix719 · 2025-05-16T19:40:20Z

Let me know if there’s anything else that needs updating

Signed-off-by: “niladrix719” <[email protected]>

Warashi

The implementation is LGTM.
I want to add a note comment for the behavior.

Warashi · 2025-05-19T00:31:01Z

pkg/app/piped/platformprovider/ecs/ecs.go

-	ModifyListeners(ctx context.Context, listenerArns []string, routingTrafficCfg RoutingTrafficConfig) error
+	ModifyListeners(ctx context.Context, listenerArns []string, routingTrafficCfg RoutingTrafficConfig) (modifiedRuleArns []string, err error)


Please add comments for methods to note that it returns a non-nil modifiedRuleArns value even when returning a non-nil error because it differs from the usual behavior of the go codes.

Added a note

Signed-off-by: “niladrix719” <[email protected]>

pkg/app/piped/platformprovider/ecs/client.go

Co-authored-by: Shinnosuke Sawada-Dazai <[email protected]> Signed-off-by: Niladri Adhikary <[email protected]>

Warashi

LGTM 👍🏻
Thank you!

t-kikuc

Great, thank you!!!

…ipe-cd#5842) * ECS: Add UI Logs for Listener Rules in TRAFFIC_ROUTING and ROLLBACK Signed-off-by: “niladrix719” <[email protected]> * resolved conflicts Signed-off-by: “niladrix719” <[email protected]> * applied requested changes Signed-off-by: “niladrix719” <[email protected]> * fix: lint Signed-off-by: “niladrix719” <[email protected]> * added note for ModifyListeners method Signed-off-by: “niladrix719” <[email protected]> * Update pkg/app/piped/platformprovider/ecs/client.go Co-authored-by: Shinnosuke Sawada-Dazai <[email protected]> Signed-off-by: Niladri Adhikary <[email protected]> --------- Signed-off-by: “niladrix719” <[email protected]> Signed-off-by: Niladri Adhikary <[email protected]> Co-authored-by: Shinnosuke Sawada-Dazai <[email protected]> Signed-off-by: Tushar240503 <[email protected]>

…ipe-cd#5842) * ECS: Add UI Logs for Listener Rules in TRAFFIC_ROUTING and ROLLBACK Signed-off-by: “niladrix719” <[email protected]> * resolved conflicts Signed-off-by: “niladrix719” <[email protected]> * applied requested changes Signed-off-by: “niladrix719” <[email protected]> * fix: lint Signed-off-by: “niladrix719” <[email protected]> * added note for ModifyListeners method Signed-off-by: “niladrix719” <[email protected]> * Update pkg/app/piped/platformprovider/ecs/client.go Co-authored-by: Shinnosuke Sawada-Dazai <[email protected]> Signed-off-by: Niladri Adhikary <[email protected]> --------- Signed-off-by: “niladrix719” <[email protected]> Signed-off-by: Niladri Adhikary <[email protected]> Co-authored-by: Shinnosuke Sawada-Dazai <[email protected]>

…ipe-cd#5842) * ECS: Add UI Logs for Listener Rules in TRAFFIC_ROUTING and ROLLBACK Signed-off-by: “niladrix719” <[email protected]> * resolved conflicts Signed-off-by: “niladrix719” <[email protected]> * applied requested changes Signed-off-by: “niladrix719” <[email protected]> * fix: lint Signed-off-by: “niladrix719” <[email protected]> * added note for ModifyListeners method Signed-off-by: “niladrix719” <[email protected]> * Update pkg/app/piped/platformprovider/ecs/client.go Co-authored-by: Shinnosuke Sawada-Dazai <[email protected]> Signed-off-by: Niladri Adhikary <[email protected]> --------- Signed-off-by: “niladrix719” <[email protected]> Signed-off-by: Niladri Adhikary <[email protected]> Co-authored-by: Shinnosuke Sawada-Dazai <[email protected]> Signed-off-by: Arya Soni <[email protected]>

niladrix719 requested review from khanhtc1202, ffjlabo, t-kikuc and Warashi as code owners May 16, 2025 17:32

github-actions bot added the area/go label May 16, 2025

niladrix719 force-pushed the ecs-log-listener-rules#5835 branch from 0178125 to fc25083 Compare May 16, 2025 17:40

niladrix719 added 2 commits May 16, 2025 23:18

ECS: Add UI Logs for Listener Rules in TRAFFIC_ROUTING and ROLLBACK

e73d864

Signed-off-by: “niladrix719” <[email protected]>

resolved conflicts

d0ced61

Signed-off-by: “niladrix719” <[email protected]>

niladrix719 force-pushed the ecs-log-listener-rules#5835 branch from 1d3fe2f to d0ced61 Compare May 16, 2025 17:48

t-kikuc requested changes May 16, 2025

View reviewed changes

niladrix719 and others added 2 commits May 16, 2025 23:34

Merge branch 'master' into ecs-log-listener-rules#5835

2b069b0

Signed-off-by: Niladri Adhikary <[email protected]>

applied requested changes

8bde09d

Signed-off-by: “niladrix719” <[email protected]>

niladrix719 requested a review from t-kikuc May 16, 2025 19:40

fix: lint

ca341d8

Signed-off-by: “niladrix719” <[email protected]>

Warashi reviewed May 19, 2025

View reviewed changes

niladrix719 and others added 2 commits May 19, 2025 11:58

added note for ModifyListeners method

35315ac

Signed-off-by: “niladrix719” <[email protected]>

Merge branch 'master' into ecs-log-listener-rules#5835

19f5643

Warashi reviewed May 19, 2025

View reviewed changes

pkg/app/piped/platformprovider/ecs/client.go Outdated Show resolved Hide resolved

Update pkg/app/piped/platformprovider/ecs/client.go

ce1a4e0

Co-authored-by: Shinnosuke Sawada-Dazai <[email protected]> Signed-off-by: Niladri Adhikary <[email protected]>

Warashi approved these changes May 19, 2025

View reviewed changes

t-kikuc approved these changes May 20, 2025

View reviewed changes

t-kikuc added the cherry-pick label May 20, 2025

t-kikuc merged commit d137010 into pipe-cd:master May 20, 2025
17 checks passed

github-actions bot mentioned this pull request May 28, 2025

[bot] Release v0.52.0 #5888

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ECS: Add UI Logs for Listener Rules in TRAFFIC_ROUTING and ROLLBACK #5842

ECS: Add UI Logs for Listener Rules in TRAFFIC_ROUTING and ROLLBACK #5842

Uh oh!

niladrix719 commented May 16, 2025 •

edited

Loading

Uh oh!

t-kikuc left a comment

Uh oh!

t-kikuc May 16, 2025

Uh oh!

t-kikuc May 16, 2025

Uh oh!

t-kikuc May 16, 2025

Uh oh!

niladrix719 commented May 16, 2025

Uh oh!

Warashi left a comment

Uh oh!

Warashi May 19, 2025

Uh oh!

niladrix719 May 19, 2025

Uh oh!

Uh oh!

Warashi left a comment

Uh oh!

t-kikuc left a comment

Uh oh!

Uh oh!

Uh oh!

	in.LogPersister.Infof("Successfully modified ELB listener rules: %s", strings.Join(modifiedRules, ", "))
	in.LogPersister.Infof("Successfully modified %d ELB listener rules: %s", len(modifiedRules), strings.Join(modifiedRules, ", "))

		ModifyListeners(ctx context.Context, listenerArns []string, routingTrafficCfg RoutingTrafficConfig) error
		ModifyListeners(ctx context.Context, listenerArns []string, routingTrafficCfg RoutingTrafficConfig) (modifiedRuleArns []string, err error)

ECS: Add UI Logs for Listener Rules in TRAFFIC_ROUTING and ROLLBACK #5842

ECS: Add UI Logs for Listener Rules in TRAFFIC_ROUTING and ROLLBACK #5842

Uh oh!

Conversation

niladrix719 commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

t-kikuc left a comment

Choose a reason for hiding this comment

Uh oh!

t-kikuc May 16, 2025

Choose a reason for hiding this comment

Uh oh!

t-kikuc May 16, 2025

Choose a reason for hiding this comment

Uh oh!

t-kikuc May 16, 2025

Choose a reason for hiding this comment

Uh oh!

niladrix719 commented May 16, 2025

Uh oh!

Warashi left a comment

Choose a reason for hiding this comment

Uh oh!

Warashi May 19, 2025

Choose a reason for hiding this comment

Uh oh!

niladrix719 May 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Warashi left a comment

Choose a reason for hiding this comment

Uh oh!

t-kikuc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

niladrix719 commented May 16, 2025 •

edited

Loading