You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/11/18 09:50:44 UTC

[GitHub] [airflow] highfly22 opened a new pull request #12441: Fix file permission issue when running git-sync in the KubernetesExecutor mode

highfly22 opened a new pull request #12441:
URL: https://github.com/apache/airflow/pull/12441


   …tor mode.
   
   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   ---
   When using git-sync in the Kubernetes, dag folder is empty because the ssh key file cannot be accessed by git-sync user.
   This commit try to fix this by add securityContext.fsGroup parameter in the pod template.
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] gmontanola commented on pull request #12441: Fix file permission issue when running git-sync in the KubernetesExecutor mode

Posted by GitBox <gi...@apache.org>.
gmontanola commented on pull request #12441:
URL: https://github.com/apache/airflow/pull/12441#issuecomment-758365725


   Thanks for this fix @highfly22! I was trying to understand why setting up a private repo with SSH keys resulted in failure. I could not even get logs from the webserver, not sure why a 403 was issued, but applying your fix solved it too.
   
   `HTTP response body: b'{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"pods is forbidden: User \\"system:serviceaccount:airflow:airflow-webserver\\" cannot list resource \\"pods/log\\" in API group \\"\\" in the namespace \\"airflow\\"","reason":"Forbidden","details":{"kind":"pods"},"code":403}\n'`
   
   @potiuk is there anything left in order to merge this PR? Let me know if I can help  :)
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ianstanton commented on pull request #12441: Fix file permission issue when running git-sync in the KubernetesExecutor mode

Posted by GitBox <gi...@apache.org>.
ianstanton commented on pull request #12441:
URL: https://github.com/apache/airflow/pull/12441#issuecomment-790222727


   @potiuk Can you give this another look? Do you think this is good to merge?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] highfly22 commented on pull request #12441: Fix file permission issue when running git-sync in the KubernetesExecutor mode

Posted by GitBox <gi...@apache.org>.
highfly22 commented on pull request #12441:
URL: https://github.com/apache/airflow/pull/12441#issuecomment-730186825


   @potiuk I have no idea on the following failed case. Could you give any clue?
   
   ```
   self = <kubernetes_tests.test_kubernetes_pod_operator_backcompat.TestKubernetesPodOperatorSystem testMethod=test_xcom_push>
   
       def test_xcom_push(self):
           return_value = '{"foo": "bar"\n, "buzz": 2}'
           args = [f'echo \'{return_value}\' > /airflow/xcom/return.json']
           k = KubernetesPodOperator(
               namespace='default',
               image="ubuntu:16.04",
               cmds=["bash", "-cx"],
               arguments=args,
               labels={"foo": "bar"},
               name="test",
               task_id="task",
               in_cluster=False,
               do_xcom_push=True,
           )
           context = create_context(k)
           self.assertEqual(k.execute(context), json.loads(return_value))
           actual_pod = self.api_client.sanitize_for_serialization(k.pod)
           volume = self.api_client.sanitize_for_serialization(PodDefaults.VOLUME)
           volume_mount = self.api_client.sanitize_for_serialization(PodDefaults.VOLUME_MOUNT)
           container = self.api_client.sanitize_for_serialization(PodDefaults.SIDECAR_CONTAINER)
           self.expected_pod['spec']['containers'][0]['args'] = args
           self.expected_pod['spec']['containers'][0]['volumeMounts'].insert(0, volume_mount)  # noqa
           self.expected_pod['spec']['volumes'].insert(0, volume)
           self.expected_pod['spec']['containers'].append(container)
   >       self.assertEqual(self.expected_pod, actual_pod)
   E       AssertionError: {'api[40 chars]': {'namespace': 'default', 'name': <ANY>, 'an[998 chars]'}]}} != {'api[40 chars]': {'annotations': {}, 'labels': {'foo': 'bar'[1012 chars]'}]}}
   E         {'apiVersion': 'v1',
   E          'kind': 'Pod',
   E          'metadata': {'annotations': {},
   E                       'labels': {'airflow_version': '2.0.0b3',
   E                                  'dag_id': 'dag',
   E                                  'execution_date': '2016-01-01T0100000100-a2f50a31f',
   E                                  'foo': 'bar',
   E                                  'kubernetes_pod_operator': 'True',
   E                                  'task_id': 'task',
   E                                  'try_number': '1'},
   E       -               'name': <ANY>,
   E       +               'name': 'test-6c6eea1bc6974613815ef7ad04989ce8',
   E                       'namespace': 'default'},
   E          'spec': {'affinity': {},
   E                   'containers': [{'args': ['echo \'{"foo": "bar"\n'
   E                                            ', "buzz": 2}\' > '
   E                                            '/airflow/xcom/return.json'],
   E                                   'command': ['bash', '-cx'],
   E                                   'env': [],
   E                                   'envFrom': [],
   E                                   'image': 'ubuntu:16.04',
   E                                   'name': 'base',
   E                                   'ports': [],
   E                                   'resources': {},
   E                                   'volumeMounts': [{'mountPath': '/airflow/xcom',
   E                                                     'name': 'xcom'}]},
   E                                  {'command': ['sh',
   E                                               '-c',
   E                                               'trap "exit 0" INT; while true; do sleep '
   E                                               '30; done;'],
   E                                   'image': 'alpine',
   E                                   'name': 'airflow-xcom-sidecar',
   E                                   'resources': {'requests': {'cpu': '1m'}},
   E                                   'volumeMounts': [{'mountPath': '/airflow/xcom',
   E                                                     'name': 'xcom'}]}],
   E                   'hostNetwork': False,
   E                   'imagePullSecrets': [],
   E                   'initContainers': [],
   E       -           'nodeSelector': {},
   E                   'restartPolicy': 'Never',
   E                   'securityContext': {},
   E                   'serviceAccountName': 'default',
   E                   'tolerations': [],
   E                   'volumes': [{'emptyDir': {}, 'name': 'xcom'}]}}
   
   kubernetes_tests/test_kubernetes_pod_operator_backcompat.py:484: AssertionError
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] kaxil merged pull request #12441: Fix file permission issue when running git-sync in the KubernetesExecutor mode

Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #12441:
URL: https://github.com/apache/airflow/pull/12441


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ashb commented on pull request #12441: Fix file permission issue when running git-sync in the KubernetesExecutor mode

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #12441:
URL: https://github.com/apache/airflow/pull/12441#issuecomment-730295626


   @highfly22 Try rebasing -- that should have been fixed latest master.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] mik-laj commented on pull request #12441: Fix file permission issue when running git-sync in the KubernetesExecutor mode

Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #12441:
URL: https://github.com/apache/airflow/pull/12441#issuecomment-758506196


   > `HTTP response body: b'{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"pods is forbidden: User \\"system:serviceaccount:airflow:airflow-webserver\\" cannot list resource \\"pods/log\\" in API group \\"\\" in the namespace \\"airflow\\"","reason":"Forbidden","details":{"kind":"pods"},"code":403}\n'`
   
   https://github.com/apache/airflow/pull/12598 looks related.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] github-actions[bot] commented on pull request #12441: Fix file permission issue when running git-sync in the KubernetesExecutor mode

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12441:
URL: https://github.com/apache/airflow/pull/12441#issuecomment-787209800


   This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] highfly22 commented on pull request #12441: Fix file permission issue when running git-sync in the KubernetesExecutor mode

Posted by GitBox <gi...@apache.org>.
highfly22 commented on pull request #12441:
URL: https://github.com/apache/airflow/pull/12441#issuecomment-734627143


   > @highfly22 Try rebasing -- that should have been fixed latest master.
   
   Rebasing fixed that error. All test are passed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] github-actions[bot] commented on pull request #12441: Fix file permission issue when running git-sync in the KubernetesExecutor mode

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12441:
URL: https://github.com/apache/airflow/pull/12441#issuecomment-729581008


   The PR should be OK to be merged with just subset of tests as it does not modify Core of Airflow. The committers might merge it or can add a label 'full tests needed' and re-run it to run all tests if they see it is needed!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #12441: Fix file permission issue when running git-sync in the KubernetesExecutor mode

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #12441:
URL: https://github.com/apache/airflow/pull/12441#issuecomment-729579091


   Would you mind adding a test case for it @highfly22 ? We have this new, fantastic pytest-based test framework for helm chart and it is super easy to add new tests there! look at `chart/tests`


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] highfly22 commented on pull request #12441: Fix file permission issue when running git-sync in the KubernetesExecutor mode

Posted by GitBox <gi...@apache.org>.
highfly22 commented on pull request #12441:
URL: https://github.com/apache/airflow/pull/12441#issuecomment-730095421


   > Would you mind adding a test case for it @highfly22 ? We have this new, fantastic pytest-based test framework for helm chart and it is super easy to add new tests there! look at `chart/tests`
   
   I have added a test in the test_pod_template_file.py.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org