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

Add minReadySeconds & strategy support #2672

Merged
merged 3 commits into from
May 18, 2022

Conversation

cmk-pcs
Copy link
Contributor

@cmk-pcs cmk-pcs commented May 10, 2022

Proposed changes

Adding support for deployment strategy and minReadySeconds to helm chart.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@brianehlert
Copy link
Collaborator

@cmk-pcs Could you please open a corresponding issue to help us track your PR?
Thank you!

@cmk-pcs
Copy link
Contributor Author

cmk-pcs commented May 11, 2022

@cmk-pcs Could you please open a corresponding issue to help us track your PR? Thank you!

Thanks, added: #2677

Copy link
Member

@lucacome lucacome left a comment

Choose a reason for hiding this comment

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

Hi @cmk-pcs

Thanks for the PR!
Would you mind also adding the new fields and comments in deployments/helm-chart/values.yaml?

Could you also add the equivalent for the DaemonSet?

Thanks.

@lucacome lucacome requested review from a team, haywoodsh, jjngx and shaun-nx May 13, 2022 01:06
@cmk-pcs cmk-pcs requested a review from lucacome May 13, 2022 06:53
Co-authored-by: Luca Comellini <[email protected]>
@cmk-pcs cmk-pcs requested a review from lucacome May 17, 2022 06:38
@lucacome lucacome requested a review from ciarams87 May 17, 2022 15:43
@codecov-commenter
Copy link

Codecov Report

Merging #2672 (8c9ce50) into main (8b53e1e) will increase coverage by 0.08%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2672      +/-   ##
==========================================
+ Coverage   53.39%   53.48%   +0.08%     
==========================================
  Files          52       52              
  Lines       14706    14751      +45     
==========================================
+ Hits         7853     7889      +36     
- Misses       6591     6599       +8     
- Partials      262      263       +1     
Impacted Files Coverage Δ
internal/configs/virtualserver.go 95.13% <0.00%> (-0.19%) ⬇️
internal/k8s/status.go 34.23% <0.00%> (-0.08%) ⬇️
internal/configs/ingress.go 76.49% <0.00%> (-0.06%) ⬇️
internal/configs/version2/http.go 0.00% <0.00%> (ø)
pkg/apis/configuration/validation/virtualserver.go 94.74% <0.00%> (ø)
pkg/apis/configuration/validation/policy.go 94.47% <0.00%> (+0.04%) ⬆️
internal/k8s/configuration.go 95.86% <0.00%> (+0.38%) ⬆️
internal/k8s/controller.go 11.37% <0.00%> (+0.49%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@lucacome lucacome merged commit aab63d3 into nginx:main May 18, 2022
@cmk-pcs cmk-pcs deleted the features/additional-deployment-support branch May 19, 2022 19:46
@ciarams87 ciarams87 added the enhancement Pull requests for new features/feature enhancements label Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Pull requests for new features/feature enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants