MachinePool: Properly handle labels & taints #2069
Conversation
With this commit, MachinePool grows two new statusfields: `OwnedLabels`
and `OwnedTaints`. These keep track of the labels and taints that were
added to the remote MachineSets at the behest of this MachinePool.
Conversely, this allows us to identify which ones were added on the
remote side, and thus shouldn't be touched by our controller.
We also fix the logic for matching taints, which are "keyed" by the
combination of the `Key` and `Effect` fields.
To summarize the behavior with this commit in play:
- Labels and taints listed in MachinePool.Spec are synced to the remote
MachineSets.
- Labels and taints that are *not* in MachinePool.Spec are removed from
the remote MachineSets *iff* they are present in
MachinePool.Status.Owned{Labels|Taints}.
- Upon successful syncing, we update
MachinePool.Status.Owned{Labels|Taints} to match the spec.
The effect is that editing MachinePool.Spec to remove a label or taint
should cause that label/taint to be removed from the remote MachineSets,
BUT labels/taints that only exist on the remote MachineSets remain
untouched.
HIVE-2035
HIVE-2276
HIVE-2279
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 2uasimojo 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 |
openshift-ci
bot
added
the
approved
|
@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. |
|
Superseded by 2082 |
| } | ||
|
|
||
| if gTaint, foundTaint := gTaintsByID[tid]; foundTaint && gTaint.Value != rTaint.Value { |
There was a problem hiding this comment.
BUG: if the rMS has duplicates, we'll replace the first one, but then because we deleted it from gTaintsByID, subsequent dups will be added as is.
| } | ||
| sort.SliceStable(taints, func(i, j int) bool { |
There was a problem hiding this comment.
POSSIBLE BUG: Does taint order matter to k8s? If so, this is not the right thing to do. We should in fact be conveying the ordering from the MachinePool to the extent that is feasible.
With this commit, MachinePool grows two new statusfields:
OwnedLabelsandOwnedTaints. These keep track of the labels and taints that were added to the remote MachineSets at the behest of this MachinePool. Conversely, this allows us to identify which ones were added on the remote side, and thus shouldn't be touched by our controller.We also fix the logic for matching taints, which are "keyed" by the combination of the
KeyandEffectfields.To summarize the behavior with this commit in play:
MachinePool.Status.Owned{Labels|Taints}.
The effect is that editing MachinePool.Spec to remove a label or taint should cause that label/taint to be removed from the remote MachineSets, BUT labels/taints that only exist on the remote MachineSets remain untouched.
HIVE-2035
HIVE-2276
HIVE-2279
HIVE-2268