You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "amoghrajesh (via GitHub)" <gi...@apache.org> on 2023/02/20 05:38:15 UTC
[GitHub] [airflow] amoghrajesh opened a new pull request, #29624: Can't configure Kubernetes and Celery workers in Helm Chart
amoghrajesh opened a new pull request, #29624:
URL: https://github.com/apache/airflow/pull/29624
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #28880
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
The "workers" section in the values.yaml is only used to configure the celery executor and celerykubernetes executors. We should split the common section of the workers section into celery vs kubernetes so that configuration is easier.
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
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](https://github.com/apache/airflow/tree/main/newsfragments).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #29624: Can't configure Kubernetes and Celery workers in Helm Chart
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29624:
URL: https://github.com/apache/airflow/pull/29624#issuecomment-1437306653
> I believe some of the `CeleryKubernetesExecutor` users use the same configuration for celery and k8s, if we separate the configurations in two sections, they will need to duplicate them. With my suggestion, they can keep them identical, or override them/some of them in the section k8s_pod_template
Hmm. I think the whole reason to have CeleryK8S executor was to make them "different" - I think characteristics (memory, CPU etc. ) of "always running" celery worker Pod - which usually has N Python interpreter started (celery parallelism) is pretty much always different than K8S Pod (which always runs 1 task). I think it would be rather counter-productive to commonalize those settings
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] closed pull request #29624: Can't configure Kubernetes and Celery workers in Helm Chart
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #29624: Can't configure Kubernetes and Celery workers in Helm Chart
URL: https://github.com/apache/airflow/pull/29624
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #29624: Can't configure Kubernetes and Celery workers in Helm Chart
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #29624:
URL: https://github.com/apache/airflow/pull/29624#issuecomment-1504306705
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.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] hussein-awala commented on pull request #29624: Can't configure Kubernetes and Celery workers in Helm Chart
Posted by "hussein-awala (via GitHub)" <gi...@apache.org>.
hussein-awala commented on PR #29624:
URL: https://github.com/apache/airflow/pull/29624#issuecomment-1437300580
@amoghrajesh or @potiuk can you please update the description and link the PR to the issue?
I think we can add a new value `k8s_pod_template` and use it when the executor is `CeleryKubernetesExecutor`:
- if the executor is `CeleryExecutor`: load conf from `.Values.workers`
- if the executor is `CeleryKubernetesExecutor`: load celery worker conf from `.Values.workers` and pod template conf from `.Values.k8s_pod_template` if it exists, and if not we load them from `.Values.workers` or `.Values` as we do for now
- if the executor is `KubernetesExecutor`: we load pod template conf from `.Values.k8s_pod_template` if exists, and if not we load them from `.Values.workers` or `.Values`
I believe some of the `CeleryKubernetesExecutor` users use the same configuration for celery and k8s, if we separate the configurations in two sections, they will need to duplicate them. With my suggestion, they can keep them identical, or override them/some of them in the section k8s_pod_template
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #29624: Can't configure Kubernetes and Celery workers in Helm Chart
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29624:
URL: https://github.com/apache/airflow/pull/29624#issuecomment-1437263567
I think the idea in the #28800 was to be able to specify some parameters in the Helm Chart Values in a similar way for K8S as it si for celery. What it would likely mean - is merging the parameters specified in the values.yaml into the pod_template_file (providing defaults) and making it possible to run K8S executor even without a pod template file.
The pod-template file is nice and super flexible, but as I imagine this one - it would be a nice way to configure most of the POD parameters in a very similar way for Celery and K8S pods, which seems to me like a good idea. It is kind of strange (byt justified in most complex cases) to have the parameters specified in pod_template_file. but having a static values (defaults?) specified in Helm chart would make it much easier to configure the installation for simple cases where pod template file flexibility is not really needed.
@jedcunningham @dimberman @dstandish - WDYT about such a feature?
This was - I think the original idea behind this feature (at least this is how I understood it). A
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] amoghrajesh commented on pull request #29624: Can't configure Kubernetes and Celery workers in Helm Chart
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on PR #29624:
URL: https://github.com/apache/airflow/pull/29624#issuecomment-1441543267
@hussein-awala @potiuk I kinda like the idea of separating the differences between CeleryK8sExecutor and CeleryExecutor. @hussein-awala's idea seems reasonable but requires a fair bit of work to be done while installing if you need to specify the properties. What consensus should we arrive at?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #29624: Can't configure Kubernetes and Celery workers in Helm Chart
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29624:
URL: https://github.com/apache/airflow/pull/29624#issuecomment-1445151205
I think... Let's try what @hussein-awala proposes :)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #29624: Can't configure Kubernetes and Celery workers in Helm Chart
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29624:
URL: https://github.com/apache/airflow/pull/29624#issuecomment-1437309061
(BTW. @amoghrajesh - adding `Closes #PR` inside comment does not work :)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] amoghrajesh commented on pull request #29624: Can't configure Kubernetes and Celery workers in Helm Chart
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on PR #29624:
URL: https://github.com/apache/airflow/pull/29624#issuecomment-1437079339
Thanks for explaining that @hussein-awala
Made the changes in sync with what was asked in the issue description: #28880. Maybe I understood the issue wrong. Could you go through that issue and explain to me where I went wrong?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] hussein-awala commented on pull request #29624: Can't configure Kubernetes and Celery workers in Helm Chart
Posted by "hussein-awala (via GitHub)" <gi...@apache.org>.
hussein-awala commented on PR #29624:
URL: https://github.com/apache/airflow/pull/29624#issuecomment-1436973598
I think this is not necessary, because the Kubernetes workers are not created by the helm chart, it just creates and configures the celery workers.
For the K8S workers, they are created by the schedulers (a worker for each task), and you can configure them by providing a pod template in [the configuration](https://airflow.apache.org/docs/apache-airflow/stable/configurations-ref.html#pod-template-file) or override the [pod_template](https://airflow.apache.org/docs/apache-airflow/stable/core-concepts/executor/kubernetes.html#pod-override) for a single task.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] amoghrajesh commented on pull request #29624: Can't configure Kubernetes and Celery workers in Helm Chart
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on PR #29624:
URL: https://github.com/apache/airflow/pull/29624#issuecomment-1437336039
Thanks, my bad. Pulled it out of the comment @potiuk
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org