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

Kubernetes worker pod doesn't use docker container entrypoint #12766

Merged
merged 5 commits into from
Dec 7, 2020

Conversation

dimberman
Copy link
Contributor

@dimberman dimberman commented Dec 2, 2020

Fixes issue on openshift caused by KubernetesExecutor pods not running
via the entrypoint script

Addresses #12602


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@boring-cyborg boring-cyborg bot added area:dev-tools area:helm-chart Airflow Helm Chart provider:cncf-kubernetes Kubernetes provider related issues labels Dec 2, 2020
@dimberman dimberman requested a review from ashb December 2, 2020 22:18
@ashb ashb added the full tests needed We need to run full set of tests for this PR to merge label Dec 3, 2020
@github-actions github-actions bot removed the full tests needed We need to run full set of tests for this PR to merge label Dec 3, 2020
@github-actions
Copy link

github-actions bot commented Dec 5, 2020

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@github-actions
Copy link

github-actions bot commented Dec 7, 2020

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@potiuk potiuk self-requested a review December 7, 2020 11:19
potiuk added a commit to PolideaInternal/airflow that referenced this pull request Dec 7, 2020
The change is backwards-compatible. It still allows to pass airflow
command without "airflow" as first parameter, but you can now
also pass "airflow" and the rest of the parameters will
be treated as "airflow" command parameters.

Documentation is updated to reflect the entrypoint behaviour
including _CMD option in SQL connections.

Part of apache#12762 and apache#12602

Partially extracted from  apache#12766
@ashb
Copy link
Member

ashb commented Dec 7, 2020

Please rebase onto mainline once #12878 has been merged.

potiuk added a commit that referenced this pull request Dec 7, 2020
The change is backwards-compatible. It still allows to pass airflow
command without "airflow" as first parameter, but you can now
also pass "airflow" and the rest of the parameters will
be treated as "airflow" command parameters.

Documentation is updated to reflect the entrypoint behaviour
including _CMD option in SQL connections.

Part of #12762 and #12602

Partially extracted from  #12766
Fixes issue on openshift caused by KubernetesExecutor pods not running
via the entrypoint script
if [[ ${AIRFLOW_COMMAND} =~ ^(scheduler|worker|flower)$ ]]; then
if [[ -n "${AIRFLOW__CELERY__BROKER_URL_CMD=}" ]]; then
if [[ ${AIRFLOW_COMMAND} =~ ^(scheduler|celery|worker|flower)$ ]]; then
if [[ -n "${AIRFLOW__CELERY__BROKER_URL_CMD=}" ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if [[ -n "${AIRFLOW__CELERY__BROKER_URL_CMD=}" ]]; then
if [[ -n "${AIRFLOW__CELERY__BROKER_URL_CMD=}" ]]; then

# The Bash and python commands still should verify the basic connections so they are run after the
# DB check but before the broker check

Copy link
Member

@ashb ashb Dec 7, 2020

Choose a reason for hiding this comment

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

Could you add back/keep this comment.

UPGRADING_TO_2.0.md Outdated Show resolved Hide resolved
UPGRADING_TO_2.0.md Outdated Show resolved Hide resolved
dimberman and others added 3 commits December 7, 2020 10:02
Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>
@dimberman dimberman merged commit 190066c into apache:master Dec 7, 2020
@dimberman dimberman deleted the fix-entrypoint-for-openshift branch December 7, 2020 21:54
potiuk added a commit that referenced this pull request Dec 13, 2020
The change is backwards-compatible. It still allows to pass airflow
command without "airflow" as first parameter, but you can now
also pass "airflow" and the rest of the parameters will
be treated as "airflow" command parameters.

Documentation is updated to reflect the entrypoint behaviour
including _CMD option in SQL connections.

Part of #12762 and #12602

Partially extracted from  #12766

(cherry picked from commit 4d44faa)
potiuk pushed a commit that referenced this pull request Dec 13, 2020
* Kubernetes worker pod doesn't use docker container entrypoint

Fixes issue on openshift caused by KubernetesExecutor pods not running
via the entrypoint script

* fix

* Update UPGRADING_TO_2.0.md

Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>

* fix UPDGRADING

* @ashb comments

Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>
(cherry picked from commit 190066c)
kaxil pushed a commit that referenced this pull request Jan 21, 2021
The change is backwards-compatible. It still allows to pass airflow
command without "airflow" as first parameter, but you can now
also pass "airflow" and the rest of the parameters will
be treated as "airflow" command parameters.

Documentation is updated to reflect the entrypoint behaviour
including _CMD option in SQL connections.

Part of #12762 and #12602

Partially extracted from  #12766

(cherry picked from commit 4d44faa)
kaxil pushed a commit that referenced this pull request Jan 21, 2021
* Kubernetes worker pod doesn't use docker container entrypoint

Fixes issue on openshift caused by KubernetesExecutor pods not running
via the entrypoint script

* fix

* Update UPGRADING_TO_2.0.md

Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>

* fix UPDGRADING

* @ashb comments

Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>
(cherry picked from commit 190066c)
kaxil pushed a commit that referenced this pull request Jan 22, 2021
The change is backwards-compatible. It still allows to pass airflow
command without "airflow" as first parameter, but you can now
also pass "airflow" and the rest of the parameters will
be treated as "airflow" command parameters.

Documentation is updated to reflect the entrypoint behaviour
including _CMD option in SQL connections.

Part of #12762 and #12602

Partially extracted from  #12766

(cherry picked from commit 4d44faa)
kaxil pushed a commit that referenced this pull request Jan 22, 2021
* Kubernetes worker pod doesn't use docker container entrypoint

Fixes issue on openshift caused by KubernetesExecutor pods not running
via the entrypoint script

* fix

* Update UPGRADING_TO_2.0.md

Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>

* fix UPDGRADING

* @ashb comments

Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>
(cherry picked from commit 190066c)
@dimberman dimberman added this to the Airflow 2.0.1 milestone Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dev-tools area:helm-chart Airflow Helm Chart provider:cncf-kubernetes Kubernetes provider related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kubernetes worker pod doesn't use docker container entrypoint, removing OpenShift support
3 participants