[AIRFLOW-3281] Fix Kubernetes operator with git-sync

[dimberman]

Make sure you have checked all steps below.

Jira

• My PR addresses the following Airflow Jira issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"
  • https://issues.apache.org/jira/browse/AIRFLOW-3281
  • In case you are fixing a typo in the documentation you can prepend your commit with [AIRFLOW-XXX], code changes always need a Jira issue.

Description

• Here are some details about my PR, including screenshots of any UI changes:

Currently the implementation of git-sync is broken because:

• git-sync clones the repository in /tmp and not in airflow-dags volume
• git-sync add a link to point to the revision required but it is not
  taken into account in AIRFLOW__CORE__DAGS_FOLDER

Dags/logs hostPath volume has been added (needed if airflow run in
kubernetes in local environment)

In this PR I also removed worker_dags_folder config because IMO is redundant. In fact is possible to define it in dags_folder using a specific airflow-configmap for the workers

Tests

• My PR adds the following unit tests OR does not need testing for this extremely good reason:

To avoid false positive in CI load_examples is set to False
otherwise DAGs from airflow/example_dags are always loaded. In this
way is possible to test import in DAGs.

To do so KubernetesExecutorTest now uses a specific folder airflowexample_dags_kubernetes to run test instead of airflow/example_dags and the DAG example_kubernetes_annotation imports a an external module

Commits

• My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
  1. Subject is separated from body by a blank line
  2. Subject is limited to 50 characters (not including Jira issue reference)
  3. Subject does not end with a period
  4. Subject uses the imperative mood ("add", not "adding")
  5. Body wraps at 72 characters
  6. Body explains "what" and "why", not "how"

Documentation

• In case of new functionality, my PR adds documentation that describes how to use it.
  • When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added.
  • All the public functions and the classes in the PR contain docstrings that explain what it does

Code Quality

• Passes flake8

[dimberman]

@kaxil How should I name/describe this commit? Should I just change it to the name/description of #3770?

[dimberman]

@kaxil Let's make sure we're still doing codecov once the tests pass. Not sure if removing this would turn it off.

[dimberman]

Oh wait... is none of the docker-compose stuff in 1-10-test?

[odracci]

@dimberman I tried to merge #3770 into 1.10-test in this branch https://github.com/odracci/incubator-airflow/tree/1-10-with-git-sync-v2
The build is green, only flake8 fails

[kaxil]

Thank you guys, yes @dimberman you can just change it to the name/description of #3770? And yup docker-compose stuff is not in this branch

[dimberman]

@kaxil I'm seeing a lot of flake8 issues. Is it possible that some commit changed our flake standards from when these PRs were made? Does flake matter for this minor release since these are just cherry picks?

[dimberman]

I'm fine to fix the flake8 with an extra commit but also don't know whether we want to add in commits that aren't in master to the test branches.

[kaxil]

It is fine to add flake8 commit as it just changes code formatting. The next release would be 1.11 or 2.0 which would be a direct branch of the master. So it is good.

[dimberman]

Great! I'll have time for that either tonight or tomorrow night (@odracci you're also welcome to take a stab at it should be pretty simple. Just lemme know if you do so I don't duplicate work).

[odracci]

@kaxil @dimberman The missing piece is #3772
I cherry-picked that PR in my branch but I still see (less) flake8 issues.
Problem is that PR has been merged on August 21st and it changed a lot of files, result is that PR overwrites already merged code.
At this point I'm not even sure if I did the right choice when I resolved the conflicts in my branch 🤞
@dimberman please proceed with your solution

[kaxil]

Sorry, I merged it and had to revert which caused this piece to fail. May well have to create a new PR.

[feng-tao]

@kaxil, what is the issue?

[kaxil]

Trying to resolve conflicts :D so that we can include #3770 and your DAG-level access commit

[feng-tao]

hey @kaxil , that's great news. I think we may need to revert this commit(b5b9287#diff-a7b22c07c43739c8eb0850a6fd6f7eb8) in v1-10-test branch as it is already included in master branch and cherry-pick the same commit from master branch. I think the original author creates a separate commit for v10.1 release. Once that commit is reverted, thing should be much easier. And we should include this critical fix as well(bf45855) once the commit is resolved.

[feng-tao]

@kaxil, thanks for running the release :)

[kaxil]

@feng-tao I have included that commit as well.

@dimberman I have spent some time today and cherry-picked and resolved some conflicts (with some help from this PR - thank you guys), it would be great if you can verify if everything that was needed is there. And then we will try to resolve issues with tests if any.

[kaxil]

Travis has passed on v1-10-test (Travis link) after 2 sleepless nights 🎉 🎉 .

All of the changes for Kuberentes and DAG Level Access has been included.

@dimberman @feng-tao

[feng-tao]

wow!!! Thanks for the great work @kaxil for driving the release!