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

feat: rollback windows. Fixes #574 #2394

Merged
merged 18 commits into from
Nov 25, 2022
Merged

Conversation

alexef
Copy link
Member

@alexef alexef commented Nov 3, 2022

fixes: #574

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.

fixes: argoproj#574
Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
fixes: argoproj#574
Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2022

Go Published Test Results

1 800 tests   1 800 ✔️  2m 30s ⏱️
   102 suites         0 💤
       1 files           0

Results for commit 4d32999.

♻️ This comment has been updated with latest results.

@codecov
Copy link

codecov bot commented Nov 3, 2022

Codecov Report

Base: 81.55% // Head: 81.58% // Increases project coverage by +0.02% 🎉

Coverage data is based on head (4d32999) compared to base (8987009).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2394      +/-   ##
==========================================
+ Coverage   81.55%   81.58%   +0.02%     
==========================================
  Files         124      124              
  Lines       18931    18959      +28     
==========================================
+ Hits        15439    15467      +28     
  Misses       2702     2702              
  Partials      790      790              
Impacted Files Coverage Δ
rollout/bluegreen.go 68.30% <100.00%> (ø)
rollout/canary.go 75.48% <100.00%> (ø)
rollout/sync.go 80.13% <100.00%> (+0.76%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2022

E2E Tests Published Test Results

    2 files      2 suites   1h 36m 2s ⏱️
  92 tests   87 ✔️ 3 💤 2
186 runs  178 ✔️ 6 💤 2

For more details on these failures, see this check.

Results for commit 4d32999.

♻️ This comment has been updated with latest results.

@alexef
Copy link
Member Author

alexef commented Nov 4, 2022

@jessesuen here's my first attempt at this. I went for the revision count option, the time based one can also be added later on

@zachaller
Copy link
Collaborator

Could you also add some e2e tests for this change? I would like to get the behavior tested there as well.

@alexef
Copy link
Member Author

alexef commented Nov 8, 2022

add some e2e tests for this change

will do. I also need to make sure unit tests cover changes so that build is green

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
@zachaller
Copy link
Collaborator

Can we also get a little bit of documentation on this as well? I will get this reviewed this week as well.

@alexef alexef added this to the v1.4 milestone Nov 15, 2022
@alexef alexef added the ready-for-review Ready for final review label Nov 15, 2022
@zachaller zachaller marked this pull request as draft November 17, 2022 15:43
@zachaller zachaller removed the ready-for-review Ready for final review label Nov 17, 2022
…nalysis when the window is detected

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
@alexef alexef marked this pull request as ready for review November 18, 2022 11:32
@alexef
Copy link
Member Author

alexef commented Nov 18, 2022

@zachaller / @jessesuen can you have another look at this?
We identified the issue that was preventing the windows from working for us, updated the code and the e2e and I believe now the feature is ready.

@alexef alexef added the ready-for-review Ready for final review label Nov 18, 2022
@sonarcloud
Copy link

sonarcloud bot commented Nov 23, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

No Coverage information No Coverage information
0.0% 0.0% Duplication

@zachaller zachaller merged commit c3e305d into argoproj:master Nov 25, 2022
@alexef alexef deleted the rollback-window branch November 25, 2022 16:08
jandersen-plaid pushed a commit to jandersen-plaid/argo-rollouts that referenced this pull request Nov 26, 2022
* feature: introduce rollback windows

fixes: argoproj#574
Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* feature: introduce rollback windows - generated files

fixes: argoproj#574
Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* ran codegen again

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* More unit tests. New e2e

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* More tests to make codecov happy

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* increas lint timeout

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* Exclude Experiment RS when computing rollback window

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* Add documentation around new feature rollbackWindow

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* Fix rollback window; cancel pauses and abort and skip to the end of analysis when the window is detected

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-review Ready for final review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rollback windows for fast-tracked rollbacks
2 participants