Merge pull request #1767 from FabianKramm/main

fix: proxy kubelets loadbalancer
loft-sh · May 13, 2024 · 975951c · 975951c
2 parents ce165cb + 328089a
commit 975951c
Show file tree

Hide file tree

Showing 7 changed files with 93 additions and 42 deletions.
diff --git a/chart/templates/_coredns.tpl b/chart/templates/_coredns.tpl
@@ -10,7 +10,9 @@ Corefile: |-
       errors
       health
       ready
+      {{- if .Values.networking.advanced.proxyKubelets.byHostname }}
       rewrite name regex .*\.nodes\.vcluster\.com kubernetes.default.svc.cluster.local
+      {{- end }}
       kubernetes cluster.local in-addr.arpa ip6.arpa {
           {{- if .Values.controlPlane.coredns.embedded }}
           kubeconfig /data/vcluster/admin.conf

diff --git a/chart/templates/service.yaml b/chart/templates/service.yaml
@@ -28,7 +28,7 @@ spec:
       {{- end }}
       nodePort: {{ .Values.controlPlane.service.httpsNodePort }}
       protocol: TCP
-   {{- if or .Values.networking.advanced.proxyKubelets.byHostname .Values.networking.advanced.proxyKubelets.byIP }}
+   {{- if and .Values.networking.advanced.proxyKubelets.byHostname (not (eq .Values.controlPlane.service.spec.type "LoadBalancer")) }}
     - name: kubelet
       port: 10250
       {{- if not .Values.experimental.isolatedControlPlane.headless }}

diff --git a/chart/tests/service_test.yaml b/chart/tests/service_test.yaml
@@ -17,7 +17,6 @@ tests:
       networking:
         advanced:
           proxyKubelets:
-            byIP: false
             byHostname: false
     asserts:
       - hasDocuments:
@@ -34,6 +33,43 @@ tests:
             protocol: TCP
             port: 443
 
+  - it: should not create kubelet port 2
+    set:
+      controlPlane:
+        service:
+          spec:
+            type: LoadBalancer
+    asserts:
+      - hasDocuments:
+          count: 1
+      - lengthEqual:
+          path: spec.ports
+          count: 1
+      - contains:
+          path: spec.ports
+          content:
+            name: https
+            nodePort: 0
+            targetPort: 8443
+            protocol: TCP
+            port: 443
+
+  - it: should create kubelet port
+    asserts:
+      - hasDocuments:
+          count: 1
+      - lengthEqual:
+          path: spec.ports
+          count: 2
+      - contains:
+          path: spec.ports
+          content:
+            name: kubelet
+            nodePort: 0
+            targetPort: 8443
+            protocol: TCP
+            port: 10250
+
   - it: service defaults
     release:
       name: my-release

diff --git a/pkg/controllers/resources/nodes/fake_syncer.go b/pkg/controllers/resources/nodes/fake_syncer.go
@@ -33,14 +33,16 @@ var (
 
 func NewFakeSyncer(ctx *synccontext.RegisterContext, nodeService nodeservice.Provider) (syncer.Object, error) {
 	return &fakeNodeSyncer{
-		nodeServiceProvider: nodeService,
-		fakeKubeletIPs:      ctx.Config.Networking.Advanced.ProxyKubelets.ByIP,
+		nodeServiceProvider:  nodeService,
+		fakeKubeletIPs:       ctx.Config.Networking.Advanced.ProxyKubelets.ByIP,
+		fakeKubeletHostnames: ctx.Config.Networking.Advanced.ProxyKubelets.ByHostname,
 	}, nil
 }
 
 type fakeNodeSyncer struct {
-	nodeServiceProvider nodeservice.Provider
-	fakeKubeletIPs      bool
+	nodeServiceProvider  nodeservice.Provider
+	fakeKubeletIPs       bool
+	fakeKubeletHostnames bool
 }
 
 func (r *fakeNodeSyncer) Resource() client.Object {
@@ -74,7 +76,7 @@ func (r *fakeNodeSyncer) FakeSyncToVirtual(ctx *synccontext.SyncContext, name ty
 	}
 
 	ctx.Log.Infof("Create fake node %s", name.Name)
-	return ctrl.Result{}, CreateFakeNode(ctx.Context, r.fakeKubeletIPs, r.nodeServiceProvider, ctx.VirtualClient, name.Name)
+	return ctrl.Result{}, createFakeNode(ctx.Context, r.fakeKubeletIPs, r.fakeKubeletHostnames, r.nodeServiceProvider, ctx.VirtualClient, name.Name)
 }
 
 func (r *fakeNodeSyncer) FakeSync(ctx *synccontext.SyncContext, vObj client.Object) (ctrl.Result, error) {
@@ -143,9 +145,10 @@ func newGUID() string {
 	return random.String(8) + "-" + random.String(4) + "-" + random.String(4) + "-" + random.String(4) + "-" + random.String(12)
 }
 
-func CreateFakeNode(
+func createFakeNode(
 	ctx context.Context,
 	fakeKubeletIPs bool,
+	fakeKubeletHostnames bool,
 	nodeServiceProvider nodeservice.Provider,
 	virtualClient client.Client,
 	name string,
@@ -228,12 +231,7 @@ func CreateFakeNode(
 				Type:               corev1.NodeReady,
 			},
 		},
-		Addresses: []corev1.NodeAddress{
-			{
-				Address: GetNodeHost(node.Name),
-				Type:    corev1.NodeHostName,
-			},
-		},
+		Addresses: []corev1.NodeAddress{},
 		DaemonEndpoints: corev1.NodeDaemonEndpoints{
 			KubeletEndpoint: corev1.DaemonEndpoint{
 				Port: constants.KubeletPort,
@@ -254,6 +252,13 @@ func CreateFakeNode(
 		Images: []corev1.ContainerImage{},
 	}
 
+	if fakeKubeletHostnames {
+		node.Status.Addresses = append(node.Status.Addresses, corev1.NodeAddress{
+			Address: GetNodeHost(node.Name),
+			Type:    corev1.NodeHostName,
+		})
+	}
+
 	if fakeKubeletIPs {
 		nodeIP, err := nodeServiceProvider.GetNodeIP(ctx, name)
 		if err != nil {

diff --git a/pkg/controllers/resources/nodes/nodeservice/node_service.go b/pkg/controllers/resources/nodes/nodeservice/node_service.go
@@ -6,6 +6,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/loft-sh/vcluster/pkg/constants"
 	"github.com/loft-sh/vcluster/pkg/util/translate"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
@@ -25,8 +26,6 @@ var (
 	ServiceClusterLabel = "vcluster.loft.sh/belongs-to"
 	// ServiceNodeLabel specifies which node this service represents
 	ServiceNodeLabel = "vcluster.loft.sh/node"
-	// KubeletPort is the port we pretend the kubelet is running under
-	KubeletPort = int32(10250)
 	// KubeletTargetPort is the port vcluster will run under
 	KubeletTargetPort = 8443
 )
@@ -149,7 +148,7 @@ func (n *nodeServiceProvider) GetNodeIP(ctx context.Context, name string) (strin
 	}
 
 	// create the new service
-	targetPort := intstr.FromInt(KubeletTargetPort)
+	targetPort := intstr.FromInt32(int32(KubeletTargetPort))
 	if vclusterService.Spec.Selector == nil {
 		targetPort = intstr.IntOrString{}
 	}
@@ -166,7 +165,7 @@ func (n *nodeServiceProvider) GetNodeIP(ctx context.Context, name string) (strin
 			Ports: []corev1.ServicePort{
 				{
 					Name:       "kubelet",
-					Port:       KubeletPort,
+					Port:       constants.KubeletPort,
 					TargetPort: targetPort,
 				},
 			},
@@ -198,8 +197,8 @@ func (n *nodeServiceProvider) GetNodeIP(ctx context.Context, name string) (strin
 }
 
 func (n *nodeServiceProvider) updateNodeServiceEndpoints(ctx context.Context, nodeServiceName string) error {
-	vclusterServiceEndpoints := &corev1.Endpoints{}
-	err := n.currentNamespaceClient.Get(ctx, types.NamespacedName{Name: n.serviceName, Namespace: n.currentNamespace}, vclusterServiceEndpoints)
+	vClusterServiceEndpoints := &corev1.Endpoints{}
+	err := n.currentNamespaceClient.Get(ctx, types.NamespacedName{Name: n.serviceName, Namespace: n.currentNamespace}, vClusterServiceEndpoints)
 	if err != nil {
 		return errors.Wrap(err, "get vcluster service endpoints")
 	}
@@ -214,14 +213,19 @@ func (n *nodeServiceProvider) updateNodeServiceEndpoints(ctx context.Context, no
 	result, err := controllerutil.CreateOrPatch(ctx, n.currentNamespaceClient, nodeServiceEndpoints, func() error {
 		// build new subsets
 		newSubsets := []corev1.EndpointSubset{}
-		for _, subset := range vclusterServiceEndpoints.Subsets {
+		for _, subset := range vClusterServiceEndpoints.Subsets {
 			newPorts := []corev1.EndpointPort{}
 			for _, p := range subset.Ports {
-				if p.Name != "kubelet" {
+				if p.Name != "https" {
 					continue
 				}
 
-				newPorts = append(newPorts, p)
+				newPorts = append(newPorts, corev1.EndpointPort{
+					Name:        "kubelet",
+					Port:        p.Port,
+					Protocol:    p.Protocol,
+					AppProtocol: p.AppProtocol,
+				})
 			}
 
 			newAddresses := []corev1.EndpointAddress{}

diff --git a/pkg/controllers/resources/nodes/syncer.go b/pkg/controllers/resources/nodes/syncer.go
@@ -52,11 +52,12 @@ func NewSyncer(ctx *synccontext.RegisterContext, nodeServiceProvider nodeservice
 	return &nodeSyncer{
 		enableScheduler: ctx.Config.ControlPlane.Advanced.VirtualScheduler.Enabled,
 
-		enforceNodeSelector: true,
-		nodeSelector:        nodeSelector,
-		clearImages:         ctx.Config.Sync.FromHost.Nodes.ClearImageStatus,
-		useFakeKubelets:     ctx.Config.Networking.Advanced.ProxyKubelets.ByHostname || ctx.Config.Networking.Advanced.ProxyKubelets.ByIP,
-		fakeKubeletIPs:      ctx.Config.Networking.Advanced.ProxyKubelets.ByIP,
+		enforceNodeSelector:  true,
+		nodeSelector:         nodeSelector,
+		clearImages:          ctx.Config.Sync.FromHost.Nodes.ClearImageStatus,
+		useFakeKubelets:      ctx.Config.Networking.Advanced.ProxyKubelets.ByHostname || ctx.Config.Networking.Advanced.ProxyKubelets.ByIP,
+		fakeKubeletIPs:       ctx.Config.Networking.Advanced.ProxyKubelets.ByIP,
+		fakeKubeletHostnames: ctx.Config.Networking.Advanced.ProxyKubelets.ByHostname,
 
 		physicalClient:      ctx.PhysicalManager.GetClient(),
 		virtualClient:       ctx.VirtualManager.GetClient(),
@@ -66,17 +67,18 @@ func NewSyncer(ctx *synccontext.RegisterContext, nodeServiceProvider nodeservice
 }
 
 type nodeSyncer struct {
-	nodeSelector        labels.Selector
-	physicalClient      client.Client
-	virtualClient       client.Client
-	unmanagedPodCache   client.Reader
-	nodeServiceProvider nodeservice.Provider
-	enforcedTolerations []*corev1.Toleration
-	enableScheduler     bool
-	clearImages         bool
-	enforceNodeSelector bool
-	useFakeKubelets     bool
-	fakeKubeletIPs      bool
+	nodeSelector         labels.Selector
+	physicalClient       client.Client
+	virtualClient        client.Client
+	unmanagedPodCache    client.Reader
+	nodeServiceProvider  nodeservice.Provider
+	enforcedTolerations  []*corev1.Toleration
+	enableScheduler      bool
+	clearImages          bool
+	enforceNodeSelector  bool
+	useFakeKubelets      bool
+	fakeKubeletIPs       bool
+	fakeKubeletHostnames bool
 }
 
 func (s *nodeSyncer) Resource() client.Object {

diff --git a/pkg/controllers/resources/nodes/translate.go b/pkg/controllers/resources/nodes/translate.go
@@ -140,11 +140,13 @@ func (s *nodeSyncer) translateUpdateStatus(ctx *synccontext.SyncContext, pNode *
 		}
 
 		// translate addresses
-		newAddresses := []corev1.NodeAddress{
-			{
+		newAddresses := []corev1.NodeAddress{}
+
+		if s.fakeKubeletHostnames {
+			newAddresses = append(newAddresses, corev1.NodeAddress{
 				Address: GetNodeHost(vNode.Name),
 				Type:    corev1.NodeHostName,
-			},
+			})
 		}
 
 		if s.fakeKubeletIPs {