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 <[email protected]>

go.mod

@@ -7,8 +7,8 @@ require (
 	github.com/onsi/ginkgo/v2 v2.20.2
 	github.com/onsi/gomega v1.34.2
 	github.com/pkg/errors v0.9.1
-	github.com/prometheus/client_golang v1.20.2
-	github.com/submariner-io/admiral v0.19.0-m3
+	github.com/prometheus/client_golang v1.20.4
+	github.com/submariner-io/admiral v0.19.0-m3.0.20241007113622-538061402b91
 	github.com/submariner-io/shipyard v0.19.0-m3
 	k8s.io/api v0.31.0
 	k8s.io/apimachinery v0.31.0

go.sum

@@ -328,8 +328,8 @@ github.com/pquerna/cachecontrol v0.0.0-20171018203845-0dec1b30a021/go.mod h1:prY
 github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw=
 github.com/prometheus/client_golang v0.9.3/go.mod h1:/TN21ttK/J9q6uSwhBd54HahCDft0ttaMvbicHlPoso=
 github.com/prometheus/client_golang v1.0.0/go.mod h1:db9x61etRT2tGnBNRi70OPL5FsnadC4Ky3P0J6CfImo=
-github.com/prometheus/client_golang v1.20.2 h1:5ctymQzZlyOON1666svgwn3s6IKWgfbjsejTMiXIyjg=
-github.com/prometheus/client_golang v1.20.2/go.mod h1:PIEt8X02hGcP8JWbeHyeZ53Y/jReSnHgO035n//V5WE=
+github.com/prometheus/client_golang v1.20.4 h1:Tgh3Yr67PaOv/uTqloMsCEdeuFTatm5zIq5+qNN23vI=
+github.com/prometheus/client_golang v1.20.4/go.mod h1:PIEt8X02hGcP8JWbeHyeZ53Y/jReSnHgO035n//V5WE=
 github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910/go.mod h1:MbSGuTsp3dbXC40dX6PRTWyKYBIrTGTE9sqQNg2J8bo=
 github.com/prometheus/client_model v0.0.0-20190129233127-fd36f4220a90/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
 github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
@@ -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-m3 h1:LTkYxCvB8S1210P2FZtCb6dzjaPpIgBrRQxZkH/snDo=
-github.com/submariner-io/admiral v0.19.0-m3/go.mod h1:xRpP1rDOblEdPHr0qrC+plcTNfShYJAOH2fexqOmI1A=
+github.com/submariner-io/admiral v0.19.0-m3.0.20241007113622-538061402b91 h1:DEYBbJPyE9xrokFDNoBe5heFPxsBiM4fi+xEnynUkg0=
+github.com/submariner-io/admiral v0.19.0-m3.0.20241007113622-538061402b91/go.mod h1:Mm9PhtbIvai0Tl+8yzrItzoTJoWpvYKT2t6cIXLNvsU=
 github.com/submariner-io/shipyard v0.19.0-m3 h1:NliwAktRPF4OsLj1TDgpaOJD/bmmZW/FH9+mJmWgxbk=
 github.com/submariner-io/shipyard v0.19.0-m3/go.mod h1:BY1ceSnPz1/hN5F9uljcSzy5n5qgAOENsIvZpJ+XPOU=
 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