[v1alpah4] rollout fix #23
Conversation
sadysnaat
requested review from
abhinavnagaraj,
jzhoucliqr,
LochanRn and
snehala27
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sadysnaat The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sadysnaat The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Hi @sadysnaat. Thanks for your PR. I'm waiting for a spectrocloud member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
| @@ -110,6 +111,20 @@ func (r *KubeadmControlPlaneReconciler) upgradeControlPlane( | |||
| return ctrl.Result{}, errors.Wrap(err, "failed to upgrade kubelet config map") | |||
| } | |||
|
|
|||
| // this should be already handled by the defaulting webhook, but during rolling upgrade it is possible that | |||
There was a problem hiding this comment.
I'm not sure if we still need this.
This was for the CAPI upgrade when RolloutStrategy was introduced, the clusters created before that upgrade does not have the strategy, so we need to make sure after that upgrade the strategy should be set.
For all the clusters we have right now, they should already have this set, so probably don't need this change.
|
Closed by #26 |
No description provided.