fix: keep pods healthy when tenant and host run different K8s versions#4037
Open
flomedja wants to merge 4 commits into
Open
fix: keep pods healthy when tenant and host run different K8s versions#4037flomedja wants to merge 4 commits into
flomedja wants to merge 4 commits into
Conversation
Pods could get stuck at Ready=False (so their Deployments and StatefulSets
never became ready) when a tenant cluster ran an older Kubernetes version
than the host, for example a 1.32/1.33 tenant on a 1.34/1.35 host. The
syncer logged "field is immutable" or "object has been modified" errors.
There were two causes:
- The syncer overwrote the host's QOS class with the tenant's before
patching, and K8s 1.32+ rejects QOS class changes. We now leave the host
QOS class alone and keep the tenant's value on the tenant pod.
- A 1.34+ host sets observedGeneration on pod conditions, but a tenant on
K8s < 1.34 drops that field. This made the syncer think the status kept
changing and overwrite the real Ready=True condition every reconcile. We
now ignore observedGeneration when the tenant doesn't store it.
Adds unit and e2e regression tests.
Fixes loft-sh#3578
loft-bot
reviewed
Jun 26, 2026
anvesh-loft
reviewed
Jun 26, 2026
anvesh-loft
reviewed
Jun 26, 2026
anvesh-loft
left a comment
anvesh-loft
left a comment
There was a problem hiding this comment.
hi @flomedja , added one comment for you to review !
The earlier fix only stripped the top-level observedGeneration, but the host kubelet also sets it per pod condition. Those got copied to the virtual pod and, on a tenant running K8s < 1.34, the apiserver kept dropping them, so we re-sent them every reconcile. Now we strip them from the conditions too. Also from review: extract patcher.CopyBidirectionalWithEq to drop the duplicated branch, log once when the suppression path is active, and add nil-version tests.
janekbaraniewski
requested changes
Jul 6, 2026
Discover the host cluster version a single time on startup and carry it on the register context, instead of NewSyncer and NewTranslator each querying and parsing it. Also fixes the mislabeled "virtual cluster version" error messages in the pods syncer that were actually wrapping the host fetch. Minor cleanups from PR review: drop the unused virtual return from the pod condition copy, use slices.Clone, guard the resize check against an unknown host version, and tidy verbose comments.
flomedja
force-pushed
the
ENGNODE-105-sync-issues-with-k8s-hosts-134-or-135
branch
from
July 6, 2026 17:31
d9a60e1 to
2f5bc6c
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2f5bc6c. Configure here.
…ontext to sync e2e polls While a host pod is terminating, the syncer kept issuing the same status update every reconcile. It compared the raw host status even though QOSClass (and ObservedGeneration on K8s < 1.34) are deliberately kept from the virtual side, so those fields always looked changed. Now we compare against the status we actually want the pod to have, so nothing updates unless something really changed. Also add .WithContext(ctx) to the Eventually/Consistently polls in the test_core/sync e2e tests so they respect the test's timeout and cancellation.
zerbitx
reviewed
Jul 9, 2026
|
|
||
| hostClusterVersion, err := vClusterOptions.HostClient.Discovery().ServerVersion() | ||
| if err != nil { | ||
| return nil, errors.Wrap(err, "get host cluster version") |
Contributor
There was a problem hiding this comment.
Let's move this to after the private nodes check on 422 since private nodes won't need it and this could error out and fail here.
zerbitx
reviewed
Jul 9, 2026
| // Compare against the status we actually want the virtual pod to have, not the raw host | ||
| // status: QOSClass is immutable and ObservedGeneration is dropped by virtual apiservers | ||
| // on K8s < 1.34, so those fields legitimately differ and must not trigger an update. | ||
| desiredStatus := *event.Host.Status.DeepCopy() |
Contributor
There was a problem hiding this comment.
Should the desired status setup be the same here and in the Diff in translate?
Perhaps with a helper to call here and here
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
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)resolves #
Pods could get stuck at
Ready=False, so their Deployments and StatefulSetsvnever became ready — when a tenant cluster ran an older Kubernetes version than the host. This showed up with a 1.32/1.33 tenant on a 1.34/1.35 host (e.g. EKS), and the syncer loggedfield is immutableorobject has been modifiederrors.Fixes #3578
Resolves ENGNODE-105
Root causes
1. QOS class (the "field is immutable" error) : The pod syncer copied the tenant's QOS class onto the host object before
patching. A later status update then tried to write that QOS class to the host, which Kubernetes 1.32+ rejects because
status.qosClassis immutable. We now leave the host QOS class untouched and keep the tenant's own value on the tenantpod, so no immutable-field patch is ever produced.
2. observedGeneration (the stuck
Ready=False)A host kubelet on K8s 1.34+ sets
observedGenerationon pod status and conditions. A tenant apiserver on K8s < 1.34 drops that field on write (PodObservedGenerationTrackingis alpha/off in 1.33 and absent before). That made the object cache (what we sent) and the informer (what got stored) disagree on every reconcile. The syncer read this as a "conditions changed" event and kept overwriting the realReady=Truecondition with stale data. We now ignoreobservedGenerationin that comparison when the tenant cluster strips it.Please provide a short message that should be published in the vcluster release notes
Fixed an issue where vCluster pods stayed Ready=False (blocking their Deployments and StatefulSets) when the tenant cluster ran an older Kubernetes version than the control plane cluster.
What else do we need to know?
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:
Note
Medium Risk
Touches core pod status reconciliation and bidirectional condition sync; behavior changes only for version-aware paths but affects all pod sync when host and virtual versions differ.
Overview
Fixes pod sync when the virtual cluster is older than the host (e.g. tenant 1.32/1.33 on a 1.34+ control plane), where workloads could stay
Ready=Falseor hit immutable-field errors.Pod status sync no longer treats host-only fields as changes on every reconcile: QOSClass stays on the virtual pod (and is not written onto the host before patch), and
observedGenerationon status/conditions is ignored for diff/update when the virtual apiserver on K8s < 1.34 drops it on write. Condition bidirectional copy uses a custom equality via newCopyBidirectionalWithEq, and deleting-host-pod status propagation compares against a desired status with the same rules.Startup discovers host cluster version once and threads
HostClusterVersion/VirtualClusterVersionthrough controller/register context (replacing per-syncer discovery calls). In-place host resize is skipped when host version is unknown or < 1.35.Tests: unit coverage for QOS/
observedGeneration/conditions; e2e adds regression cases for virtual QOS and stableReady, plusWithContext(ctx)on GinkgoEventually/Consistentlyacross sync e2e specs.Reviewed by Cursor Bugbot for commit e8d0226. Bugbot is set up for automated code reviews on this repo. Configure here.