Chart: Allow hybrid executors

[LipuFei]

Airflow 2.10.0 has the new feature to use multiple executors. The value schema check in the current Helm chart doesn't allow this because it has a strict list of allowed values. I removed this enum check list to enable custom and combinations of executors.

Another change is the executor label in the scheduler Deployment. I removed this label because it has no functional use. Reasons are:

1. Characters such as : are not allowed in label values. This prevents me from using short names such as airflow.providers.amazon.aws.executors.ecs.AwsEcsExecutor:AwsEcsExecutor
2. The label value can have max 63 characters. Even if (1) is okay, we still need to do some string truncating.

Another change is the executor label in the scheduler Deployment. Because Kubernetes allows max 63 characters for label values, and hybrid executor values can easily exceed this limit, therefore I added a trunc -63 to prevent this. This value has no functional use AFAIK, so doing a truncate here will not cause any functional problems.

[hussein-awala]

I think we can sanitize the value or add it when it contains only acceptable characters and when it is shorter than 64 characters.

[LipuFei]

In my humble opinion, sanitizing this value is a little too much work (complexity) for something that has no functional use. I'd prefer to remove it then making it too complex...

[hussein-awala]

What about adding two new values (executors which is an array of executors, and defaultExecutor) and deprecate this one 🤔

[LipuFei]

Just realise we are talking about the same thing... 😄

What if we keep this as a list of enums? Then, in the value, we concatenate them with , into a comma separated list? Downside is, this is still a fixed list of executors. You cannot specify custom executors or use short names such as airflow.providers.amazon.aws.executors.ecs.AwsEcsExecutor:AwsEcsExecutor.

[jedcunningham]

We have a ton of conditionals like if CeleryExecutor so we can provision the right stuff for the release, but you've not accounted for any of that with this change. Try it with CeleryExecutor,KubernetesExecutor, for example, to see the problems this brings.

I think @hussein-awala idea of an array of executors makes sense, but I agree the shortname/enum thing is definitely a problem. We will need to come up with something here.

[LipuFei]

We have a ton of conditionals like if CeleryExecutor so we can provision the right stuff for the release, but you've not accounted for any of that with this change. Try it with CeleryExecutor,KubernetesExecutor, for example, to see the problems this brings.

I think @hussein-awala idea of an array of executors makes sense, but I agree the shortname/enum thing is definitely a problem. We will need to come up with something here.

I agree. After some digging, I indeed see a lot of references to .Values.executor everywhere, not to mention the executor references to .Values.config.core.executor and they need to be the same...

I will work on this slowly, excluding the short name support. It's a lot more complex than I initially expected.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.