-
Notifications
You must be signed in to change notification settings - Fork 527
fix: upgrade broken for VMAS + managed identities #4021
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4021 +/- ##
==========================================
+ Coverage 73.69% 73.72% +0.03%
==========================================
Files 147 147
Lines 23164 23177 +13
==========================================
+ Hits 17070 17087 +17
+ Misses 4979 4976 -3
+ Partials 1115 1114 -1
Continue to review full report at Codecov.
|
logger.Infoln(fmt.Sprintf("Evaluating if agent resource: %s needs to be removed", resourceName)) | ||
removeRole := true | ||
// The usage of NormalizeResourcesForK8sMasterUpgrade across the code base seems to indicate that | ||
// agentPoolsToPreserve == nil => master node upgrade |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you mean if agentPoolsToPreserve has a length of 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right, I wrote it with this in mind
https://github.com/Azure/aks-engine/blob/v0.57.0/pkg/operations/kubernetesupgrade/upgrader.go#L201
@@ -1,5 +1,6 @@ | |||
{ | |||
"env": { | |||
"UPGRADE_CLUSTER": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jackfrancis, jadarsie 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:
Approvers can indicate their approval by writing |
Reason for Change:
Upgrade fails for VMAS clusters if its VMs own a
roleAssignment
resource.The problem is that
NormalizeResourcesForK8sMasterUpgrade
is not filteringroleAssignments
resources and ARM complains because theseroleAssignments
resources depend on missing VM resources.stdout message:
Notes:
Credit Where Due:
Does this change contain code from or inspired by another project?
Requirements: