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

fix: enable CSI addons on upgrade when appropriate #2347

Merged
merged 2 commits into from Nov 26, 2019

Conversation

mboersma
Copy link
Member

Reason for Change:
Follows up on #2345 to fix the enabling csi-azuredisk-* and csi-azurefile-* components on upgrade. Also addresses a review comment on the previous PR.

Issue Fixed:
Refs #2337

Requirements:

Notes:

@codecov
Copy link

codecov bot commented Nov 22, 2019

Codecov Report

Merging #2347 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2347      +/-   ##
==========================================
+ Coverage    71.9%   71.91%   +0.01%     
==========================================
  Files         130      130              
  Lines       23395    23404       +9     
==========================================
+ Hits        16823    16832       +9     
  Misses       5546     5546              
  Partials     1026     1026

o.KubernetesConfig.Addons[i] = defaultCloudNodeManagerAddonsConfig
// Ensure cloud-node-manager and CSI components are enabled on appropriate upgrades
if isUpgrade && to.Bool(o.KubernetesConfig.UseCloudControllerManager) &&
common.IsKubernetesVersionGe(o.OrchestratorVersion, "1.16.0") {
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a UT that validates that a 1.15-->1.16 upgrade w/ all three of these addons disabled initially is statically overwritten to enabled=true (which is what I interpret this back-compat logic to be doing)

@mboersma mboersma added this to Under Review in backlog via automation Nov 25, 2019
@mboersma mboersma added this to the v0.45.0 milestone Nov 25, 2019
@mboersma
Copy link
Member Author

/hold

Pending additional unit tests.

@acs-bot acs-bot added size/L and removed size/S labels Nov 25, 2019
@mboersma
Copy link
Member Author

/hold cancel

Copy link
Member

@jackfrancis jackfrancis left a comment

Choose a reason for hiding this comment

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

/lgtm

@acs-bot
Copy link

acs-bot commented Nov 26, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackfrancis, mboersma

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [jackfrancis,mboersma]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@acs-bot acs-bot merged commit 80fe859 into Azure:master Nov 26, 2019
backlog automation moved this from Under Review to Done Nov 26, 2019
@mboersma mboersma deleted the fix-cnm-four branch November 26, 2019 17:09
mboersma added a commit that referenced this pull request Nov 26, 2019
* fix: enable CSI addons on upgrade when appropriate

* test: add coverage for 1.16 upgrade w/ UseCloudControllerManager
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
backlog
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants