Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The individual components should be deployable without setting the executor first #583

Open
4 tasks done
karakanb opened this issue May 18, 2022 · 2 comments
Open
4 tasks done
Labels
kind/enhancement kind - new features or changes status/needs-discussion status - this needs discussion

Comments

@karakanb
Copy link
Contributor

Checks

Motivation

I am trying to transition from KubernetesExecutor to CeleryKubernetesExecutor gradually. My goal is to do this transition as safe as possible without any disruptions, and do every change incrementally.

What I want to do is:

  • Enable workers, deploy, ensure they can connect to Redis and they are working fine.
  • Enable flower, deploy, ensure all is good.
  • Change all my tasks to run on kubernetes queue, so that they'll behave the same way.
  • Change the executor config to CeleryKubernetesExecutor
  • Test individual tasks piece by piece by changing the queue.
  • Finally, if all is well, roll it out fully.

I would like to be able to do this, because at each step I want to make sure all is good before I transition the pipelines, and I have a mixed set of workloads that I would like to keep on Kubernetes, while ensuring that after the rollout everything will keep running the same way.

As of now, I am not able to do this gradual switch because of the simple check on the helm chart.

Error: UPGRADE FAILED: execution error at (airflow/templates/_helpers/validate-values.tpl:43:5): If `airflow.executor=KubernetesExecutor`, then all of [`workers.enabled`, `flower.enabled`, `redis.enabled`] should be `false`!

Implementation

Remove the check for disabling workers / redis / flower.

Are you willing & able to help?

  • I am able to submit a PR!
  • I can help test the feature!
@karakanb karakanb added the kind/enhancement kind - new features or changes label May 18, 2022
@thesuperzapper
Copy link
Member

@karakanb The values validations are intended to ensure people don't deploy non-sensible combinations of values in the chart (as there are MANY options).

Is there a reason why you can't set your airflow.executor to CeleryKubernetesExecutor at the same time as your other changes to enable flower/redis/etc?

I believe you can change your DAG tasks to use the kubernetes queue while still using the KubernetesExecutor, so that you can enable CeleryKubernetesExecutor without some of the tasks unexpectedly swapping to the Celery workers, either by:

  1. Setting the AIRFLOW__OPERATORS__DEFAULT_QUEUE to kubernetes
  2. Manually set queue="kubernetes" in ALL your DAG task definitions

@karakanb
Copy link
Contributor Author

I can do that, but that means that I have to deploy all of the changes at once, including potentially blocking the dag runs in case something is wrong, e.g. a task being forgotten to moved to a default queue, or a task setting an explicit queue somewhere; therefore, it seems to be the safer option to be able to incrementally deploy things and ensure they work step by step without breaking the current flow.

Would you be interested in a solution like skip-checks or sth similar that would allow skipping those checks for cases like this?

@thesuperzapper thesuperzapper added this to Unsorted in Issue Triage and PR Tracking via automation Jun 6, 2022
@thesuperzapper thesuperzapper added the status/needs-discussion status - this needs discussion label Jun 6, 2022
@thesuperzapper thesuperzapper moved this from Unsorted to Triage | Needs Discussion in Issue Triage and PR Tracking Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement kind - new features or changes status/needs-discussion status - this needs discussion
Projects
Issue Triage and PR Tracking
Triage | Needs Discussion
Development

No branches or pull requests

2 participants