Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
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
90 changes: 90 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,96 @@ func PodSyncSpec() {
}).WithPolling(constants.PollingInterval).WithTimeout(constants.PollingTimeoutLong).Should(Succeed())
})
})

It("should preserve virtual pod QOS class", func(ctx context.Context) {
// Regression test for https://github.com/loft-sh/vcluster/issues/3578, where pods
// got stuck at Ready=False. The syncer used to copy the virtual QOS class onto the
// host pod, so a later status update tried to change the host's QOS class. K8s 1.32+
// treats that field as immutable and rejects the update.
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", func(ctx context.Context) {
// Regression test for https://github.com/loft-sh/vcluster/issues/3578, where pods
// kept switching back to Ready=False because the syncer wrongly thought their
// conditions had changed on every reconcile.
//
// This only happens when the host runs K8s >= 1.34 and the virtual cluster runs
// < 1.34. On a same-version cluster the bug can't reproduce, so the real check
// lives in the unit test TestDiffPodStatusObservedGeneration.
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
31 changes: 11 additions & 20 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 @@ -93,15 +93,14 @@ func New(ctx *synccontext.RegisterContext) (syncertypes.Object, error) {
return nil, fmt.Errorf("failed to create scheduling config: %w", err)
}

hostClusterVersionInfo, err := ctx.Config.HostClient.Discovery().ServerVersion()
if err != nil {
return nil, fmt.Errorf("failed to get virtual cluster version : %w", err)
}

hostClusterVersion, err := utilversion.ParseSemantic(hostClusterVersionInfo.String())
if err != nil {
// This should never happen
return nil, fmt.Errorf("failed to parse host cluster version : %w", err)
// The host cluster version is discovered once at startup and carried on the context.
// It is nil in unit tests, which callers treat as "version unknown".
var hostClusterVersion *utilversion.Version
if ctx.HostClusterVersion != nil {
hostClusterVersion, err = utilversion.ParseSemantic(ctx.HostClusterVersion.String())
if err != nil {
return nil, fmt.Errorf("failed to parse host cluster version: %w", err)
}
}

return &podSyncer{
Expand Down Expand Up @@ -370,16 +369,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 Expand Up @@ -431,7 +420,9 @@ func (s *podSyncer) Sync(ctx *synccontext.SyncContext, event *synccontext.SyncEv
}

func (s *podSyncer) resizeHostPodContainerResourcesInPlace(ctx *synccontext.SyncContext, event *synccontext.SyncEvent[*corev1.Pod]) error {
if s.hostClusterVersion.LessThan(utilversion.MustParseSemantic("1.35.0")) {
// in-place pod resize is only available on the host from K8s 1.35; skip it when the host
// version is older or not known.
if s.hostClusterVersion == nil || s.hostClusterVersion.LessThan(utilversion.MustParseSemantic("1.35.0")) {
return nil
}

Expand Down
55 changes: 51 additions & 4 deletions pkg/controllers/resources/pods/translate/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package translate
import (
"encoding/json"
"fmt"
"slices"
"strings"

"github.com/loft-sh/vcluster/pkg/mappings"
Expand All @@ -20,8 +21,9 @@ 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(
// Sync conditions. We only keep the reconciled host side; the virtual conditions are
// recomputed below when vPod.Status is overwritten with the host status.
_, event.Host.Status.Conditions = t.conditionsCopyBidirectional(
event.VirtualOld.Status.Conditions,
event.Virtual.Status.Conditions,
event.HostOld.Status.Conditions,
Expand All @@ -32,11 +34,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 a few virtual values that the host
// must not overwrite:
// - QOSClass: the host treats it as immutable (since K8s 1.32), so it can't be changed.
// - ObservedGeneration: a virtual cluster on K8s < 1.34 doesn't store it, so copying the
// host value would keep looking like a change and patch the status on every reconcile.
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
vPod.Status.Conditions = stripConditionObservedGenerations(vPod.Status.Conditions)
}
err := t.convertResourceClaimStatuses(ctx, vPod, pPod.GetNamespace())
if err != nil {
return err
Expand Down Expand Up @@ -281,3 +291,40 @@ func hasToleration(tolerations []corev1.Toleration, tol corev1.Toleration) bool
}
return false
}

// virtualClusterStripsObservedGeneration reports whether the virtual apiserver throws away
// ObservedGeneration when a pod status is written. This happens 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 works like patcher.CopyBidirectional for pod conditions. On
// K8s < 1.34 the virtual cluster does not store ObservedGeneration, so this function ignores that field
// when checking for changes to avoid reacting to a value that was never saved. It does not
// change the conditions, 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)
}

return patcher.CopyBidirectionalWithEq(virtualOld, virtual, hostOld, host, func(a, b []corev1.PodCondition) bool {
return apiequality.Semantic.DeepEqual(
stripConditionObservedGenerations(a),
stripConditionObservedGenerations(b),
)
})
}

// stripConditionObservedGenerations returns a copy of the conditions with ObservedGeneration
// set to zero on each one. It is only used to compare conditions while ignoring that field.
func stripConditionObservedGenerations(conditions []corev1.PodCondition) []corev1.PodCondition {
out := slices.Clone(conditions)
for i := range out {
out[i].ObservedGeneration = 0
}
return out
}
Loading
Loading