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: Add ability to restart maxUnavailable pods to BlueGreen strategy #937

Merged
merged 15 commits into from
Feb 5, 2021

Conversation

khhirani
Copy link
Contributor

@khhirani khhirani commented Jan 12, 2021

Closes #866

Signed-off-by: khhirani [email protected]

@khhirani khhirani force-pushed the bluegreen-maxunavailable branch from c169f95 to 0e5f490 Compare January 12, 2021 09:30
@codecov-io
Copy link

codecov-io commented Jan 12, 2021

Codecov Report

Merging #937 (0e85f9d) into master (01055cf) will decrease coverage by 0.00%.
The diff coverage is 77.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #937      +/-   ##
==========================================
- Coverage   81.60%   81.59%   -0.01%     
==========================================
  Files          99       99              
  Lines        8732     8739       +7     
==========================================
+ Hits         7126     7131       +5     
- Misses       1151     1152       +1     
- Partials      455      456       +1     
Impacted Files Coverage Δ
utils/defaults/defaults.go 95.34% <0.00%> (-4.66%) ⬇️
utils/replicaset/replicaset.go 89.70% <100.00%> (+0.19%) ⬆️

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 01055cf...0e85f9d. Read the comment docs.

@khhirani khhirani changed the title feat: Add ability to reestart maxUnavailable pods to BlueGreen strategy feat: Add ability to restart maxUnavailable pods to BlueGreen strategy Jan 12, 2021
// Value can be an absolute number (ex: 5) or a percentage of total pods at the start of update (ex: 10%).
// Absolute number is calculated from percentage by rounding down.
// This can not be 0 if MaxSurge is 0.
// By default, a fixed value of 1 is used.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to figure out what the real default is (what kubernetes chooses). I originally thought it was 20%, then yesterday I thought it was 0, and now I see this comment (which you copied from canary) stating it's 1. But now I'm reading documentation says it's 25%:
https://kubernetes.io/docs/concepts/workloads/controllers/deployment/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The answer is:

Copy link
Member

@jessesuen jessesuen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any tests for this

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 4, 2021

Kudos, SonarCloud Quality Gate passed!

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
11.8% 11.8% Duplication

@khhirani khhirani merged commit 7384684 into argoproj:master Feb 5, 2021
@khhirani khhirani deleted the bluegreen-maxunavailable branch February 5, 2021 02:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to restart up to maxUnavailable pods at a time for a bluegreen rollout
3 participants