-
Notifications
You must be signed in to change notification settings - Fork 302
Autoscaler followup #15746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Autoscaler followup #15746
Conversation
…ty when the autoscaler feature flag has been disabled. Fixes rancher#15741
…on when only mouse interactions were occurring Fixes rancher#15739
…exclusive with the autoscaler config
| }, | ||
| { | ||
| path: FIELDS.AUTOSCALER_MIN, | ||
| rules: ['requiredInt', 'isPositive', 'isAutoscalerMaxGreaterThanMin'], |
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.
I put the isAutoscalerMaxGreaterThanMin in both min and max fields because these don't get evaluated until a user focuses on a field and I wanted to ensure that whichever field is touched the user will be informed.
ef4b992 to
3d6b75e
Compare
mantis-toboggan-md
left a comment
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.
| :mode="mode" | ||
| :label="t('cluster.machinePool.autoscaler.enable', undefined, true)" | ||
| :disabled="busy" | ||
| :disabled="value.pool.etcdRole || value.pool.controlPlaneRole" |
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.
I think this input (and the other autoscaling inputs?) should also be disabled if busy is 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.
Put it back in each of three locations
- Removed requiredInt validation since UnitInput converts to integer anyway - Added `busy` back for disabled.


Summary
Added some code to make autoscaler semi-mutually-exclusive with nodes that have the etcd or control plane roles. We do this by discouraging and attempting to disable autoscaler when the etcd and control plane roles are selected.
Also:
Fixes #15741
Fixes #15739
close-bork.mp4
Areas or cases that should be tested
Areas which could experience regressions
See above
Screenshot/Video
autoscaler-mutex.mp4
Checklist
Admin,Standard UserandUser Base