fix(controller): prevent negative vsvc weights on a replica scaledown following a canary abort for istio trafficrouting #3467

shubhajyoti-bagchi-groww · 2024-03-23T19:06:17Z

fixes: #3470

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 with a list of types and scopes found here, (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 · 2024-03-23T19:15:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.81%. Comparing base (8405f2e) to head (b2e5fd1).
Report is 53 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3467      +/-   ##
==========================================
+ Coverage   81.83%   82.81%   +0.97%     
==========================================
  Files         135      135              
  Lines       20688    16998    -3690     
==========================================
- Hits        16931    14077    -2854     
+ Misses       2883     2034     -849     
- Partials      874      887      +13

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2024-03-23T19:15:45Z

Go Published Test Results

2 142 tests 2 142 ✅ 2m 51s ⏱️
119 suites 0 💤
1 files 0 ❌

Results for commit b2e5fd1.

♻️ This comment has been updated with latest results.

github-actions · 2024-03-23T20:00:25Z

E2E Tests Published Test Results

4 files 4 suites 3h 45m 52s ⏱️
109 tests 101 ✅ 6 💤 2 ❌
448 runs 412 ✅ 24 💤 12 ❌

For more details on these failures, see this check.

Results for commit b2e5fd1.

♻️ This comment has been updated with latest results.

…m 1.36.1 to 1.36.3 (argoproj#3452) chore(deps): bump github.com/aws/aws-sdk-go-v2/service/cloudwatch Bumps [github.com/aws/aws-sdk-go-v2/service/cloudwatch](https://github.com/aws/aws-sdk-go-v2) from 1.36.1 to 1.36.3. - [Release notes](https://github.com/aws/aws-sdk-go-v2/releases) - [Commits](aws/aws-sdk-go-v2@service/ec2/v1.36.1...service/ssm/v1.36.3) --- updated-dependencies: - dependency-name: github.com/aws/aws-sdk-go-v2/service/cloudwatch dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: shubhajyoti-bagchi-groww <[email protected]>

Signed-off-by: shubhajyoti-bagchi-groww <[email protected]>

…1.10.0 (argoproj#3462) chore(deps): bump slsa-framework/slsa-github-generator Bumps [slsa-framework/slsa-github-generator](https://github.com/slsa-framework/slsa-github-generator) from 1.9.1 to 1.10.0. - [Release notes](https://github.com/slsa-framework/slsa-github-generator/releases) - [Changelog](https://github.com/slsa-framework/slsa-github-generator/blob/main/CHANGELOG.md) - [Commits](slsa-framework/slsa-github-generator@v1.9.1...v1.10.0) --- updated-dependencies: - dependency-name: slsa-framework/slsa-github-generator dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: shubhajyoti-bagchi-groww <[email protected]>

…ancingv2 from 1.30.3 to 1.30.4 (argoproj#3461) chore(deps): bump github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 Bumps [github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2](https://github.com/aws/aws-sdk-go-v2) from 1.30.3 to 1.30.4. - [Release notes](https://github.com/aws/aws-sdk-go-v2/releases) - [Commits](aws/aws-sdk-go-v2@service/s3/v1.30.3...service/s3/v1.30.4) --- updated-dependencies: - dependency-name: github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: shubhajyoti-bagchi-groww <[email protected]>

Signed-off-by: shubhajyoti-bagchi-groww <[email protected]>

sonarcloud · 2024-03-24T10:59:37Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

shubhajyoti-bagchi-groww · 2024-03-25T14:29:51Z

@zachaller Would you be able to take a look at this ?

OpenGuidou · 2024-03-25T15:27:03Z

Hi,
Thanks for this proposal.

If I understand well, this case occurs when the stable.AvailableReplicas > rollout.spec.Replicas, when we use HPA, right ?
If so, does the e2e test really testing this use case ?

shubhajyoti-bagchi-groww · 2024-03-25T16:15:53Z

Hi @OpenGuidou, thanks for the response

Right, this happens in cases whenever stable.AvailableReplicas > rollout.spec.Replicas

HPA is one way where stable.AvailableReplicas > rollout.spec.Replicas without any manual intervention
However, in the issue in which we faced, we had a need to reduce the number of replicas of the rollout which was in aborted state and came across the issue.

The e2e test here focuses on breaking the existing flow using one of the scenarios, without this fix. It kind of reproduces exactly what we faced.

OpenGuidou

LGTM, thanks !

Eslam-mohammed-anwar · 2024-03-26T06:12:21Z

we have a similar issue #3470

Eslam-mohammed-anwar · 2024-03-29T12:57:08Z

@zachaller when should it be released ?

shubhajyoti-bagchi-groww changed the title ~~controller: Fixed negative canary weight bug for istio trafficrouting~~ fix: prevent negative canary weights on a replica scaledown following a canary abort for istio trafficrouting Mar 23, 2024

shubhajyoti-bagchi-groww changed the title ~~fix: prevent negative canary weights on a replica scaledown following a canary abort for istio trafficrouting~~ fix: prevent negative vsvc weights on a replica scaledown following a canary abort for istio trafficrouting Mar 23, 2024

shubhajyoti-bagchi-groww changed the title ~~fix: prevent negative vsvc weights on a replica scaledown following a canary abort for istio trafficrouting~~ fix(controller): prevent negative vsvc weights on a replica scaledown following a canary abort for istio trafficrouting Mar 23, 2024

shubhajyoti-bagchi-groww force-pushed the controller-trafficrouting-fix branch from 1cebbea to 75e1d30 Compare March 23, 2024 19:25

dependabot bot and others added 9 commits March 24, 2024 16:06

controller: Fixed negative canary weight bug for istio trafficrouting

146937b

Signed-off-by: shubhajyoti-bagchi-groww <[email protected]>

Added e2e test to validate the fix

fdb70a6

Signed-off-by: shubhajyoti-bagchi-groww <[email protected]>

Removed local changes

d938c0b

Signed-off-by: shubhajyoti-bagchi-groww <[email protected]>

linted as per golangci-lint run

36d343f

Signed-off-by: shubhajyoti-bagchi-groww <[email protected]>

docs: typo in BlueGreen (argoproj#3463)

5bba5aa

Signed-off-by: shubhajyoti-bagchi-groww <[email protected]>

Removed sleep and waited for 1 replicas

728f0a8

Signed-off-by: shubhajyoti-bagchi-groww <[email protected]>

shubhajyoti-bagchi-groww force-pushed the controller-trafficrouting-fix branch from 24ff7b5 to 728f0a8 Compare March 24, 2024 10:37

shubhajyoti-bagchi-groww and others added 2 commits March 24, 2024 16:07

Merge branch 'master' into controller-trafficrouting-fix

39506b3

Moved up canary weight test to match PR title

b2e5fd1

Signed-off-by: shubhajyoti-bagchi-groww <[email protected]>

OpenGuidou approved these changes Mar 25, 2024

View reviewed changes

zachaller approved these changes Mar 26, 2024

View reviewed changes

zachaller merged commit 4b1cc3b into argoproj:master Mar 26, 2024
24 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(controller): prevent negative vsvc weights on a replica scaledown following a canary abort for istio trafficrouting #3467

fix(controller): prevent negative vsvc weights on a replica scaledown following a canary abort for istio trafficrouting #3467

shubhajyoti-bagchi-groww commented Mar 23, 2024 •

edited by zachaller

Loading

codecov bot commented Mar 23, 2024 •

edited

Loading

github-actions bot commented Mar 23, 2024 •

edited

Loading

github-actions bot commented Mar 23, 2024 •

edited

Loading

sonarcloud bot commented Mar 24, 2024

shubhajyoti-bagchi-groww commented Mar 25, 2024

OpenGuidou commented Mar 25, 2024

shubhajyoti-bagchi-groww commented Mar 25, 2024

OpenGuidou left a comment

Eslam-mohammed-anwar commented Mar 26, 2024

Eslam-mohammed-anwar commented Mar 29, 2024

fix(controller): prevent negative vsvc weights on a replica scaledown following a canary abort for istio trafficrouting #3467

fix(controller): prevent negative vsvc weights on a replica scaledown following a canary abort for istio trafficrouting #3467

Conversation

shubhajyoti-bagchi-groww commented Mar 23, 2024 • edited by zachaller Loading

codecov bot commented Mar 23, 2024 • edited Loading

Codecov Report

github-actions bot commented Mar 23, 2024 • edited Loading

Go Published Test Results

github-actions bot commented Mar 23, 2024 • edited Loading

E2E Tests Published Test Results

sonarcloud bot commented Mar 24, 2024

Quality Gate passed

shubhajyoti-bagchi-groww commented Mar 25, 2024

OpenGuidou commented Mar 25, 2024

shubhajyoti-bagchi-groww commented Mar 25, 2024

OpenGuidou left a comment

Choose a reason for hiding this comment

Eslam-mohammed-anwar commented Mar 26, 2024

Eslam-mohammed-anwar commented Mar 29, 2024

shubhajyoti-bagchi-groww commented Mar 23, 2024 •

edited by zachaller

Loading

codecov bot commented Mar 23, 2024 •

edited

Loading

github-actions bot commented Mar 23, 2024 •

edited

Loading

github-actions bot commented Mar 23, 2024 •

edited

Loading