Discovery resource version err (#10118)

Co-authored-by: changelog-bot <changelog-bot>
Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>

changelog/v1.14.32/discovery-watchlabels-bugfix.yaml

@@ -0,0 +1,7 @@
+changelog:
+  - type: FIX
+    issueLink: https://github.com/solo-io/gloo/issues/8635
+    description: >
+      Fix a bug that caused discovered Upstreams to not reflect the updated state of parent Services discovered using
+      watchLabels
+    resolvesIssue: false

projects/gloo/pkg/discovery/discovery.go

@@ -44,7 +44,6 @@ type UpstreamDiscovery struct {
 	discoveryPlugins       []DiscoveryPlugin
 	lock                   sync.Mutex
 	latestDesiredUpstreams map[DiscoveryPlugin]v1.UpstreamList
-	extraSelectorLabels    map[string]string
 }
 
 type EndpointDiscovery struct {
@@ -95,7 +94,7 @@ func NewUpstreamDiscovery(
 // launch a goroutine for all the UDS plugins
 func (d *UpstreamDiscovery) StartUds(opts clients.WatchOpts, discOpts Opts) (chan error, error) {
 	aggregatedErrs := make(chan error)
-	d.extraSelectorLabels = opts.Selector
+
 	for _, uds := range d.discoveryPlugins {
 		upstreams, errs, err := uds.DiscoverUpstreams(d.watchNamespaces, d.writeNamespace, opts, discOpts)
 		if err != nil {
@@ -136,12 +135,14 @@ func (d *UpstreamDiscovery) Resync(ctx context.Context) error {
 	for uds, desiredUpstreams := range d.latestDesiredUpstreams {
 		udsName := strings.Replace(reflect.TypeOf(uds).String(), "*", "", -1)
 		udsName = strings.Replace(udsName, ".", "", -1)
+
+		// selecting on the discovery plugin label will get all Upstreams created by Discovery
+		// there is no need to select on watchLabels as these are not present on Upstream metadata, nor are they
+		// necessary to identify Upstreams that may need to be resynced
 		selector := map[string]string{
 			"discovered_by": udsName,
 		}
-		for k, v := range d.extraSelectorLabels {
-			selector[k] = v
-		}
+
 		logger.Debugw("reconciling upstream details", zap.Any("upstreams", desiredUpstreams))
 		if err := d.upstreamReconciler.Reconcile(d.writeNamespace, desiredUpstreams, uds.UpdateUpstream, clients.ListOpts{
 			Ctx:      ctx,

test/kube2e/gateway/gateway_test.go

@@ -1366,6 +1366,7 @@ var _ = Describe("Kube2e: gateway", func() {
 		})
 	})
 
+	// as of v1.17 these tests are replaced by the discovery watchlabels suite in the new kubernetes e2e tests
 	Context("upstream discovery", func() {
 		var createdServices []string
 
@@ -1494,6 +1495,64 @@ var _ = Describe("Kube2e: gateway", func() {
 			}, "15s", "0.5s").ShouldNot(BeNil())
 		})
 
+		It("Modifies discovered Upstream when Service is modified", func() {
+			// This tests the fix for a bug whereby Discovery would fail to update discovered Upstreams on Service
+			// updates when watchLabels were enabled
+
+			watchedKey, watchedValue := "watchKey", "watchValue"
+			bonusKey, bonusValue, modifiedBonusValue := "foo", "bar", "modified-bar"
+
+			// set watchLabels for Discovery to select on
+			labels := map[string]string{
+				watchedKey: watchedValue,
+			}
+			setWatchLabels(labels)
+
+			// add an additional label which we'll modify later
+			labels[bonusKey] = bonusValue
+
+			svcName := "uds-test-service"
+			createServiceWithWatchedLabels(svcName, labels)
+
+			// confirm the Upstream has the Service's labels in its DiscoveryMetadata
+			Eventually(func() (map[string]string, error) {
+				us, err := getUpstream(svcName)
+				if err != nil {
+					return nil, err
+				}
+				return us.DiscoveryMetadata.GetLabels(), nil
+			}, "15s", "0.5s").Should(BeEquivalentTo(
+				map[string]string{
+					"gloo":     svcName,
+					watchedKey: watchedValue,
+					bonusKey:   bonusValue,
+				},
+			))
+
+			// get the service, modify its bonus label, and update it
+			svc, err := resourceClientset.KubeClients().CoreV1().Services(namespace).Get(ctx, svcName, metav1.GetOptions{})
+			Expect(err).NotTo(HaveOccurred())
+			svc.Labels[bonusKey] = modifiedBonusValue
+
+			_, err = resourceClientset.KubeClients().CoreV1().Services(namespace).Update(ctx, svc, metav1.UpdateOptions{})
+			Expect(err).NotTo(HaveOccurred())
+
+			// expect the discovered Upstream's DiscoveryMetadata to eventually reflect the updated label
+			Eventually(func() (map[string]string, error) {
+				us, err := getUpstream(svcName)
+				if err != nil {
+					return nil, err
+				}
+				return us.DiscoveryMetadata.GetLabels(), nil
+			}, "15s", "0.5s").Should(BeEquivalentTo(
+				map[string]string{
+					"gloo":     svcName,
+					watchedKey: watchedValue,
+					bonusKey:   modifiedBonusValue,
+				},
+			))
+		})
+
 		It("Does not discover upstream with no label when watched labels are set", func() {
 			watchedKey := "A"
 			watchedValue := "B"