Use identifying labels for lookup of EPS on service EPS update

...instead of all labels otherwise, on update to the service labels,
the existing EPS is not found resulting in a new one created.

Related to #1646

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>

go.mod

@@ -8,7 +8,7 @@ require (
 	github.com/onsi/gomega v1.34.2
 	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.20.4
-	github.com/submariner-io/admiral v0.19.0-rc1
+	github.com/submariner-io/admiral v0.19.0-rc1.0.20241007115040-cd5bf4d54261
 	github.com/submariner-io/shipyard v0.19.0-rc1
 	k8s.io/api v0.31.1
 	k8s.io/apimachinery v0.31.1

go.sum

@@ -389,8 +389,8 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO
 github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
 github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
 github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
-github.com/submariner-io/admiral v0.19.0-rc1 h1:UBYQmbnsaICgs6Qyid5egN8uc7L4Ql0DTBx3sb4vihA=
-github.com/submariner-io/admiral v0.19.0-rc1/go.mod h1:2bmvHxnJ0piOMriKolIJ4/o5negeZ7ge/NIEeARXNS8=
+github.com/submariner-io/admiral v0.19.0-rc1.0.20241007115040-cd5bf4d54261 h1:b39I5740FgONLFymnRR36wtvjqXnwzC85A/Bwaw3dgw=
+github.com/submariner-io/admiral v0.19.0-rc1.0.20241007115040-cd5bf4d54261/go.mod h1:2bmvHxnJ0piOMriKolIJ4/o5negeZ7ge/NIEeARXNS8=
 github.com/submariner-io/shipyard v0.19.0-rc1 h1:KaQnYlGLZjUFAepgpHW0pmM9J74sjrOl5A1hsdLtWFQ=
 github.com/submariner-io/shipyard v0.19.0-rc1/go.mod h1:HrnoErg/rM3r+QjUePY0x/SZi1xaeyjuQ+JTEc1bkMM=
 github.com/tidwall/pretty v1.0.0/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk=

pkg/agent/controller/clusterip_service_test.go

@@ -198,6 +198,41 @@ func testClusterIPServiceInOneCluster() {
 		})
 	})
 
+	When("the labels for an exported service are updated", func() {
+		JustBeforeEach(func() {
+			t.cluster1.createService()
+			t.cluster1.createServiceExport()
+			t.awaitNonHeadlessServiceExported(&t.cluster1)
+		})
+
+		It("should update the existing EndpointSlice labels", func() {
+			existingEPS := findEndpointSlices(t.cluster1.localEndpointSliceClient, t.cluster1.service.Namespace,
+				t.cluster1.service.Name, t.cluster1.clusterID)[0]
+
+			By("Updating service labels")
+
+			newLabelName := "new-label"
+			newLabelValue := "new-value"
+
+			t.cluster1.service.Labels[newLabelName] = newLabelValue
+			t.cluster1.serviceEndpointSlices[0].Labels[newLabelName] = newLabelValue
+
+			t.cluster1.updateServiceEndpointSlices()
+
+			Eventually(func() map[string]string {
+				eps, err := t.cluster1.localEndpointSliceClient.Get(context.TODO(), existingEPS.Name, metav1.GetOptions{})
+				Expect(err).To(Succeed())
+
+				return eps.GetLabels()
+			}).Should(HaveKeyWithValue(newLabelName, newLabelValue))
+
+			newSlices := findEndpointSlices(t.cluster1.localEndpointSliceClient, t.cluster1.service.Namespace,
+				t.cluster1.service.Name, t.cluster1.clusterID)
+			Expect(newSlices).To(HaveLen(1))
+			Expect(newSlices[0].Name).To(Equal(existingEPS.Name))
+		})
+	})
+
 	When("the session affinity is configured for an exported service", func() {
 		BeforeEach(func() {
 			t.cluster1.service.Spec.SessionAffinity = corev1.ServiceAffinityClientIP

pkg/agent/controller/service_endpoint_slices.go

@@ -28,7 +28,9 @@ import (
 	"github.com/pkg/errors"
 	"github.com/submariner-io/admiral/pkg/federate"
 	"github.com/submariner-io/admiral/pkg/log"
+	"github.com/submariner-io/admiral/pkg/resource"
 	"github.com/submariner-io/admiral/pkg/syncer"
+	"github.com/submariner-io/admiral/pkg/util"
 	"github.com/submariner-io/lighthouse/pkg/constants"
 	discovery "k8s.io/api/discovery/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -176,29 +178,7 @@ func (c *ServiceEndpointSliceController) onServiceEndpointSlice(obj runtime.Obje
 		return nil, false
 	}
 
-	if op == syncer.Delete {
-		list, err := c.localClient.List(context.TODO(), metav1.ListOptions{
-			LabelSelector: k8slabels.SelectorFromSet(returnEPS.Labels).String(),
-		})
-		if err != nil {
-			logger.Error(err, "Error listing EndpointSlice resources for delete")
-			return nil, true
-		}
-
-		if len(list.Items) == 0 {
-			logger.V(log.DEBUG).Infof("Existing EndpointSlice not found with labels: %#v", returnEPS.Labels)
-			return nil, false
-		}
-
-		returnEPS.Name = list.Items[0].GetName()
-	}
-
-	name := returnEPS.Name
-	if name == "" {
-		name = returnEPS.GenerateName
-	}
-
-	logger.V(logLevel).Infof("Returning EndpointSlice %s/%s: %s", serviceEPS.Namespace, name,
+	logger.V(logLevel).Infof("Returning EndpointSlice %s/%s: %s", serviceEPS.Namespace, returnEPS.GenerateName,
 		endpointSliceStringer{returnEPS})
 
 	return returnEPS, false
@@ -346,12 +326,48 @@ func (c *ServiceEndpointSliceController) isHeadless() bool {
 }
 
 func (c *ServiceEndpointSliceController) Distribute(ctx context.Context, obj runtime.Object) error {
-	return c.federator.Distribute(ctx, obj) //nolint:wrapcheck // No need to wrap here
+	toDistribute := resource.MustToUnstructured(obj)
+	labels := toDistribute.GetLabels()
+
+	identifyingLabels := map[string]string{}
+	if c.isHeadless() {
+		identifyingLabels[constants.LabelSourceName] = labels[constants.LabelSourceName]
+	} else {
+		identifyingLabels[mcsv1a1.LabelServiceName] = labels[mcsv1a1.LabelServiceName]
+		identifyingLabels[constants.LabelSourceNamespace] = labels[constants.LabelSourceNamespace]
+		identifyingLabels[constants.MCSLabelSourceCluster] = labels[constants.MCSLabelSourceCluster]
+	}
+
+	_, _, err := util.CreateOrUpdateWithOptions[*unstructured.Unstructured](ctx, util.CreateOrUpdateOptions[*unstructured.Unstructured]{
+		Client: resource.ForDynamic(c.localClient),
+		Obj:    toDistribute,
+		MutateOnUpdate: func(obj *unstructured.Unstructured) (*unstructured.Unstructured, error) {
+			return util.CopyImmutableMetadata(obj, toDistribute), nil
+		},
+		IdentifyingLabels: identifyingLabels,
+	})
+
+	return err
 }
 
 func (c *ServiceEndpointSliceController) Delete(ctx context.Context, obj runtime.Object) error {
 	if c.isHeadless() {
-		return c.federator.Delete(ctx, obj) //nolint:wrapcheck // No need to wrap here
+		list, err := c.localClient.List(ctx, metav1.ListOptions{
+			LabelSelector: k8slabels.Set(map[string]string{
+				constants.LabelSourceName: resource.MustToMeta(obj).GetLabels()[constants.LabelSourceName],
+			}).String(),
+		})
+		if err != nil {
+			return errors.Wrap(err, "error listing EndpointSlice resources for delete")
+		}
+
+		if len(list.Items) == 0 {
+			logger.V(log.DEBUG).Infof("Existing EndpointSlice not found for service EPS %q",
+				resource.MustToMeta(obj).GetLabels()[constants.LabelSourceName])
+			return nil
+		}
+
+		return c.localClient.Delete(ctx, list.Items[0].GetName(), metav1.DeleteOptions{}) //nolint:wrapcheck // No need to wrap here
 	}
 
 	// For a non-headless service, we never delete the single exported EPS - we update its endpoint condition based on