jedcunningham commented on a change in pull request #16117:
URL: https://github.com/apache/airflow/pull/16117#discussion_r641596500
##########
File path: chart/values.yaml
##########
@@ -919,19 +929,22 @@ config:
colored_console_log: 'False'
metrics:
statsd_on: '{{ ternary "True" "False" .Values.statsd.enabled }}'
- statsd_port: 9125
+ statsd_port: '{{ .Values._ports.statsdIngest }}'
Review comment:
```suggestion
statsd_port: '{{ .Values.ports.statsdIngest }}'
```
This should be the service port as Airflow will go through the service.
##########
File path: chart/values.schema.json
##########
@@ -2153,8 +2153,64 @@
}
}
},
+ "_ports": {
+ "description": "All container ports used by chart.",
+ "type": "object",
+ "x-docsSection": "Ports",
+ "additionalProperties": false,
+ "properties": {
+ "flowerUI": {
+ "description": "Flower UI port.",
+ "type": "integer",
+ "default": 5555,
+ "const": 5555
+ },
+ "airflowUI": {
+ "description": "Airflow UI port.",
+ "type": "integer",
+ "default": 8080,
+ "const": 8080
Review comment:
I know this will block changes to the port when values.yaml is validated
with values.schema.yaml, but having these "hard coded" in values.yaml where
people are expecting to find changeable parameters would definitely be
confusing.
If we do want to force them to use a certain container port, we should just
hard code it in the templates instead (and in most cases we are talking about
1, maybe 2 places if we use named ports in services). It'd be easy to block
attempted changes via `config.webserver.web_server_port`, but env vars are
harder to comprehensively block. Another plus, our values is big enough as it
is.
##########
File path: chart/values.yaml
##########
@@ -919,19 +929,22 @@ config:
colored_console_log: 'False'
metrics:
statsd_on: '{{ ternary "True" "False" .Values.statsd.enabled }}'
- statsd_port: 9125
+ statsd_port: '{{ .Values._ports.statsdIngest }}'
statsd_prefix: airflow
statsd_host: '{{ printf "%s-statsd" .Release.Name }}'
webserver:
+ web_server_port: '{{ .Values._ports.airflowUI }}'
enable_proxy_fix: 'True'
# For Airflow 1.10
rbac: 'True'
celery:
+ worker_log_server_port: '{{ .Values._ports.workerLogs }}'
+ flower_port: '{{ .Values._ports.flowerUI }}'
worker_concurrency: 16
scheduler:
# statsd params included for Airflow 1.10 backward compatibility; moved to
[metrics] in 2.0
statsd_on: '{{ ternary "True" "False" .Values.statsd.enabled }}'
- statsd_port: 9125
+ statsd_port: '{{ .Values._ports.statsdIngest }}'
Review comment:
```suggestion
statsd_port: '{{ .Values.ports.statsdIngest }}'
```
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
