fix: abort scaledown stable RS for canary with traffic routing

[huikang]

Signed-off-by: Hui Kang hui.kang@salesforce.com

close #1292

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
• The title of the PR is (a) conventional, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
• I've signed my commits with DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My builds are green. Try syncing with master if they are not.
• My organization is added to USERS.md.

[codecov]

Codecov Report

Merging #1331 (154229c) into master (503c520) will decrease coverage by 0.13%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master    #1331      +/-   ##
==========================================
- Coverage   81.40%   81.27%   -0.14%     
==========================================
  Files         106      107       +1     
  Lines        9778     9824      +46     
==========================================
+ Hits         7960     7984      +24     
- Misses       1278     1297      +19     
- Partials      540      543       +3     

Impacted Files	Coverage Δ
utils/istio/multicluster.go	45.00% <45.00%> (ø)
rollout/controller.go	78.35% <100.00%> (ø)
rollout/trafficrouting.go	88.46% <100.00%> (ø)
utils/replicaset/canary.go	82.40% <100.00%> (ø)
utils/defaults/defaults.go	85.71% <0.00%> (ø)
rollout/trafficrouting/ambassador/ambassador.go	84.28% <0.00%> (+0.70%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba21c6c...154229c. Read the comment docs.

[jessesuen]

@huikang I think we should be fixing this in a different way.

Instead of modifying CalculateReplicaCountsForCanary to consider if we see ro.Status.Abort, I think it will be cleaner to modify GetCanaryReplicasOrWeight so that it always returns 100 if we are in a fully promoted state.

Something like:

// GetCanaryReplicasOrWeight either returns a static set of replicas or a weight percentage
func GetCanaryReplicasOrWeight(rollout *v1alpha1.Rollout) (*int32, int32) {
	if rollout.Status.PromoteFull || rollout.Status.CurrentPodHash == rollout.Status.StableRS {
		return nil, 100
	}

WDYT?

[huikang]

Hi, @jessesuen , your approach is more generic than my last one. PR is updated. Thanks for your feedback.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication