Merge pull request #235 from aamoyel/main

feat: add condition to controlPlaneServiceTemplate in KubevirtCluster…
kubernetes-sigs · May 16, 2023 · 1b99674 · 1b99674
2 parents f42af14 + 3d35fd1
commit 1b99674
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 4 deletions.
diff --git a/api/v1alpha1/kubevirtcluster_types.go b/api/v1alpha1/kubevirtcluster_types.go
@@ -103,7 +103,8 @@ type SSHKeys struct {
 
 // ControlPlaneServiceTemplate describes the template for the control plane service.
 type ControlPlaneServiceTemplate struct {
-	// Service metadata allows to set labels and annotations for the service.
+	// Service metadata allows to set labels, annotations and namespace for the service.
+	// When infraClusterSecretRef is used, ControlPlaneService take the kubeconfig namespace by default if metadata.namespace is not specified.
 	// This field is optional.
 	// +kubebuilder:pruning:PreserveUnknownFields
 	// +nullable

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_kubevirtclusters.yaml
@@ -62,8 +62,10 @@ spec:
                   to modify the service metadata and the service type.
                 properties:
                   metadata:
-                    description: Service metadata allows to set labels and annotations
-                      for the service. This field is optional.
+                    description: Service metadata allows to set labels, annotations
+                      and namespace for the service. When infraClusterSecretRef is
+                      used, ControlPlaneService take the kubeconfig namespace by default
+                      if metadata.namespace is not specified. This field is optional.
                     nullable: true
                     type: object
                     x-kubernetes-preserve-unknown-fields: true

diff --git a/controllers/kubevirtcluster_controller.go b/controllers/kubevirtcluster_controller.go
@@ -54,6 +54,14 @@ type KubevirtClusterReconciler struct {
 	Log          logr.Logger
 }
 
+func GetLoadBalancerNamespace(kc *infrav1.KubevirtCluster, infraClusterNamespace string ) string {
+	// Use namespace specified in Service Template if exist
+	if kc.Spec.ControlPlaneServiceTemplate.ObjectMeta.Namespace != "" {
+		return kc.Spec.ControlPlaneServiceTemplate.ObjectMeta.Namespace
+	}
+	return infraClusterNamespace
+}
+
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=kubevirtclusters,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=kubevirtclusters/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups="",resources=services;,verbs=get;list;watch;create;update;patch;delete
@@ -107,8 +115,10 @@ func (r *KubevirtClusterReconciler) Reconcile(goctx gocontext.Context, req ctrl.
 		return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
 	}
 
+	loadBalancerNamespace := GetLoadBalancerNamespace(kubevirtCluster, infraClusterNamespace)
+
 	// Create a helper for managing a service hosting the load-balancer.
-	externalLoadBalancer, err := loadbalancer.NewLoadBalancer(clusterContext, infraClusterClient, infraClusterNamespace)
+	externalLoadBalancer, err := loadbalancer.NewLoadBalancer(clusterContext, infraClusterClient, loadBalancerNamespace)
 	if err != nil {
 		return ctrl.Result{}, errors.Wrapf(err, "failed to create helper for managing the externalLoadBalancer")
 	}

diff --git a/controllers/kubevirtcluster_controller_test.go b/controllers/kubevirtcluster_controller_test.go
@@ -25,6 +25,7 @@ var (
 	clusterName               string
 	kubevirtClusterName       string
 	kubevirtCluster           *infrav1.KubevirtCluster
+	kubeconfigNamespace       string
 	cluster                   *clusterv1.Cluster
 	fakeClient                client.Client
 	kubevirtClusterReconciler controllers.KubevirtClusterReconciler
@@ -175,4 +176,26 @@ var _ = Describe("Reconcile", func() {
 			Expect(err).Should(HaveOccurred())
 		})
 	})
+
+	Context("Compute Control Plane LB service namespace values precedence before it's created", func() {
+		BeforeEach(func() {
+			clusterName = "test-cluster"
+			kubevirtClusterName = "test-kubevirt-cluster"
+			kubeconfigNamespace = "kubeconfig-namespace"
+			cluster = testing.NewCluster(kubevirtClusterName, kubevirtCluster)
+		})
+
+		AfterEach(func() {})
+
+		It("should use provided LB namespace if its set", func() {
+			kubevirtCluster = testing.NewKubevirtClusterWithNamespacedLB(kubevirtClusterName, kubevirtClusterName, "lb-namespace")
+			ns := controllers.GetLoadBalancerNamespace(kubevirtCluster, kubeconfigNamespace)
+			Expect(ns).To(Equal("lb-namespace"))
+		})
+		It("should use kubeconfig namespace if LB namespace is not set", func() {
+			kubevirtCluster = testing.NewKubevirtClusterWithNamespacedLB(kubevirtClusterName, kubevirtClusterName, "")
+			ns := controllers.GetLoadBalancerNamespace(kubevirtCluster, kubeconfigNamespace)
+			Expect(ns).To(Equal(kubeconfigNamespace))
+		})
+	})
 })
diff --git a/pkg/testing/common.go b/pkg/testing/common.go
@@ -46,6 +46,29 @@ func NewKubevirtCluster(clusterName, kubevirtName string) *infrav1.KubevirtClust
 	}
 }
 
+func NewKubevirtClusterWithNamespacedLB(clusterName, kubevirtName string, lbNamespace string) *infrav1.KubevirtCluster {
+	return &infrav1.KubevirtCluster{
+		TypeMeta: metav1.TypeMeta{},
+		ObjectMeta: metav1.ObjectMeta{
+			Name: kubevirtName,
+			OwnerReferences: []metav1.OwnerReference{
+				{
+					APIVersion: clusterv1.GroupVersion.String(),
+					Kind:       "Cluster",
+					Name:       clusterName,
+				},
+			},
+		},
+		Spec: infrav1.KubevirtClusterSpec {
+			ControlPlaneServiceTemplate: infrav1.ControlPlaneServiceTemplate{
+				ObjectMeta: metav1.ObjectMeta {
+					Namespace: lbNamespace,
+				},
+			},
+		},
+	}
+}
+
 func NewKubevirtMachine(kubevirtMachineName, machineName string) *infrav1.KubevirtMachine {
 	return &infrav1.KubevirtMachine{
 		TypeMeta: metav1.TypeMeta{},