ashb commented on a change in pull request #14152:
URL: https://github.com/apache/airflow/pull/14152#discussion_r573663354
##########
File path: chart/templates/_helpers.yaml
##########
@@ -15,6 +15,32 @@
# specific language governing permissions and limitations
# under the License.
+{{/*
+Expand the name of the chart.
+*/}}
+{{- define "airflow.name" -}}
+{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }}
+{{- end }}
+
+{{/*
+Create a default fully qualified app name.
+We truncate at 63 chars because some Kubernetes name fields are limited to
this (by the DNS naming spec).
+If release name contains chart name it will be used as a full name.
+*/}}
+{{- define "airflow.fullname" -}}
+{{- if .Values.fullnameOverride }}
Review comment:
This is not in the chart README.
##########
File path: chart/templates/rbac/pod-cleanup-role.yaml
##########
@@ -18,7 +18,7 @@
################################
## Airflow Cleanup Role
#################################
-{{- if and .Values.rbacEnabled .Values.cleanup.enabled }}
+{{- if and .Values.rbac.create .Values.cleanup.enabled }}
Review comment:
Why `rbac.create` here, but `cleanup.serviceAccount.create` elsewhere?
More specifically: why do we need _two_ different config options for each
role we create?
##########
File path: chart/templates/_helpers.yaml
##########
@@ -15,6 +15,32 @@
# specific language governing permissions and limitations
# under the License.
+{{/*
+Expand the name of the chart.
+*/}}
+{{- define "airflow.name" -}}
+{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }}
+{{- end }}
+
+{{/*
+Create a default fully qualified app name.
+We truncate at 63 chars because some Kubernetes name fields are limited to
this (by the DNS naming spec).
+If release name contains chart name it will be used as a full name.
+*/}}
+{{- define "airflow.fullname" -}}
+{{- if .Values.fullnameOverride }}
Review comment:
Why do we need two separate override values?
##########
File path: chart/templates/_helpers.yaml
##########
@@ -15,6 +15,32 @@
# specific language governing permissions and limitations
# under the License.
+{{/*
+Expand the name of the chart.
+*/}}
+{{- define "airflow.name" -}}
+{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }}
Review comment:
`nameOverride` is not in the chart readme.
----------------------------------------------------------------
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]
