bugfix: propagate eds changes when annotations change. (#10411)

Author: yuval-k

changelog/v1.18.0-rc3/fix-eds-update-on-add-labels.yaml

@@ -0,0 +1,7 @@
+changelog:
+  - type: NON_USER_FACING
+    issueLink: https://github.com/solo-io/solo-projects/issues/7301
+    resolvesIssue: true
+    description: >-
+      Fixes a bug where EDS updates were not triggered when adding annotations to a service.
+      Non-user-facing as this was in unreleased version.

projects/gateway2/krtcollections/endpoints.go

@@ -136,12 +136,17 @@ type EndpointsForUpstream struct {
 func NewEndpointsForUpstream(us UpstreamWrapper, logger *zap.Logger) *EndpointsForUpstream {
 	// start with a hash of the cluster name. technically we dont need it for krt, as we can compare the upstream name. but it helps later
 	// to compute the hash we present envoy with.
+	// add the upstream hash to the clustername, so that if it changes the envoy cluster will become warm again.
+	clusterName := GetEndpointClusterName(us.Inner)
+
 	h := fnv.New64()
 	h.Write([]byte(us.Inner.GetMetadata().Ref().String()))
+	// As long as we hash the upstream in the cluster name (due to envoy cluster warming bug), we
+	// also need to include that in the hash
+	// see: https://github.com/envoyproxy/envoy/issues/13009
+	h.Write([]byte(clusterName))
 	upstreamHash := h.Sum64()
 
-	// add the upstream hash to the clustername, so that if it changes the envoy cluster will become warm again.
-	clusterName := GetEndpointClusterName(us.Inner)
 	return &EndpointsForUpstream{
 		LbEps:       make(map[PodLocality][]EndpointWithMd),
 		ClusterName: clusterName,

projects/gateway2/krtcollections/endpoints_test.go

@@ -240,6 +240,39 @@ func TestEndpointsForUpstreamWithDiscoveredUpstream(t *testing.T) {
 	g.Expect(h1).NotTo(Equal(h2), "not expected %v, got %v", h1, h2)
 }
 
+// Note: if we find out the envoy bug was fixed (where changed clusters don't warm until EDS updates)
+// this test can be removed.
+func TestEndpointsForUpstreamWhenUpstreamLabelsAdded(t *testing.T) {
+	g := gomega.NewWithT(t)
+
+	usGen := func() UpstreamWrapper {
+		return UpstreamWrapper{
+			Inner: &gloov1.Upstream{
+				Metadata: &core.Metadata{Name: "name", Namespace: "ns"},
+				UpstreamType: &gloov1.Upstream_Kube{
+					Kube: &kubernetes.UpstreamSpec{
+						ServiceName:      "svc",
+						ServiceNamespace: "ns",
+						ServicePort:      8080,
+					},
+				},
+			},
+		}
+	}
+	// 2 copies
+	us1 := usGen()
+	us2 := usGen()
+	us2.Inner.DiscoveryMetadata = &gloov1.DiscoveryMetadata{
+		Labels: map[string]string{
+			"extra": "label",
+		},
+	}
+	efu1 := NewEndpointsForUpstream(us1, nil)
+	efu2 := NewEndpointsForUpstream(us2, nil)
+
+	g.Expect(efu1.LbEpsEqualityHash).NotTo(Equal(efu2.LbEpsEqualityHash))
+}
+
 func TestEndpoints(t *testing.T) {
 	testCases := []struct {
 		name     string

projects/gateway2/proxy_syncer/cla.go

@@ -31,10 +31,10 @@ func (c EndpointResources) Equals(in EndpointResources) bool {
 
 // TODO: this is needed temporary while we don't have the per-upstream translation done.
 // once the plugins are fixed to support it, we can have the proxy translation skip upstreams/endpoints and remove this collection
-func newEnvoyEndpoints(glooEndpoints krt.Collection[krtcollections.EndpointsForUpstream]) krt.Collection[EndpointResources] {
+func newEnvoyEndpoints(glooEndpoints krt.Collection[krtcollections.EndpointsForUpstream], dbg *krt.DebugHandler) krt.Collection[EndpointResources] {
 	clas := krt.NewCollection(glooEndpoints, func(_ krt.HandlerContext, ep krtcollections.EndpointsForUpstream) *EndpointResources {
 		return TransformEndpointToResources(ep)
-	})
+	}, krt.WithDebugging(dbg), krt.WithName("EnvoyEndpoints"))
 	return clas
 }
 

projects/gateway2/proxy_syncer/proxy_syncer.go

@@ -406,7 +406,7 @@ func (s *ProxySyncer) Init(ctx context.Context, dbg *krt.DebugHandler) error {
 		s.k8sGwExtensions.KRTExtensions().Endpoints()...,
 	), withDebug, krt.WithName("EndpointIRs"))
 
-	clas := newEnvoyEndpoints(endpointIRs)
+	clas := newEnvoyEndpoints(endpointIRs, dbg)
 
 	kubeGateways := SetupCollectionDynamic[gwv1.Gateway](
 		ctx,

projects/gateway2/setup/ggv2setup_test.go

@@ -115,6 +115,8 @@ func init() {
 }
 
 func TestScenarios(t *testing.T) {
+	t.Skip("skipping test till we deflake it")
+
 	writer.set(t)
 	os.Setenv("POD_NAMESPACE", "gwtest")
 	testEnv := &envtest.Environment{

projects/gloo/pkg/translator/translator.go

@@ -415,6 +415,8 @@ func GetEndpointClusterName(clusterName string, upstream *v1.Upstream) (string,
 	if err != nil {
 		return "", err
 	}
+	//note: we add the upstream hash here because of
+	// https://github.com/envoyproxy/envoy/issues/13009
 	endpointClusterName := fmt.Sprintf("%s-%d", clusterName, hash)
 	return endpointClusterName, nil
 }