Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.17] Ensure endpoints for upstreams are listed within watchNamespaces #9881

Merged
merged 1 commit into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading