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

fix(install): fix issue where chart metadata is not being saved on helm install #5455

Merged
merged 1 commit into from Mar 15, 2019
Merged

Conversation

bacongobbler
Copy link
Member

In #5365, I changed the Chart field in the Release object to be omitted from the output so that helm status would not display that information when helm status -o json was requested. However, that resulted in helm install omitting that information when serializing the data to be stored in the release secret.

By adding the field back and stripping out the chart metadata before printing, we're able to achieve the original goal of stripping out that metadata from helm status without causing any serialization issues.

This is a quick n' dirty hack to get helm install working again as it should. @adamreese pointed me to a solution in the Go standard library where we can translate the binary data in the release object into a struct for general consumption. I'll file a new ticket to track that refactoring work.

…elm install`

Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>
@bacongobbler bacongobbler added awaiting review bug Categorizes issue or PR as related to a bug. v3.x Issues and Pull Requests related to the major version v3 labels Mar 15, 2019
@helm-bot helm-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 15, 2019
@bacongobbler bacongobbler added this to In Progress in Helm 3 via automation Mar 15, 2019
@bacongobbler bacongobbler moved this from In Progress to Awaiting Review in Helm 3 Mar 15, 2019
@@ -53,6 +53,9 @@ func newStatusCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
return err
}

// strip chart metadata from the output
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a FIXME note. We should come up with a better solution for dumping the chart with the release.

Copy link
Member

Choose a reason for hiding this comment

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

Just noticed the issue you created 🐑

Helm 3 automation moved this from Awaiting Review to LGTM Mar 15, 2019
@bacongobbler bacongobbler merged commit 8f37ab4 into helm:dev-v3 Mar 15, 2019
Helm 3 automation moved this from LGTM to Done Mar 15, 2019
@bacongobbler bacongobbler deleted the fix-list branch March 15, 2019 16:31
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. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. v3.x Issues and Pull Requests related to the major version v3
Projects
No open projects
Helm 3
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants