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 2021/01/04 20:48:22 UTC

[GitHub] [airflow] mickaelgervais opened a new pull request #10606: Use private docker repository for K8S Operator & sidecar container

mickaelgervais opened a new pull request #10606:
URL: https://github.com/apache/airflow/pull/10606


   closes: #10605
   related: #10605
   
   
   ---
   **Use private docker repository for K8S Operator & sidecar container**
   
   @mik-laj 


----------------------------------------------------------------
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] zoomzoomTnT commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
zoomzoomTnT commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r483139637



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -292,6 +299,15 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
         # Attach sidecar
         self.extract_xcom = extract_xcom
 
+    def qualify_image_url(self, image: str) -> str:
+        if not self.docker_repository:
+            return image
+
+        if image.startswith(self.docker_repository):
+            return image
+
+        return self.docker_repository + '/' + image

Review comment:
       The tag 'latest' might not be available, please add image tag here @mickaelgervais




----------------------------------------------------------------
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] zoomzoomTnT commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
zoomzoomTnT commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r483151830



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -237,7 +243,8 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
 
         # Pod Container
         self.container = k8s.V1Container(name='base')
-        self.container.image = image
+        self.container.image = self.qualify_image_url(image)

Review comment:
       Please revert this line, because self.container.image can already be a full qualified string with private registry in the beginning. Also, this private registry for main container image might be different from xcom-sidecar's private registry, coupling these together will cause more problems. This feature should only solve the hard coded bug for xcom-sidecar image on line 64: https://github.com/apache/airflow/blob/master/airflow/kubernetes/pod_generator.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



[GitHub] [airflow] dimberman commented on pull request #10606: Use private docker repository for K8S Operator & sidecar container

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


   Also, another option that might work here which I'm going to look into this weekend would be to just nix the sidecar container entirely. I think we might be able to get away with a "kubectl cp" to pull the info,


-- 
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] omkark95 commented on pull request #10606: Use private docker repository for K8S Operator & sidecar container

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


   > @omka here is a workaround: https://hackmd.io/bhMoDP-zQgOKzBFr4MlB8g
   
   @mik-laj 
   No luck with this too. Seems like this one is only applicable for 1.10.11 and above. 
   
   <img width="539" alt="MicrosoftTeams-image (1)" src="https://user-images.githubusercontent.com/35721532/103754379-47e97080-5032-11eb-9825-857f79476ab8.png">
   
   This still pulls the images from dockerhub. 
   
   Could you confirm the minimum version this fix works on? or the version which was tested? 


----------------------------------------------------------------
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] zoomzoomTnT commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
zoomzoomTnT commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r483153531



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -224,6 +228,8 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
         else:
             self.ud_pod = pod
 
+        self.docker_repository = docker_repository

Review comment:
       Would like to update this field name to be more specific to xcom-sidecar




----------------------------------------------------------------
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] omkark95 commented on pull request #10606: Use private docker repository for K8S Operator & sidecar container

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


   > @omka here is a workaround: https://hackmd.io/bhMoDP-zQgOKzBFr4MlB8g
   
   @mik-laj  Tried it unfortunately it still is pulling from docker hub.
   
   ![image](https://user-images.githubusercontent.com/35721532/103650020-680f2600-4f85-11eb-971c-05ebecc714d1.png)
   
   We use the same docker image across web, worker and scheduler. 
   ![MicrosoftTeams-image (1)](https://user-images.githubusercontent.com/35721532/103650051-72c9bb00-4f85-11eb-96e0-5e520866b0e2.png)
   


----------------------------------------------------------------
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] zoomzoomTnT commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
zoomzoomTnT commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r483153124



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -292,6 +299,15 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
         # Attach sidecar
         self.extract_xcom = extract_xcom
 
+    def qualify_image_url(self, image: str) -> str:

Review comment:
       Would like to update this method name to be more specific to xcom-sidecar




----------------------------------------------------------------
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] dimberman commented on pull request #10606: Use private docker repository for K8S Operator & sidecar container

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


   @mickaelgervais can you please rebase this PR to 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] zoomzoomTnT commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
zoomzoomTnT commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r483139637



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -292,6 +299,15 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
         # Attach sidecar
         self.extract_xcom = extract_xcom
 
+    def qualify_image_url(self, image: str) -> str:
+        if not self.docker_repository:
+            return image
+
+        if image.startswith(self.docker_repository):
+            return image
+
+        return self.docker_repository + '/' + image

Review comment:
       The implicit tag 'latest' might not be available, please add explicit image tag here @mickaelgervais




----------------------------------------------------------------
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 commented on pull request #10606: Use private docker repository for K8S Operator & sidecar container

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


   This needs a rebase on latest Master, please


----------------------------------------------------------------
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] stale[bot] closed pull request #10606: Use private docker repository for K8S Operator & sidecar container

Posted by GitBox <gi...@apache.org>.
stale[bot] closed pull request #10606:
URL: https://github.com/apache/airflow/pull/10606


   


----------------------------------------------------------------
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] omkark95 edited a comment on pull request #10606: Use private docker repository for K8S Operator & sidecar container

Posted by GitBox <gi...@apache.org>.
omkark95 edited a comment on pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#issuecomment-755197874


   > @omka here is a workaround: https://hackmd.io/bhMoDP-zQgOKzBFr4MlB8g
   
   @mik-laj 
   No luck with this too. Seems like this one is only applicable for 1.10.11 and above. We are on 1.10.9.
   
   <img width="539" alt="MicrosoftTeams-image (1)" src="https://user-images.githubusercontent.com/35721532/103754379-47e97080-5032-11eb-9825-857f79476ab8.png">
   
   This still pulls the images from dockerhub. 
   
   Could you confirm the minimum version this fix works on? or the version which was tested? 


----------------------------------------------------------------
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] mickaelgervais commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
mickaelgervais commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r484424833



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -237,7 +243,8 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
 
         # Pod Container
         self.container = k8s.V1Container(name='base')
-        self.container.image = image
+        self.container.image = self.qualify_image_url(image)

Review comment:
       Yes you're right. I don't want to change the default behavior for the default image. I just want to fix the side-car hard coded  image path.




----------------------------------------------------------------
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] zoomzoomTnT commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
zoomzoomTnT commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r483139637



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -292,6 +299,15 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
         # Attach sidecar
         self.extract_xcom = extract_xcom
 
+    def qualify_image_url(self, image: str) -> str:
+        if not self.docker_repository:
+            return image
+
+        if image.startswith(self.docker_repository):
+            return image
+
+        return self.docker_repository + '/' + image

Review comment:
       The tag 'latest' might not be available, please add image tag here




----------------------------------------------------------------
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] mickaelgervais commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
mickaelgervais commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r484425495



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -292,6 +299,15 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
         # Attach sidecar
         self.extract_xcom = extract_xcom
 
+    def qualify_image_url(self, image: str) -> str:

Review comment:
       Maybe it's simpler to add a parameter for the side car image full path instead of using a docker registry? This is specific but will match to your comments. Is it ok for you?




----------------------------------------------------------------
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] zoomzoomTnT commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
zoomzoomTnT commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r483139637



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -292,6 +299,15 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
         # Attach sidecar
         self.extract_xcom = extract_xcom
 
+    def qualify_image_url(self, image: str) -> str:
+        if not self.docker_repository:
+            return image
+
+        if image.startswith(self.docker_repository):
+            return image
+
+        return self.docker_repository + '/' + image

Review comment:
       The implicit tag 'latest' might not be available, please add explicit image tag here @mickaelgervais




----------------------------------------------------------------
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] omkark95 commented on pull request #10606: Use private docker repository for K8S Operator & sidecar container

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


   @turbaszek @kaxil @mickaelgervais  any update on this merge? This is impacting our airflow deployment for any large DAG runs. 
   
   We already tried updating the `pod_generator.py` file (v1.10.14) and for our older deployment `pod_request_factory.py` (v1.10.9) to point to our private docker repo. But the DAG executions still picks up the alpine image from docker hub instead of private repo.
   
   Airflow web, scheduler and workers all contain this change for alpine image.


----------------------------------------------------------------
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] zoomzoomTnT commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
zoomzoomTnT commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r483139637



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -292,6 +299,15 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
         # Attach sidecar
         self.extract_xcom = extract_xcom
 
+    def qualify_image_url(self, image: str) -> str:
+        if not self.docker_repository:
+            return image
+
+        if image.startswith(self.docker_repository):
+            return image
+
+        return self.docker_repository + '/' + image

Review comment:
       The tag 'latest' might not be available, please add explicit image tag here @mickaelgervais




----------------------------------------------------------------
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] omkark95 edited a comment on pull request #10606: Use private docker repository for K8S Operator & sidecar container

Posted by GitBox <gi...@apache.org>.
omkark95 edited a comment on pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#issuecomment-754626650


   > @omka here is a workaround: https://hackmd.io/bhMoDP-zQgOKzBFr4MlB8g
   
   @mik-laj  Tried it unfortunately it still is pulling from docker hub.
   
   ![image](https://user-images.githubusercontent.com/35721532/103650020-680f2600-4f85-11eb-971c-05ebecc714d1.png)
   
   We use the same docker image across web, worker and scheduler. 
   
   <img width="539" alt="MicrosoftTeams-image (1)" src="https://user-images.githubusercontent.com/35721532/103650633-5b3f0200-4f86-11eb-8edb-eda55960c4c4.png">
   


----------------------------------------------------------------
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] zoomzoomTnT commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
zoomzoomTnT commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r484540189



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -292,6 +299,15 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
         # Attach sidecar
         self.extract_xcom = extract_xcom
 
+    def qualify_image_url(self, image: str) -> str:

Review comment:
       absolutely!




----------------------------------------------------------------
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] zoomzoomTnT commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
zoomzoomTnT commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r483154267



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -292,6 +299,15 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
         # Attach sidecar
         self.extract_xcom = extract_xcom
 
+    def qualify_image_url(self, image: str) -> str:
+        if not self.docker_repository:
+            return image
+
+        if image.startswith(self.docker_repository):
+            return image
+
+        return self.docker_repository + '/' + image

Review comment:
       Please add a explicit image tag variable since the implicit tag 'latest' might not exist




----------------------------------------------------------------
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] zoomzoomTnT commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
zoomzoomTnT commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r483151830



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -237,7 +243,8 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
 
         # Pod Container
         self.container = k8s.V1Container(name='base')
-        self.container.image = image
+        self.container.image = self.qualify_image_url(image)

Review comment:
       Please revert this line, because self.container.image can already be a full qualified string with private registry in the beginning. Also, this private registry for main container image might be different from xcom-sidecar's private registry, coupling these together will cause more problems. If am understanding correctly, this feature should only solve the hard coded bug for xcom-sidecar image on line 64: https://github.com/apache/airflow/blob/master/airflow/kubernetes/pod_generator.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



[GitHub] [airflow] mickaelgervais commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
mickaelgervais commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r484425706



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -224,6 +228,8 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
         else:
             self.ud_pod = pod
 
+        self.docker_repository = docker_repository

Review comment:
       Ok! this match my previous comment! ;)




----------------------------------------------------------------
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] zoomzoomTnT commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
zoomzoomTnT commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r483151830



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -237,7 +243,8 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
 
         # Pod Container
         self.container = k8s.V1Container(name='base')
-        self.container.image = image
+        self.container.image = self.qualify_image_url(image)

Review comment:
       Please revert this line, because self.container.image can be a full qualified string with private registry in the beginning.




----------------------------------------------------------------
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] zoomzoomTnT commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
zoomzoomTnT commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r483151830



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -237,7 +243,8 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
 
         # Pod Container
         self.container = k8s.V1Container(name='base')
-        self.container.image = image
+        self.container.image = self.qualify_image_url(image)

Review comment:
       Please revert this line, because self.container.image can already be a full qualified string with private registry in the beginning. This feature should only solve the hard coded bug for xcom-sidecar image on line 64: https://github.com/apache/airflow/blob/master/airflow/kubernetes/pod_generator.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



[GitHub] [airflow] mickaelgervais commented on pull request #10606: Use private docker repository for K8S Operator & sidecar container

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


   Hi @dimberman , Excuse me for the delay...
   I've left the projet where I have introduce Airflow, so if you can finish this MR it's a pleasure! :)
   Di not hesitate to contact me if needed, I'll answer faster
   Regards


-- 
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] dimberman commented on pull request #10606: Use private docker repository for K8S Operator & sidecar container

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


   @mickaelgervais Hi Mickael, are you interested in completing this?


-- 
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] dimberman commented on pull request #10606: Use private docker repository for K8S Operator & sidecar container

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


   @mickaelgervais are you interested in finishing this? If not I can pick it up.


-- 
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] dimberman commented on pull request #10606: Use private docker repository for K8S Operator & sidecar container

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


   @mickaelgervais can you please rebase this PR to 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] omkark95 removed a comment on pull request #10606: Use private docker repository for K8S Operator & sidecar container

Posted by GitBox <gi...@apache.org>.
omkark95 removed a comment on pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#issuecomment-754626650


   > @omka here is a workaround: https://hackmd.io/bhMoDP-zQgOKzBFr4MlB8g
   
   @mik-laj  Tried it unfortunately it still is pulling from docker hub.
   
   ![image](https://user-images.githubusercontent.com/35721532/103650020-680f2600-4f85-11eb-971c-05ebecc714d1.png)
   
   We use the same docker image across web, worker and scheduler. 
   
   <img width="539" alt="MicrosoftTeams-image (1)" src="https://user-images.githubusercontent.com/35721532/103650633-5b3f0200-4f86-11eb-8edb-eda55960c4c4.png">
   


----------------------------------------------------------------
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] dimberman commented on pull request #10606: Use private docker repository for K8S Operator & sidecar container

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


   @mickaelgervais I'm a bit confused, would this not be doable with an image_pull_secret?


-- 
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 #10606: Use private docker repository for K8S Operator & sidecar container

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


   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] zoomzoomTnT commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
zoomzoomTnT commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r483151830



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -237,7 +243,8 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
 
         # Pod Container
         self.container = k8s.V1Container(name='base')
-        self.container.image = image
+        self.container.image = self.qualify_image_url(image)

Review comment:
       Please revert this line, because self.container.image can be a full qualified string with private registry in the beginning. This feature should only solve the hard coded bug for xcom-sidecar image on line 64: https://github.com/apache/airflow/blob/master/airflow/kubernetes/pod_generator.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



[GitHub] [airflow] github-actions[bot] closed pull request #10606: Use private docker repository for K8S Operator & sidecar container

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #10606:
URL: https://github.com/apache/airflow/pull/10606


   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] zoomzoomTnT commented on a change in pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
zoomzoomTnT commented on a change in pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#discussion_r483153531



##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -224,6 +228,8 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
         else:
             self.ud_pod = pod
 
+        self.docker_repository = docker_repository

Review comment:
       Would like to update this field name to be more specific to xcom-sidecar. My initial approach to this issue was just adding a field for xcom-sidecar-image, doesn't need to split image to image and docker_repo.




----------------------------------------------------------------
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 #10606: Use private docker repository for K8S Operator & sidecar container

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


   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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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



[GitHub] [airflow] omkark95 edited a comment on pull request #10606: Use private docker repository for K8S Operator & sidecar container

Posted by GitBox <gi...@apache.org>.
omkark95 edited a comment on pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#issuecomment-754626650


   > @omka here is a workaround: https://hackmd.io/bhMoDP-zQgOKzBFr4MlB8g
   
   @mik-laj  Tried it unfortunately it still is pulling from docker hub.
   
   ![image](https://user-images.githubusercontent.com/35721532/103650020-680f2600-4f85-11eb-971c-05ebecc714d1.png)
   
   We use the same docker image across web, worker and scheduler. 
   
   


----------------------------------------------------------------
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] boring-cyborg[bot] commented on pull request #10606: Feature/airflow 10605

Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #10606:
URL: https://github.com/apache/airflow/pull/10606#issuecomment-682031457


   Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
   Here are some useful points:
   - Pay attention to the quality of your code (flake8, pylint and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/master/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
   - In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/master/docs/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
   - Consider using [Breeze environment](https://github.com/apache/airflow/blob/master/BREEZE.rst) for testing locally, itโ€™s a heavy docker but it ships with a working Airflow and a lot of integrations.
   - Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
   - Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
   - Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#coding-style-and-best-practices).
   Apache Airflow is a community-driven project and together we are making it better ๐Ÿš€.
   In case of doubts contact the developers at:
   Mailing List: dev@airflow.apache.org
   Slack: https://apache-airflow-slack.herokuapp.com/
   


----------------------------------------------------------------
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 #10606: Use private docker repository for K8S Operator & sidecar container

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


   @omka here is a workaround: https://hackmd.io/bhMoDP-zQgOKzBFr4MlB8g


----------------------------------------------------------------
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