Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 93 additions & 0 deletions e2e/test_core/sync/test_pods.go
Original file line number Diff line number Diff line change
Expand Up @@ -895,6 +895,99 @@ func PodSyncSpec() {
}).WithPolling(constants.PollingInterval).WithTimeout(constants.PollingTimeoutLong).Should(Succeed())
})
})

It("should preserve virtual pod QOS class (Bug 1 regression)", func(ctx context.Context) {
// Regression test for https://github.com/loft-sh/vcluster/issues/3578 (Bug 1).
// The syncer used to overwrite the host QOS class with the virtual one before patching,
// so a later status update sent the wrong QOS class. K8s 1.32+ rejects that as
// "field is immutable", which left pods stuck at Ready=False.
suffix := random.String(6)
ns := "pod-qos-test-" + suffix
createTestNamespace(ctx, ns)

podName := "qos-" + suffix
By("Creating a pod with no resource requests (virtual QOS class: BestEffort)", func() {
_, err := vClusterClient.CoreV1().Pods(ns).Create(ctx, &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{Name: podName},
Spec: corev1.PodSpec{
Containers: []corev1.Container{{
Name: testingContainerName,
Image: testingContainerImage,
ImagePullPolicy: corev1.PullIfNotPresent,
SecurityContext: defaultSecurityContext(),
}},
},
}, metav1.CreateOptions{})
Expect(err).To(Succeed())
})

By("Waiting for the pod to be Running", func() {
waitPodRunning(ctx, podName, ns)
})

By("Verifying the virtual pod QOS class is BestEffort (not overwritten from host)", func() {
vpod, err := vClusterClient.CoreV1().Pods(ns).Get(ctx, podName, metav1.GetOptions{})
Expect(err).To(Succeed())
Expect(vpod.Status.QOSClass).To(Equal(corev1.PodQOSBestEffort),
"virtual pod QOS class should reflect the virtual apiserver's computation (BestEffort for no resource requests), not the host pod's QOS class")
})
})

It("should keep pod Ready condition stable after reaching Ready=True (Bug 2 regression)", func(ctx context.Context) {
// Regression test for https://github.com/loft-sh/vcluster/issues/3578 (Bug 2).
// A K8s 1.34+ host kubelet sets ObservedGeneration on pod conditions. A virtual
// cluster on K8s < 1.34 strips that field on write, so the object cache (what we
// sent) and the informer (what was stored) disagree every reconcile. That false
// "conditions changed" signal caused needless host status updates and, together
// with Bug 1, made pods flap to Ready=False.
//
// This only reproduces the bug when the host is K8s >= 1.34 and the virtual cluster
// is < 1.34; on a same-version cluster it passes trivially. The real boundary guard
// is the unit test TestDiffPodStatusObservedGeneration; this is a best-effort check.
suffix := random.String(6)
ns := "pod-cond-stable-test-" + suffix
createTestNamespace(ctx, ns)

podName := "cond-stable-" + suffix
By("Creating a pod", func() {
_, err := vClusterClient.CoreV1().Pods(ns).Create(ctx, &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{Name: podName},
Spec: corev1.PodSpec{
Containers: []corev1.Container{{
Name: testingContainerName,
Image: testingContainerImage,
ImagePullPolicy: corev1.PullIfNotPresent,
SecurityContext: defaultSecurityContext(),
}},
},
}, metav1.CreateOptions{})
Expect(err).To(Succeed())
})

readyCondition := And(
HaveField("Type", corev1.PodReady),
HaveField("Status", corev1.ConditionTrue),
)

By("Waiting for the pod to reach Running phase with Ready=True", func() {
waitPodRunning(ctx, podName, ns)
Eventually(func(g Gomega) {
pod, err := vClusterClient.CoreV1().Pods(ns).Get(ctx, podName, metav1.GetOptions{})
g.Expect(err).To(Succeed())
g.Expect(pod.Status.Conditions).To(ContainElement(readyCondition),
"pod Ready condition is not yet True: %v", pod.Status.Conditions)
}).WithPolling(constants.PollingInterval).WithTimeout(constants.PollingTimeoutLong).Should(Succeed())
})

By("Verifying Ready condition stays True and does not flap", func() {
Consistently(func(g Gomega) {
pod, err := vClusterClient.CoreV1().Pods(ns).Get(ctx, podName, metav1.GetOptions{})
g.Expect(err).To(Succeed())
g.Expect(pod.Status.Conditions).To(ContainElement(readyCondition),
"pod Ready condition flapped away from True: %v", pod.Status.Conditions)
}).WithPolling(constants.PollingInterval).WithTimeout(constants.PollingTimeoutShort).Should(Succeed())
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
})
})
},
)
}
Expand Down
10 changes: 0 additions & 10 deletions pkg/controllers/resources/pods/syncer.go
Comment thread
flomedja marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -370,16 +370,6 @@ func (s *podSyncer) Sync(ctx *synccontext.SyncContext, event *synccontext.SyncEv
return ctrl.Result{}, err
}

// ignore the QOSClass field while updating pod status when there is a
Comment thread
cursor[bot] marked this conversation as resolved.
// mismatch in this field value on vcluster and host. This field
// has become immutable from k8s 1.32 version and patch fails if
// syncer tries to update this field.
// This needs to be done before patch object is created when
// NewSyncerPatcher() is called so that there are no
// differences found in host QOSClass and virtual QOSClass and
// a patch event for this field is not created
event.Host.Status.QOSClass = event.Virtual.Status.QOSClass

// patch objects
patch, err := patcher.NewSyncerPatcher(ctx, event.Host, event.Virtual, patcher.TranslatePatches(ctx.Config.Sync.ToHost.Pods.Patches, false))
if err != nil {
Expand Down
63 changes: 60 additions & 3 deletions pkg/controllers/resources/pods/translate/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (

func (t *translator) Diff(ctx *synccontext.SyncContext, event *synccontext.SyncEvent[*corev1.Pod]) error {
// sync conditions
event.Virtual.Status.Conditions, event.Host.Status.Conditions = patcher.CopyBidirectional(
event.Virtual.Status.Conditions, event.Host.Status.Conditions = t.conditionsCopyBidirectional(
Comment thread
janekbaraniewski marked this conversation as resolved.
Outdated
event.VirtualOld.Status.Conditions,
event.Virtual.Status.Conditions,
event.HostOld.Status.Conditions,
Expand All @@ -32,11 +32,19 @@ func (t *translator) Diff(ctx *synccontext.SyncContext, event *synccontext.SyncE
vPod := event.Virtual
pPod := event.Host

// We have to reset the QOSClass to the original value, otherwise we might get an error
// that the field is immutable
// Copy the host status onto the virtual pod, but keep the virtual values for fields the
// host must not overwrite:
// - QOSClass is immutable on the host (since K8s 1.32), so we always keep the virtual value.
// - ObservedGeneration is dropped by virtual apiservers that don't track it (K8s < 1.34).
// Syncing the host value there would cause infinite status-patch churn: the object cache
// records what we sent while the informer returns the stored (zero) value.
originalQOSClass := vPod.Status.QOSClass
originalObservedGeneration := vPod.Status.ObservedGeneration
vPod.Status = *pPod.Status.DeepCopy()
vPod.Status.QOSClass = originalQOSClass
if t.virtualClusterStripsObservedGeneration() {
Comment thread
janekbaraniewski marked this conversation as resolved.
vPod.Status.ObservedGeneration = originalObservedGeneration
}
err := t.convertResourceClaimStatuses(ctx, vPod, pPod.GetNamespace())
if err != nil {
return err
Expand Down Expand Up @@ -281,3 +289,52 @@ func hasToleration(tolerations []corev1.Toleration, tol corev1.Toleration) bool
}
return false
}

// virtualClusterStripsObservedGeneration reports whether the virtual apiserver drops
// ObservedGeneration on write. It does on K8s < 1.34, where the PodObservedGenerationTracking
// feature gate is off by default.
func (t *translator) virtualClusterStripsObservedGeneration() bool {
return t.virtualClusterVersion != nil &&
t.virtualClusterVersion.LessThan(k8sPodObservedGenerationMinVersion)
}

// conditionsCopyBidirectional is patcher.CopyBidirectional for pod conditions, except it
// ignores ObservedGeneration when the virtual cluster strips it (K8s < 1.34) to avoid
// detecting a phantom change every reconcile. It returns the original slices, so a real
// ObservedGeneration from the host is still synced.
func (t *translator) conditionsCopyBidirectional(
virtualOld, virtual, hostOld, host []corev1.PodCondition,
) (newVirtual, newHost []corev1.PodCondition) {
if !t.virtualClusterStripsObservedGeneration() {
return patcher.CopyBidirectional(virtualOld, virtual, hostOld, host)
}

newVirtual = virtual
newHost = host
if !apiequality.Semantic.DeepEqual(
stripConditionObservedGenerations(virtualOld),
stripConditionObservedGenerations(virtual),
) {
newHost = virtual
} else if !apiequality.Semantic.DeepEqual(
stripConditionObservedGenerations(hostOld),
stripConditionObservedGenerations(host),
) {
newVirtual = host
}
return newVirtual, newHost
Comment thread
flomedja marked this conversation as resolved.
Outdated
}

// stripConditionObservedGenerations returns a shallow copy of conditions with
// ObservedGeneration zeroed on each entry. Used only for equality comparisons.
func stripConditionObservedGenerations(conditions []corev1.PodCondition) []corev1.PodCondition {
if conditions == nil {
return nil
}
out := make([]corev1.PodCondition, len(conditions))
Comment thread
janekbaraniewski marked this conversation as resolved.
Outdated
copy(out, conditions)
for i := range out {
out[i].ObservedGeneration = 0
}
return out
}
Loading
Loading