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

v3: helm template does not render NOTES.txt #6901

Open
vainikkaj opened this issue Nov 7, 2019 · 46 comments
Open

v3: helm template does not render NOTES.txt #6901

vainikkaj opened this issue Nov 7, 2019 · 46 comments
Labels
bug Categorizes issue or PR as related to a bug. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. keep open v3.x Issues and Pull Requests related to the major version v3

Comments

@vainikkaj
Copy link

Helm version:

$ h3 version
version.BuildInfo{Version:"v3.0.0-rc.3", GitCommit:"2ed206799b451830c68bff30af2a52879b8b937a", GitTreeState:"clean", GoVersion:"go1.13.4"}

Steps to recreate:

$ h3 create foo
Creating foo
$ h3 template foo -s templates/NOTES.txt --notes
Error: unknown flag: --notes
$ tree foo
foo
├── charts
├── Chart.yaml
├── templates
│   ├── deployment.yaml
│   ├── _helpers.tpl
│   ├── ingress.yaml
│   ├── NOTES.txt
│   ├── serviceaccount.yaml
│   ├── service.yaml
│   └── tests
│       └── test-connection.yaml
└── values.yaml

How feature works in Helm 2.x

$ helm template bar -x templates/NOTES.txt --notes
---
# Source: bar/templates/NOTES.txt
1. Get the application URL by running these commands:
 export POD_NAME=$(kubectl get pods --namespace default -l "app.kubernetes.io/name=bar,app.kubernetes.io/instance=release-name" -o jsonpath="{.items[0].metadata.name}")
 echo "Visit http://127.0.0.1:8080 to use your application"
 kubectl port-forward $POD_NAME 8080:80
@hickeyma
Copy link
Contributor

hickeyma commented Nov 7, 2019

@vainikkaj Good catch. This looks like it might have been a regression. It seems to have been removed in #5365. @bacongobbler Was this by design or unintentionally?

@hickeyma hickeyma added question/support v3.x Issues and Pull Requests related to the major version v3 labels Nov 7, 2019
@hickeyma
Copy link
Contributor

hickeyma commented Nov 7, 2019

@bacongobbler If unintentional, I'll push a PR for it.

@bacongobbler
Copy link
Member

this might've been an unintentional regression.

@thomastaylor312 thomastaylor312 added this to the 3.0.0 milestone Nov 7, 2019
@thomastaylor312 thomastaylor312 added the bug Categorizes issue or PR as related to a bug. label Nov 7, 2019
@hickeyma
Copy link
Contributor

hickeyma commented Nov 7, 2019

ok, will push a pr later or tomorrow morning

@hickeyma hickeyma self-assigned this Nov 8, 2019
@bacongobbler
Copy link
Member

bacongobbler commented Nov 11, 2019

Hey @hickeyma, do you think you'll have time to look into this before we cut 3.0? If not we should probably move this over to 3.1. Thanks!

@vainikkaj
Copy link
Author

I'm keeping my fingers crossed for 3.0 fix 😃

@hickeyma
Copy link
Contributor

@bacongobbler Sorry but due to some unexpected events, I have unable to get push it. After talking on slack, I will leave it to you as you said you might get it in.

@mattfarina mattfarina modified the milestones: 3.0.0, 3.1.0 Nov 12, 2019
@mattfarina
Copy link
Collaborator

I tried to fix this but found a nuance we need to talk about first. This is why I'm moving it to 3.1.0.

In Helm v2 when running helm template with the --notes flag it generates the notes for that chart and all sub-charts. Each is separated like the YAML parts are and the individual source is noted.

In Helm v2 when running helm install with the --render-subchart-notes all of the notes are combined into one output. For example, if you installed stable/wordpress using that flag the mariadb and wordpress notes would be back to back with no separator. Personally, I find this to be a confusing UX. I only noticed it now because I don't tend to use the --render-subchart-notes flag.

In Helm v3 the rendering process was based around the install process rather than a separate flow like Helm v2. In a deeply nested part of Helm the notes are generated and separated from the other manifests. If the sub-chart notes are rendered they are all combined the way helm v2 install presents them.

This means we have lost the ability to render each of the notes output as a separate file for helm template. The feature will not be exactly the same for v3.

I have some questions I would like feedback on...

  • What, if any, are the impacts of rendering one notes output instead of each of the subcharts separately?
  • Should there be headings between each of the charts/sub-charts when rendering notes? This would affect both helm template and helm install (and other commands)
  • On helm template should the sub-chart notes be hidden behind an addition flag like they are on helm install?

Feedback on this is appreciated so that we can make sure the experience provided by showing notes works moving forward.

@technosophos
Copy link
Member

Given this, I think @mattfarina is right in delaying this to 3.1. Trying to rush something out the door without thinking through the UX would lock us into an anit-pattern from now until Helm 4, since fixing it would require a breaking change.

technosophos added a commit to technosophos/k8s-helm that referenced this issue Dec 10, 2019
Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
technosophos added a commit that referenced this issue Dec 17, 2019
Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
@bacongobbler
Copy link
Member

We haven't heard of any activity/feedback from the OP on this ticket, so I'm going to remove this from the 3.1.0 milestone until further feedback to the concerns raised from the other maintainers have been addressed. Thanks!

@bacongobbler bacongobbler removed this from the 3.1.0 milestone Jan 27, 2020
@ttauveron
Copy link

As a temporary workaround, you can run the following command :

helm install chart --dry-run  --generate-name

This will render the NOTES.txt file.

mattfarina pushed a commit that referenced this issue Mar 23, 2020
Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
(cherry picked from commit ab79732)
@hickeyma hickeyma removed their assignment May 13, 2020
@brendarearden
Copy link

rancher/rancher#26999 we attempted to use the work around provided by @ttauveron but since --dry-run doesn't support CRDs we are still having difficulty displaying notes for all templates - would like to see --notes support added back in

@AceHack
Copy link

AceHack commented Oct 14, 2020

Yes would also like to see this, do we know when it can be fixed for template?

@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 Jan 14, 2021
@AceHack
Copy link

AceHack commented Jan 16, 2021

Bump

@Dentrax
Copy link

Dentrax commented Feb 23, 2022

Unfortunately this requires an actual cluster, unlike helm template.

Bump. It would be useful to test this in template command by passing a --notes flag. For CI testig purposes.

@kennedy
Copy link

kennedy commented Mar 29, 2022

Bump

@consideRatio
Copy link
Contributor

It is a relevant feature, but I think there is a need to help out drive development about it.

Current behavior of --render-subchart-notes

Its relevant to overview the current behavior if we consider changing it. I conclude that --render-subchart-notes is silently ignored when used in the helm template, but for helm install it is functioning correctly and as documented.

The documentation says if set, render subchart notes along with the parent, so practically, the documentation is also currently only valid for the helm install situation as the along with the parent part isn't true anyhow for when used with helm template.

Steps to create a dummy chart for debugging

# create helm chart "a" with nested charts
# a has subchart b and c
# b has subchart d
cd /tmp
helm create a && echo "a FINDME" > a/templates/NOTES.txt
cd a/charts
helm create b && echo "b FINDME" > b/templates/NOTES.txt
helm create c && echo "c FINDME" > c/templates/NOTES.txt
cd b/charts
helm create d && echo "d FINDME" > d/templates/NOTES.txt
cd ../../..

Reproduction of broken helm template --render-subchart-notes .

Nothing is rendered when --render-subchart-notes is used with helm template.

# check for any NOTES.txt being rendered
helm template --render-subchart-notes . | grep FINDME

Output from helm install --dry-run --render-subchart-notes test .

Note that the ordering of these, the ordering seems random.

<CHART A TEMPLATES>
NOTES:
<NOTES FROM CHART B>
<NOTES FROM CHART A>
<NOTES FROM CHART D>
<NOTES FROM CHART C>

What UX makes sense?

In #6901 (comment) @bacongobbler asked for input on what UX to go for.

My short-term UX suggestion

  • Add --render-notes flag for helm template and helm install alongside --render-subchart-notes to toggle if the root chart's notes should be rendered. Let --render-notes be false by default for helm template, but true by default for helm install, to match current behavior.
  • Update the help text for --render-subchart-notes note about its mention of along with the parent, something that should be controlled by render-notes now.
  • For now, stick with current behavior from helm install with how notes are to be rendered for helm template. The current behavior is to render notes after all other templates has been rendered, with the separator line NOTES:. The notes are rendered one after the other in a random order without additional new-line spacing or titles.

My long-term UX suggestion

I think we should go for the short-term suggestion above at first and not go straight to this, as this involves breaking changes etc.

When helm template is used, we can see the following separators between templates.

  • I think we should treat NOTES.txt as any other rendered template with separators that include a mention of where they come from.
    ---
    # Source: a/templates/tests/test-connection.yaml
    
  • I think NOTES.txt templates should be rendered last.
  • I think NOTES.txt from the main chart and its subcharts should be ordered deterministically when rendered, so that if we have a structure like a,b,c,d above the ordering would be a>b>d>c where d comes before c because it is a dependency of b which evaluates in full before c.

Now what?

  • Everyone: please provide feedback to the short-term UX proposal above.
  • Maintainers: please help with yes/no decisions - is there a proposal available that you encourage work towards at this point in time?
  • Maintainers: please help with who-does-what decisions - do you wish for someone in the community to work towards this, or do you wish to do it yourself?

@jglick
Copy link

jglick commented May 23, 2022

we should treat NOTES.txt as any other rendered template

If you mean to do so by default, this could break tooling which expects the output of helm template to be (multidocument) YAML. Would be safer to render it as comments (prepending # to every line):

---
# Source: xxx/templates/service-account.yaml
apiVersion: v1
kind: ServiceAccount
metadata:
  name: xxx
  namespace: xxx
---
# Source: xxx/templates/NOTES.txt
# Here is how you use this chart:
# Run: la la la

which seems to be tolerated

$ helm template xxx | yq e -ojson | jq -s
[
  {
    "apiVersion": "v1",
    "kind": "ServiceAccount",
    "metadata": {
      "name": "xxx",
      "namespace": "xxx"
    }
  },
  null
]

@PowerSurge1
Copy link

Whatever we do here, let's consider argoproj/argo-cd#4733 as a use case. It looks like ArgoCD uses 'helm template' and then applies the result, rather than executing helm directly. Looks like both sides want the NOTES.TXT available.

@Dentrax
Copy link

Dentrax commented Jun 14, 2022

Any roadmap planned here?

@sandipchitale
Copy link

I also wish there was a way to generate some manifests for any Kubernetes Resources or CRDs during regular install/upgrade/ rollback, but not actually deploy them. Maybe templates with special extension like .generate.yml. These will capture the exact set of .Values and .Capabilities and .Release objects. These could then be referenced in Notes.txt file. The output could go to some output directories that capture the release name and number as seen in helm history and install/upgrade/rollback flavor so that user knows which install/upgrade/rollback generated them. Such generated manifests could be invoked by the user later to run one-off jobs etc later to make some changes in deployed release but use consistently generated manifests.

@sandipchitale
Copy link

Actually, I just realized that can declare a manifest template in NOTES.txt and it will not be deployed but will be rendered:

NOTES.txt

Other notes stuff

---
# Release Name:      {{ .Release.Name }}
# Release Namespace: {{ .Release.Namespace }}
# Release Revision:  {{ .Release.Revision }}
# Release Install:   {{ .Release.IsInstall }}
# Release Upgrade:   {{ .Release.IsUpgrade }}
---
# templates/service.yaml
apiVersion: v1
kind: Service
metadata:
  name: {{ include "gen.fullname" . }}
  labels:
    {{- include "gen.labels" . | nindent 4 }}
spec:
  type: {{ .Values.service.type }}
  ports:
    - port: {{ .Values.service.port }}
      targetPort: http
      protocol: TCP
      name: http
  selector:
    {{- include "gen.selectorLabels" . | nindent 4 }}

krajorama added a commit to grafana/mimir that referenced this issue Jun 23, 2022
Add a NOTES.txt that prints the endpoints that will be usable by the user
for interacting with mimir/gem.

Currently helm does not support rendering the NOTES for testing.
Rage/vote here: helm/helm#6901

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
krajorama added a commit to grafana/mimir that referenced this issue Jun 23, 2022
* Helm: add NOTES.txt

Add a NOTES.txt that prints the endpoints that will be usable by the user
for interacting with mimir/gem.

Currently helm does not support rendering the NOTES for testing.
Rage/vote here: helm/helm#6901

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
@consideRatio
Copy link
Contributor

consideRatio commented Jun 28, 2022

This is a non-breaking proposal that addresses this issue.

Add --render-notes to helm [template|install]

  • --render-notes determines if top level chart NOTES.txt is rendered.
  • --render-subchart-notes determines if subchart NOTES.txt are rendered.
  • For backward compatibility, --render-notes is by default made false for helm template and true for helm install.

@eshepelyuk
Copy link

@consideRatio what about helm upgrade ?

masonmei pushed a commit to udmire/mimir that referenced this issue Jul 11, 2022
* Helm: add NOTES.txt

Add a NOTES.txt that prints the endpoints that will be usable by the user
for interacting with mimir/gem.

Currently helm does not support rendering the NOTES for testing.
Rage/vote here: helm/helm#6901

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
@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.

@alfsch
Copy link

alfsch commented Nov 28, 2022

For me this issue is still open

@jglick
Copy link

jglick commented Nov 28, 2022

Yes it was closed simply due to lack of activity.

It seems

- uses: actions/stale@v3.0.14
has not been updated to pick up actions/stale#789 which would at least let it show up in grey rather than purple. Maybe #11498 would fix that.

@eugentius
Copy link

/remove stale

@abdennour
Copy link

/reopen please

@Dentrax
Copy link

Dentrax commented Jan 20, 2023

/reopen
/cc @mattfarina

@joejulian joejulian reopened this Jan 20, 2023
@joejulian joejulian added keep open help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. and removed Stale labels Jan 20, 2023
@davinkevin
Copy link

+1 on this one. We have no way to test the code generated by NOTES.txt without a cluster connected (because install|upgrade requires a cluster, even if --dry-run and --debug are set)

@mohittalele
Copy link

any update on this issue ?

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. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. keep open v3.x Issues and Pull Requests related to the major version v3
Projects
None yet
Development

No branches or pull requests