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

Go SDK installing Kubernetes resources to the wrong namespaces #8780

Closed
object88 opened this issue Sep 21, 2020 · 12 comments · May be fixed by #8785
Closed

Go SDK installing Kubernetes resources to the wrong namespaces #8780

object88 opened this issue Sep 21, 2020 · 12 comments · May be fixed by #8785
Labels
bug Categorizes issue or PR as related to a bug. Stale

Comments

@object88
Copy link
Contributor

I am working on a standalone Go binary that uses the helm Go APIs to deploy a chart. The tricky part is that the binary is a long-running service running in a particular namespace, and it's deploying arbitrary helm charts to other, arbitrary namespaces. The perhaps more tricky part is that this binary is expected to be concurrency-safe, so more than one deployment may happen at once.

The problem that I am experiencing is when that the service (running in namespace nhi) deploys a chart, the helm chart itself lands in the desired namespaces ("default"), but the kubernetes resources described by the chart land in the nhi namespaces.

This situation has been discussed in a number of other issues, but I am not clear at this point if this is the expected behavior (and if so, I don't understand why), or if it's an acknowledged bug. I think that I have read (and I may be wrong) that the target namespace get derived from the kube context, but I am not sure that works for me given the constraints mentioned above.

I have a demo project that illustrates the problem: https://github.com/object88/namespaced-helm-install. Relevant code snippet:

	c := cli.New()

	a := &action.Configuration{
		Capabilities: chartutil.DefaultCapabilities,
	}
	debug := func(msg string, args ...interface{}) {
		fmt.Printf(fmt.Sprintf("** %s\n", msg), args...)
	}
	a.Init(c.RESTClientGetter(), "default", "", debug)

	act := action.NewInstall(a)
	act.Atomic = true
	act.Namespace = "default"
	act.ReleaseName = "foo"

	ch, _ := loader.Load("/opt/charts/foo")
	act.Run(ch, nil)

The test.sh command will build and push the "server" docker image to the nhi namespace, and deploy the server. (if you run this, you may need to manually create that namespace. Also, you will need to comment this out code to build and publish the docker image, or change to a different dockerhub repo) The server then attempts to deploy a different chart, "foo" to the "default" namespace. After running test.sh, you can run the following commands:

$ helm list --all-namespaces
NAME              	NAMESPACE	REVISION	UPDATED                                	STATUS  	CHART                        	APP VERSION
foo               	default  	1       	2020-09-21 17:46:08.694358671 +0000 UTC	deployed	foo-0.0.0
nhi               	nhi      	1       	2020-09-21 10:46:06.468626 -0700 PDT   	deployed	namespaced-helm-install-0.1.0	1.16.0

$ kubectl get all --namespace default
NAME                 TYPE        CLUSTER-IP   EXTERNAL-IP   PORT(S)   AGE
service/kubernetes   ClusterIP   10.96.0.1    <none>        443/TCP   34d

$ kubectl get all --namespace nhi
NAME                                              READY   STATUS    RESTARTS   AGE
pod/foo-nginx-deployment-6696cf858-2cqrc          1/1     Running   0          5m8s
pod/nhi-namespaced-helm-install-9fc497b7d-svqlw   1/1     Running   0          5m11s

NAME                                  TYPE        CLUSTER-IP     EXTERNAL-IP   PORT(S)   AGE
service/nhi-namespaced-helm-install   ClusterIP   10.104.221.1   <none>        80/TCP    5m11s

NAME                                          READY   UP-TO-DATE   AVAILABLE   AGE
deployment.apps/foo-nginx-deployment          1/1     1            1           5m9s
deployment.apps/nhi-namespaced-helm-install   1/1     1            1           5m11s

NAME                                                    DESIRED   CURRENT   READY   AGE
replicaset.apps/foo-nginx-deployment-6696cf858          1         1         1       5m8s
replicaset.apps/nhi-namespaced-helm-install-9fc497b7d   1         1         1       5m11s

Contents of go.mod:

module ...

go 1.15

require (
	[SNIP]
	helm.sh/helm/v3 v3.3.3
)

Output of kubectl version:

$ kubectl version
Client Version: version.Info{Major:"1", Minor:"19", GitVersion:"v1.19.2", GitCommit:"f5743093fd1c663cb0cbc89748f730662345d44d", GitTreeState:"clean", BuildDate:"2020-09-16T21:51:21Z", GoVersion:"go1.15.2", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.3", GitCommit:"2e7996e3e2712684bc73f0dec0200d64eec7fe40", GitTreeState:"clean", BuildDate:"2020-05-20T12:43:34Z", GoVersion:"go1.13.9", Compiler:"gc", Platform:"linux/amd64"}

Cloud Provider/Platform (AKS, GKE, Minikube etc.):

minikube

Thanks for any feedback, corrections for my code, or otherwise.

@object88
Copy link
Contributor Author

object88 commented Sep 21, 2020

I think this may be related to #8685.

@mattfarina mattfarina added the bug Categorizes issue or PR as related to a bug. label Sep 22, 2020
@mattfarina
Copy link
Collaborator

I understand the confusion and in Helm v4 we may be able to more easily refine the API to make this easier. There are things I would change to make the experience better.

I believe this is a bug. One complicated enough I don't have a quick fix for.

Here is what I see happening. When action.Init() is run a namespace is passed in. Init sets the namespace for the for saving the release information. Along with this a KubeClient is created where no namespace is specified. The client, using factories, figures out what the client should be and sets the namespace. For example, if my local kubeconfig is set at the "foo" namespace that will be the namespace the KubeClient uses.

When KubeClient.Create() is run this detected namespace is the one used to upload the resources to.

In the Helm client this works as expected because of the way it's built. But, the SDK can have issues.

There is no way, currently, to set the namespace for the KubeClient to use.

I manually tested all of this out. Now we need to find the best way to fix this issue.

@philipp1992
Copy link

jup im experiencing the same issues, which makes using the go client pretty much unusable. the release gets installed in the targeted namespaced, but the resources get applied to the namespace in the kubeconfig (default in my case)

@ganesanarun
Copy link

We are experiencing the same issue as @philipp1992

@bacongobbler
Copy link
Member

bacongobbler commented Nov 2, 2020

There is a way to change the namespace parameter in the KubeClient after it’s been instantiated. You just need to acquire a handle to the client... Which I do not believe is readily available from within the action.Configuration object:

https://github.com/helm/helm/blob/master/pkg/kube/client.go#L63

https://github.com/helm/helm/blob/master/pkg/kube/client.go#L143

@github-actions
Copy link

github-actions bot commented Feb 1, 2021

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Feb 1, 2021
@github-actions github-actions bot closed this as completed Mar 4, 2021
@mtiller
Copy link

mtiller commented Jul 23, 2021

This seems to have been closed, but as I read this, no solution is provided. This really does make using the SDK extremely difficult. I have to manually code my namespace into all my templates. Is there really nothing that can be done in Helm here? As @bacongobbler points out, if the underlying KubeClient were simply exported (I think that is what he is getting at), then I could resolve this.

@hickeyma hickeyma removed the Stale label Jul 26, 2021
@hickeyma hickeyma reopened this Jul 27, 2021
@mtiller
Copy link

mtiller commented Jul 29, 2021

Just a note to anybody who comes across this issue. The solution proposed in #9171 worked for me (e.g., setting HELM_NAMESPACE before initializing any bits of the Helm SDK). Hopefully a future version of the SDK will address this because without that workaround, using the SDK is quite difficult.

@mattfarina
Copy link
Collaborator

mattfarina commented Jul 29, 2021

The issue here is that the kube client namespace cannot be set or changed easily programmatically. I dealt with it not long ago and feel everyone's frustration. I did manage to come up with a work around for the project I'm working on.

First, what's happening...

a.Init(c.RESTClientGetter(), "default", "", debug)

When this line (from the original snippet) is executed two things initially happen...

helm/pkg/action/action.go

Lines 365 to 372 in e9e6b07

func (cfg *Configuration) Init(getter genericclioptions.RESTClientGetter, namespace, helmDriver string, log DebugLog) error {
kc := kube.New(getter)
kc.Log = log
lazyClient := &lazyClient{
namespace: namespace,
clientFn: kc.Factory.KubernetesClientSet,
}

The first this is that a kube client is created. Later that is attached to the action configurations KubeClient property. No namespace is set here. This is what will be used to build and push k8s resources to the server.

The second thing is a lazyclient is created with a namespace. This is for the storage of the Helm release record. The release record and the Kubernetes resource locations can be different.

So far we have a blank namespace, which will go with the default per your kubeconfig, and one tied to a namespace.

It's worth noting that the lazyclient namespace which is injected into the storage drivers isn't something that's easily changed with the API. But, there is a way to change that, too.

After a.Init is run, there is now a.KubeClient that is typed as an interface (details here). That interface provides no access to changing the Namespace. And, per our backwards compatibility guidelines... we aren't changing until Helm v4.

There is no native way to set the KubeClient namespace. I lost hours trying to solve for this.

I've found a work around for custom applications. The basics of it are in this file.

Instead of using action configuration... we use our own wrapper just to add a method...

// Configuration is a composite type of Helm's Configuration type
type Configuration struct {
	*action.Configuration
}

Then we added a SetNamespace function to the struct. This is able to change the KubeClient namespace along with changing the namespace to where the Helm release objects are stored. The code here can be picked apart for your needs. It shows how to deal with both situations and deals with different drivers that Helm itself provides. It worked for all our tests because we handle the memory drivers, too.

By switching on the KubeClient type we can deal with the native namespace handling for an implementation, which do exist, instead of dealing with what's provided by the interface.

Note, there is technically no need to create a function or extend the action configuration. In the main line code above the relevant switch statements could be used. We just did that to easily contain the code.

@github-actions
Copy link

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Oct 28, 2021
@bacongobbler
Copy link
Member

#10230

@fnikolai
Copy link

fnikolai commented Feb 27, 2024

#9171 didn't solve the problem for me.

What seems to work is the solution of Hypher

var actionConfig action.Configuration

err := actionConfig.Init(
	configFlags,
	namespace,
	"secret",
	func(format string, v ...interface{}) {
		fmt.Sprintf(format, v)
	},
)

if err != nil {
	fmt.Println(err)
	os.Exit(1)
}

// When actionConfig.Init is called it sets up the driver with the default namespace.
// We need to change the namespace to honor the release namespace.
// https://github.com/helm/helm/issues/9171
actionConfig.KubeClient.(*kube.Client).Namespace = namespace

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Categorizes issue or PR as related to a bug. Stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants