Reapply "fix: patch virtual instead of physical and always add host ip annotations to physical (#3147)" (#3151)

This reverts commit 7f97d20.

(cherry picked from commit 15de457)

Author: johannesfrey

pkg/controllers/resources/pods/syncer.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"reflect"
 	"slices"
-	"strings"
 	"time"
 
 	nodev1 "k8s.io/api/node/v1"
@@ -252,19 +251,6 @@ func (s *podSyncer) SyncToHost(ctx *synccontext.SyncContext, event *synccontext.
 		if pPod.Spec.NodeName == "" {
 			return ctrl.Result{}, nil
 		}
-
-		if s.fakeKubeletIPs {
-			nodeService, err := s.ensureNodeService(ctx, pPod)
-			if err != nil {
-				if kerrors.IsNotFound(err) {
-					return ctrl.Result{RequeueAfter: time.Second}, nil
-				}
-				return ctrl.Result{}, err
-			}
-
-			pPod.Annotations[translatepods.HostIPAnnotation] = nodeService.Spec.ClusterIP
-			pPod.Annotations[translatepods.HostIPsAnnotation] = nodeService.Spec.ClusterIP
-		}
 	}
 
 	err = pro.ApplyPatchesHostObject(ctx, nil, pPod, event.Virtual, ctx.Config.Sync.ToHost.Pods.Patches, false)
@@ -349,21 +335,6 @@ func (s *podSyncer) Sync(ctx *synccontext.SyncContext, event *synccontext.SyncEv
 		return patcher.DeleteVirtualObjectWithOptions(ctx, event.Virtual, event.Host, "node name is different between the two", &client.DeleteOptions{GracePeriodSeconds: &minimumGracePeriodInSeconds})
 	}
 
-	if s.fakeKubeletIPs && event.Host.Status.HostIP != "" {
-		nodeService, err := s.ensureNodeService(ctx, event.Host)
-		if err != nil {
-			if kerrors.IsNotFound(err) {
-				return ctrl.Result{RequeueAfter: time.Second}, nil
-			}
-			return ctrl.Result{}, err
-		}
-
-		event.Host.Status.HostIP = nodeService.Spec.ClusterIP
-		event.Host.Status.HostIPs = []corev1.HostIP{
-			{IP: nodeService.Spec.ClusterIP},
-		}
-	}
-
 	// validate virtual pod before syncing it to the host cluster
 	if s.podSecurityStandard != "" {
 		valid, err := s.isPodSecurityStandardsValid(ctx, event.Virtual, ctx.Log)
@@ -424,6 +395,10 @@ func (s *podSyncer) Sync(ctx *synccontext.SyncContext, event *synccontext.SyncEv
 	// update the virtual pod if the spec has changed
 	err = s.podTranslator.Diff(ctx, event)
 	if err != nil {
+		if kerrors.IsNotFound(err) {
+			return ctrl.Result{RequeueAfter: time.Second}, nil
+		}
+
 		return ctrl.Result{}, err
 	}
 
@@ -547,17 +522,6 @@ func (s *podSyncer) assignNodeToPod(ctx *synccontext.SyncContext, pObj *corev1.P
 	return nil
 }
 
-func (s *podSyncer) ensureNodeService(ctx *synccontext.SyncContext, pPod *corev1.Pod) (*corev1.Service, error) {
-	serviceName := translate.SafeConcatName(translate.VClusterName, "node", strings.ReplaceAll(pPod.Spec.NodeName, ".", "-"))
-
-	nodeService := &corev1.Service{}
-	err := ctx.CurrentNamespaceClient.Get(ctx.Context, types.NamespacedName{Name: serviceName, Namespace: ctx.CurrentNamespace}, nodeService)
-	if err != nil {
-		return nil, fmt.Errorf("get node service: %w", err)
-	}
-	return nodeService, nil
-}
-
 func (s *podSyncer) applyLimitByClasses(ctx *synccontext.SyncContext, virtual *corev1.Pod) bool {
 	return s.applyLimitByPriorityClass(ctx, virtual) || s.applyLimitByRuntimeClass(ctx, virtual)
 }

pkg/controllers/resources/pods/syncer_test.go

@@ -602,6 +602,10 @@ func TestSync(t *testing.T) {
 		{IP: "3.3.3.3"},
 	}
 
+	pPodFakeKubeletHostIPs := pPodFakeKubelet.DeepCopy()
+	pPodFakeKubeletHostIPs.Annotations[podtranslate.HostIPAnnotation] = pVclusterService.Spec.ClusterIP
+	pPodFakeKubeletHostIPs.Annotations[podtranslate.HostIPsAnnotation] = pVclusterService.Spec.ClusterIP
+
 	vPodWithNodeName := &corev1.Pod{
 		ObjectMeta: vObjectMeta,
 		Spec: corev1.PodSpec{
@@ -675,9 +679,14 @@ func TestSync(t *testing.T) {
 			Name:                 "Fake Kubelet enabled with Node sync",
 			InitialVirtualState:  []runtime.Object{testNode.DeepCopy(), vPodWithNodeName, vNamespace.DeepCopy()},
 			InitialPhysicalState: []runtime.Object{testNode.DeepCopy(), pVclusterNodeService.DeepCopy(), pPodFakeKubelet.DeepCopy()},
+			// The virtual pod should have the host IPs of the node service in its status.
 			ExpectedVirtualState: map[schema.GroupVersionKind][]runtime.Object{
 				corev1.SchemeGroupVersion.WithKind("Pod"): {vPodWithHostIP},
 			},
+			// The physical pod should have the host IPs of the node service in its annotations.
+			ExpectedPhysicalState: map[schema.GroupVersionKind][]runtime.Object{
+				corev1.SchemeGroupVersion.WithKind("Pod"): {pPodFakeKubeletHostIPs},
+			},
 			Sync: func(ctx *synccontext.RegisterContext) {
 				ctx.Config.Sync.FromHost.Nodes.Selector.All = true
 				ctx.Config.Networking.Advanced.ProxyKubelets.ByIP = true

pkg/controllers/resources/pods/translate/diff.go

@@ -2,13 +2,15 @@ package translate
 
 import (
 	"encoding/json"
+	"fmt"
 	"strings"
 
 	"github.com/loft-sh/vcluster/pkg/patcher"
 	"github.com/loft-sh/vcluster/pkg/syncer/synccontext"
 	"github.com/loft-sh/vcluster/pkg/util/translate"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -83,11 +85,26 @@ func (t *translator) Diff(ctx *synccontext.SyncContext, event *synccontext.SyncE
 		delete(event.Host.Annotations, OwnerSetKind)
 	}
 
+	if t.fakeKubeletIPs && event.Host.Status.HostIP != "" {
+		nodeService, err := ensureNodeService(ctx, event.Host)
+		if err != nil {
+			return err
+		}
+
+		event.Virtual.Status.HostIP = nodeService.Spec.ClusterIP
+		event.Virtual.Status.HostIPs = []corev1.HostIP{
+			{IP: nodeService.Spec.ClusterIP},
+		}
+
+		event.Host.Annotations[HostIPAnnotation] = nodeService.Spec.ClusterIP
+		event.Host.Annotations[HostIPsAnnotation] = nodeService.Spec.ClusterIP
+	}
+
 	return nil
 }
 
 func GetExcludedAnnotations(pPod *corev1.Pod) []string {
-	annotations := []string{ClusterAutoScalerAnnotation, OwnerReferences, OwnerSetKind, NamespaceAnnotation, NameAnnotation, UIDAnnotation, ServiceAccountNameAnnotation, HostsRewrittenAnnotation, VClusterLabelsAnnotation}
+	annotations := []string{ClusterAutoScalerAnnotation, OwnerReferences, OwnerSetKind, NamespaceAnnotation, NameAnnotation, UIDAnnotation, ServiceAccountNameAnnotation, HostsRewrittenAnnotation, VClusterLabelsAnnotation, HostIPAnnotation, HostIPsAnnotation}
 	if pPod != nil {
 		for _, v := range pPod.Spec.Volumes {
 			if v.Projected != nil {
@@ -113,6 +130,17 @@ func GetExcludedAnnotations(pPod *corev1.Pod) []string {
 	return annotations
 }
 
+func ensureNodeService(ctx *synccontext.SyncContext, pPod *corev1.Pod) (*corev1.Service, error) {
+	serviceName := translate.SafeConcatName(translate.VClusterName, "node", strings.ReplaceAll(pPod.Spec.NodeName, ".", "-"))
+
+	nodeService := &corev1.Service{}
+	err := ctx.CurrentNamespaceClient.Get(ctx.Context, types.NamespacedName{Name: serviceName, Namespace: ctx.CurrentNamespace}, nodeService)
+	if err != nil {
+		return nil, fmt.Errorf("get node service: %w", err)
+	}
+	return nodeService, nil
+}
+
 // Changeable fields within the pod:
 // - spec.containers[*].image
 // - spec.initContainers[*].image

pkg/controllers/resources/pods/translate/translator.go

@@ -442,7 +442,7 @@ func (t *translator) translateVolumes(ctx *synccontext.SyncContext, pPod *corev1
 		}
 		if pPod.Spec.Volumes[i].DownwardAPI != nil {
 			for j := range pPod.Spec.Volumes[i].DownwardAPI.Items {
-				translateFieldRef(pPod.Spec.Volumes[i].DownwardAPI.Items[j].FieldRef, t.fakeKubeletIPs, t.schedulingConfig.IsSchedulerFromVirtualCluster(pPod.Spec.SchedulerName))
+				translateFieldRef(pPod.Spec.Volumes[i].DownwardAPI.Items[j].FieldRef, t.fakeKubeletIPs)
 			}
 		}
 		if pPod.Spec.Volumes[i].ISCSI != nil && pPod.Spec.Volumes[i].ISCSI.SecretRef != nil {
@@ -508,7 +508,7 @@ func (t *translator) translateProjectedVolume(
 		}
 		if projectedVolume.Sources[i].DownwardAPI != nil {
 			for j := range projectedVolume.Sources[i].DownwardAPI.Items {
-				translateFieldRef(projectedVolume.Sources[i].DownwardAPI.Items[j].FieldRef, t.fakeKubeletIPs, t.schedulingConfig.IsSchedulerFromVirtualCluster(pPod.Spec.SchedulerName))
+				translateFieldRef(projectedVolume.Sources[i].DownwardAPI.Items[j].FieldRef, t.fakeKubeletIPs)
 			}
 		}
 		if projectedVolume.Sources[i].ServiceAccountToken != nil {
@@ -607,7 +607,7 @@ func (t *translator) translateProjectedVolume(
 	return nil
 }
 
-func translateFieldRef(fieldSelector *corev1.ObjectFieldSelector, fakeKubeletIPs, enableScheduler bool) {
+func translateFieldRef(fieldSelector *corev1.ObjectFieldSelector, fakeKubeletIPs bool) {
 	if fieldSelector == nil {
 		return
 	}
@@ -632,11 +632,11 @@ func translateFieldRef(fieldSelector *corev1.ObjectFieldSelector, fakeKubeletIPs
 		fieldSelector.FieldPath = "metadata.annotations['" + ServiceAccountNameAnnotation + "']"
 	// translate downward API references for status.hostIP(s) only when both virtual scheduler & fakeKubeletIPs are enabled
 	case "status.hostIP":
-		if fakeKubeletIPs && enableScheduler {
+		if fakeKubeletIPs {
 			fieldSelector.FieldPath = "metadata.annotations['" + HostIPAnnotation + "']"
 		}
 	case "status.hostIPs":
-		if fakeKubeletIPs && enableScheduler {
+		if fakeKubeletIPs {
 			fieldSelector.FieldPath = "metadata.annotations['" + HostIPsAnnotation + "']"
 		}
 	}
@@ -645,7 +645,7 @@ func translateFieldRef(fieldSelector *corev1.ObjectFieldSelector, fakeKubeletIPs
 func (t *translator) TranslateContainerEnv(ctx *synccontext.SyncContext, envVar []corev1.EnvVar, envFrom []corev1.EnvFromSource, vPod *corev1.Pod, serviceEnvMap map[string]string) ([]corev1.EnvVar, []corev1.EnvFromSource, error) {
 	envNameMap := make(map[string]struct{})
 	for j, env := range envVar {
-		translateDownwardAPI(&envVar[j], t.fakeKubeletIPs, t.schedulingConfig.IsSchedulerFromVirtualCluster(vPod.Spec.SchedulerName))
+		translateDownwardAPI(&envVar[j], t.fakeKubeletIPs)
 		if env.ValueFrom != nil && env.ValueFrom.ConfigMapKeyRef != nil && env.ValueFrom.ConfigMapKeyRef.Name != "" {
 			envVar[j].ValueFrom.ConfigMapKeyRef.Name = mappings.VirtualToHostName(ctx, envVar[j].ValueFrom.ConfigMapKeyRef.Name, vPod.Namespace, mappings.ConfigMaps())
 		}
@@ -686,14 +686,14 @@ func (t *translator) TranslateContainerEnv(ctx *synccontext.SyncContext, envVar
 	return envVar, envFrom, nil
 }
 
-func translateDownwardAPI(env *corev1.EnvVar, fakeKubeletIPs, enableScheduler bool) {
+func translateDownwardAPI(env *corev1.EnvVar, fakeKubeletIPs bool) {
 	if env.ValueFrom == nil {
 		return
 	}
 	if env.ValueFrom.FieldRef == nil {
 		return
 	}
-	translateFieldRef(env.ValueFrom.FieldRef, fakeKubeletIPs, enableScheduler)
+	translateFieldRef(env.ValueFrom.FieldRef, fakeKubeletIPs)
 }
 
 func (t *translator) translateDNSConfig(pPod *corev1.Pod, vPod *corev1.Pod, nameServer string) {