From 3513f20eeb998177720d25e346e822ec5f2efc70 Mon Sep 17 00:00:00 2001
From: Aswin Suryanarayanan <asuryana@redhat.com>
Date: Thu, 19 Dec 2024 11:38:10 -0500
Subject: [PATCH] Change loadbalancer type for HCP deployments
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The Kubevirt(HCP) clusters do not have their loadbalancer installed and rely
on the host loadblancer( like netallb) when a loadbalancer is created.So when
the Submariner exposes the submariner-gateway loadbalancer service with
ServiceExternalTrafficPolicy locally does not work, as the traffic from
the remotecluster first lands on the host cluster. Hence the traffic policy
need to be updated to cluster in these deployments.

A new parameter hostedCluster is added to submariner config, if set the
traffic policy will be set to cluster

Signed-off-by: Aswin Suryanarayanan <asuryana@redhat.com>
---
 api/v1alpha1/submariner_types.go                    | 13 +++++++++++--
 config/crd/bases/submariner.io_submariners.yaml     |  2 ++
 controllers/submariner/loadbalancer_resources.go    |  8 +++++++-
 controllers/submariner/submariner_controller.go     |  1 +
 .../submariner/submariner_controller_test.go        |  2 ++
 pkg/embeddedyamls/yamls.go                          |  6 ++++++
 6 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/api/v1alpha1/submariner_types.go b/api/v1alpha1/submariner_types.go
index 7dc385e15..8bc224353 100644
--- a/api/v1alpha1/submariner_types.go
+++ b/api/v1alpha1/submariner_types.go
@@ -160,10 +160,14 @@ type SubmarinerSpec struct {
 	// Enable NAT between clusters.
 	// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Enable NAT"
 	// +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:com.tectonic.ui:booleanSwitch"}
-	NatEnabled bool `json:"natEnabled"`
-
+	NatEnabled          bool `json:"natEnabled"`
 	AirGappedDeployment bool `json:"airGappedDeployment,omitempty"`
 
+	// Is the cluster a hosted cluster.
+	// +operator-sdk:csv:customresourcedefinitions:type=status,displayName="Hosted Cluster"
+	// +operator-sdk:csv:customresourcedefinitions:type=status,xDescriptors={"urn:alm:descriptor:com.tectonic.ui:text"}
+	HostedCluster bool `json:"hostedCluster,omitempty"`
+
 	// Enable automatic Load Balancer in front of the gateways.
 	// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Enable Load Balancer"
 	// +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:com.tectonic.ui:booleanSwitch"}
@@ -228,6 +232,11 @@ type SubmarinerStatus struct {
 
 	AirGappedDeployment bool `json:"airGappedDeployment,omitempty"`
 
+	// Is the cluster a hosted cluster.
+	// +operator-sdk:csv:customresourcedefinitions:type=status,displayName="Hosted Cluster"
+	// +operator-sdk:csv:customresourcedefinitions:type=status,xDescriptors={"urn:alm:descriptor:com.tectonic.ui:text"}
+	HostedCluster bool `json:"hostedCluster,omitempty"`
+
 	ColorCodes string `json:"colorCodes,omitempty"`
 
 	// The current cluster ID.
diff --git a/config/crd/bases/submariner.io_submariners.yaml b/config/crd/bases/submariner.io_submariners.yaml
index 89d0ce972..0393c0c40 100644
--- a/config/crd/bases/submariner.io_submariners.yaml
+++ b/config/crd/bases/submariner.io_submariners.yaml
@@ -137,6 +137,8 @@ spec:
               haltOnCertificateError:
                 description: Halt on certificate error (so the pod gets restarted).
                 type: boolean
+              hostedCluster:
+                type: boolean
               imageOverrides:
                 additionalProperties:
                   type: string
diff --git a/controllers/submariner/loadbalancer_resources.go b/controllers/submariner/loadbalancer_resources.go
index c29ca97ef..813aead60 100644
--- a/controllers/submariner/loadbalancer_resources.go
+++ b/controllers/submariner/loadbalancer_resources.go
@@ -91,6 +91,8 @@ func (r *Reconciler) getOCPPlatformType(ctx context.Context) (string, error) {
 }
 
 func newLoadBalancerService(instance *v1alpha1.Submariner, platformTypeOCP string) *corev1.Service {
+	externalTrafficPolicy := corev1.ServiceExternalTrafficPolicyTypeLocal
+
 	var svcAnnotations map[string]string
 
 	switch platformTypeOCP {
@@ -104,6 +106,10 @@ func newLoadBalancerService(instance *v1alpha1.Submariner, platformTypeOCP strin
 			"service.kubernetes.io/ibm-load-balancer-cloud-provider-ip-type":                   "public",
 			"service.kubernetes.io/ibm-load-balancer-cloud-provider-vpc-health-check-protocol": "http",
 		}
+	case string(configv1.KubevirtPlatformType):
+		if instance.Spec.HostedCluster {
+			externalTrafficPolicy = corev1.ServiceExternalTrafficPolicyTypeCluster
+		}
 	default:
 		svcAnnotations = map[string]string{}
 	}
@@ -115,7 +121,7 @@ func newLoadBalancerService(instance *v1alpha1.Submariner, platformTypeOCP strin
 			Annotations: svcAnnotations,
 		},
 		Spec: corev1.ServiceSpec{
-			ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeLocal,
+			ExternalTrafficPolicy: externalTrafficPolicy,
 			Type:                  corev1.ServiceTypeLoadBalancer,
 			Selector: map[string]string{
 				// Traffic is directed to the active gateway
diff --git a/controllers/submariner/submariner_controller.go b/controllers/submariner/submariner_controller.go
index c50c69576..e341d9452 100644
--- a/controllers/submariner/submariner_controller.go
+++ b/controllers/submariner/submariner_controller.go
@@ -222,6 +222,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
 	instance.Status.Version = instance.Spec.Version
 	instance.Status.NatEnabled = instance.Spec.NatEnabled
 	instance.Status.AirGappedDeployment = instance.Spec.AirGappedDeployment
+	instance.Status.HostedCluster = instance.Spec.HostedCluster
 	instance.Status.ColorCodes = instance.Spec.ColorCodes
 	instance.Status.ClusterID = instance.Spec.ClusterID
 	instance.Status.GlobalCIDR = instance.Spec.GlobalCIDR
diff --git a/controllers/submariner/submariner_controller_test.go b/controllers/submariner/submariner_controller_test.go
index ccb45f641..4707d441a 100644
--- a/controllers/submariner/submariner_controller_test.go
+++ b/controllers/submariner/submariner_controller_test.go
@@ -78,6 +78,7 @@ func testReconciliation() {
 		BeforeEach(func() {
 			t.submariner.Spec.NatEnabled = true
 			t.submariner.Spec.AirGappedDeployment = true
+			t.submariner.Spec.HostedCluster = true
 		})
 
 		It("should populate general Submariner resource Status fields from the Spec", func(ctx SpecContext) {
@@ -86,6 +87,7 @@ func testReconciliation() {
 			updated := t.getSubmariner(ctx)
 			Expect(updated.Status.NatEnabled).To(BeTrue())
 			Expect(updated.Status.AirGappedDeployment).To(BeTrue())
+			Expect(updated.Status.HostedCluster).To(BeTrue())
 			Expect(updated.Status.ClusterID).To(Equal(t.submariner.Spec.ClusterID))
 			Expect(updated.Status.GlobalCIDR).To(Equal(t.submariner.Spec.GlobalCIDR))
 			Expect(updated.Status.NetworkPlugin).To(Equal(t.clusterNetwork.NetworkPlugin))
diff --git a/pkg/embeddedyamls/yamls.go b/pkg/embeddedyamls/yamls.go
index d8c3679dd..a1be0aaf0 100644
--- a/pkg/embeddedyamls/yamls.go
+++ b/pkg/embeddedyamls/yamls.go
@@ -248,6 +248,9 @@ spec:
               haltOnCertificateError:
                 description: Halt on certificate error (so the pod gets restarted).
                 type: boolean
+              hostedCluster:
+                description: Is the cluster a hosted cluster.
+                type: boolean
               imageOverrides:
                 additionalProperties:
                   type: string
@@ -840,6 +843,9 @@ spec:
                 required:
                 - mismatchedContainerImages
                 type: object
+              hostedCluster:
+                description: Is the cluster a hosted cluster.
+                type: boolean
               loadBalancerStatus:
                 description: The status of the load balancer DaemonSet.
                 properties: