From 7019732f0838d18d40d665056c4b4015dfa73761 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Tue, 20 Aug 2024 11:40:16 -0400 Subject: [PATCH] Propagate exported Service session affinity to ServiceImport ...to align with the MCS spec. If multiple clusters export a service, the fields are merged, ie for SessionAffinity, if any exported service has "ClientIP" then the ServiceImport will as well; for SessionAffinityConfig, the ServiceImport will inherit the first exported non-nil SessionAffinityConfig. Fixes https://github.com/submariner-io/lighthouse/issues/1610 Signed-off-by: Tom Pantelis --- pkg/agent/controller/agent.go | 3 ++- .../controller/clusterip_service_test.go | 25 +++++++++++++++-- pkg/agent/controller/controller_suite_test.go | 27 +++++++++++++------ pkg/agent/controller/service_import.go | 14 ++++++++-- 4 files changed, 56 insertions(+), 13 deletions(-) diff --git a/pkg/agent/controller/agent.go b/pkg/agent/controller/agent.go index 60bafd87..b8905180 100644 --- a/pkg/agent/controller/agent.go +++ b/pkg/agent/controller/agent.go @@ -254,7 +254,8 @@ func (a *Controller) serviceExportToServiceImport(obj runtime.Object, _ int, op serviceImport.Spec = mcsv1a1.ServiceImportSpec{ Ports: []mcsv1a1.ServicePort{}, Type: svcType, - SessionAffinityConfig: new(corev1.SessionAffinityConfig), + SessionAffinity: svc.Spec.SessionAffinity, + SessionAffinityConfig: svc.Spec.SessionAffinityConfig, } serviceImport.Status = mcsv1a1.ServiceImportStatus{ diff --git a/pkg/agent/controller/clusterip_service_test.go b/pkg/agent/controller/clusterip_service_test.go index 1fc9932a..bf7255b6 100644 --- a/pkg/agent/controller/clusterip_service_test.go +++ b/pkg/agent/controller/clusterip_service_test.go @@ -193,6 +193,21 @@ func testClusterIPServiceInOneCluster() { }) }) + When("the session affinity is configured for an exported service", func() { + BeforeEach(func() { + t.cluster1.service.Spec.SessionAffinity = corev1.ServiceAffinityClientIP + t.cluster1.service.Spec.SessionAffinityConfig = &corev1.SessionAffinityConfig{ + ClientIP: &corev1.ClientIPConfig{TimeoutSeconds: ptr.To(int32(10))}, + } + }) + + It("should be propagated to the ServiceImport", func() { + t.cluster1.createService() + t.cluster1.createServiceExport() + t.awaitNonHeadlessServiceExported(&t.cluster1) + }) + }) + When("two Services with the same name in different namespaces are exported", func() { It("should correctly export both services", func() { t.cluster1.createService() @@ -306,8 +321,9 @@ func testClusterIPServiceInOneCluster() { Namespace: t.cluster1.service.Namespace, }, Spec: mcsv1a1.ServiceImportSpec{ - Type: mcsv1a1.ClusterSetIP, - Ports: t.aggregatedServicePorts, + Type: mcsv1a1.ClusterSetIP, + Ports: t.aggregatedServicePorts, + SessionAffinity: corev1.ServiceAffinityNone, }, Status: mcsv1a1.ServiceImportStatus{ Clusters: []mcsv1a1.ClusterStatus{{Cluster: t.cluster1.clusterID}}, @@ -357,6 +373,11 @@ func testClusterIPServiceInTwoClusters() { BeforeEach(func() { t = newTestDiver() + + t.cluster2.service.Spec.SessionAffinity = corev1.ServiceAffinityClientIP + t.cluster2.service.Spec.SessionAffinityConfig = &corev1.SessionAffinityConfig{ + ClientIP: &corev1.ClientIPConfig{TimeoutSeconds: ptr.To(int32(10))}, + } }) JustBeforeEach(func() { diff --git a/pkg/agent/controller/controller_suite_test.go b/pkg/agent/controller/controller_suite_test.go index 93bdbe65..cdc151a9 100644 --- a/pkg/agent/controller/controller_suite_test.go +++ b/pkg/agent/controller/controller_suite_test.go @@ -181,9 +181,10 @@ func newTestDiver() *testDriver { }, }, Spec: corev1.ServiceSpec{ - ClusterIP: "10.253.9.1", - Selector: map[string]string{"app": "test"}, - Ports: []corev1.ServicePort{toServicePort(port1), toServicePort(port2)}, + ClusterIP: "10.253.9.1", + Selector: map[string]string{"app": "test"}, + Ports: []corev1.ServicePort{toServicePort(port1), toServicePort(port2)}, + SessionAffinity: corev1.ServiceAffinityNone, }, }, serviceExport: &mcsv1a1.ServiceExport{ @@ -259,9 +260,10 @@ func newTestDiver() *testDriver { Namespace: serviceNamespace, }, Spec: corev1.ServiceSpec{ - ClusterIP: "10.253.10.1", - Selector: map[string]string{"app": "test"}, - Ports: []corev1.ServicePort{toServicePort(port1), toServicePort(port2)}, + ClusterIP: "10.253.10.1", + Selector: map[string]string{"app": "test"}, + Ports: []corev1.ServicePort{toServicePort(port1), toServicePort(port2)}, + SessionAffinity: corev1.ServiceAffinityNone, }, }, serviceExport: &mcsv1a1.ServiceExport{ @@ -783,8 +785,9 @@ func (t *testDriver) awaitAggregatedServiceImport(sType mcsv1a1.ServiceImportTyp Namespace: test.RemoteNamespace, }, Spec: mcsv1a1.ServiceImportSpec{ - Type: sType, - Ports: []mcsv1a1.ServicePort{}, + Type: sType, + Ports: []mcsv1a1.ServicePort{}, + SessionAffinity: corev1.ServiceAffinityNone, }, } @@ -796,6 +799,14 @@ func (t *testDriver) awaitAggregatedServiceImport(sType mcsv1a1.ServiceImportTyp for _, c := range clusters { expServiceImport.Status.Clusters = append(expServiceImport.Status.Clusters, mcsv1a1.ClusterStatus{Cluster: c.clusterID}) + + if c.service.Spec.SessionAffinity != corev1.ServiceAffinityNone { + expServiceImport.Spec.SessionAffinity = c.service.Spec.SessionAffinity + } + + if c.service.Spec.SessionAffinityConfig != nil { + expServiceImport.Spec.SessionAffinityConfig = c.service.Spec.SessionAffinityConfig + } } } diff --git a/pkg/agent/controller/service_import.go b/pkg/agent/controller/service_import.go index ce639e70..bd11e81d 100644 --- a/pkg/agent/controller/service_import.go +++ b/pkg/agent/controller/service_import.go @@ -300,8 +300,10 @@ func (c *ServiceImportController) Distribute(ctx context.Context, obj runtime.Ob }, }, Spec: mcsv1a1.ServiceImportSpec{ - Type: localServiceImport.Spec.Type, - Ports: []mcsv1a1.ServicePort{}, + Type: localServiceImport.Spec.Type, + Ports: []mcsv1a1.ServicePort{}, + SessionAffinity: localServiceImport.Spec.SessionAffinity, + SessionAffinityConfig: localServiceImport.Spec.SessionAffinityConfig, }, Status: mcsv1a1.ServiceImportStatus{ Clusters: []mcsv1a1.ClusterStatus{ @@ -339,6 +341,14 @@ func (c *ServiceImportController) Distribute(ctx context.Context, obj runtime.Ob c.serviceExportClient.removeStatusCondition(ctx, serviceName, serviceNamespace, mcsv1a1.ServiceExportConflict, typeConflictReason) + if existing.Spec.SessionAffinity == "" || existing.Spec.SessionAffinity == corev1.ServiceAffinityNone { + existing.Spec.SessionAffinity = localServiceImport.Spec.SessionAffinity + } + + if existing.Spec.SessionAffinityConfig == nil { + existing.Spec.SessionAffinityConfig = localServiceImport.Spec.SessionAffinityConfig + } + var added bool existing.Status.Clusters, added = slices.AppendIfNotPresent(existing.Status.Clusters,