Delete MachineAutoscalers that would get MaxReplicas==0 #2229
Conversation
|
/assign @lleshchi |
openshift-ci
bot
added
the
approved
| @@ -853,25 +853,29 @@ func (r *ReconcileMachinePool) syncMachineAutoscalers( | |||
| maLog.WithField("desired", minReplicas). | |||
| WithField("observed", rMA.Spec.MinReplicas). | |||
| Info("min replicas out of sync") | |||
| rMA.Spec.MinReplicas = minReplicas | |||
| remoteMachineAutoscalers.Items[j].Spec.MinReplicas = minReplicas | |||
There was a problem hiding this comment.
Note to reviewers: Had to change this block to operate on the original (it was previously operating on the loop variable, which was an ephemeral copy scoped to this block) so we can detect maxReplicas==0 below in the deletion discovery path.
| assert.Equal(t, *eMS.Spec.Replicas, *rMS.Spec.Replicas, "Replicas for %s", rMS.Name) | ||
| assert.Equal(t, eMS.Generation, rMS.Generation, "Generation for %s", rMS.Name) |
There was a problem hiding this comment.
ntr: mostly-unrelated change, makes debugging easier.
MachineAutoscalers are not allowed to have MaxReplicas==0. PR openshift#2215 / bce2d47 partially fixed the scenario where a MachinePool's autoscaling maxReplicas is less than the number of AZs by causing such MAs not to be *created*. However, when reducing the MachinePool's maxReplicas to below the number of AZs, we were still trying to update *existing* MAs to have MaxReplicas==0. This commit adjusts the logic to delete them instead. HIVE-2415
2uasimojo
force-pushed
the
HIVE-2415/scale-down
branch
from
fab74b5
to
8d2b041
Compare
|
/override ci/prow/security addressed by #2231 |
|
@2uasimojo: Overrode contexts on behalf of 2uasimojo: ci/prow/security In response to this:
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. |
|
@2uasimojo: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2229 +/- ##
==========================================
+ Coverage 58.29% 58.31% +0.01%
==========================================
Files 182 182
Lines 25691 25692 +1
==========================================
+ Hits 14977 14982 +5
+ Misses 9455 9451 -4
Partials 1259 1259
|
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 2uasimojo, lleshchi 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 |
|
/cherry-pick mce-2.5 |
|
@2uasimojo: new pull request created: #2233 In response to this:
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. |
MachineAutoscalers are not allowed to have MaxReplicas==0.
PR #2215 / bce2d47 partially fixed the scenario where a MachinePool's autoscaling maxReplicas is less than the number of AZs by causing such MAs not to be created. However, when reducing the MachinePool's maxReplicas to below the number of AZs, we were still trying to update existing MAs to have MaxReplicas==0. This commit adjusts the logic to delete them instead.
HIVE-2415