fix: Retains the name of the ingress for backwards compatibility

[j1wonpark]

When an existing user upgrades from Helm chart 1.10.0 to 1.11.0-dev, the ingress name is different, so they will try to create a new one. And finally it fails. (#34196)
Therefore, to ensure backward compatibility, add airflow- to the middle of the ingress name to maintain the name before 1.10.0.

---

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

[boring-cyborg]

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:

• Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits will help you with that.
• In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
• Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
• Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
• Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
• Be sure to read the Airflow Coding style.
• Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
  Apache Airflow is a community-driven project and together we are making it better 🚀.
  In case of doubts contact the developers at:
  Mailing List: dev@airflow.apache.org
  Slack: https://s.apache.org/airflow-slack

[potiuk]

Thanks for opening the issue and PR, but I believe this is not the right way to fix it @phil-park

I believe the problem is in fullname definition. I think the problem is that if you use "airflow" as Release.Name the new "airflow.fullname" macro shortens to just .Release.Name. I think that was the intention @fgalind1 - but it indeed causes backwards compatibility problem.

Was there any particular reason to shorten the fullname in this case @fgalind1 other than avoiding accidental duplication? I am not sure if there ia any way to avoid backwards compatibilty problems, but my vote would go to removing the shortening and leaving "airflow-airflow" if someone uses "airflow" release name (even if it looks duplicate).

{{- define "airflow.fullname" -}}
  {{- if not .Values.useStandardNaming }}
    {{- .Release.Name }}
  {{- else if .Values.fullnameOverride }}
    {{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }}
  {{- else }}
    {{- $name := default .Chart.Name .Values.nameOverride }}
    {{- if contains $name .Release.Name }}.   # <---------- HERE 
      {{- .Release.Name | trunc 63 | trimSuffix "-" }}
    {{- else }}
      {{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }}
    {{- end }}
  {{- end }}
{{- end }}

[fgalind1]

I think the problem is that if you use "airflow" as Release.Name the new "airflow.fullname" macro shortens to just .Release.Name.

yep that's correct

Was there any particular reason to shorten the fullname in this case @fgalind1 other than avoiding accidental duplication? I am not sure if there ia any way to avoid backwards compatibilty problems, but my vote would go to removing the shortening and leaving "airflow-airflow" if someone uses "airflow" release name (even if it looks duplicate).

Just to make it standard as that is the most common pattern I've seen in mostly all upstream charts for the fullname implementation, e.g:

• https://github.com/bitnami/charts/blob/main/bitnami/common/templates/_names.tpl#L31-L32
• https://github.com/jfrog/charts/blob/master/stable/artifactory/templates/_helpers.tpl#L25-L26
• https://github.com/prometheus-community/helm-charts/blob/main/charts/prometheus/templates/_helpers.tpl#L56-L57

Maybe if we want to keep it for backwards compatibility we could add an option that opts-in/out that if condition like .Values.disableShortenFullname or .Values.allowShortenFullname? Maybe to something like:

{{- define "airflow.fullname" -}}
  {{- if not .Values.useStandardNaming }}
    {{- .Release.Name }}
  {{- else if .Values.fullnameOverride }}
    {{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }}
  {{- else }}
    {{- $name := default .Chart.Name .Values.nameOverride }}
    {{- if and .Values.disableShortenFullname (contains $name .Release.Name) }}.
      {{- .Release.Name | trunc 63 | trimSuffix "-" }}
    {{- else }}
      {{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }}
    {{- end }}
  {{- end }}
{{- end }}

@potiuk any thoughts?

[potiuk]

@potiuk any thoughts?

Good for me, though this inflates options we have :)

[j1wonpark]

I'm actually using the name airflow-airflow-ingress. I know this is actually a bad name, but it's hard to risk removing ingress and recreating it. (There are quite a few airflows in operation.) I would appreciate it if backward compatibility was provided.

[potiuk]

I'm actually using the name airflow-airflow-ingress. I know this is actually a bad name, but it's hard to risk removing ingress and recreating it. (There are quite a few airflows in operation.) I would appreciate it if backward compatibility was provided.

Yeah. I wish there was an easy way to provide new defaults, while keeping the old ones if someone used them in the past.

[jedcunningham]

@fgalind1, is this list incorrect then and we missed the ingress changes, or is this only a problem when useStandardNaming is enabled?

[fgalind1]

@fgalind1, is this list incorrect then and we missed the ingress changes, or is this only a problem when useStandardNaming is enabled?

@jedcunningham that's a good point, and yes ingress should be included there. I've made a PR #34354 to cover that and clarify when that happens at least with the current implementation at this moment

[hussein-awala]

I wonder if we should close this PR and the linked issue (#34196) since we've merged #34354

[j1wonpark]

I found out that the reason why renaming (recreating) the ingress failed was because I used Helm's wait option.
It's okay to close this PR and issue.