feat(helm): add --app-version flag to 'helm install/upgrade'

[eraac]

Closes #3555

New pull request for #4961, sorry about that

If applicable:

• this PR contains documentation
• this PR contains unit tests

[eraac]

That commit include some changes according to #4961 (comment) and adding new boolean into Chart metadata (by default false) for allowing or not the flag --app-version

[marshallford]

Is this gtg?

[eraac]

Is this gtg?

@marshallford yes, waiting for a review at least

[bacongobbler]

Thank you for the PR.

Before we can consider reviewing this further, would you please document how chart developers can allow users to set the --app-version flag for their chart. From looking at this code, by default, any chart that omits the overrideAppVersion field (i.e. all charts of the legacy charts) will be unable to use this unless they opt in. Users need to know what they need to do to allow users to override the app version field in their charts.

If you're thinking of porting this over to Helm 3, perhaps @adamreese should take a look at this PR. I'm still concerned about the overall design of allowing users to override chart metadata as part of helm install.

Thanks!

[eraac]

If you're thinking of porting this over to Helm 3, perhaps @adamreese should take a look at this PR. I'm still concerned about the overall design of allowing users to override chart metadata as part of helm install.

@bacongobbler I haven't look next major version yet, what I should know about it ? Of course, i think porting this in helm 3

By the way I have make changes according to the review

[darekmydlarz]

Hello, what is the status of this PR?

[Stono]

would still really like this feature.

[unittolabs]

Any updates about merging this? This is a very useful feature, hope to get it =)

[eraac]

Circleci fail, but i have no idea why. If someone has explication, i can fix it (i guess)

[bacongobbler]

#!/bin/bash -eo pipefail
.circleci/test.sh
Running 'make build'
GOBIN=/go/src/k8s.io/helm/bin go install  -tags '' -ldflags '-w -s -X k8s.io/helm/pkg/version.GitCommit=b5f34b33bb0a61800ed83066f9654c5b5fa29ed0 -X k8s.io/helm/pkg/version.GitTreeState=clean' k8s.io/helm/cmd/...
Running 'make verify-docs'
GOBIN=/go/src/k8s.io/helm/bin go install  -tags '' -ldflags '-w -s -X k8s.io/helm/pkg/version.GitCommit=b5f34b33bb0a61800ed83066f9654c5b5fa29ed0 -X k8s.io/helm/pkg/version.GitTreeState=clean' k8s.io/helm/cmd/...
Creating /root/.helm 
Creating /root/.helm/repository 
Creating /root/.helm/repository/cache 
Creating /root/.helm/repository/local 
Creating /root/.helm/plugins 
Creating /root/.helm/starters 
Creating /root/.helm/cache/archive 
Creating /root/.helm/repository/repositories.yaml 
Adding stable repo with URL: https://kubernetes-charts.storage.googleapis.com 
Adding local repo with URL: http://127.0.0.1:8879/charts 
$HELM_HOME has been configured at /root/.helm.
Not installing Tiller due to 'client-only' flag having been set
--- /tmp/tmp.xDUewHmInp/docs/helm/helm_install.md	2019-06-07 13:13:04.965128551 +0000
+++ docs/helm/helm_install.md	2019-06-07 13:10:32.990535551 +0000
@@ -83,7 +83,7 @@ helm install [CHART] [flags]
       --ca-file string           Verify certificates of HTTPS-enabled servers using this CA bundle
       --cert-file string         Identify HTTPS client using this SSL certificate file
       --dep-up                   Run helm dependency update before installing the chart
-      --description string       Specify a description for the release
+      --description string       specify a description for the release
       --devel                    Use development versions, too. Equivalent to version '>0.0.0-0'. If --version is set, this is ignored.
       --dry-run                  Simulate an install
   -h, --help                     help for install
helm docs are out of date. Please run "make docs"
Makefile:150: recipe for target 'verify-docs' failed
make: *** [verify-docs] Error 1
Exited with code 2

[eraac]

Thanks @bacongobbler, CircleCI pass successfully now, and no conflict 🎉
What's the next step?

[bacongobbler]

I'd suggest reaching out and finding another maintainer to review this. I'm uncomfortable merging a PR proposing a design that I don't believe is the right path forward for the chart spec as per my last comment made in #4961 (comment), doubly so now that this PR introduces conditional logic in the chart metadata (via overrideAppVersion).

Having a few extra maintainers vet the design would help determine a better path forward for this use case. I'm focusing primarily on Helm 3's ongoing development and security fixes for Helm 2, which is why I have not been spending much time looking at features for Helm 2 such as this one.

[iorlas]

Okay. I've been upgrading to the helm this week, since tiller was finally ditched and 3.0alpha is released.

And I have encountered the same problems as all you, guys. And I faced the fact that I can't use appVersion in my workflow. I need ability to set appVersion when I helm install it. But on the second thought... it is so wrong, I cannot just ignore this pull request. Let me elaborate.

Helm install

First, we need to understand where the problem starts.

Actually, helm install has 2 behaviours:

1. Install remote package

Feels nice and play, similar to the apt, pip, gem, yum, npm - any package manager. Even newbie will handle helm here.

2. Install local ... wait a minute!

When we install the local chart, it is not a package yet. As it is not a package and we may not need to make one, do we need to edit anything? Maybe... we need to install it directly, every time, since it is handy. This is where the problem begins.

The Use Case

User has a project. User has CI/CD. CD manages the versioning process. User does not want the repo thing, since project is private or personal. User wants to helm install it.

User wants to use Chart.appVersion, but he is unable to edit it within CD flow, but user won't edit it manually, since it feels redundant.

User is stuck.

Options

User has options:

1. Ignore Chart.appVersion and use Values.image.tag/Values.appVersion

@bacongobbler gave us a good snippet:

{{ default .Values.appVersion .Chart.AppVersion }}

Common approach, but feels weird, since we will have two appVersions, it is not conventional, makes harder to understand the codebase for a newbie. It creates a variety lf approaches for the same configurational problem, which should be simple and convenient, with one source of appVersion

2. Force the helm team to add ability to edit Chart.yaml using helm install arguments

It solves the problem in much more convenient approach when the first one: you will use the same variable, same flow, everything is perfect and shiny! But, it will mangle whole helm install thing with package-related arguments, will lead to possible bugs with override mechanics and, more important, break the logic and open a path for proposals to add more wild arguments to the install, leading to the hell of the bug reports and increased learning curve.

Immutability

I totally agree with the points @bacongobbler gave. First, immutability. If we will add arguments like so, what is the difference would be between the values and chart yaml files? What logic would be taken when someone will propose to add one more argument, then one more from the values? It'll be a chaos.

If we need Chart.yaml and values.yaml, we need to know why it is split. And we have some reasons:

Chart.yaml - for release variables, which are bound to the release as artifacts, like appVersion, which is used to glue multiple remote artifacts together. So whole ChartPackage will be a release, like a black box. It should not be accessible when we "install the software"

Values.yaml - variables and its defaults. Like arguments for the black box, for a function. It could be supplied, could be overridden. I would even call it arguments.yaml or defaults.yaml

Second, we already have approach which solves the problem, @bacongobbler snippet:

{{ default .Values.appVersion .Chart.AppVersion }}

The real problem

The real problem is, helm install is used wrong. More than that, it is implemented wrong. It should not allow to install the Chart Template, it should ask for a package instead.

Using helm install as helm package && helm install leads to the proposals to add helm package arguments to the helm install. And we already have --app-version argument in the helm package command! Why on earth you would need it anywhere else?!

Proposal

1. helm install should work only with packages
2. helm package-and-install could be implemented as a shortcut, but it will need to accept both package and install arguments

[eraac]

helm package doesn't allow to move the helm chart outside the repository (which is bad because developer can update all the helm-chart)

[wasker]

I'd like to add another vote for incorporating this change. We rarely change the chart itself, and only vary image tag and a bunch of meta-information about deployment. In our scenario helm list is not helpful by showing the version that baked into the chart vs. what's actually there.

Please accept this change.

[bacongobbler]

@iorlas I like your breakdown and your suggestions. Thank you for your insight and feedback, and you make a perfect point RE: this case has already been solved if you helm package --app-version 0.1.0 ./mychart && helm install ./mychart-VERSION.tgz --generate-name. Perhaps we should consider that as the path forward.

[iorlas]

helm package doesn't allow to move the helm chart outside the repository (which is bad because developer can update all the helm-chart)

Problem: helm package packages files into archive, so if we will package it and shove into the repository, it won't be easy to customise it, since you will need to extract it first, then package again and then publish.

@eraac, am I correct in understanding problem you describe?
What is the repository you are talking about? Helm repo or git repo? Since Helm repo is deprecated right now in favor of k8s facility, I assume you are talking about git repos.

So, again, we are not talking about the real problem. Currently Helm consists of multiple idioms. Especially: Chart, Package.

Package

Package is a build artifact, release, application. Package is supposed to be customisable using variables(set/values).

Treat it like a build: you have a binary or minimised js/css.

Think for a minute, are your application builds supposed to be customisable more than configuration allows? No

Chart

Chart is a template. It is like a source code. You can customise it, take some parts out of it, add another.

But when you change the source code, are you complaining you are not able to run is directly on you production environment, w/o compiling it first? We are able in some stacks, but under the hood it is the same compile-then-run, so you won't be able to run it w/o compiling

Sooo....

The real problem all you guys are struggling: you don't want to use Package idiom. You want to skip it when you are working on the non-public project, so it doesn't make sense to "create some reusable package"

And I'm with you all on it!

Although I feel like it is solid, clean way to distinguish build-specific and stage-specific configuration. I create package as build artifact, then installing it on multiple stages.

Use-case: You have small, pet-project, even a startup, you don't want to deal with such complications

Problem: You see Helm as orchestration utility, which consists of: templates, input values. And for you, there are no reasons to have anything beside that!

Reasoning

As per my previous comment, helm package && helm install is a way Helm works. There are no reasons to change that. It will simplify Helm a lot, but will leave packaging mechanism w/o many important features, leading to the more complicated process of installing foreign packages.

And, this exact pull request will break distinguish between package and install. It will make harder for newbies to learn the Helm. It will break the Helm!

Proposal

@bacongobbler, we need to simplify Helm, we need to support both medium/big and small projects. Small project don't need that. Project app could run the code using go run in production and it is fine, since development time does matter to test the idea.

In this effort, I would suggest to add new command

helm run

Arguments: helm package + helm upgrade --install, except some non-viable for such usage, like --repo or --devel, up to discussion

Function: render the templates state using all arguments, install it directly w/o creating the package archive

It will behave like helm upgrade --install, simplifying upgrade and install in exchange. It will work as helm package, but giving working k8s resources release as a product.

So new project won't have a need to deal with packages, upgrades: you can just helm create and helm run!

[iorlas]

By the way, there is one use-case described here #3555 (comment)

I ran into something where I need to pin to version 0.6.0 of an app, and the chart version has no relation to the app version.

But I see that helm run or helm package && helm install should solve it.

[wasker]

In our case it's not a "pet project", but a bunch of microservices that are following the same template (code, CI/CD, k8s infra) and share a bunch of CI/CD and k8s setup bits. All of them are owned by different teams and are deployed on separate schedules in same or different k8s clusters. There's really no need in publishing Helm packages, because those packages won't be consumed outside of CI/CD context which builds each microservice. Variables that are important for our scenario are: chart version (i.e. version of the template that is used to install the microservice) and an app version (i.e. version of the microservice which was installed with the template).

[iorlas]

@wasker Is isn't pet project, but it still needs "simplified" behavior: write some scripts, install/upgrade it directly. Am I right? It is what I'm trying to describe.

Still, package is not necessary sharable artifact, but it is the way to deliver your app into your cluster. Publishing is a like an additional feature allowed by such design.

How you set chart version? Are you ok with setting it manually? It supposed to be, since it is the "scrips version".

What would you think if you had such command as I described, helm run? So, helm install would install some remote/local package from the repo, but helm run will install your scripts w/o packaging, allowing to set all variables as cmd parameters?

And what would you think about using different commands to install remove package and install local scripts?

[wasker]

@iorlas I'm not attached to the actual command we'd have to use for that.

[gabemeola]

A "package and install" command would be very useful and simplify the workflow a bit. Currently I need to find the name and version of the packaged file. Having it "in one go" would be nice.

[iorlas]

Currently I need to find the name and version of the packaged file. Having it "in one go" would be nice.

Same thing here, so we had to hardcode in CI variables, changing it manually, otherwise it won't deploy.

I'm not helm contributor/maintainer, but maybe I'll be able to dedicate some time later this month for PoC. We really need it...

[bacongobbler]

Closing as we're not planning to move this PR forward. A command that wraps the logic of helm package and helm install would solve the use case originally described. Let's carry on with that discussion back in #3555. Thanks everyone!

[FloLaco]

According to @iorlas and @bacongobbler , there is a workaround if we use {{ default .Values.appVersion .Chart.AppVersion }}. Maybe it's a stupid question, but where to use it ? Chart.yaml does not accept any function.

Error: cannot load Chart.yaml: error converting YAML to JSON: yaml: invalid map key: map[interface {}]interface {}{"default .Values.appVersion .Chart.AppVersion":interface {}(nil)}

[f3man]

Oh, it is a pity.

My case is like that(IMO a lot of projects have the same)

I have a common chart for all my applications so the actual apps use this chart. However, the lifecycle of the app and the lifecycle of the chart are different. They are living in different repos and being deployed by different CI/CD pipelines
The chart has its own version and it is changed not so often. I thought Chart.version is supposed to represent chart version.
The app is changed very often. Chart.appVersion looks like an applicable field to represent the app version.

In my case helm install/upgrade --app-version $(Build.BuildId) would be perfect, but it looks like it is what it is