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

Hard coded static routes not respecting namespace #409

Closed
jbury opened this issue Jan 25, 2022 · 8 comments
Closed

Hard coded static routes not respecting namespace #409

jbury opened this issue Jan 25, 2022 · 8 comments

Comments

@jbury
Copy link
Contributor

jbury commented Jan 25, 2022

I'm one of the few users that can't install nats using helm, hence my recent changes adding support to the install scripts/bootstrapping images to support namespacing. In using the install scripts, I'm installing nats in the nats namespace. Unfortunately, the default configuration for the nats-config ConfigMap hard-codes some routes that aren't valid if you're running in any namespace other than default. I believe the relevant version of the config map is this one, though I'm not 100% sure as the configmaps themselves don't contain a reference to their original filename.

https://github.com/nats-io/k8s/blob/main/nats-server/simple-nats.yml

I'm not sure how this should be handled, though I'm more than happy to actually do the implementation myself if it'll take some legwork.

The immediate fix I've implemented is that I manually edited the configmap to replace

        nats://nats-0.nats.default.svc:6222
        nats://nats-1.nats.default.svc:6222
        nats://nats-2.nats.default.svc:6222

with

        nats://nats-0.nats.nats.svc:6222
        nats://nats-1.nats.nats.svc:6222
        nats://nats-2.nats.nats.svc:6222

Which I've tested and it works just fine, but I'd prefer to fix it for everyone using the install script, and not just me :) Any ideas/guidance?

@wallyqs
Copy link
Member

wallyqs commented Jan 25, 2022

that's right, those would be the required changes to connect. I think that one workaround that might work is in the config yaml to do this instead:

 env:
        - name: CLUSTER_ADVERTISE
          value: $(POD_NAME).nats.$(POD_NAMESPACE).svc
        - name: CLUSTER_ROUTES
          value: "[ nats-0.nats.$(POD_NAMESPACE).svc,nats-1.nats.$(POD_NAMESPACE).svc,nats-2.nats.$(POD_NAMESPACE).svc ]"

and then in the ConfigMap change the configuration so that it becomes:

routes = $CLUSTER_ROUTES

https://github.com/nats-io/k8s/blob/main/nats-server/simple-nats.yml#L13-L17

@Jarema
Copy link
Member

Jarema commented Feb 7, 2022

@wallyqs wouldn't #421 and using short domain names by default clear this out?

@jbury
Copy link
Contributor Author

jbury commented Mar 11, 2022

@Jarema correct me if I'm wrong, but it looks like the short domain names you're talking about still hard-code the namespace (default), and that's the main issue here.

@Jarema
Copy link
Member

Jarema commented Mar 31, 2022

@jbury no, the namespace is calculated ("$(POD_NAME).%s.$(POD_NAMESPACE)" (include "nats.fullname").

@jbury
Copy link
Contributor Author

jbury commented Aug 12, 2022

Oho, that'd do the trick then, I think @Jarema.

@wallyqs On a scale from 1-10, 1 being "jbury is the official mantainer now" to 10 being "It should be 1:1", how maintained are these non-helm setup scripts compared to the helm setup process? Is it worth me even patching these? Is there a plan to just remove these setup scripts altogether?

@wallyqs
Copy link
Member

wallyqs commented Aug 12, 2022

I think we should remove some of them but we could keep a few, like the simple-nats one to keep as a reference.

@jbury
Copy link
Contributor Author

jbury commented Aug 12, 2022

@wallyqs I have no vision for what to keep/what to nuke, but if one of ya'll maintainers wants to clean out the stuff we explicitly don't want, I'd be happy to make this change in whatever is left over.

@jbury
Copy link
Contributor Author

jbury commented Aug 14, 2022

Per convo in 396, I'm just closing this one altogether and letting @wallyqs deal with the scripts cleanup however he pleases.

@jbury jbury closed this as completed Aug 14, 2022
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

3 participants