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

k8s gateway cluster names #10357

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

k8s gateway cluster names #10357

wants to merge 39 commits into from

Conversation

jenshu
Copy link

@jenshu jenshu commented Nov 15, 2024

Description

(reopening the PR that was migrated to k8sgateway#10286, which was originally copied from #10255)

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!

API changes

Code changes

CI changes

Docs changes

Context

Interesting decisions

Testing steps

Notes for reviewers

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

stevenctl and others added 30 commits November 5, 2024 10:35
@solo-changelog-bot
Copy link

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

@jenshu jenshu changed the title Feat/stevenctl/cls name k8s gateway cluster names Nov 15, 2024
const (
// KubeSourceResourceLabel indicates the kind of resource that the synthetic
// resource is based on.
KubeSourceResourceLabel = "~internal.solo.io/kubernetes-source-resource"
Copy link
Author

Choose a reason for hiding this comment

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

getting errors like: "caller":"discovery/discovery.go:151","msg":"failed reconciling upstreams","version":"1.0.0-ci1","discovered_by":"kubernetesplugin","upstreams":14,"error":"reconciling resource kube-system-kube-dns-53: creating kube resource kube-system-kube-dns-53: Upstream.gloo.solo.io \"kube-system-kube-dns-53\" is invalid: [metadata.labels: Invalid value: \"~internal.solo.io/kubernetes-name\": prefix part a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'), when running this in a cluster

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants