Improve Helm post-setup NOTES #11269

soumynathan · 2020-04-30T21:12:44Z

This PR fixes the NOTES.txt file to reflight the correct
action at the upper level, when preflight alone is installed

Fixes: #11245
Signed-off-by: Swaminathan Vasudevan svasudevan@suse.com

maintainer-s-little-helper · 2020-04-30T21:12:46Z

Please set the appropriate release note label.

coveralls · 2020-04-30T21:39:26Z

Coverage decreased (-0.02%) to 44.443% when pulling f33e8df on soumynathan:helm-prints-misleading-notes-with-preflight-alone into 4bdb361 on cilium:master.

joestringer

Nice work, the output you posted on the other issue looks accurate and helpful.

We might consider adding an extra piece of notes below this chunk to mention Hubble; other than that I don't think any of the toplevel charts need specific logic to generate different notes.

Two more comments below on phrasing.

joestringer · 2020-04-30T22:00:45Z

install/kubernetes/cilium/templates/NOTES.txt

@@ -1,4 +1,10 @@
-You have successfully installed {{ title .Chart.Name }}.
+{{- if (and (.Values.preflight.enabled) (not (.Values.agent.enabled)) (not (.Values.config.enabled)) (not (.Values.operator.enabled))) }}
+    You have successfully completed the prefight check.


I'm actually wondering about the phrasing here. The only thing we know was successful at this point in time is that the YAMLs were generated and injected into the cluster.

Looking at the preflight check documentation, we actually suggest using another command to monitor the status of the prefilght check until it becomes successful. I wonder if those instructions would be appropriate to include in the snippet here?

Will check it out and see what makes sense.

install/kubernetes/cilium/templates/NOTES.txt

soumynathan · 2020-04-30T22:09:36Z

Nice work, the output you posted on the other issue looks accurate and helpful.

We might consider adding an extra piece of notes below this chunk to mention Hubble; other than that I don't think any of the toplevel charts need specific logic to generate different notes.

Two more comments below on phrasing.

You are right, I missed the hubble-ui, since it has already merged, we can add that too.

install/kubernetes/cilium/charts/hubble-ui/templates/NOTES.txt

joestringer · 2020-05-01T00:17:13Z

install/kubernetes/cilium/templates/NOTES.txt

+    You have successfully ran the prefight check.
+    Now make sure to check the number of READY pods is the same as the number of running cilium pods.
+    Then make sure the cilium preflight deployment is also marked READY 1/1.
+    If you have an issues please refer to the CNP Validation section in the upgrade guide.


I recognize that the user likely already has the upgrade docs up at this point so this should make sense without needing to add a direct link to the upgrade guide.

aanm · 2020-05-01T16:27:37Z

test-me-please

install/kubernetes/cilium/charts/hubble-ui/templates/NOTES.txt

gandro

Blocking the merge until https://github.com/cilium/cilium/pull/11269/files#r419407275 has been discussed.

This PR fixes the NOTES.txt file to reflect the correct action at the upper level, when preflight alone is installed. Also adds the NOTES.txt file that was missing for hubble-ui and handles the hubble-ui and hubble-cli text in the upper level NOTES.txt file. Fixes: cilium#11245 Signed-off-by: Swaminathan Vasudevan <svasudevan@suse.com>

gandro

Awesome, thank you!

gandro · 2020-05-05T08:43:14Z

test-me-please

gandro · 2020-05-12T08:14:32Z

Note to backporters: When backporting this commit, please do not backport the file creation of NOTES.txt in the hubble-ui folder, only the changes to the cilium chart. We don't have the hubble-ui chart in the 1.7 branch.

soumynathan requested a review from a team April 30, 2020 21:12

maintainer-s-little-helper bot added the dont-merge/needs-release-note label Apr 30, 2020

maintainer-s-little-helper bot added this to In progress in 1.8.0 Apr 30, 2020

joestringer added the release-note/misc This PR makes changes that have no direct user impact. label Apr 30, 2020

maintainer-s-little-helper bot removed the dont-merge/needs-release-note label Apr 30, 2020

joestringer changed the title ~~Helm prints misleading NOTES upon preflight install~~ Improve Helm post-setup NOTES Apr 30, 2020

joestringer requested changes Apr 30, 2020

View reviewed changes

soumynathan force-pushed the helm-prints-misleading-notes-with-preflight-alone branch from 9b522ba to e333e39 Compare April 30, 2020 22:51

joestringer added the needs-backport/1.7 label Apr 30, 2020

maintainer-s-little-helper bot added this to Needs backport from master in 1.7.4 Apr 30, 2020

joestringer reviewed Apr 30, 2020

View reviewed changes

install/kubernetes/cilium/charts/hubble-ui/templates/NOTES.txt Outdated Show resolved Hide resolved

joestringer requested a review from a team April 30, 2020 23:51

joestringer approved these changes May 1, 2020

View reviewed changes

joestringer reviewed May 1, 2020

View reviewed changes

aanm approved these changes May 1, 2020

View reviewed changes

gandro reviewed May 4, 2020

View reviewed changes

install/kubernetes/cilium/charts/hubble-ui/templates/NOTES.txt Outdated Show resolved Hide resolved

maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 4, 2020

gandro requested changes May 4, 2020

View reviewed changes

gandro removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 4, 2020

soumynathan force-pushed the helm-prints-misleading-notes-with-preflight-alone branch from e333e39 to f33e8df Compare May 4, 2020 17:34

joestringer requested a review from gandro May 4, 2020 19:45

gandro approved these changes May 5, 2020

View reviewed changes

aanm added the area/helm Impacts helm charts and user deployment experience label May 5, 2020

maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 5, 2020

joestringer merged commit 5f246ba into cilium:master May 5, 2020

1.8.0 automation moved this from In progress to Merged May 5, 2020

christarazi mentioned this pull request May 8, 2020

v1.7 backports 2020-05-08 #11441

Merged

christarazi added backport-pending/1.7 and removed needs-backport/1.7 labels May 8, 2020

maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.7 in 1.7.4 May 8, 2020

christarazi removed the backport-pending/1.7 label May 9, 2020

maintainer-s-little-helper bot removed this from Backport pending to v1.7 in 1.7.4 May 9, 2020

christarazi added the needs-backport/1.7 label May 9, 2020

maintainer-s-little-helper bot added this to Needs backport from master in 1.7.4 May 9, 2020

qmonnet mentioned this pull request May 12, 2020

v1.7 backports 2020-05-12 #11488

Merged

qmonnet added backport-pending/1.7 and removed needs-backport/1.7 labels May 12, 2020

maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.7 in 1.7.4 May 12, 2020

christarazi added backport-done/1.7 and removed backport-pending/1.7 labels May 13, 2020

maintainer-s-little-helper bot moved this from Backport pending to v1.7 to Backport done to v1.7 in 1.7.4 May 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve Helm post-setup NOTES #11269

Improve Helm post-setup NOTES #11269

soumynathan commented Apr 30, 2020

maintainer-s-little-helper bot commented Apr 30, 2020

coveralls commented Apr 30, 2020 •

edited

joestringer left a comment

joestringer Apr 30, 2020

soumynathan Apr 30, 2020

soumynathan commented Apr 30, 2020

joestringer May 1, 2020

aanm commented May 1, 2020

gandro left a comment

gandro left a comment

gandro commented May 5, 2020

gandro commented May 12, 2020 •

edited

Improve Helm post-setup NOTES #11269

Improve Helm post-setup NOTES #11269

Conversation

soumynathan commented Apr 30, 2020

maintainer-s-little-helper bot commented Apr 30, 2020

coveralls commented Apr 30, 2020 • edited

joestringer left a comment

Choose a reason for hiding this comment

joestringer Apr 30, 2020

Choose a reason for hiding this comment

soumynathan Apr 30, 2020

Choose a reason for hiding this comment

soumynathan commented Apr 30, 2020

joestringer May 1, 2020

Choose a reason for hiding this comment

aanm commented May 1, 2020

gandro left a comment

Choose a reason for hiding this comment

gandro left a comment

Choose a reason for hiding this comment

gandro commented May 5, 2020

gandro commented May 12, 2020 • edited

coveralls commented Apr 30, 2020 •

edited

gandro commented May 12, 2020 •

edited