Added Ports to NodePortConfig and expose fixes (#247)

* fix NodePort service update

* updated crd docs

charts/k3k/crds/k3k.io_clusters.yaml

@@ -125,10 +125,24 @@ spec:
                     type: object
                   nodePort:
                     properties:
-                      enabled:
-                        type: boolean
-                    required:
-                    - enabled
+                      etcdPort:
+                        description: |-
+                          ETCDPort is the port on each node on which the ETCD service is exposed when type is NodePort.
+                          If not specified, a port will be allocated (default: 30000-32767)
+                        format: int32
+                        type: integer
+                      serverPort:
+                        description: |-
+                          ServerPort is the port on each node on which the K3s server service is exposed when type is NodePort.
+                          If not specified, a port will be allocated (default: 30000-32767)
+                        format: int32
+                        type: integer
+                      servicePort:
+                        description: |-
+                          ServicePort is the port on each node on which the K3s service is exposed when type is NodePort.
+                          If not specified, a port will be allocated (default: 30000-32767)
+                        format: int32
+                        type: integer
                     type: object
                 type: object
               mode:

cli/cmds/cluster/create.go

@@ -100,9 +100,7 @@ func createAction(config *CreateConfig) cli.ActionFunc {
 		cluster := newCluster(name, cmds.Namespace(), config)
 
 		cluster.Spec.Expose = &v1alpha1.ExposeConfig{
-			NodePort: &v1alpha1.NodePortConfig{
-				Enabled: true,
-			},
+			NodePort: &v1alpha1.NodePortConfig{},
 		}
 
 		// add Host IP address as an extra TLS-SAN to expose the k3k cluster

docs/crds/crd-docs.md

@@ -196,7 +196,9 @@ _Appears in:_
 
 | Field | Description | Default | Validation |
 | --- | --- | --- | --- |
-| `enabled` _boolean_ |  |  |  |
+| `serverPort` _integer_ | ServerPort is the port on each node on which the K3s server service is exposed when type is NodePort.<br />If not specified, a port will be allocated (default: 30000-32767) |  |  |
+| `servicePort` _integer_ | ServicePort is the port on each node on which the K3s service is exposed when type is NodePort.<br />If not specified, a port will be allocated (default: 30000-32767) |  |  |
+| `etcdPort` _integer_ | ETCDPort is the port on each node on which the ETCD service is exposed when type is NodePort.<br />If not specified, a port will be allocated (default: 30000-32767) |  |  |
 
 
 #### PersistenceConfig

pkg/apis/k3k.io/v1alpha1/types.go

@@ -160,7 +160,18 @@ type LoadBalancerConfig struct {
 }
 
 type NodePortConfig struct {
-	Enabled bool `json:"enabled"`
+	// ServerPort is the port on each node on which the K3s server service is exposed when type is NodePort.
+	// If not specified, a port will be allocated (default: 30000-32767)
+	// +optional
+	ServerPort *int32 `json:"serverPort,omitempty"`
+	// ServicePort is the port on each node on which the K3s service is exposed when type is NodePort.
+	// If not specified, a port will be allocated (default: 30000-32767)
+	// +optional
+	ServicePort *int32 `json:"servicePort,omitempty"`
+	// ETCDPort is the port on each node on which the ETCD service is exposed when type is NodePort.
+	// If not specified, a port will be allocated (default: 30000-32767)
+	// +optional
+	ETCDPort *int32 `json:"etcdPort,omitempty"`
 }
 
 type ClusterStatus struct {

pkg/controller/cluster/cluster.go

@@ -180,12 +180,11 @@ func (c *ClusterReconciler) reconcileCluster(ctx context.Context, cluster *v1alp
 		cluster.Status.ServiceCIDR = defaultClusterServiceCIDR
 	}
 
-	log.Info("creating cluster service")
-
-	serviceIP, err := c.createClusterService(ctx, cluster, s)
+	service, err := c.ensureClusterService(ctx, cluster)
 	if err != nil {
 		return err
 	}
+	serviceIP := service.Spec.ClusterIP
 
 	if err := c.createClusterConfigs(ctx, cluster, s, serviceIP); err != nil {
 		return err
@@ -290,30 +289,36 @@ func (c *ClusterReconciler) createClusterConfigs(ctx context.Context, cluster *v
 	return nil
 }
 
-func (c *ClusterReconciler) createClusterService(ctx context.Context, cluster *v1alpha1.Cluster, s *server.Server) (string, error) {
-	// create cluster service
-	clusterService := s.Service(cluster)
+func (c *ClusterReconciler) ensureClusterService(ctx context.Context, cluster *v1alpha1.Cluster) (*v1.Service, error) {
+	log := ctrl.LoggerFrom(ctx)
+	log.Info("ensuring cluster service")
 
-	if err := controllerutil.SetControllerReference(cluster, clusterService, c.Scheme); err != nil {
-		return "", err
+	service := server.Service(cluster)
+
+	createdClusterService := &v1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      service.Name,
+			Namespace: service.Namespace,
+		},
 	}
-	if err := c.Client.Create(ctx, clusterService); err != nil {
-		if !apierrors.IsAlreadyExists(err) {
-			return "", err
+	result, err := controllerutil.CreateOrUpdate(ctx, c.Client, createdClusterService, func() error {
+		if err := controllerutil.SetControllerReference(cluster, createdClusterService, c.Scheme); err != nil {
+			return err
 		}
+
+		createdClusterService.Spec = service.Spec
+		return nil
+	})
+	if err != nil {
+		return nil, err
 	}
 
-	var service v1.Service
-
-	objKey := ctrlruntimeclient.ObjectKey{
-		Namespace: cluster.Namespace,
-		Name:      server.ServiceName(cluster.Name),
-	}
-	if err := c.Client.Get(ctx, objKey, &service); err != nil {
-		return "", err
+	key := client.ObjectKeyFromObject(createdClusterService)
+	if result != controllerutil.OperationResultNone {
+		log.Info("ensuring cluster service", "key", key, "result", result)
 	}
 
-	return service.Spec.ClusterIP, nil
+	return createdClusterService, nil
 }
 
 func (c *ClusterReconciler) server(ctx context.Context, cluster *v1alpha1.Cluster, server *server.Server) error {

pkg/controller/cluster/cluster_test.go

@@ -6,10 +6,12 @@ import (
 	"time"
 
 	"github.com/rancher/k3k/pkg/apis/k3k.io/v1alpha1"
+	"github.com/rancher/k3k/pkg/controller/cluster/server"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	corev1 "k8s.io/api/core/v1"
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/ptr"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -25,17 +27,19 @@ var _ = Describe("Cluster Controller", func() {
 		)
 
 		BeforeEach(func() {
-			createdNS := &corev1.Namespace{ObjectMeta: v1.ObjectMeta{GenerateName: "ns-"}}
+			createdNS := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{GenerateName: "ns-"}}
 			err := k8sClient.Create(context.Background(), createdNS)
 			Expect(err).To(Not(HaveOccurred()))
 			namespace = createdNS.Name
 		})
 
-		When("created with a default spec", func() {
+		When("creating a Cluster", func() {
 
-			It("should have been created with some defaults", func() {
-				cluster := &v1alpha1.Cluster{
-					ObjectMeta: v1.ObjectMeta{
+			var cluster *v1alpha1.Cluster
+
+			BeforeEach(func() {
+				cluster = &v1alpha1.Cluster{
+					ObjectMeta: metav1.ObjectMeta{
 						GenerateName: "clusterset-",
 						Namespace:    namespace,
 					},
@@ -43,7 +47,9 @@ var _ = Describe("Cluster Controller", func() {
 
 				err := k8sClient.Create(ctx, cluster)
 				Expect(err).To(Not(HaveOccurred()))
+			})
 
+			It("will be created with some defaults", func() {
 				Expect(cluster.Spec.Mode).To(Equal(v1alpha1.SharedClusterMode))
 				Expect(cluster.Spec.Agents).To(Equal(ptr.To[int32](0)))
 				Expect(cluster.Spec.Servers).To(Equal(ptr.To[int32](1)))
@@ -63,6 +69,63 @@ var _ = Describe("Cluster Controller", func() {
 					WithPolling(time.Second).
 					Should(Equal(expectedHostVersion))
 			})
+
+			When("exposing the cluster with nodePort and custom posrts", func() {
+				It("will have a NodePort service with the specified port exposed", func() {
+					cluster.Spec.Expose = &v1alpha1.ExposeConfig{
+						NodePort: &v1alpha1.NodePortConfig{
+							ServerPort:  ptr.To[int32](30010),
+							ServicePort: ptr.To[int32](30011),
+							ETCDPort:    ptr.To[int32](30012),
+						},
+					}
+
+					err := k8sClient.Update(ctx, cluster)
+					Expect(err).To(Not(HaveOccurred()))
+
+					var service v1.Service
+
+					Eventually(func() v1.ServiceType {
+						serviceKey := client.ObjectKey{
+							Name:      server.ServiceName(cluster.Name),
+							Namespace: cluster.Namespace,
+						}
+
+						err := k8sClient.Get(ctx, serviceKey, &service)
+						Expect(client.IgnoreNotFound(err)).To(Not(HaveOccurred()))
+						return service.Spec.Type
+					}).
+						WithTimeout(time.Second * 30).
+						WithPolling(time.Second).
+						Should(Equal(v1.ServiceTypeNodePort))
+
+					servicePorts := service.Spec.Ports
+					Expect(servicePorts).NotTo(BeEmpty())
+					Expect(servicePorts).To(HaveLen(3))
+
+					Expect(servicePorts).To(ContainElement(
+						And(
+							HaveField("Name", "k3s-server-port"),
+							HaveField("Port", BeEquivalentTo(6443)),
+							HaveField("NodePort", BeEquivalentTo(30010)),
+						),
+					))
+					Expect(servicePorts).To(ContainElement(
+						And(
+							HaveField("Name", "k3s-service-port"),
+							HaveField("Port", BeEquivalentTo(443)),
+							HaveField("NodePort", BeEquivalentTo(30011)),
+						),
+					))
+					Expect(servicePorts).To(ContainElement(
+						And(
+							HaveField("Name", "k3s-etcd-port"),
+							HaveField("Port", BeEquivalentTo(2379)),
+							HaveField("NodePort", BeEquivalentTo(30012)),
+						),
+					))
+				})
+			})
 		})
 	})
 })

pkg/controller/cluster/server/service.go

@@ -8,51 +8,69 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
-func (s *Server) Service(cluster *v1alpha1.Cluster) *v1.Service {
-	serviceType := v1.ServiceTypeClusterIP
-	if cluster.Spec.Expose != nil {
-		if cluster.Spec.Expose.NodePort != nil {
-			if cluster.Spec.Expose.NodePort.Enabled {
-				serviceType = v1.ServiceTypeNodePort
-			}
-		}
-	}
-
-	return &v1.Service{
+func Service(cluster *v1alpha1.Cluster) *v1.Service {
+	service := &v1.Service{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "Service",
 			APIVersion: "v1",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      ServiceName(s.cluster.Name),
+			Name:      ServiceName(cluster.Name),
 			Namespace: cluster.Namespace,
 		},
 		Spec: v1.ServiceSpec{
-			Type: serviceType,
+			Type: v1.ServiceTypeClusterIP,
 			Selector: map[string]string{
 				"cluster": cluster.Name,
 				"role":    "server",
 			},
-			Ports: []v1.ServicePort{
-				{
-					Name:     "k3s-server-port",
-					Protocol: v1.ProtocolTCP,
-					Port:     serverPort,
-				},
-				{
-					Name:       "k3s-service-port",
-					Protocol:   v1.ProtocolTCP,
-					Port:       servicePort,
-					TargetPort: intstr.FromInt(serverPort),
-				},
-				{
-					Name:     "k3s-etcd-port",
-					Protocol: v1.ProtocolTCP,
-					Port:     etcdPort,
-				},
-			},
 		},
 	}
+
+	k3sServerPort := v1.ServicePort{
+		Name:     "k3s-server-port",
+		Protocol: v1.ProtocolTCP,
+		Port:     serverPort,
+	}
+
+	k3sServicePort := v1.ServicePort{
+		Name:       "k3s-service-port",
+		Protocol:   v1.ProtocolTCP,
+		Port:       servicePort,
+		TargetPort: intstr.FromInt(serverPort),
+	}
+
+	etcdPort := v1.ServicePort{
+		Name:     "k3s-etcd-port",
+		Protocol: v1.ProtocolTCP,
+		Port:     etcdPort,
+	}
+
+	if cluster.Spec.Expose != nil {
+		nodePortConfig := cluster.Spec.Expose.NodePort
+		if nodePortConfig != nil {
+			service.Spec.Type = v1.ServiceTypeNodePort
+
+			if nodePortConfig.ServerPort != nil {
+				k3sServerPort.NodePort = *nodePortConfig.ServerPort
+			}
+			if nodePortConfig.ServicePort != nil {
+				k3sServicePort.NodePort = *nodePortConfig.ServicePort
+			}
+			if nodePortConfig.ETCDPort != nil {
+				etcdPort.NodePort = *nodePortConfig.ETCDPort
+			}
+		}
+	}
+
+	service.Spec.Ports = append(
+		service.Spec.Ports,
+		k3sServicePort,
+		etcdPort,
+		k3sServerPort,
+	)
+
+	return service
 }
 
 func (s *Server) StatefulServerService() *v1.Service {

tests/cluster_test.go

@@ -64,9 +64,7 @@ var _ = When("a cluster is installed", func() {
 			Spec: v1alpha1.ClusterSpec{
 				TLSSANs: []string{hostIP},
 				Expose: &v1alpha1.ExposeConfig{
-					NodePort: &v1alpha1.NodePortConfig{
-						Enabled: true,
-					},
+					NodePort: &v1alpha1.NodePortConfig{},
 				},
 			},
 		}
@@ -130,9 +128,7 @@ var _ = When("a cluster is installed", func() {
 			Spec: v1alpha1.ClusterSpec{
 				TLSSANs: []string{hostIP},
 				Expose: &v1alpha1.ExposeConfig{
-					NodePort: &v1alpha1.NodePortConfig{
-						Enabled: true,
-					},
+					NodePort: &v1alpha1.NodePortConfig{},
 				},
 			},
 		}
@@ -154,7 +150,6 @@ var _ = When("a cluster is installed", func() {
 		serverPod := serverPods.Items[0]
 
 		fmt.Fprintf(GinkgoWriter, "deleting pod %s/%s\n", serverPod.Namespace, serverPod.Name)
-		// GracePeriodSeconds: ptr.To[int64](0)
 		err = k8s.CoreV1().Pods(namespace).Delete(ctx, serverPod.Name, v1.DeleteOptions{})
 		Expect(err).To(Not(HaveOccurred()))