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

Feat/stevenctl/cls name #10286

Open
wants to merge 33 commits into
base: main
Choose a base branch
from
Open

Feat/stevenctl/cls name #10286

wants to merge 33 commits into from

Conversation

nfuden
Copy link
Contributor

@nfuden nfuden commented Nov 6, 2024

** this is the PR to use **

Originally #10255

Description

https://github.com/solo-io/solo-projects/issues/7105

This will make cluster names for kube services look like kube-svc_name_namespace_port or kube-svc_reviews_bookinfo_8080. If we add other resources besides a kube service, this also lets us get things like istio-se_helloworld_backend-ns_5000.

This will only happen for Gateway v2 translation. We can disable it using the env var GG_K8S_GW_LEGACY_CLUSTER_NAMES.

API changes

We can disable it using the env var GG_K8S_GW_LEGACY_CLUSTER_NAMES.

Code changes

  • XDS translator now takes options that can customize this new behavior. We instantiate translator separately for v2, so that is the only path to enable it.
  • Added another function to customize the generated name rather than modifying the existing cluster name function (it has over 100 usages! mostly in unit tests)
  • Use internal labels to propagate the info in a structured way.

Docs changes

We should document this as a breaking change; users who rely on the old format should use the env var or update their EnvoyFilters or whatever config relies on XDS names...

Context

https://github.com/solo-io/solo-projects/issues/7105

Interesting decisions

  • Use internal labels to propagate the info in a structured way. This was done so we can support more than just Upstream_Kube upstreams.
  • Functional options for translator constructor. I didn't feel like updating the many references to this constructor, so varargs help. In hindsight, a second constructor could have also solved this.

Testing steps

TODO!

@solo-changelog-bot
Copy link

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

@github-actions github-actions bot added keep pr updated signals bulldozer to keep pr up to date with base branch work in progress signals bulldozer to keep pr open (don't auto-merge) labels Nov 6, 2024
Copy link

github-actions bot commented Nov 6, 2024

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

https://gloo-edge--pr10286-feat-stevenctl-cls-n-4b40vjt4.web.app

(expires Wed, 20 Nov 2024 00:46:21 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 77c2b86e287749579b7ff9cadb81e099042ef677

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 work in progress signals bulldozer to keep pr open (don't auto-merge)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants