Repository: kubernetes-sigs/cluster-api-provider-kubevirt (go)

Issue:
Cannot change LB service namespace when infraClusterSecretRef is used (kubeconfig by default)
**What steps did you take and what happened:**
When we use a "remote" cluster-api cluster to bootstrap virtual machines on an external kubevirt cluster, we currently used infraClusterSecretRef to provide a kubeconfig file to the controller.
Actually, we can bootstrap clusters VMs in specific namespaces but LoadBalancer, NodePort or ClusterIP services are currently created in the namespace specified in the kubeconfig (or default if not set). With this implementation, kubeconfig are not reusable for multiple clusters in multiple namespaces and Services cannot make calls to control-plane api-server pods from default namespace.

**What did you expect to happen:**
Like labels and annotations in controlPlaneServiceTemplate.metadata Object, i think it would be great to use the namespace provided in metadata to create the service in a selected namespace.

Here is the CRD currently maintained:
![image](https://user-images.githubusercontent.com/73712310/214596915-ac54e9a8-aa3d-4baf-bc7f-5f57ac5eb6bc.png)



**Environment:**

- Cluster-api version: v1.3.2
- Cluster-api-provider-kubevirt version: v0.1.4
- Kubernetes version: (use `kubectl version`): v1.26.1
- KubeVirt version: v0.58.0
- OS (e.g. from `/etc/os-release`): Ubuntu 22.04.1

/kind bug

Pull request:
feat: add condition to controlPlaneServiceTemplate in KubevirtCluster…
Fix #219 

Patch:
diff --git a/api/v1alpha1/kubevirtcluster_types.go b/api/v1alpha1/kubevirtcluster_types.go
index c07f070..e327b53 100644
--- 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
index acde4cf..532e0eb 100644
--- 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
index 632f121..d2b5913 100644
--- 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")
 	}


Test patch:
diff --git a/controllers/kubevirtcluster_controller_test.go b/controllers/kubevirtcluster_controller_test.go
index 11b5066..a16dcc0 100644
--- 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
index 572cc62..fe53b13 100644
--- 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{},