-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
KEP-3243: Update the design to mutate the label selector based on matchLabelKeys at api-server instead of the scheduler handling it #5033
Open
mochizuki875
wants to merge
2
commits into
kubernetes:master
Choose a base branch
from
mochizuki875:matchlabelkeys-to-podtopologyspread
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+87
−37
Open
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -307,15 +307,7 @@ required) or even code snippets. If there's any ambiguity about HOW your | |||||
proposal will be implemented, this is the place to discuss them. | ||||||
--> | ||||||
|
||||||
A new field named `MatchLabelKeys` will be added to `TopologySpreadConstraint`. | ||||||
Currently, when scheduling a pod, the `LabelSelector` defined in the pod is used | ||||||
to identify the group of pods over which spreading will be calculated. | ||||||
`MatchLabelKeys` adds another constraint to how this group of pods is identified: | ||||||
the scheduler will use those keys to look up label values from the incoming pod; | ||||||
and those key-value labels are ANDed with `LabelSelector` to select the group of | ||||||
existing pods over which spreading will be calculated. | ||||||
|
||||||
A new field named `MatchLabelKeys` will be introduced to`TopologySpreadConstraint`: | ||||||
A new optional field named `MatchLabelKeys` will be introduced to`TopologySpreadConstraint`. | ||||||
```go | ||||||
type TopologySpreadConstraint struct { | ||||||
MaxSkew int32 | ||||||
|
@@ -333,22 +325,53 @@ type TopologySpreadConstraint struct { | |||||
} | ||||||
``` | ||||||
|
||||||
Examples of use are as follows: | ||||||
When a Pod is created, kube-apiserver will obtain the labels from the pod | ||||||
by the keys in `matchLabelKeys` and `key in (value)` is merged to `LabelSelector` | ||||||
of `TopologySpreadConstraint`. | ||||||
|
||||||
For example, when this sample Pod is created, | ||||||
|
||||||
```yaml | ||||||
topologySpreadConstraints: | ||||||
- maxSkew: 1 | ||||||
topologyKey: kubernetes.io/hostname | ||||||
whenUnsatisfiable: DoNotSchedule | ||||||
matchLabelKeys: | ||||||
- app | ||||||
- pod-template-hash | ||||||
apiVersion: v1 | ||||||
kind: Pod | ||||||
metadata: | ||||||
name: sample | ||||||
labels: | ||||||
app: sample | ||||||
... | ||||||
topologySpreadConstraints: | ||||||
- maxSkew: 1 | ||||||
topologyKey: kubernetes.io/hostname | ||||||
whenUnsatisfiable: DoNotSchedule | ||||||
labelSelector: {} | ||||||
matchLabelKeys: # ADDED | ||||||
- app | ||||||
``` | ||||||
|
||||||
kube-apiserver modifies the `labelSelector` like the following: | ||||||
|
||||||
```diff | ||||||
topologySpreadConstraints: | ||||||
- maxSkew: 1 | ||||||
topologyKey: kubernetes.io/hostname | ||||||
whenUnsatisfiable: DoNotSchedule | ||||||
labelSelector: | ||||||
+ matchExpressions: | ||||||
+ - key: app | ||||||
+ operator: In | ||||||
+ values: | ||||||
+ - sample | ||||||
matchLabelKeys: | ||||||
- app | ||||||
``` | ||||||
|
||||||
The scheduler plugin `PodTopologySpread` will obtain the labels from the pod | ||||||
labels by the keys in `matchLabelKeys`. The obtained labels will be merged | ||||||
to `labelSelector` of `topologySpreadConstraints` to filter and group pods. | ||||||
The pods belonging to the same group will be part of the spreading in | ||||||
`PodTopologySpread`. | ||||||
Currently, scheduler will also be aware of `matchLabelKeys` and | ||||||
gracefully handle the same labels. | ||||||
This originates from the previous implementation only scheduler | ||||||
obtains the pod labels by the keys in `matchLabelKeys` and uses | ||||||
them with `labelSelector` to filter and group pods for spreading. | ||||||
In the near future, the scheduler implementation related to | ||||||
`matchLabelKeys` will be removed. | ||||||
|
||||||
Finally, the feature will be guarded by a new feature flag. If the feature is | ||||||
disabled, the field `matchLabelKeys` is preserved if it was already set in the | ||||||
|
@@ -532,7 +555,9 @@ enhancement: | |||||
|
||||||
In the event of an upgrade, kube-apiserver will start to accept and store the field `MatchLabelKeys`. | ||||||
|
||||||
In the event of a downgrade, kube-scheduler will ignore `MatchLabelKeys` even if it was set. | ||||||
In the event of a downgrade, kube-apiserver will reject pod creation with `matchLabelKeys` in `TopologySpreadConstraint`. | ||||||
But, regarding existing pods, we leave `matchLabelKeys` and generated `LabelSelector` even after downgraded. | ||||||
kube-scheduler will ignore `MatchLabelKeys` even if it was set. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto
Suggested change
|
||||||
|
||||||
### Version Skew Strategy | ||||||
|
||||||
|
@@ -659,7 +684,7 @@ feature flags will be enabled on some API servers and not others during the | |||||
rollout. Similarly, consider large clusters and how enablement/disablement | ||||||
will rollout across nodes. | ||||||
--> | ||||||
It won't impact already running workloads because it is an opt-in feature in scheduler. | ||||||
It won't impact already running workloads because it is an opt-in feature in apiserver and scheduler. | ||||||
But during a rolling upgrade, if some apiservers have not enabled the feature, they will not | ||||||
be able to accept and store the field "MatchLabelKeys" and the pods associated with these | ||||||
apiservers will not be able to use this feature. As a result, pods belonging to the | ||||||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
We should keep this part.