Skip to content

Commit

Permalink
[1.17] Ensure endpoints for upstreams are listed within watchNamespac…
Browse files Browse the repository at this point in the history
…es (#9881)

* Ensure endpoints for upstreams are listed within watchNamespaces

* add changelog

* codegen

* add tests

* newline

* update helm tests

---------

Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>
  • Loading branch information
davidjumani and soloio-bulldozer[bot] authored Aug 9, 2024
1 parent 39dc4e5 commit 6d1b50c
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 5 deletions.
6 changes: 6 additions & 0 deletions changelog/v1.17.2/fix-eds-error.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
changelog:
- type: FIX
issueLink: https://github.com/solo-io/gloo/issues/5885
resolvesIssue: false
description: Fix a bug that causes edge to try to list endpoints across all namespaces when no upstreams exist.

6 changes: 6 additions & 0 deletions projects/gloo/pkg/plugins/kubernetes/eds.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,12 @@ func newEndpointWatcherForUpstreams(kubeFactoryFactory func(ns []string) KubePlu
}
}

// If there are no upstreams to watch (eg: if discovery is disabled), namespaces remains an empty list.
// When creating the InformerFactory, by convention, an empty namespace list means watch all namespaces.
// To ensure that we only watch what we are supposed to, fallback to WatchNamespaces if namespaces is an empty list.
if len(namespaces) == 0 {
namespaces = settings.GetWatchNamespaces()
}
kubeFactory := kubeFactoryFactory(namespaces)
// this can take a bit of time some make sure we are still in business
if opts.Ctx.Err() != nil {
Expand Down
9 changes: 9 additions & 0 deletions projects/gloo/pkg/plugins/kubernetes/eds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,16 @@ var _ = Describe("Eds", func() {
Expect(err).NotTo(HaveOccurred())
watcher.List("foo", clients.ListOpts{Ctx: ctx})
Expect(func() {}).NotTo(Panic())
})

It("should default to watchNamespaces if no upstreams exist", func() {
watchNamespaces := []string{"gloo-system"}
_, err := newEndpointWatcherForUpstreams(func(namespaces []string) KubePluginSharedFactory {
Expect(namespaces).To(Equal(watchNamespaces))
return mockSharedFactory
},
mockCache, v1.UpstreamList{}, clients.WatchOpts{Ctx: ctx}, &v1.Settings{WatchNamespaces: watchNamespaces})
Expect(err).NotTo(HaveOccurred())
})

Context("Istio integration", func() {
Expand Down
5 changes: 0 additions & 5 deletions test/kube2e/helm/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,11 +256,6 @@ var _ = Describe("Kube2e: helm", func() {
// Since the production recommendation is to disable discovery, we remove it from the list of deployments to check to consider gloo is healthy
glooDeploymentsToCheck = []string{"gloo", "gateway-proxy"}

additionalInstallArgs = []string{
// Setting `settings.disableKubernetesDestinations` && `global.glooRbac.namespaced` leads to panic in gloo
// Ref: https://github.com/solo-io/gloo/issues/8801
"--set", "global.glooRbac.namespaced=false",
}
additionalInstallArgs = append(additionalInstallArgs, valuesForProductionRecommendations...)

expectGatewayProxyIsReady = func() {
Expand Down

0 comments on commit 6d1b50c

Please sign in to comment.