devspace-sh / devspace

DevSpace - The Fastest Developer Tool for Kubernetes ⚡ Automate your deployment workflow with DevSpace and develop software directly inside Kubernetes.
https://devspace.sh
Apache License 2.0
4.3k stars 360 forks source link

Odd namespace behaviour incluster for kubectl deploy #2745

Open kuuji opened 11 months ago

kuuji commented 11 months ago

What happened?

I'm trying to deploy a preview using devspace in CI (runners running in kubernetes) in a different namespace than where devspace is running. This is using the kubectl deploy method.

Despite providing the namespace via an arg -> devspace deploy -n ci-tplkjs-1 devspace ends up deploying the preview in the same namespace as the runner (ci).

There are interesting things to note here and I tried a few things with various level of success.

What did you expect to happen instead?

setting namespace via devspace deploy -n should be enough to pass down to the manifests when running incluster.

How can we reproduce the bug? (as minimally and precisely as possible)

For this you need

Local Environment:

Anything else we need to know?

Initially I thought this was the reason https://github.com/devspace-sh/devspace/blob/main/pkg/devspace/kubectl/util/util.go#L31 but according to https://github.com/devspace-sh/devspace/blob/main/pkg/devspace/kubectl/util/util.go#L35-L37 it should only override it if the namespace was empty and it shouldn't be by that point.

I'm down to look deeper into this, I didn't get a chance yet and thought to post here first in case maybe this is intended?

lizardruss commented 11 months ago

@kuuji Thanks for submitting this issue! Yes this does seem odd, and the recommended usage is to use devspace use namespace [NAMESPACE] before running devspace deploy. This results in updating the your kube config to have a namespace set as well, which is more reliable for advanced uses of DevSpace, where other processes that use the kube config are involved.

I'll bring this up for discussion to see if we either want to fix this for the simpler cases, or officially deprecate the flag in favor of devspace use namespace

kuuji commented 11 months ago

@kuuji Thanks for submitting this issue! Yes this does seem odd, and the recommended usage is to use devspace use namespace [NAMESPACE] before running devspace deploy. This results in updating the your kube config to have a namespace set as well, which is more reliable for advanced uses of DevSpace, where other processes that use the kube config are involved.

I'll bring this up for discussion to see if we either want to fix this for the simpler cases, or officially deprecate the flag in favor of devspace use namespace

@lizardruss I cannot use devspace use namespace while running incluster. There is no kubeconfig when incluster, it's using the service account and the default namespace becomes the namespace the pod is in. So sadly this isn't an option.

I don't think the flag should be deprecated if there is no other way for incluster.

edit: I did try that as well ;)

lizardruss commented 11 months ago

@kuuji Ah I see, I skimmed over the incluster part. I'm surprised that setting the namespace in the deployment config didn't work. We'll take a look, but it may be some time before this gets prioritized.

kuuji commented 11 months ago

@lizardruss I found a bit of time to dig and got to the bottom of it. I'll put up a fix today or tomorrow.

the TLDR; is it's because of the way the manifests are rendered in https://github.com/devspace-sh/devspace/blob/main/pkg/devspace/deploy/deployer/kubectl/builder.go#L111-L166 when incluster. This is specifically because we run kubectl to render the manifests and when you're incluster kubectl will add the namespace you're in to all the manifests.

This doesn't affect inlineManifests, because we don't use kubectl to render that. And this doesn't affect when you're not incluster because the namespace kubectl inject will be based on what's in your kubeconfig.