Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

set maxUnavailable to 1 in addons-manager deployment #2962

Merged
merged 1 commit into from
Jul 19, 2022

Conversation

ggpaue
Copy link
Contributor

@ggpaue ggpaue commented Jul 18, 2022

What this PR does / why we need it

set maxUnavailable to 1 for addons-manager deployment

Which issue(s) this PR fixes

Fixes #2884

Describe testing done for PR

Manual tested the change on kind cluster:

  1. setup single control plane kind cluster
  2. apply addons-manager.yaml to setup addons-manager deployment
  3. use a new image to update addons-manager deployment
  4. Verify addons-manager deployment can be updated

Release note

package-based-lcm:  set maxUnavailable to 1 in addons-manager deployment

PR Checklist

  • Squash the commits into one or a small number of logical commits
  • Use good commit messages
  • Ensure PR contains terms all contributors can understand and links all contributors can access

Additional information

Special notes for your reviewer

@vijaykatam
Copy link
Contributor

Change looks good to me. Were you able to test upgrade on an addons-manager previously deployed on a single control plane node to the new version?

@ggpaue
Copy link
Contributor Author

ggpaue commented Jul 18, 2022

Change looks good to me. Were you able to test upgrade on an addons-manager previously deployed on a single control plane node to the new version?

Yes, I used a local kind cluster, single node and tested deployment upgrade. Verified that maxUnavailable 25% will fail upgrade but set it to 1 will succeed

@codecov
Copy link

codecov bot commented Jul 19, 2022

Codecov Report

Merging #2962 (fceb1e4) into main (e921082) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2962      +/-   ##
==========================================
- Coverage   44.00%   43.98%   -0.02%     
==========================================
  Files         416      416              
  Lines       41852    41852              
==========================================
- Hits        18415    18410       -5     
- Misses      21715    21720       +5     
  Partials     1722     1722              
Impacted Files Coverage Δ
addons/controllers/clusterbootstrap_controller.go 63.28% <0.00%> (-0.62%) ⬇️
pkg/v1/tkg/tkgpackageclient/package_update.go 85.00% <0.00%> (+1.42%) ⬆️

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 e921082...fceb1e4. Read the comment docs.

Copy link
Contributor

@vijaykatam vijaykatam left a comment

Choose a reason for hiding this comment

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

LGTM

@vijaykatam vijaykatam added the ok-to-merge PRs should be labelled with this before merging label Jul 19, 2022
@ggpaue ggpaue merged commit 541ca31 into vmware-tanzu:main Jul 19, 2022
abhijit-dev82 pushed a commit to abhijit-dev82/tanzu-framework that referenced this pull request Jul 27, 2022
)

Signed-off-by: Guanpeng Gao <gguanpeng@vmware.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-not-required ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Addons manager update on a single control plane deployment fails to progress
3 participants