Skip to content

Commit 78f1d59

Browse files
scottrigbyscottrigby
authored
Merge pull request #301 from z4ce/joboutput_hip
Add HIP for obtaining output from hook jobs and pods
2 parents 71a5de3 + 93255c6 commit 78f1d59

File tree

2 files changed

+255
-0
lines changed

2 files changed

+255
-0
lines changed

‎hips/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,4 @@ restricted markdown format and can be found in the
2727
- [hip-0015: Annotation for Images used by Helm Charts](hip-0015.md)
2828
- [hip-0017: Helm OCI MediaType Registration](hip-0017.md)
2929
- [hip-0018: Include repository URL and tarball digest in chart's release metadata](hip-0018.md)
30+
- [hip-0019: New annotations for displaying hook output](hip-0019.md)

‎hips/hip-0019.md‎

Lines changed: 254 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,254 @@
1+
---
2+
hip: 0019
3+
title: "New annotations for displaying hook output"
4+
authors: [ "Ian Zink <[email protected]>" ]
5+
created: "2023-01-26"
6+
type: "feature"
7+
status: "draft"
8+
---
9+
10+
## Abstract
11+
12+
This proposes a new annotation to indicate that the output from the hook should be displayed to the user.
13+
14+
## Motivation
15+
16+
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 a Helm chart can not be successfully installed.
17+
18+
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, a certain amount of ephemeral storage, memory, or CPUs available. You might want to validate the service key they provided was correct or that that database they entered is reachable. Letting a Helm install and/or upgrade fail, and then make the user debug this to see why it failed is a poor user experience. These things can all be done with checks enabled by the hook proposed in this HIP.
19+
20+
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.
21+
22+
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 a Helm install failure.
23+
24+
## Rationale
25+
26+
There are other ways that this could be implemented. For example, we could have a separate preflight hook type. However, this new hook type wouldn't be handled at all by previous versions of Helm. With this design, it requires minimal changes to Helm and allows for backwards compatibility.
27+
28+
Another strategy could be for Helm to include Troubleshoot.sh as a dependent library, but this could result in too tight of a coupling between the projects and lower overall flexibility and adaptability.
29+
30+
## Specification
31+
32+
Templates may include the following annotations on Jobs or Pods:
33+
34+
```yaml
35+
"helm.sh/hook": pre-install, pre-upgrade
36+
"helm.sh/hook-output-log-policy": hook-failed # or hook-succeeded or hook-failed,hook-succeeded
37+
```
38+
39+
40+
`helm.sh/hook-output-log-policy` would indicate that Helm should display the output of the Job to the user.
41+
42+
Additionally, a new user flag should be created `--no-log-output` that would skip the output of logs.
43+
44+
Additionally, there will be a new item added to the action SDK configuration to allow SDK consumers to get the output.
45+
By default this output will be discarded by writing to `io.Discard`, but an SDK consumer can overwrite the HookOutputFunc to provide a custom writer.
46+
47+
48+
```go
49+
type Configuration struct {
50+
...
51+
// Called with container name and returns and expects writer that will receive the log output
52+
HookOutputFunc func(namespace, pod, container string) io.Writer
53+
}
54+
```
55+
56+
## Backwards compatibility
57+
58+
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. The logs would be written to `stderr`, further mitigating any breaking changes to scripts using `helm` CLI.
59+
60+
## Security implications
61+
62+
Potentially the preflight checks could check for security misconfigurations that could enhance the security of the chart deployment.
63+
64+
## How to teach this
65+
66+
In the first instance, documentation plus the help text for `helm install` would explain the feature.
67+
68+
An example template could be provided in documentation showing how to use this feature with a generic command used in a hook.
69+
70+
A more advanced example showing how to use the new feature with Troubleshoot.sh to provide preflight checks could be linked in the documentation, provided directly in the documentation, or provided on the Troubleshoot.sh documentation site independently.
71+
72+
## Reference implementation
73+
74+
[Pull Request for Documentation ](https://github.com/helm/helm-www/pull/1242)
75+
76+
[Pull Request for Helm](https://github.com/helm/helm/pull/10309) - most upvoted open PR
77+
78+
79+
## Rejected ideas
80+
N/A
81+
82+
## Open issues
83+
N/A
84+
85+
## References
86+
87+
[Troubleshoot.sh](https://troubleshoot.sh/) - the tool that is the motivation for this HIP.
88+
89+
[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.
90+
91+
## Reference - Examples Usage
92+
93+
### Example using `false`
94+
95+
Template:
96+
```yaml
97+
apiVersion: batch/v1
98+
kind: Job
99+
metadata:
100+
name: "{{ .Release.Name }}-false-job"
101+
labels:
102+
app.kubernetes.io/managed-by: {{ .Release.Service | quote }}
103+
app.kubernetes.io/instance: {{ .Release.Name | quote }}
104+
app.kubernetes.io/version: {{ .Chart.AppVersion }}
105+
helm.sh/chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
106+
annotations:
107+
"helm.sh/hook": pre-install, pre-upgrade
108+
"helm.sh/hook-output-log-policy": hook-failed, hook-suceeded
109+
"helm.sh/hook-weight": "-5"
110+
"helm.sh/hook-delete-policy": hook-succeeded, hook-failed
111+
112+
spec:
113+
backoffLimit: 0
114+
template:
115+
metadata:
116+
name: "{{ .Release.Name }}"
117+
labels:
118+
app.kubernetes.io/managed-by: {{ .Release.Service | quote }}
119+
app.kubernetes.io/instance: {{ .Release.Name | quote }}
120+
helm.sh/chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
121+
spec:
122+
restartPolicy: Never
123+
containers:
124+
- name: post-install-job
125+
image: "alpine:3.18"
126+
command: ["sh", "-c", "echo foo ; false"]
127+
```
128+
129+
What it should loook when running:
130+
131+
```text
132+
$ helm install ./ my-release
133+
Logs for pod: my-release-false-job-bgbz6, container: pre-install-job
134+
foo
135+
Error: INSTALLATION FAILED: failed pre-install: job failed: BackoffLimitExceeded
136+
```
137+
138+
### Example using Troubleshoot Preflight Checks
139+
140+
```yaml
141+
apiVersion: batch/v1
142+
kind: Job
143+
metadata:
144+
name: "{{ .Release.Name }}-preflight-job"
145+
labels:
146+
app.kubernetes.io/managed-by: {{ .Release.Service | quote }}
147+
app.kubernetes.io/instance: {{ .Release.Name | quote }}
148+
app.kubernetes.io/version: {{ .Chart.AppVersion }}
149+
helm.sh/chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
150+
annotations:
151+
"helm.sh/hook": pre-install, pre-upgrade
152+
"helm.sh/hook-output-log-policy": hook-failed
153+
"helm.sh/hook-weight": "-5"
154+
"helm.sh/hook-delete-policy": hook-succeeded, hook-failed
155+
156+
spec:
157+
backoffLimit: 0
158+
template:
159+
metadata:
160+
name: "{{ .Release.Name }}"
161+
labels:
162+
app.kubernetes.io/managed-by: {{ .Release.Service | quote }}
163+
app.kubernetes.io/instance: {{ .Release.Name | quote }}
164+
helm.sh/chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
165+
spec:
166+
restartPolicy: Never
167+
volumes:
168+
- name: preflights
169+
configMap:
170+
name: "{{ .Release.Name }}-preflight-config"
171+
containers:
172+
- name: post-install-job
173+
image: "replicated/preflight:latest"
174+
command: ["preflight", "--interactive=false", "/preflights/preflights.yaml"]
175+
volumeMounts:
176+
- name: preflights
177+
mountPath: /preflights
178+
179+
---
180+
apiVersion: v1
181+
kind: ConfigMap
182+
metadata:
183+
annotations:
184+
"helm.sh/hook": pre-install, pre-upgrade
185+
"helm.sh/hook-weight": "-6"
186+
"helm.sh/hook-delete-policy": hook-succeeded, hook-failed
187+
labels:
188+
app.kubernetes.io/managed-by: {{ .Release.Service | quote }}
189+
app.kubernetes.io/instance: {{ .Release.Name | quote }}
190+
app.kubernetes.io/version: {{ .Chart.AppVersion }}
191+
helm.sh/chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
192+
name: "{{ .Release.Name }}-preflight-config"
193+
data:
194+
preflights.yaml: |
195+
apiVersion: troubleshoot.sh/v1beta2
196+
kind: Preflight
197+
metadata:
198+
name: preflight-tutorial
199+
spec:
200+
collectors:
201+
{{ if eq .Values.mariadb.enabled false }}
202+
- mysql:
203+
collectorName: mysql
204+
uri: '{{ .Values.externalDatabase.user }}:{{ .Values.externalDatabase.password }}@tcp({{ .Values.externalDatabase.host }}:{{ .Values.externalDatabase.port }})/{{ .Values.externalDatabase.database }}?tls=false'
205+
{{ end }}
206+
analyzers:
207+
- clusterVersion:
208+
outcomes:
209+
- fail:
210+
when: "< 1.16.0"
211+
message: The application requires at least Kubernetes 1.16.0, and recommends 1.18.0.
212+
uri: https://kubernetes.io
213+
- warn:
214+
when: "< 1.18.0"
215+
message: Your cluster meets the minimum version of Kubernetes, but we recommend you update to 1.18.0 or later.
216+
uri: https://kubernetes.io
217+
- pass:
218+
message: Your cluster meets the recommended and required versions of Kubernetes.
219+
{{ if eq .Values.mariadb.enabled false }}
220+
- mysql:
221+
checkName: Must be MySQL 8.x or later
222+
collectorName: mysql
223+
outcomes:
224+
- fail:
225+
when: connected == false
226+
message: Cannot connect to MySQL server
227+
- fail:
228+
when: version < 8.x
229+
message: The MySQL server must be at least version 8
230+
- pass:
231+
message: The MySQL server is ready
232+
{{ end }}
233+
```
234+
235+
Which should yield the following output to stdout:
236+
237+
```text
238+
$ helm install ./ my-release
239+
Logs for pod: my-release-preflight-job-bgbz6, container: pre-install-job
240+
--- FAIL: Required Kubernetes Version
241+
--- The application requires at least Kubernetes 1.16.0, and recommends 1.18.0.
242+
--- FAIL: Must be MySQL 8.x or later
243+
--- Cannot connect to MySQL server
244+
--- FAIL preflight-tutorial
245+
FAILED
246+
name: cluster-resources status: completed completed: 1 total: 3
247+
name: mysql/mysql status: running completed: 1 total: 3
248+
name: mysql/mysql status: completed completed: 2 total: 3
249+
name: cluster-info status: running completed: 2 total: 3
250+
name: cluster-info status: completed completed: 3 total: 3
251+
252+
Error: INSTALLATION FAILED: failed pre-install: job failed: BackoffLimitExceeded
253+
254+
```

0 commit comments

Comments
 (0)