jedcunningham commented on code in PR #31066: URL: https://github.com/apache/airflow/pull/31066#discussion_r1278115766
########## chart/values.yaml: ########## @@ -25,6 +25,21 @@ fullnameOverride: "" # Provide a name to substitute for the name of the chart nameOverride: "" +# Use standard naming for all resources using airflow.fullname template +# Consider removing this later and default it to true +# to make this chart follow standard naming conventions using the fullname template. +# For now this is an opt-in switch for backwards compatibility to leverage the standard naming convention +# and being able to use fully fullnameOverride and nameOverride in all resources +# For new installations - it is recommended to set it to True to follow standard naming conventions +# For existing installations, this will rename and redeploy your resources with the new names. Be aware that +# this will recreate your deployment/statefulsets along with their persistent volume claims and data storage +# migration may be needed to keep your old data +# +# Note:fernet-key, redis-password and broker-url secrets don't use this logic yet, Review Comment: ```suggestion # Note:fernet-key,redis-password and broker-url secrets don't use this logic yet, ``` nit ########## chart/values.yaml: ########## @@ -25,6 +25,21 @@ fullnameOverride: "" # Provide a name to substitute for the name of the chart nameOverride: "" +# Use standard naming for all resources using airflow.fullname template +# Consider removing this later and default it to true +# to make this chart follow standard naming conventions using the fullname template. +# For now this is an opt-in switch for backwards compatibility to leverage the standard naming convention +# and being able to use fully fullnameOverride and nameOverride in all resources +# For new installations - it is recommended to set it to True to follow standard naming conventions +# For existing installations, this will rename and redeploy your resources with the new names. Be aware that +# this will recreate your deployment/statefulsets along with their persistent volume claims and data storage +# migration may be needed to keep your old data +# +# Note:fernet-key, redis-password and broker-url secrets don't use this logic yet, +# as this may break existing installations due to how they get installed via pre-install hook. +# We may need to think what is the best way to handle renaming for these resources in a graceful way Review Comment: ```suggestion ``` ########## chart/INSTALL: ########## @@ -12,3 +12,28 @@ kubectl create namespace airflow ## Install the chart in that namespace helm install airflow -n airflow . + +# Naming conventions Review Comment: Can you move this section to the docs instead? Maybe on the homepage? ########## chart/newsfragments/31066.significant.rst: ########## @@ -0,0 +1,8 @@ +Support naming customization on helm chart resources, some resources may be renamed during upgrade + +This is a new opt-in switch ``useStandardNaming`` for backwards compatibility to leverage the standard naming convention and being able to use fully fullnameOverride and nameOverride in all resources. Review Comment: ```suggestion This is a new opt-in switch ``useStandardNaming``, for backwards compatibility, to leverage the standard naming convention, which allows full use of fullnameOverride and nameOverride in all resources. ``` ########## chart/newsfragments/31066.significant.rst: ########## @@ -0,0 +1,8 @@ +Support naming customization on helm chart resources, some resources may be renamed during upgrade + +This is a new opt-in switch ``useStandardNaming`` for backwards compatibility to leverage the standard naming convention and being able to use fully fullnameOverride and nameOverride in all resources. + +Only the following resources will be renamed using defaults ``useStandardNaming=false``: Review Comment: ```suggestion Only the following resources will be renamed using default of ``useStandardNaming=false``: ``` -- 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]
