diff --git a/hips/hip-9999.md b/hips/hip-9999.md index 45e3c006..524dd825 100644 --- a/hips/hip-9999.md +++ b/hips/hip-9999.md @@ -1,6 +1,6 @@ --- hip: 9999 -title: "New annotations for displaying job output" +title: "New annotations for displaying hook output" authors: [ "Ian Zink " ] created: "2023-01-26" type: "feature" @@ -9,15 +9,17 @@ status: "draft" ## Abstract -This proposes a new annotation to indicate that the output from the job should be displayed to the user. +This proposes a new annotation to indicate that the output from the hook should be displayed to the user. ## Motivation -The primary motivation for this HIP is the ability to display Preflight checks before the main chart installs. This provides vital feedback to the user as to why the chart install is not proceeding. +The one motivation for this HIP is the ability to display Preflight checks before the main chart installs. This provides vital feedback to the user as to why the chart install is not proceeding. -Often it is important to verify that the kubernetes cluster you are deploying a helm chart into has certain properties. You might need to know that the cluster is of a certain version to use various APIs. You might need to know that it has ingress available, a certain amount of ephemeral storage, memory, or CPUs available. You might want to validate the the service key they provided was correct or that that database they entered is reachable. Letting a chart deploy and then finding debugging to see why it failed is a poor user experience. These things can all be done with preflight checks enabled by the hook proposed in this HIP. +Often it is important to verify that the kubernetes cluster you are deploying a helm chart into has certain properties. You might need to know that it has an ingress controller available, a certain amount of ephemeral storage, memory, or CPUs available. You might want to validate the the service key they provided was correct or that that database they entered is reachable. Letting a chart deploy and then finding debugging to see why it failed is a poor user experience. These things can all be done with preflight checks enabled by the hook proposed in this HIP. -In general, allowing chart developers to run jobs and present that feedback directly to the users could also open up additional use cases beyond just the preflight use case that motivated this HIP. I could imagine scenarios where maybe CVE warnings are presented or specific upgrade feedback is presented instead of just helm install failure. +Another common motivation for this HIP is to indicate that database migration output running as a hook should be visible to the user in the case of failure. + +In general, allowing chart developers to run jobs and present that feedback directly to the users could also open up additional use cases beyond just the preflight use case that motivated this HIP. I could imagine scenarios where maybe CVE warnings are presented or specific upgrade feedback is presented instead of just helm install failure. ## Rationale @@ -39,6 +41,18 @@ Templates could include the following annotations on Batch Jobs: Additionally a new user flag should be created `--no-log-output` that would skip the output of logs. +Additionally there will be a new item added to the action SDK configuration to allow SDK consumers to get the output. +By default this output will be written to stdout, but an SDK consumer can overwrite the HookOutputFunc to provide a custom writer. + + +```go +type Configuration struct { + ... +// Called with container name and returns and expects writer that will receive the log output + HookOutputFunc func(namespace string, pod string, container string) io.Writer +} +``` + ## Backwards compatibility The only backwards compatibility concern would be that scripts parsing `helm install` output would see some additional text in the case of logs being output. The fact that notes already make the output unstructured should mitigate any concern here. Since we already are trusting chart developers to provide output in the form of notes, this is a logical extension of that that allows the developer to provide more dynamic output. @@ -57,7 +71,10 @@ A more advanced example showing how to use the new feature with Troubleshoot.sh ## Reference implementation -The `safe-install` plugin (link in references) demonstrates what running preflights could look like, but not in the fashion implemented in this HIP. +[Pull Request for Documentation ](https://github.com/helm/helm-www/pull/1242) + +[Pull Request for Helm](https://github.com/helm/helm/pull/10309) - most upvoted open PR + ## Rejected ideas N/A @@ -66,10 +83,6 @@ N/A N/A ## References -[Pull Request for Documentation ](https://github.com/helm/helm-www/pull/1242) - -[Pull Request for Helm](https://github.com/helm/helm/pull/10309) - third most upvoted open PR - [Troubleshoot.sh](https://troubleshoot.sh/) - the tool that is the motivation for this HIP. [safe-install plugin](https://github.com/z4ce/helm-safe-install) - Plugin that provides a similiar experience to what I hope this HIP will provide natively. @@ -83,7 +96,7 @@ Template: apiVersion: batch/v1 kind: Job metadata: - name: "{{ .Release.Name }}"-false-job + name: "{{ .Release.Name }}-false-job" labels: app.kubernetes.io/managed-by: {{ .Release.Service | quote }} app.kubernetes.io/instance: {{ .Release.Name | quote }} @@ -91,7 +104,7 @@ metadata: helm.sh/chart: "{{ .Chart.Name }}-{{ .Chart.Version }}" annotations: "helm.sh/hook": pre-install, pre-upgrade - "helm.sh/hook-output-log-policy": hook-failed + "helm.sh/hook-output-log-policy": hook-failed, hook-suceeded "helm.sh/hook-weight": "-5" "helm.sh/hook-delete-policy": hook-succeeded, hook-failed @@ -108,8 +121,8 @@ spec: restartPolicy: Never containers: - name: post-install-job - image: "alpine:3.3" - command: ["bash", "-c", "echo foo ; false"] + image: "alpine:3.18" + command: ["sh", "-c", "echo foo ; false"] ``` What it should loook when running: