Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(controller): rollback should skip all steps to active rs within RollbackWindow #2953

Conversation

amazingandyyy
Copy link
Contributor

@amazingandyyy amazingandyyy commented Aug 11, 2023

Resolve #2939

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.

@amazingandyyy amazingandyyy force-pushed the fix_canary_rollback_to_active_rs_in_rollbackwindow branch 3 times, most recently from 96303fa to 9921cba Compare August 11, 2023 22:26
@amazingandyyy amazingandyyy changed the title fix: Canary rollback should skip all steps to active rs in RollbackWindow fix(canary): rollback should skip all steps to active rs in RollbackWindow Aug 11, 2023
@amazingandyyy amazingandyyy changed the title fix(canary): rollback should skip all steps to active rs in RollbackWindow fix(canary): rollback should skip all steps to active rs within RollbackWindow Aug 11, 2023
@amazingandyyy amazingandyyy force-pushed the fix_canary_rollback_to_active_rs_in_rollbackwindow branch from 9921cba to f80863b Compare August 11, 2023 22:31
@amazingandyyy amazingandyyy changed the title fix(canary): rollback should skip all steps to active rs within RollbackWindow fix(controller): rollback should skip all steps to active rs within RollbackWindow Aug 11, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 11, 2023

Go Published Test Results

2 044 tests   2 044 ✔️  2m 39s ⏱️
   118 suites         0 💤
       1 files           0

Results for commit 8b79f75.

♻️ This comment has been updated with latest results.

@amazingandyyy amazingandyyy marked this pull request as draft August 11, 2023 23:02
@amazingandyyy amazingandyyy force-pushed the fix_canary_rollback_to_active_rs_in_rollbackwindow branch 4 times, most recently from af059e3 to 262c1b4 Compare August 11, 2023 23:15
@amazingandyyy amazingandyyy marked this pull request as ready for review August 11, 2023 23:15
@amazingandyyy amazingandyyy force-pushed the fix_canary_rollback_to_active_rs_in_rollbackwindow branch 2 times, most recently from 9ce8241 to e8ac154 Compare August 11, 2023 23:20
@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (f433582) 81.75% compared to head (8b79f75) 81.76%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2953   +/-   ##
=======================================
  Coverage   81.75%   81.76%           
=======================================
  Files         134      134           
  Lines       20377    20386    +9     
=======================================
+ Hits        16659    16668    +9     
  Misses       2861     2861           
  Partials      857      857           
Files Changed Coverage Δ
rollout/canary.go 75.74% <100.00%> (+0.26%) ⬆️
utils/replicaset/replicaset.go 88.45% <100.00%> (+0.12%) ⬆️

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

@amazingandyyy amazingandyyy marked this pull request as draft August 11, 2023 23:37
@github-actions
Copy link
Contributor

github-actions bot commented Aug 12, 2023

E2E Tests Published Test Results

    4 files      4 suites   3h 39m 8s ⏱️
102 tests   88 ✔️   6 💤   8
428 runs  385 ✔️ 24 💤 19

For more details on these failures, see this check.

Results for commit 8b79f75.

♻️ This comment has been updated with latest results.

@amazingandyyy amazingandyyy force-pushed the fix_canary_rollback_to_active_rs_in_rollbackwindow branch 2 times, most recently from 1ffc1ab to be07a73 Compare August 12, 2023 00:36
@yohanb yohanb force-pushed the fix_canary_rollback_to_active_rs_in_rollbackwindow branch 2 times, most recently from c4187af to d42be54 Compare August 12, 2023 03:48
@amazingandyyy amazingandyyy force-pushed the fix_canary_rollback_to_active_rs_in_rollbackwindow branch 4 times, most recently from 521c90e to 7e7fd26 Compare August 16, 2023 18:06
@amazingandyyy amazingandyyy marked this pull request as ready for review August 16, 2023 20:01
@yohanb yohanb force-pushed the fix_canary_rollback_to_active_rs_in_rollbackwindow branch 2 times, most recently from 3b32b6b to c4940f1 Compare August 16, 2023 20:04
@amazingandyyy
Copy link
Contributor Author

@zachaller can you take a look on this, the issue is blocking us from using RollbackWindow in our production deployment.

@amazingandyyy
Copy link
Contributor Author

@zachaller want to surface this again. thanks! Happy Friday!

yohanb and others added 3 commits August 28, 2023 11:46
Resolve argoproj#2939

Signed-off-by: Andy Chen <amazingandyyy@gmail.com>
…active

Signed-off-by: Andy Chen <amazingandyyy@gmail.com>
Signed-off-by: Andy Chen <amazingandyyy@gmail.com>
@amazingandyyy amazingandyyy force-pushed the fix_canary_rollback_to_active_rs_in_rollbackwindow branch from 18ddda0 to d843ecc Compare August 28, 2023 18:46
…anary_rollback_to_active_rs_in_rollbackwindow
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
47.8% 47.8% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@zachaller zachaller merged commit 8b0215d into argoproj:master Aug 31, 2023
zachaller added a commit that referenced this pull request Aug 31, 2023
…ollbackWindow (#2953)

* fix(canary): skip steps when in rollback window and rs is still active

Resolve #2939

Signed-off-by: Andy Chen <amazingandyyy@gmail.com>

* test(canary): add case where rollback when in window and rs is still active

Signed-off-by: Andy Chen <amazingandyyy@gmail.com>

* test(replicaset): add test case for IsActive function

Signed-off-by: Andy Chen <amazingandyyy@gmail.com>

---------

Signed-off-by: Andy Chen <amazingandyyy@gmail.com>
Co-authored-by: Yohan Belval <ybelval@turo.com>
Co-authored-by: zachaller <zachaller@users.noreply.github.com>
@zachaller zachaller added the cherry-pick-completed Used once we have cherry picked the PR to all requested releases label Aug 31, 2023
@yohanb yohanb deleted the fix_canary_rollback_to_active_rs_in_rollbackwindow branch November 24, 2023 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick/release-1.6 cherry-pick-completed Used once we have cherry picked the PR to all requested releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fast-track rollback on Canary doesn't quite skip stages
3 participants