fix: revert fake kubelet ip in pod syncer (#3191) (#3195)

(cherry picked from commit b3f6fc4)

Co-authored-by: Johannes Frey <[email protected]>

Author: loft-bot

pkg/controllers/resources/pods/syncer_test.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"maps"
 	"testing"
-	"time"
 
 	"gotest.tools/assert"
 	corev1 "k8s.io/api/core/v1"
@@ -591,39 +590,6 @@ func TestSync(t *testing.T) {
 		},
 	}
 
-	testNodeName := "test123"
-	pVclusterNodeService := pVclusterService.DeepCopy()
-	pVclusterNodeService.Name = translate.SafeConcatName(testingutil.DefaultTestVClusterName, "node", testNodeName)
-
-	pPodFakeKubelet := pPodBase.DeepCopy()
-	pPodFakeKubelet.Spec.NodeName = testNodeName
-	pPodFakeKubelet.Status.HostIP = "3.3.3.3"
-	pPodFakeKubelet.Status.HostIPs = []corev1.HostIP{
-		{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{
-			NodeName: testNodeName,
-		},
-	}
-	vPodWithHostIP := vPodWithNodeName.DeepCopy()
-	vPodWithHostIP.Status.HostIP = pVclusterService.Spec.ClusterIP
-	vPodWithHostIP.Status.HostIPs = []corev1.HostIP{
-		{IP: pVclusterService.Spec.ClusterIP},
-	}
-
-	testNode := &corev1.Node{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: testNodeName,
-		},
-	}
-
 	priorityClassName := "high-priority"
 	pPriorityClass := &schedulingv1.PriorityClass{
 		ObjectMeta: metav1.ObjectMeta{
@@ -675,43 +641,6 @@ func TestSync(t *testing.T) {
 				assert.NilError(t, err)
 			},
 		},
-		{
-			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
-				syncContext, syncer := syncertesting.FakeStartSyncer(t, ctx, New)
-				_, err := syncer.(*podSyncer).Sync(syncContext, synccontext.NewSyncEventWithOld(pPodFakeKubelet, pPodFakeKubelet, vPodWithNodeName, vPodWithNodeName))
-				assert.NilError(t, err)
-			},
-		},
-		{
-			Name:                 "Fake Kubelet enabled with Node sync and node service not found",
-			InitialVirtualState:  []runtime.Object{testNode.DeepCopy(), vPodWithNodeName, vNamespace.DeepCopy()},
-			InitialPhysicalState: []runtime.Object{testNode.DeepCopy(), pPodFakeKubelet.DeepCopy()},
-			ExpectedVirtualState: map[schema.GroupVersionKind][]runtime.Object{
-				corev1.SchemeGroupVersion.WithKind("Pod"): {vPodWithNodeName},
-			},
-			Sync: func(ctx *synccontext.RegisterContext) {
-				ctx.Config.Sync.FromHost.Nodes.Selector.All = true
-				ctx.Config.Networking.Advanced.ProxyKubelets.ByIP = true
-				syncContext, syncer := syncertesting.FakeStartSyncer(t, ctx, New)
-
-				result, err := syncer.(*podSyncer).Sync(syncContext, synccontext.NewSyncEventWithOld(pPodFakeKubelet, pPodFakeKubelet, vPodWithNodeName, vPodWithNodeName))
-				assert.NilError(t, err)
-				assert.Equal(t, result.RequeueAfter, time.Second, "Should requeue if node service is not found")
-			},
-		},
 		{
 			Name:                 "From Host PriorityClasses sync enabled",
 			InitialVirtualState:  []runtime.Object{vPodWithPriorityClass, vNamespace.DeepCopy()},

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

@@ -2,15 +2,13 @@ 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"
 )
 
@@ -85,26 +83,11 @@ 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, HostIPAnnotation, HostIPsAnnotation}
+	annotations := []string{ClusterAutoScalerAnnotation, OwnerReferences, OwnerSetKind, NamespaceAnnotation, NameAnnotation, UIDAnnotation, ServiceAccountNameAnnotation, HostsRewrittenAnnotation, VClusterLabelsAnnotation}
 	if pPod != nil {
 		for _, v := range pPod.Spec.Volumes {
 			if v.Projected != nil {
@@ -130,17 +113,6 @@ 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

@@ -42,8 +42,6 @@ const (
 	ClusterAutoScalerDaemonSetAnnotation = "cluster-autoscaler.kubernetes.io/daemonset-pod"
 	ServiceAccountNameAnnotation         = "vcluster.loft.sh/service-account-name"
 	ServiceAccountTokenAnnotation        = "vcluster.loft.sh/token-"
-	HostIPAnnotation                     = "vcluster.loft.sh/host-ip"
-	HostIPsAnnotation                    = "vcluster.loft.sh/host-ips"
 )
 
 var (
@@ -115,7 +113,6 @@ func NewTranslator(ctx *synccontext.RegisterContext, eventRecorder record.EventR
 		hostPriorityClassesSyncEnabled: ctx.Config.Sync.FromHost.PriorityClasses.Enabled,
 		priorityClassesSyncEnabled:     ctx.Config.Sync.ToHost.PriorityClasses.Enabled,
 		schedulingConfig:               schedulingConfig,
-		fakeKubeletIPs:                 ctx.Config.Networking.Advanced.ProxyKubelets.ByIP,
 
 		mountPhysicalHostPaths: ctx.Config.ControlPlane.HostPathMapper.Enabled && !ctx.Config.ControlPlane.HostPathMapper.Central,
 
@@ -148,7 +145,6 @@ type translator struct {
 	hostPriorityClassesSyncEnabled bool
 	priorityClassesSyncEnabled     bool
 	schedulingConfig               scheduling.Config
-	fakeKubeletIPs                 bool
 
 	virtualLogsPath       string
 	virtualPodLogsPath    string
@@ -438,7 +434,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)
+				translateFieldRef(pPod.Spec.Volumes[i].DownwardAPI.Items[j].FieldRef)
 			}
 		}
 		if pPod.Spec.Volumes[i].ISCSI != nil && pPod.Spec.Volumes[i].ISCSI.SecretRef != nil {
@@ -504,7 +500,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)
+				translateFieldRef(projectedVolume.Sources[i].DownwardAPI.Items[j].FieldRef)
 			}
 		}
 		if projectedVolume.Sources[i].ServiceAccountToken != nil {
@@ -603,7 +599,7 @@ func (t *translator) translateProjectedVolume(
 	return nil
 }
 
-func translateFieldRef(fieldSelector *corev1.ObjectFieldSelector, fakeKubeletIPs bool) {
+func translateFieldRef(fieldSelector *corev1.ObjectFieldSelector) {
 	if fieldSelector == nil {
 		return
 	}
@@ -626,22 +622,13 @@ func translateFieldRef(fieldSelector *corev1.ObjectFieldSelector, fakeKubeletIPs
 		fieldSelector.FieldPath = "metadata.annotations['" + UIDAnnotation + "']"
 	case "spec.serviceAccountName":
 		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 {
-			fieldSelector.FieldPath = "metadata.annotations['" + HostIPAnnotation + "']"
-		}
-	case "status.hostIPs":
-		if fakeKubeletIPs {
-			fieldSelector.FieldPath = "metadata.annotations['" + HostIPsAnnotation + "']"
-		}
 	}
 }
 
 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)
+		translateDownwardAPI(&envVar[j])
 		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())
 		}
@@ -682,14 +669,14 @@ func (t *translator) TranslateContainerEnv(ctx *synccontext.SyncContext, envVar
 	return envVar, envFrom, nil
 }
 
-func translateDownwardAPI(env *corev1.EnvVar, fakeKubeletIPs bool) {
+func translateDownwardAPI(env *corev1.EnvVar) {
 	if env.ValueFrom == nil {
 		return
 	}
 	if env.ValueFrom.FieldRef == nil {
 		return
 	}
-	translateFieldRef(env.ValueFrom.FieldRef, fakeKubeletIPs)
+	translateFieldRef(env.ValueFrom.FieldRef)
 }
 
 func (t *translator) translateDNSConfig(pPod *corev1.Pod, vPod *corev1.Pod, nameServer string) {

test/e2e/syncer/pods/pods.go

@@ -82,9 +82,6 @@ var _ = ginkgo.Describe("Pods are running in the host cluster", func() {
 		pod, err := f.HostClient.CoreV1().Pods(pPodName.Namespace).Get(f.Context, pPodName.Name, metav1.GetOptions{})
 		framework.ExpectNoError(err)
 
-		// ignore HostIP differences
-		resetHostIP(vpod, pod)
-
 		// Since k8s 1.32, status.QOSClass field has become immutable,
 		// hence we have stopeed syncing it. So ignore
 		// the differences in the status.QOSClass field
@@ -218,9 +215,6 @@ var _ = ginkgo.Describe("Pods are running in the host cluster", func() {
 		pod, err := f.HostClient.CoreV1().Pods(pPodName.Namespace).Get(f.Context, pPodName.Name, metav1.GetOptions{})
 		framework.ExpectNoError(err)
 
-		// ignore HostIP differences
-		resetHostIP(vpod, pod)
-
 		// Since k8s 1.32, status.QOSClass field has become immutable,
 		// hence we have stopeed syncing it. So ignore
 		// the differences in the status.QOSClass field
@@ -772,11 +766,6 @@ var _ = ginkgo.Describe("Pods are running in the host cluster", func() {
 	})
 })
 
-func resetHostIP(vpod, pod *corev1.Pod) {
-	vpod.Status.HostIP, pod.Status.HostIP = "", ""
-	vpod.Status.HostIPs, pod.Status.HostIPs = nil, nil
-}
-
 func ignoreQOSClassDiff(vpod, pod *corev1.Pod) {
 	pod.Status.QOSClass = vpod.Status.QOSClass
 }