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

Kubeapps with Helm3 loses raw user chart config #1423

Closed
absoludity opened this issue Jan 9, 2020 · 8 comments · Fixed by #1443
Closed

Kubeapps with Helm3 loses raw user chart config #1423

absoludity opened this issue Jan 9, 2020 · 8 comments · Fixed by #1443
Assignees

Comments

@absoludity
Copy link
Contributor

absoludity commented Jan 9, 2020

Description:

When running Kubeapps with Helm2, the user supplied raw chart config (ie. customised values.yaml) is recorded with the helm release.

Kubeapps uses this to display the config when upgrading, complete with user comments etc.

When running Kubeapps with Helm3, a helm release records only the config data, not the raw user supplied config with the comments etc.

We need to either find some way to record the raw user supplied config values for a release, or alternatively accept that on upgrade users will see their config without the explanatory comments (or are there also other issues this will cause?).

Opening as an issue both for discussion and also so it can be implemented in parallel once we decide on a solution.

Additional information you deem important (e.g. issue happens only occasionally):

Related to #1056

cc: @andresmgot , @SimonAlling , @lindhe , @latiif

@SimonAlling
Copy link
Contributor

FAQ

Q: Can't we just make a PR fixing this in Helm?

A:

This was an intentional design decision, yes. Kubernetes has a 1MB storage limit. Saving the raw file content in certain cases was going over that limit.

@bacongobbler on Slack

@andresmgot
Copy link
Contributor

As far as I can see we have two options here:

  1. Store the raw config independently. When creating a release, we can create a different secret with the raw config of the release. When requesting a release, the backend would attach the raw config as in Helm2.
    Pros: All user modifications are kept. It's transparent from the Dashboard point of view.
    Cons: We might hit the same 1MB limit problem. If a release is created/upgraded using the helm cli, the raw config won't be available.

  2. Apply the parsed config to the default values.yaml when upgrading. We can use the current YAML library to set in the default values.yaml the modified values of the installed version.
    Pros: We don't need to store any additional information in the cluster. Works even if the chart has been installed using the Helm CLI.
    Cons: If the user modifies any comment, those changes will be lost. The dashboard need to be aware if the release is from Helm 3 to apply that workflow.

None of the options are ideal but I would go with the second option because:

  • It keeps full compatibility with releases made with the helm CLI
  • AFAIK this only affects to the view in which the user upgrades and the version selected is the same than the currently installed. When the version changes, we do the values migration anyway, losing any custom comment.

WDYT?

@lindhe
Copy link
Contributor

lindhe commented Jan 9, 2020

I don't see how option (1) could hit the same 1MB problem if we store that info in a database. Would that be an option?

Of course, it would not work if helm install is run manually outside of Kubeapps...

@andresmgot
Copy link
Contributor

I don't see how option (1) could hit the same 1MB problem if we store that info in a database. Would that be an option?

In theory, a values.yaml could go over 1MB (1MB of text is a lot of text), while unlikely, it could happen. In any case, I am more worried about the other issue, causing an inconsistency between the experience using helm (the cli) and kubeapps.

@absoludity
Copy link
Contributor Author

absoludity commented Jan 10, 2020

+1 for option 2.

Also:

**Cons**: If the user modifies any comment, those changes will be lost. The dashboard need to be aware if the release is from Helm 3 to apply that workflow.

Why does the dashboard need to be aware of the difference? We can just make this the only behaviour (ie. start ignoring the helm2 release's raw config, given that it won't be available in helm3), I think?

@andresmgot
Copy link
Contributor

Why does the dashboard need to be aware of the difference? We can just make this the only behaviour (ie. start ignoring the helm2 release's raw config, given that it won't be available in helm3), I think?

Probably, I would need to verify that the result is the same. We can try that.

@SimonAlling
Copy link
Contributor

SimonAlling commented Jan 14, 2020

Should the entire values.yaml + Helm 3 problem be viewed as solved now? I was thinking maybe we should notify the user in some way that there is no point in editing or adding comments. Might be confusing to see that they're just not there otherwise.

@absoludity
Copy link
Contributor Author

Yeah, perhaps when editing values.yaml on the tab: Note: Only comments from the original chart values.yaml will be preserved.

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

Successfully merging a pull request may close this issue.

4 participants