From ed152f9daed5f119b1662c2017e59d4ba121189d Mon Sep 17 00:00:00 2001 From: Alan Amoyel Date: Fri, 21 Apr 2023 11:15:59 +0200 Subject: [PATCH 1/4] feat: add condition to controlPlaneServiceTemplate in KubevirtCluster CRD to deploy LB service in namespace provided in metadata --- api/v1alpha1/kubevirtcluster_types.go | 3 ++- controllers/kubevirtcluster_controller.go | 12 +++++++++- .../kubevirtcluster_controller_test.go | 23 +++++++++++++++++++ pkg/testing/common.go | 23 +++++++++++++++++++ 4 files changed, 59 insertions(+), 2 deletions(-) diff --git a/api/v1alpha1/kubevirtcluster_types.go b/api/v1alpha1/kubevirtcluster_types.go index c07f0701..e327b530 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/controllers/kubevirtcluster_controller.go b/controllers/kubevirtcluster_controller.go index 632f1212..d2b59132 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") } diff --git a/controllers/kubevirtcluster_controller_test.go b/controllers/kubevirtcluster_controller_test.go index 11b5066f..a16dcc0d 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 572cc623..fe53b138 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{}, From 38960ce2f8ce757baf8bd43c64ee3a33c9af08a1 Mon Sep 17 00:00:00 2001 From: Alan Amoyel Date: Tue, 25 Apr 2023 18:51:38 +0200 Subject: [PATCH 2/4] condition to use controlPlaneEndpoint values --- controllers/kubevirtcluster_controller.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/controllers/kubevirtcluster_controller.go b/controllers/kubevirtcluster_controller.go index d2b59132..a4c43885 100644 --- a/controllers/kubevirtcluster_controller.go +++ b/controllers/kubevirtcluster_controller.go @@ -164,8 +164,14 @@ func (r *KubevirtClusterReconciler) reconcileNormal(ctx *context.ClusterContext, } } + // Get the ControlPlane Host and Port manually set by the user if existing + if ctx.KubevirtCluster.Spec.ControlPlaneEndpoint.Host != "" { + ctx.KubevirtCluster.Spec.ControlPlaneEndpoint = infrav1.APIEndpoint{ + Host: ctx.KubevirtCluster.Spec.ControlPlaneEndpoint.Host, + Port: ctx.KubevirtCluster.Spec.ControlPlaneEndpoint.Port, + } // Get LoadBalancer ExternalIP if cluster Service Type is LoadBalancer - if ctx.KubevirtCluster.Spec.ControlPlaneServiceTemplate.Spec.Type == "LoadBalancer" { + } else if ctx.KubevirtCluster.Spec.ControlPlaneServiceTemplate.Spec.Type == "LoadBalancer" { lbip4, err := externalLoadBalancer.ExternalIP(ctx) if err != nil { conditions.MarkFalse(ctx.KubevirtCluster, infrav1.LoadBalancerAvailableCondition, infrav1.LoadBalancerProvisioningFailedReason, clusterv1.ConditionSeverityWarning, err.Error()) From 285f1e6e9b15a925f74d541e914fd462d85b3fb6 Mon Sep 17 00:00:00 2001 From: Alan Amoyel Date: Tue, 25 Apr 2023 18:52:55 +0200 Subject: [PATCH 3/4] Revert "feat: add condition to controlPlaneServiceTemplate in KubevirtCluster CRD to deploy LB service in namespace provided in metadata" This reverts commit ed152f9daed5f119b1662c2017e59d4ba121189d. --- api/v1alpha1/kubevirtcluster_types.go | 3 +-- controllers/kubevirtcluster_controller.go | 12 +--------- .../kubevirtcluster_controller_test.go | 23 ------------------- pkg/testing/common.go | 23 ------------------- 4 files changed, 2 insertions(+), 59 deletions(-) diff --git a/api/v1alpha1/kubevirtcluster_types.go b/api/v1alpha1/kubevirtcluster_types.go index e327b530..c07f0701 100644 --- a/api/v1alpha1/kubevirtcluster_types.go +++ b/api/v1alpha1/kubevirtcluster_types.go @@ -103,8 +103,7 @@ type SSHKeys struct { // ControlPlaneServiceTemplate describes the template for the control plane service. type ControlPlaneServiceTemplate struct { - // 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. + // Service metadata allows to set labels and annotations for the service. // This field is optional. // +kubebuilder:pruning:PreserveUnknownFields // +nullable diff --git a/controllers/kubevirtcluster_controller.go b/controllers/kubevirtcluster_controller.go index a4c43885..10ba5f3a 100644 --- a/controllers/kubevirtcluster_controller.go +++ b/controllers/kubevirtcluster_controller.go @@ -54,14 +54,6 @@ 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 @@ -115,10 +107,8 @@ 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, loadBalancerNamespace) + externalLoadBalancer, err := loadbalancer.NewLoadBalancer(clusterContext, infraClusterClient, infraClusterNamespace) 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 index a16dcc0d..11b5066f 100644 --- a/controllers/kubevirtcluster_controller_test.go +++ b/controllers/kubevirtcluster_controller_test.go @@ -25,7 +25,6 @@ var ( clusterName string kubevirtClusterName string kubevirtCluster *infrav1.KubevirtCluster - kubeconfigNamespace string cluster *clusterv1.Cluster fakeClient client.Client kubevirtClusterReconciler controllers.KubevirtClusterReconciler @@ -176,26 +175,4 @@ 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 fe53b138..572cc623 100644 --- a/pkg/testing/common.go +++ b/pkg/testing/common.go @@ -46,29 +46,6 @@ 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{}, From 3d35fd1db337d20d2ec7bd6750f74b53d8df6194 Mon Sep 17 00:00:00 2001 From: Alan Amoyel Date: Tue, 25 Apr 2023 18:58:11 +0200 Subject: [PATCH 4/4] feat: add condition to controlPlaneServiceTemplate in KubevirtCluster CRD to deploy LB service in namespace provided in metadata --- api/v1alpha1/kubevirtcluster_types.go | 3 ++- ...ure.cluster.x-k8s.io_kubevirtclusters.yaml | 6 +++-- controllers/kubevirtcluster_controller.go | 20 +++++++++------- .../kubevirtcluster_controller_test.go | 23 +++++++++++++++++++ pkg/testing/common.go | 23 +++++++++++++++++++ 5 files changed, 64 insertions(+), 11 deletions(-) diff --git a/api/v1alpha1/kubevirtcluster_types.go b/api/v1alpha1/kubevirtcluster_types.go index c07f0701..e327b530 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 acde4cfd..532e0eb4 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 10ba5f3a..d2b59132 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") } @@ -154,14 +164,8 @@ func (r *KubevirtClusterReconciler) reconcileNormal(ctx *context.ClusterContext, } } - // Get the ControlPlane Host and Port manually set by the user if existing - if ctx.KubevirtCluster.Spec.ControlPlaneEndpoint.Host != "" { - ctx.KubevirtCluster.Spec.ControlPlaneEndpoint = infrav1.APIEndpoint{ - Host: ctx.KubevirtCluster.Spec.ControlPlaneEndpoint.Host, - Port: ctx.KubevirtCluster.Spec.ControlPlaneEndpoint.Port, - } // Get LoadBalancer ExternalIP if cluster Service Type is LoadBalancer - } else if ctx.KubevirtCluster.Spec.ControlPlaneServiceTemplate.Spec.Type == "LoadBalancer" { + if ctx.KubevirtCluster.Spec.ControlPlaneServiceTemplate.Spec.Type == "LoadBalancer" { lbip4, err := externalLoadBalancer.ExternalIP(ctx) if err != nil { conditions.MarkFalse(ctx.KubevirtCluster, infrav1.LoadBalancerAvailableCondition, infrav1.LoadBalancerProvisioningFailedReason, clusterv1.ConditionSeverityWarning, err.Error()) diff --git a/controllers/kubevirtcluster_controller_test.go b/controllers/kubevirtcluster_controller_test.go index 11b5066f..a16dcc0d 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 572cc623..fe53b138 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{},