Fix invalid ServiceExport Conflict status condition

This occurs if there's more than one service exported in a cluster. The conflict checking was processing all local EndpointSlices instead of only those corresponding to the service in question. Signed-off-by: Tom Pantelis <[email protected]>
submariner-io · Nov 17, 2023 · 858ad36 · 858ad36
1 parent 847b34c
commit 858ad36
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 11 deletions.
diff --git a/pkg/agent/controller/clusterip_service_test.go b/pkg/agent/controller/clusterip_service_test.go
@@ -271,6 +271,9 @@ func testClusterIPServiceInOneCluster() {
 
 			// Ensure the resources for the first Service weren't overwritten
 			t.awaitAggregatedServiceImport(mcsv1a1.ClusterSetIP, t.cluster1.service.Name, t.cluster1.service.Namespace, &t.cluster1)
+
+			t.cluster1.ensureNoServiceExportCondition(mcsv1a1.ServiceExportConflict)
+			t.cluster1.ensureNoServiceExportCondition(mcsv1a1.ServiceExportConflict, serviceExport)
 		})
 	})
 

diff --git a/pkg/agent/controller/controller_suite_test.go b/pkg/agent/controller/controller_suite_test.go
@@ -478,8 +478,9 @@ func (c *cluster) newGlobalIngressIP(name, ip string) *unstructured.Unstructured
 	return ingressIP
 }
 
-func (c *cluster) retrieveServiceExportCondition(condType mcsv1a1.ServiceExportConditionType) *mcsv1a1.ServiceExportCondition {
-	obj, err := c.localServiceExportClient.Get(context.TODO(), c.serviceExport.Name, metav1.GetOptions{})
+func (c *cluster) retrieveServiceExportCondition(se *mcsv1a1.ServiceExport, condType mcsv1a1.ServiceExportConditionType,
+) *mcsv1a1.ServiceExportCondition {
+	obj, err := serviceExportClientFor(c.localDynClient, se.Namespace).Get(context.TODO(), se.Name, metav1.GetOptions{})
 	Expect(err).To(Succeed())
 
 	return controller.FindServiceExportStatusCondition(toServiceExport(obj).Status.Conditions, condType)
@@ -571,16 +572,28 @@ func (c *cluster) ensureLastServiceExportCondition(expected *mcsv1a1.ServiceExpo
 		resource.ToJSON(expected)))
 }
 
-func (c *cluster) ensureNoServiceExportCondition(condType mcsv1a1.ServiceExportConditionType) {
-	Consistently(func() interface{} {
-		return c.retrieveServiceExportCondition(condType)
-	}).Should(BeNil(), "Unexpected ServiceExport status condition")
+func (c *cluster) ensureNoServiceExportCondition(condType mcsv1a1.ServiceExportConditionType, serviceExports ...*mcsv1a1.ServiceExport) {
+	if len(serviceExports) == 0 {
+		serviceExports = []*mcsv1a1.ServiceExport{c.serviceExport}
+	}
+
+	for _, se := range serviceExports {
+		Consistently(func() interface{} {
+			return c.retrieveServiceExportCondition(se, condType)
+		}).Should(BeNil(), "Unexpected ServiceExport status condition")
+	}
 }
 
-func (c *cluster) awaitNoServiceExportCondition(condType mcsv1a1.ServiceExportConditionType) {
-	Eventually(func() interface{} {
-		return c.retrieveServiceExportCondition(condType)
-	}).Should(BeNil(), "Unexpected ServiceExport status condition")
+func (c *cluster) awaitNoServiceExportCondition(condType mcsv1a1.ServiceExportConditionType, serviceExports ...*mcsv1a1.ServiceExport) {
+	if len(serviceExports) == 0 {
+		serviceExports = []*mcsv1a1.ServiceExport{c.serviceExport}
+	}
+
+	for _, se := range serviceExports {
+		Eventually(func() interface{} {
+			return c.retrieveServiceExportCondition(se, condType)
+		}).Should(BeNil(), "Unexpected ServiceExport status condition")
+	}
 }
 
 func (c *cluster) awaitServiceUnavailableStatus() {

diff --git a/pkg/agent/controller/endpoint_slice.go b/pkg/agent/controller/endpoint_slice.go
@@ -237,7 +237,10 @@ func (c *EndpointSliceController) checkForConflicts(_, name, namespace string) (
 		return false, nil
 	}
 
-	epsList := c.syncer.ListLocalResources(&discovery.EndpointSlice{})
+	epsList := c.syncer.ListLocalResourcesBySelector(&discovery.EndpointSlice{}, k8slabels.SelectorFromSet(map[string]string{
+		constants.LabelSourceNamespace: namespace,
+		mcsv1a1.LabelServiceName:       name,
+	}))
 
 	var prevServicePorts []mcsv1a1.ServicePort
 	var intersectedServicePorts []mcsv1a1.ServicePort