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

Switch separate Gloo Gateway xds to proxy syncer #9310

Merged
merged 65 commits into from
Apr 14, 2024
Merged

Conversation

npolshakova
Copy link
Contributor

@npolshakova npolshakova commented Apr 2, 2024

Gloo Edge Enterprise supports a custom ext-auth and rate-limit server in the default deployment. To support ext-auth and ratelimit, this replaces the current gateway xds syncer with a proxy syncer. The proxy syncer would feed proxies in to a common in memory set of proxies.

The changes in this PR include:

  1. Minor fix to kube e2e test debugging logic to not hard code gateway name when getting the envoy config dump on test failure.

  2. Removing the k8s Gateway controller's kube service discovery logic in favor of the legacy classic Edge kube svc discovery. This means Gloo Gateway now requires disable_kubernetes_destinations=false to be set (this is the default value in the helm chart).

The proxy no longer uses upstream and instead selects kube destination:

Old:

            routeAction:
              single:
                upstream:
                  name: default-bar-svc-canary-8080
                  namespace: default

New:

            routeAction:
              single:
                kube:
                  port: 8080
                  ref:
                    name: bar-svc-canary
                    namespace: default

Similarly, the Gloo Gateway mirror plugin translates a kube destination into an upstream here instead of relying on the Gloo Gateway discovered upstream.

  1. Combines the Gloo Gateway and classic Edge xds syncer. The k8s Gateway proxies are translated separately and "synced" with the in-memory proxies in the proxy syncer. The ownership labels have been changed to be consistent with the Proxy metadata. The envoy translator syncer now syncs all proxies (both classic Edge proxies and Gloo Gateway proxies).

In order to support Gloo Gateway creating proxies in any namespace, we need to change the Gloo Gateway proxy metadata to include a proxy_namespace label. Currently, the proxyClient can only list proxies in the writeNamespace (ex. gloo-system), and will error in solo-kit if the namespace is not valid. Instead of changing solo-kit to support the empty namespace, the proxy_namespace label is added to the proxy and used to reconstruct the snapshot cache key.

Since the xds syncers are combined, we now reuse the old role metadata field to be consistent with the other proxy types (ingress, knative, gloo edge legacy, etc.). The old proxies use <proxy_namespace>~<proxy_name> to define the role. The gloo gateway role comes from the proxy-deployment ConfigMap and is in the format <owner>~<proxy_namespace>~<proxy_name>.

Old:

metadata:
   gateway:
      name: my-namespace
      namespace: gw

New:

metadata:
   role: gloo-kube-api~my-namespace~my-namespace-gw

@github-actions github-actions bot added the keep pr updated signals bulldozer to keep pr up to date with base branch label Apr 2, 2024
Copy link

github-actions bot commented Apr 2, 2024

Visit the preview URL for this PR (updated for commit d5024f7):

https://gloo-edge--pr9310-switch-to-proxy-sync-qlih4vg0.web.app

(expires Fri, 19 Apr 2024 22:21:55 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 77c2b86e287749579b7ff9cadb81e099042ef677

@solo-changelog-bot
Copy link

Issues linked to changelog:
https://github.com/solo-io/solo-projects/issues/5939

@npolshakova npolshakova changed the title [DNM] Switch to proxy syncer Switch to proxy syncer Apr 9, 2024
@npolshakova npolshakova changed the title Switch to proxy syncer Switch separate Gloo Gateway xds to proxy syncer Apr 11, 2024
Copy link
Contributor

@lgadban lgadban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fairly certain the changes to xds_syncer introduced in #9341 are not present.

Unfortunately this wasn't caught as no e2e test is in place for the status reporting

See: https://github.com/solo-io/gloo/pull/9341/files#diff-b517e67cbd4141da0c540b1b581e39c7203ffadf370c83c5865f70ef9d4bd06d

@npolshakova
Copy link
Contributor Author

I'm fairly certain the changes to xds_syncer introduced in #9341 are not present.

Unfortunately this wasn't caught as no e2e test is in place for the status reporting

See: https://github.com/solo-io/gloo/pull/9341/files#diff-b517e67cbd4141da0c540b1b581e39c7203ffadf370c83c5865f70ef9d4bd06d

The proxy syncer no longer syncs envoy, it only reconciles the proxies, so the gateway translator will handle the statuses for RouteOptions. @ilackarms

.github/workflows/pr.yaml Show resolved Hide resolved
projects/gateway/pkg/syncer/translator_syncer.go Outdated Show resolved Hide resolved
projects/gateway2/deployer/deployer_test.go Outdated Show resolved Hide resolved
projects/gateway2/proxy_syncer/queue.go Show resolved Hide resolved
projects/gloo/pkg/utils/translators.go Outdated Show resolved Hide resolved
projects/gloo/pkg/xds/cache.go Outdated Show resolved Hide resolved
projects/gloo/pkg/xds/node_hash.go Outdated Show resolved Hide resolved
test/kube2e/gateway/gateway_test.go Outdated Show resolved Hide resolved
@lgadban lgadban merged commit b0a9bcb into main Apr 14, 2024
20 checks passed
@lgadban lgadban deleted the switch-to-proxy-syncer branch April 14, 2024 00:25
@jenshu jenshu mentioned this pull request May 29, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep pr updated signals bulldozer to keep pr up to date with base branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants