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

implement KubernetesExecutor #311

Merged
merged 54 commits into from
Aug 23, 2023
Merged

implement KubernetesExecutor #311

merged 54 commits into from
Aug 23, 2023

Conversation

adwk67
Copy link
Member

@adwk67 adwk67 commented Aug 1, 2023

Description

Implements KubernetesExecutor.
Fixes #2.
Jenkins: 🟢 https://ci.stackable.tech/view/02%20Operator%20Tests%20(custom)/job/airflow-operator-it-custom/99/

Implementation summary:

  • the executor field has been removed from the top-level cluster config
  • the workers role has been replaced with a complex enum specifying one of:
    • celery: this contains the worker config/roles etc.
    • kubernetes: this contains just the logging/resources of the config, plus env-overrides (has to be included as we don't have a role object)
  • when executor = kubernetes, a pod template is deployed as config map that airflow will use to spawn pods for the DAG tasks

Tests on Openshift:

--- PASS: kuttl (3580.00s)
    --- PASS: kuttl/harness (0.00s)
        --- PASS: kuttl/harness/logging_airflow-2.4.1_openshift-true (282.85s)
        --- PASS: kuttl/harness/ldap_airflow-latest-2.6.1_ldap-authentication-insecure-tls_openshift-true_executor-celery (321.02s)
        --- PASS: kuttl/harness/mount-dags-gitsync_airflow-latest-2.6.1_openshift-true_executor-kubernetes (215.58s)
        --- PASS: kuttl/harness/logging_airflow-2.2.5_openshift-true (305.99s)
        --- PASS: kuttl/harness/logging_airflow-2.2.4_openshift-true (328.94s)
        --- PASS: kuttl/harness/mount-dags-configmap_airflow-latest-2.6.1_openshift-true_executor-kubernetes (202.57s)
        --- PASS: kuttl/harness/mount-dags-configmap_airflow-latest-2.6.1_openshift-true_executor-celery (290.25s)
        --- PASS: kuttl/harness/orphaned-resources_airflow-latest-2.6.1_openshift-true (318.29s)
        --- PASS: kuttl/harness/smoke_airflow-2.2.5_openshift-true_executor-kubernetes (292.73s)
        --- PASS: kuttl/harness/smoke_airflow-2.4.1_openshift-true_executor-kubernetes (267.12s)
        --- PASS: kuttl/harness/smoke_airflow-2.6.1_openshift-true_executor-celery (284.75s)
        --- PASS: kuttl/harness/smoke_airflow-2.4.1_openshift-true_executor-celery (335.22s)
        --- PASS: kuttl/harness/smoke_airflow-2.6.1_openshift-true_executor-kubernetes (226.21s)
        --- PASS: kuttl/harness/cluster-operation_airflow-latest-2.6.1_openshift-true (303.22s)
        --- PASS: kuttl/harness/resources_airflow-latest-2.6.1_openshift-true (209.99s)
        --- PASS: kuttl/harness/smoke_airflow-2.2.4_openshift-true_executor-celery (343.50s)
        --- PASS: kuttl/harness/smoke_airflow-2.2.5_openshift-true_executor-celery (395.81s)
        --- PASS: kuttl/harness/smoke_airflow-2.2.4_openshift-true_executor-kubernetes (314.30s)
        --- PASS: kuttl/harness/logging_airflow-2.6.1_openshift-true (239.35s)
        --- PASS: kuttl/harness/ldap_airflow-latest-2.6.1_ldap-authentication-server-verification-tls_openshift-true_executor-celery (311.55s)
        --- PASS: kuttl/harness/ldap_airflow-latest-2.6.1_ldap-authentication-no-tls_openshift-true_executor-kubernetes (202.59s)
        --- PASS: kuttl/harness/ldap_airflow-latest-2.6.1_ldap-authentication-no-tls_openshift-true_executor-celery (296.19s)
        --- PASS: kuttl/harness/ldap_airflow-latest-2.6.1_ldap-authentication-insecure-tls_openshift-true_executor-kubernetes (246.54s)
        --- PASS: kuttl/harness/mount-dags-gitsync_airflow-latest-2.6.1_openshift-true_executor-celery (251.68s)
        --- PASS: kuttl/harness/ldap_airflow-latest-2.6.1_ldap-authentication-server-verification-tls_openshift-true_executor-kubernetes (200.25s)

Definition of Done Checklist

  • Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant
  • Please make sure all these things are done and tick the boxes

Author

Reviewer

Acceptance

@adwk67 adwk67 linked an issue Aug 1, 2023 that may be closed by this pull request
@adwk67 adwk67 marked this pull request as ready for review August 14, 2023 09:52
@adwk67 adwk67 requested a review from a team August 14, 2023 09:53
@adwk67
Copy link
Member Author

adwk67 commented Aug 18, 2023

Copy link
Member

@razvan razvan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works and it's ok but IMO:

  • ExecutorConfig vs AirflowConfig - the names are confusing don't reflect the intent.
  • Not sure if it's wise to introduce so much complication just because of the affinity issue of executors. But fortunately it's an implementation detail that we can hopefully change later without breaking the CRD.

tests/templates/kuttl/mount-dags-gitsync/03-assert.yaml.j2 Outdated Show resolved Hide resolved
rust/operator-binary/src/airflow_controller.rs Outdated Show resolved Hide resolved
docs/modules/airflow/pages/index.adoc Outdated Show resolved Hide resolved
@razvan
Copy link
Member

razvan commented Aug 22, 2023

This is almost adding support for complete new product. It may be worth adding an explicit integration test for kubernetes workers and actually run a some tasks with it. This is already done.

adwk67 and others added 2 commits August 23, 2023 09:45
Co-authored-by: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com>
Co-authored-by: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com>
adwk67 and others added 2 commits August 23, 2023 09:49
Co-authored-by: Razvan-Daniel Mihai <84674+razvan@users.noreply.github.com>
@adwk67 adwk67 added this pull request to the merge queue Aug 23, 2023
Merged via the queue into main with commit 8164085 Aug 23, 2023
28 of 29 checks passed
@adwk67 adwk67 deleted the feat/kubernetes-executor branch August 23, 2023 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend Airflow operator by implementing KubernetesExecutor
2 participants