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

Allow users to specify a suffix for --advertise-host argument #888

Open
ADustyOldMuffin opened this issue Apr 26, 2022 · 3 comments
Open

Comments

@ADustyOldMuffin
Copy link

ADustyOldMuffin commented Apr 26, 2022

As found here the --advertise-host argument is built based on the pod name, and the namespace. In our instance our nodes exist in different Kubernetes clusters, but have the same podnames and namespace. What makes our nodes uniquely identifiable is their FQDN (what cluster they're in).

I know it's not supported but would it make sense to have an option to specify a suffix for these names?

So the code would look something like

suffix := ""
if b.Spec().AdvertiseSuffix != nil {
    suffix = fmt.Sprintf(".%s", *b.Spec().AdvertiseSuffix)
}

...
fmt.Sprintf("--advertise-host=$(POD_NAME).%s.%s",
    b.Cluster.DiscoveryServiceName(), b.Cluster.Namespace(), suffix)

Or something of the sort.

@chrisseto
Copy link
Contributor

I wonder if we could use the utility added by #894 to easily address this problem? Another alternative would be to use hostname -f in favor of manually constructing the --advertise-host flag.

@ADustyOldMuffin would you mind sharing the (potentially redacted) output of the following commands on the cluster(s) that you're running?

# From a cockroachDB Pod
hostname -f

# From anywhere
# If running on stock CRDB containers, you may need to run: microdnf install bind-utils
nslookup kubernetes.default.svc

@aybabtme
Copy link

Hey folks, following up on this. Would your team be open to a PR that introduces a way to customize how pods self-advertise? We're in an environment where the DNS name for each crdb pod is created externally (externalDNS), and the pod needs to advertise that name instead.

@udnay
Copy link

udnay commented May 23, 2024

From a technical standpoint I think this is fine. My concern was in the rolling restart logic that the operator was checking logical CRDB nodes across all regions vs just the stateful set that it owns in its own cluster.

e.g. if I have 3 regions
k8s-1 with nodes n1,n2,n3
k8s-2 with nodes n4,n5,n6
and
k8s-3 with nodes n7,n8,n9

Then making sure the operator in k8s-1 does not hang trying to update n4 or something.

The logic seems to be focused only on stateful sets so this likely fine. Orchestration across the clusters will need to be handled by something outside the operator though.

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

No branches or pull requests

4 participants