villebro commented on code in PR #426:
URL:
https://github.com/apache/superset-kubernetes-operator/pull/426#discussion_r4250287962
##########
charts/superset-operator/values.schema.json:
##########
@@ -52,6 +52,20 @@
"required": ["resourceName", "restartPolicy"]
}
},
+ "extraEnv": {
+ "description": "Extra environment variables for the manager container.
WATCH_NAMESPACE is reserved for the watch block. See
https://pkg.go.dev/k8s.io/api/core/v1#EnvVar";,
+ "type": "array",
+ "items": {
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "name": { "type": "string", "minLength": 1, "not": { "const":
"WATCH_NAMESPACE" } },
Review Comment:
Helm only reports `at '/extraEnv/0/name': 'not' failed` here, so the user
never sees why the name is rejected. Could we do this check in the template
instead, next to the existing replicas/leaderElection `fail`?
```suggestion
"name": { "type": "string", "minLength": 1 },
```
And in `templates/deployment.yaml`, after the existing `fail`:
```yaml
{{- range .Values.extraEnv }}
{{- if eq .name "WATCH_NAMESPACE" }}
{{- fail "extraEnv must not set WATCH_NAMESPACE; configure watch.scope and
watch.namespaces instead" }}
{{- end }}
{{- end }}
```
That fails with `extraEnv must not set WATCH_NAMESPACE; configure
watch.scope and watch.namespaces instead`. The test in
`schema_strictness_test.yaml` would then move next to the replicas test in
`optout_and_helpers_test.yaml` and check the message with `errorMessage`.
##########
charts/superset-operator/values.yaml:
##########
@@ -30,6 +30,11 @@ resources:
# See
[corev1.ContainerResizePolicy](https://pkg.go.dev/k8s.io/api/core/v1#ContainerResizePolicy).
resizePolicy: []
+# -- Extra environment variables for the manager container, such as proxy
settings or `GOMEMLIMIT`.
Review Comment:
The operator's only outbound traffic is to the Kubernetes API server, and
client-go honours `HTTPS_PROXY`. Setting a proxy without a `NO_PROXY` that
covers the API server address would send API calls through the proxy, so I'd
rather not lead with that example:
```suggestion
# -- Extra environment variables for the manager container, such as
`GOMEMLIMIT` or `GODEBUG`.
```
The chart README will need regenerating afterwards with `make codegen`.
##########
docs/reference/releases.md:
##########
@@ -27,6 +27,7 @@ This page tracks notable changes in Apache Superset
Kubernetes Operator releases
- **Helm `dnsPolicy` and `dnsConfig`.** The Helm chart now exposes `dnsPolicy`
and `dnsConfig` values for the operator manager pod
([#394](https://github.com/apache/superset-kubernetes-operator/pull/394),
[@younsl](https://github.com/younsl)).
- **Helm `priorityClassName`.** The Helm chart now exposes a
[`priorityClassName`](https://kubernetes.io/docs/concepts/scheduling-eviction/pod-priority-preemption/)
value that sets the PriorityClass of the operator manager pods. Unset by
default, which keeps the cluster default priority
([#420](https://github.com/apache/superset-kubernetes-operator/pull/420),
[@younsl](https://github.com/younsl)).
+- **Helm `extraEnv`.** The Helm chart now exposes an `extraEnv` value that
appends environment variables to the operator manager container, for example
proxy settings or a `GOMEMLIMIT` derived from the memory limit.
`WATCH_NAMESPACE` is rejected by the values schema because the `watch` block
owns it
([#426](https://github.com/apache/superset-kubernetes-operator/pull/426),
[@younsl](https://github.com/younsl)).
Review Comment:
Matching both changes above:
```suggestion
- **Helm `extraEnv`.** The Helm chart now exposes an `extraEnv` value that
appends environment variables to the operator manager container, for example a
`GOMEMLIMIT` derived from the memory limit. `WATCH_NAMESPACE` is rejected
because the `watch` block owns it
([#426](https://github.com/apache/superset-kubernetes-operator/pull/426),
[@younsl](https://github.com/younsl)).
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
