fix(pods): sanitize topology spread matchLabelKeys before host sync#3807
fix(pods): sanitize topology spread matchLabelKeys before host sync#3807officialasishkumar wants to merge 2 commits intoloft-sh:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57e52c6646
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| value, ok := podLabels[requirement.Key] | ||
| return ok && requirement.Values[0] == value |
There was a problem hiding this comment.
Strip matchLabelKeys expression even when pod label changed
This sanitization only removes a matchLabelKeys-derived expression when the selector value still equals the pod's current label value. If that label was later changed or removed (pod labels are mutable), the API-server-injected expression becomes stale but is still invalid to keep alongside MatchLabelKeys, so the host API server can still reject the synced pod on 1.34+. The filter should strip requirements keyed by MatchLabelKeys regardless of current label equality.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Updated in c605e3d: requirements keyed by MatchLabelKeys are now stripped regardless of the pod's current label value, and the test covers the stale-label case.
Kubernetes 1.34+ mutates topology spread selectors during pod creation by merging matchLabelKeys into matchExpressions. When vCluster forwards that already-mutated pod to the host API server, the host rejects the create because the same key exists in both matchLabelKeys and labelSelector. Strip only the API-server-generated key-in-value requirements before translating the selector so the host API server can apply its own mutation step again. Add a translator unit test that covers the pod-template-hash case from the reported sync failure.
57e52c6 to
c605e3d
Compare
What issue type does this pull request address? (keep at least one, remove the others)
/kind bugfix
What does this pull request do? Which issues does it resolve? (use
resolves #<issue_number>if possible)Sanitizes API-server-merged
topologySpreadConstraints.matchLabelKeysexpressions before syncing pods to the host cluster, so the host API server can apply its own mutation step without rejecting the pod as invalid.resolves #3668
Please provide a short message that should be published in the vcluster release notes
Fixed an issue where pods using
topologySpreadConstraints.matchLabelKeyscould fail to sync to host clusters running Kubernetes 1.34+.What else do we need to know?
Validation:
go test ./pkg/controllers/resources/pods/translate -run TestTranslateTopologySpreadConstraints -count=1 -vgo test ./pkg/controllers/resources/pods/...E2E Tests
Default Test Execution
The mandatory PR suite runs automatically. Only specify additional test suites below if needed.
Adding New Test Suites
When adding a new ginkgo test suite:
Additional test suites
Additional test suite(s) that will be executed before the mandatory PR suite: