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

set namespace for kubeclient during uninstall #2517

Conversation

thorbenbelow
Copy link
Contributor

@thorbenbelow thorbenbelow commented Jan 31, 2024

Fixes #1033 #1558

This should probably be adjusted to still call ensureHelmConfig or use c.Config().Flags() instead of a new helm cli object.

Signed-off-by: Thorben Below <56894536+thorbenbelow@users.noreply.github.com>
@derailed derailed added bug Something isn't working question Further information is requested InProgress Marks an issue has being worked on labels Feb 6, 2024
Copy link
Owner

@derailed derailed left a comment

Choose a reason for hiding this comment

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

@thorbenbelow Brilliant!! Thank you for your research on this!!

I've taken a pass and it looks like the issue is due to the helm api not taking into account the passed in ns and thus applies the deletions in the context ns.

cli.New call hydrates a new connection from flags and thus uninstall deletes the resources in the chart ns. (Helm deletion manifests don't specify a namespace and thus default to the context namespace.
Does this make sense?

I think we can fix this by doing those 3 things:

  1. Turn off persistent config in k9s. See client/config.go UsePersistentConfig
  2. Update ensureHelmConfig to take in configFlags vs a connection object
  3. Update the Helm uninstall to now read something like:
	flags := h.Client().Config().Flags()
	flags.Namespace = &ns
	cfg, err := ensureHelmConfig(flags, ns)
	...
	
// ensureHelmConfig return a new configuration.
func ensureHelmConfig(flags *genericclioptions.ConfigFlags, ns string) (*action.Configuration, error) {
	cfg := new(action.Configuration)
	err := cfg.Init(flags, ns, os.Getenv("HELM_DRIVER"), helmLogger)

	return cfg, err
}	

@thorbenbelow thorbenbelow deleted the helm-delete-using-wrong-namespace branch November 10, 2024 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working InProgress Marks an issue has being worked on question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Helm delete deletes only the helm entry but not the deployment.
2 participants