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-5681] Allow specification of a tag or hash for the git_sync init container #6350

Merged
merged 1 commit into from
Dec 17, 2019

Conversation

george-miller
Copy link
Contributor

Make sure you have checked all steps below.

Jira

Description

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

We want to deploy dags based on tags on a branch, and want to use the git_sync init container in our kuberenetes setup to clone the dag folder at the start of a task run.

Tests

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

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.
    • All the public functions and the classes in the PR contain docstrings that explain what it does
    • If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

@OmerJog
Copy link
Contributor

OmerJog commented Nov 6, 2019

@george-miller
The test you added failed:


55) FAIL: test_make_pod_git_sync_rev (tests.kubernetes.test_worker_configuration.TestKubernetesWorkerConfiguration)
----------------------------------------------------------------------
   Traceback (most recent call last):
    tests/kubernetes/test_worker_configuration.py line 444 in test_make_pod_git_sync_rev
      'The git_sync_rev env did not get into the init container')
   AssertionError: {'name': 'GIT_SYNC_REV',
    'value': <MagicMock name='mock.git_sync_credentials_secret.to_dict()' id='140691298444512'>,
    'value_from': None} not found in [{'name': 'GIT_SYNC_REPO',
    'value': <MagicMock name='mock.git_repo.to_dict()' id='140691299285424'>,
    'value_from': None}, {'name': 'GIT_SYNC_BRANCH',
    'value': <MagicMock name='mock.git_branch.to_dict()' id='140691292267800'>,
    'value_from': None}, {'name': 'GIT_SYNC_ROOT',
    'value': <MagicMock name='mock.git_sync_root.to_dict()' id='140691300804312'>,
    'value_from': None}, {'name': 'GIT_SYNC_DEST', 'value': 'repo', 'value_from': None}, {'name': 'GIT_SYNC_REV', 'value': 'sampletag', 'value_from': None}, {'name': 'GIT_SYNC_DEPTH', 'value': '1', 'value_from': None}, {'name': 'GIT_SYNC_ONE_TIME', 'value': 'true', 'value_from': None}, {'name': 'GIT_SYNC_USERNAME',
    'value': <MagicMock name='mock.git_user.to_dict()' id='140691301425448'>,
    'value_from': None}, {'name': 'GIT_SYNC_PASSWORD',
    'value': <MagicMock name='mock.git_password.to_dict()' id='140691301994848'>,
    'value_from': None}, {'name': 'GIT_SYNC_USERNAME',
    'value': None,
    'value_from': {'config_map_key_ref': None,
                   'field_ref': None,
                   'resource_field_ref': None,
                   'secret_key_ref': {'key': 'GIT_SYNC_USERNAME',
                                      'name': <MagicMock name='mock.git_sync_credentials_secret.to_dict()' id='140691298444512'>,
                                      'optional': None}}}, {'name': 'GIT_SYNC_PASSWORD',
    'value': None,
    'value_from': {'config_map_key_ref': None,
                   'field_ref': None,
                   'resource_field_ref': None,
                   'secret_key_ref': {'key': 'GIT_SYNC_PASSWORD',
                                      'name': <MagicMock name='mock.git_sync_credentials_secret.to_dict()' id='140691298444512'>,
                                      'optional': None}}}, {'name': 'GIT_SSH_KEY_FILE', 'value': '/etc/git-secret/ssh', 'value_from': None}, {'name': 'GIT_SYNC_SSH', 'value': 'true', 'value_from': None}, {'name': 'GIT_KNOWN_HOSTS', 'value': 'true', 'value_from': None}, {'name': 'GIT_SSH_KNOWN_HOSTS_FILE',
    'value': '/etc/git-secret/known_hosts',
    'value_from': None}] : The git_sync_rev env did not get into the init container

@ashb ashb added the k8s label Dec 12, 2019
@kaxil kaxil requested a review from dimberman December 17, 2019 01:28
@kaxil
Copy link
Member

kaxil commented Dec 17, 2019

The change looks good to me, tests might need fixing. If you can get a fix out soon we can cherry-pick this to 1.10.7

cc @dimberman WDYT of this PR?

Copy link
Contributor

@dimberman dimberman left a comment

Choose a reason for hiding this comment

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

@george-miller fantastic addition. Once tests are passing I'll merge 👍

@dimberman
Copy link
Contributor

55) FAIL: test_make_pod_git_sync_rev (tests.kubernetes.test_worker_configuration.TestKubernetesWorkerConfiguration)
----------------------------------------------------------------------
   Traceback (most recent call last):
    tests/kubernetes/test_worker_configuration.py line 444 in test_make_pod_git_sync_rev
      'The git_sync_rev env did not get into the init container')
   AssertionError: {'name': 'GIT_SYNC_REV',
    'value': <MagicMock name='mock.git_sync_credentials_secret.to_dict()' id='140691298444512'>,
    'value_from': None} not found in [{'name': 'GIT_SYNC_REPO',
    'value': <MagicMock name='mock.git_repo.to_dict()' id='140691299285424'>,
    'value_from': None}, {'name': 'GIT_SYNC_BRANCH',
    'value': <MagicMock name='mock.git_branch.to_dict()' id='140691292267800'>,
    'value_from': None}, {'name': 'GIT_SYNC_ROOT',
    'value': <MagicMock name='mock.git_sync_root.to_dict()' id='140691300804312'>,
    'value_from': None}, {'name': 'GIT_SYNC_DEST', 'value': 'repo', 'value_from': None}, {'name': 'GIT_SYNC_REV', 'value': 'sampletag', 'value_from': None}, {'name': 'GIT_SYNC_DEPTH', 'value': '1', 'value_from': None}, {'name': 'GIT_SYNC_ONE_TIME', 'value': 'true', 'value_from': None}, {'name': 'GIT_SYNC_USERNAME',
    'value': <MagicMock name='mock.git_user.to_dict()' id='140691301425448'>,
    'value_from': None}, {'name': 'GIT_SYNC_PASSWORD',
    'value': <MagicMock name='mock.git_password.to_dict()' id='140691301994848'>,
    'value_from': None}, {'name': 'GIT_SYNC_USERNAME',
    'value': None,
    'value_from': {'config_map_key_ref': None,
                   'field_ref': None,
                   'resource_field_ref': None,
                   'secret_key_ref': {'key': 'GIT_SYNC_USERNAME',
                                      'name': <MagicMock name='mock.git_sync_credentials_secret.to_dict()' id='140691298444512'>,
                                      'optional': None}}}, {'name': 'GIT_SYNC_PASSWORD',
    'value': None,
    'value_from': {'config_map_key_ref': None,
                   'field_ref': None,
                   'resource_field_ref': None,
                   'secret_key_ref': {'key': 'GIT_SYNC_PASSWORD',
                                      'name': <MagicMock name='mock.git_sync_credentials_secret.to_dict()' id='140691298444512'>,
                                      'optional': None}}}, {'name': 'GIT_SSH_KEY_FILE', 'value': '/etc/git-secret/ssh', 'value_from': None}, {'name': 'GIT_SYNC_SSH', 'value': 'true', 'value_from': None}, {'name': 'GIT_KNOWN_HOSTS', 'value': 'true', 'value_from': None}, {'name': 'GIT_SSH_KNOWN_HOSTS_FILE',
    'value': '/etc/git-secret/known_hosts',
    'value_from': None}] : The git_sync_rev env did not get into the init container```

Looks like it's your new test that's failing.

@ashb ashb force-pushed the git_sync_allow_rev_specification branch from 56709a1 to 9933ab4 Compare December 17, 2019 10:42
…init container

We want to deploy dags based on tags on a branch, and want to use the git_sync init container in our kuberenetes setup to clone the dag folder at the start of a task run.

Co-authored-by: Ash Berlin-Taylor <[email protected]>
@ashb ashb force-pushed the git_sync_allow_rev_specification branch from 9933ab4 to 7ee0f32 Compare December 17, 2019 14:37
@kaxil kaxil merged commit 2645f9d into apache:master Dec 17, 2019
kaxil pushed a commit that referenced this pull request Dec 19, 2019
ashb pushed a commit that referenced this pull request Dec 19, 2019
kaxil pushed a commit that referenced this pull request Dec 19, 2019
KKcorps pushed a commit to KKcorps/airflow that referenced this pull request Dec 21, 2019
galuszkak pushed a commit to FlyrInc/apache-airflow that referenced this pull request Mar 5, 2020
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.

6 participants