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

AIRFLOW-6062 Watch worker pods from all namespaces #8546

Merged
merged 1 commit into from Apr 26, 2020

Conversation

mppetkov
Copy link
Contributor

@mppetkov mppetkov commented Apr 24, 2020

The scheduler currently monitors pods only from the Kubernetes namespace provided in the configuration. The initial fix has been implemented in this PR - #7123 but it looks like it was not actually a complete fix.
I am extending the previous fix with additional change in the KubernetesJobWatcher to watch all namespaces in the Kubernetes cluster.


Make sure to mark the boxes below before creating PR: [x]

  • Description above provides context of the change
  • Unit tests coverage for changes (not needed for documentation changes)
  • Target Github ISSUE in description if exists
  • Commits follow "How to write a good git commit message"
  • Relevant documentation is updated including usage instructions.
  • I will engage committers as explained in Contribution Workflow Example.

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.
Read the Pull Request Guidelines for more information.

@dimberman dimberman added this to the Airflow 1.10.11 milestone Apr 26, 2020
@dimberman dimberman merged commit a8bcc1a into apache:master Apr 26, 2020
@boring-cyborg
Copy link

boring-cyborg bot commented Apr 26, 2020

Awesome work, congrats on your first merged pull request!

@dimberman
Copy link
Contributor

Thank you for adding this @mppetkov!

@mppetkov mppetkov deleted the AIRFLOW-6062 branch April 26, 2020 16:25
@hcheng4
Copy link

hcheng4 commented May 13, 2020

[Not a contribution]

Hi folks,

This PR is going to break some Airflow deployments that do not have cluster-scoped RBAC to list all pods in all namespaces.

Are there any active discussions around how to handle this better, or are we going to expect all Airflow KubernetesExecutor-based deployments to have cluster-scoped roles to list across all namespaces? This is a change to our security model and will probably prevent us from being able to pick up 1.10.11.

@dimberman
Copy link
Contributor

@hcheng4 that's a really good point I noticed that in the tests. We need to have a larger discussion (maybe make this a "multi-namespace" mode)

@hcheng4
Copy link

hcheng4 commented Jul 1, 2020

[Not a contribution]

@dimberman a multi-namespace mode sounds like a great idea. Is there a discussion already ongoing? If there's agreement design-wise, I can see if there's appetite and bandwidth here for a patch contribution.

It may be the case that longer-term the restrictions of an in-namespace Kubernetes deployment are too much and it's undesirable to support that going forward, but that doesn't feel like the type of compatibility-breaking change that's appropriate for a patch release IMHO.

@mppetkov
Copy link
Contributor Author

mppetkov commented Jul 2, 2020

One more vote for the generic multi-namespace approach, it would be amazing if we have functionality like that. Actually, that's why I have contributed the fix. In my company we need to run each worker Pod into a specific namespace based on a condition.

From what I understand, Airflow needs to provide the following options in the airflow.cfg to make this approach generic.

- default_namespace: 'namespace1'
Airflow will create each worker Pod in this specific namespace unless the value is not overwritten.

- allowed_namespaces: ['*']
Airflow can create a worker Pod in all namespaces (probably the whitelist property should have * as a value)

- allowed_namespaces: ['namespace1', 'namespace2', 'namespace3']
Airflow can create a worker Pod in any namespace that is part of the allowed_namespaces property.

- denied_namespaces: ['namespace4', 'namespace5', 'namespace6']
Airflow cannot create a worker Pod in any namespace that is part of the denied_namespaces property.

What do you think about this approach?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Scheduler Scheduler or dag parsing Issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants