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 2022/03/01 17:23:42 UTC
[GitHub] [airflow] potiuk opened a new pull request #21905: Add missing parameters for Kubernetes 23 library tests
potiuk opened a new pull request #21905:
URL: https://github.com/apache/airflow/pull/21905
Kubernetes 23.3.0 is more picky when it comes to values passed to
Pod Generator - it requires:
* imagePullPolicy
* dnsPolicy
* restartPolicy
to be not None.
This might be only problem with our tests but in order to make it
compatible with deprecated pod generator, the pod generator assign now
defaults for those values if not set.
<!--
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/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/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/main/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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r816996009
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
I actually fixed it to follow the "latest" rule.
--
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] potiuk commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r816999159
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
This is what I get when I do it :(
```
@image_pull_policy.setter
def image_pull_policy(self, image_pull_policy):
"""Sets the image_pull_policy of this V1Container.
Image pull policy. One of Always, Never, IfNotPresent. Defaults to Always if :latest tag is specified, or IfNotPresent otherwise. Cannot be updated. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images Possible enum values: - `\"Always\"` means that kubelet always attempts to pull the latest image. Container will fail If the pull fails. - `\"IfNotPresent\"` means that kubelet pulls if the image isn't present on disk. Container will fail if the image isn't present and the pull fails. - `\"Never\"` means that kubelet never pulls an image, but only uses a local image. Container will fail if the image isn't present # noqa: E501
:param image_pull_policy: The image_pull_policy of this V1Container. # noqa: E501
:type: str
"""
allowed_values = ["Always", "IfNotPresent", "Never"] # noqa: E501
if self.local_vars_configuration.client_side_validation and image_pull_policy not in allowed_values: # noqa: E501
raise ValueError(
"Invalid value for `image_pull_policy` ({0}), must be one of {1}" # noqa: E501
> .format(image_pull_policy, allowed_values)
)
E ValueError: Invalid value for `image_pull_policy` (None), must be one of ['Always', 'IfNotPresent', 'Never']
```
--
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] jedcunningham commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r816994716
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
Does it complain if you just don’t set it to None? e.g.
```suggestion
if image_pull_policy:
self.container.image_pull_policy = image_pull_policy
```
--
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] jedcunningham commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r817030264
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
Yeah, that's with 23.3.0. Trying with Airflows PodGenerator now.
--
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] potiuk commented on pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#issuecomment-1055721120
Fixed. The tests are monstrous now :)
--
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] potiuk edited a comment on pull request #21905: Fix handling some None parameters in kubernetes 23 libs
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#issuecomment-1055741487
> Probably worth updating the title and description here.
Done
--
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] potiuk commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r816997289
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
Yep it fails.
--
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] jedcunningham commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r816995153
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
^ if that pattern works, that's probably the best for all of these?
--
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] jedcunningham commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r817015523
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
That's really unfortunate. This sure feels like a bug on their side to be honest.
--
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] jedcunningham commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r817023833
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
e.g:
```
>>> from kubernetes import client
>>> container = client.V1Container(name="foo")
>>> # good so far
>>> container.image_pull_policy
>>> # still good
>>> container.image_pull_policy = None
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/local/lib/python3.7/site-packages/kubernetes/client/models/v1_container.py", line 298, in image_pull_policy
.format(image_pull_policy, allowed_values)
ValueError: Invalid value for `image_pull_policy` (None), must be one of ['Always', 'IfNotPresent', 'Never']
```
--
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] jedcunningham commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r816988957
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
It's unfortunate it's required now, as this will behave differently... k8s does `IfNotPresent` unless the tag is `latest`, in which case it does `Always`.
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -187,15 +187,15 @@ def __init__(
self.spec = k8s.V1PodSpec(containers=[])
self.spec.security_context = security_context
self.spec.tolerations = tolerations
- self.spec.dns_policy = dnspolicy
+ self.spec.dns_policy = dnspolicy if dnspolicy else "Default"
Review comment:
The default is actually `ClusterFirst` (see https://kubernetes.io/docs/reference/kubernetes-api/workload-resources/pod-v1/#hostname-and-name-resolution).
--
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] potiuk commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r817029367
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
Did you try kubernetes 23.3.0 ?
--
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] jedcunningham commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r817021426
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
I'm experimenting with V1Container, and I can only reproduce this when I set `container.image_pull_policy`. How'd it even get set if it didn't get in the `if`?
--
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] potiuk commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r817020802
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,6 +178,11 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
+ if not image_pull_policy:
+ if image and image.endswith("latest"):
Review comment:
Ah yeah. but it should be ` or ":" not in 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r817032785
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
I Just tried it again with this (just to be sure) and it failes in 23.3.0 (it works with 22.* though):
```
if not image_pull_policy:
self.container.image_pull_policy = image_pull_policy
self.container.ports = ports or []
self.container.resources = resources
self.container.volume_mounts = volume_mounts or []
# Pod Spec
self.spec = k8s.V1PodSpec(containers=[])
self.spec.security_context = security_context
self.spec.tolerations = tolerations
if not dnspolicy:
self.spec.dns_policy = dnspolicy
self.spec.scheduler_name = schedulername
self.spec.host_network = hostnetwork
self.spec.affinity = affinity
self.spec.service_account_name = service_account_name
self.spec.init_containers = init_containers
self.spec.volumes = volumes or []
self.spec.node_selector = node_selectors
if not restart_policy:
self.spec.restart_policy = restart_policy
self.spec.priority_class_name = priority_class_name
```
--
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] potiuk commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r817038757
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
RIGHT. I think I reached the limit of :facepalm: today :)
```
if not -> if:
```
--
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] potiuk commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r816997176
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -187,15 +187,15 @@ def __init__(
self.spec = k8s.V1PodSpec(containers=[])
self.spec.security_context = security_context
self.spec.tolerations = tolerations
- self.spec.dns_policy = dnspolicy
+ self.spec.dns_policy = dnspolicy if dnspolicy else "Default"
Review comment:
Well.. That's ..... unexpected :D.
So you say "Default" is not "default" :facepalm:
--
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] potiuk commented on pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#issuecomment-1055693868
All should be fixed @jedcunningham
--
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] jedcunningham commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r816990778
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -187,15 +187,15 @@ def __init__(
self.spec = k8s.V1PodSpec(containers=[])
self.spec.security_context = security_context
self.spec.tolerations = tolerations
- self.spec.dns_policy = dnspolicy
+ self.spec.dns_policy = dnspolicy if dnspolicy else "Default"
Review comment:
(Yeah, confusing that "default" isn't the default 🤷♂️)
--
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] potiuk commented on pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#issuecomment-1055739804
Sorry for that @jedcunningham. I think I need to go to sleep. :)
--
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] potiuk commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r817029367
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
Did you try kubernetes 23.3.30 ?
--
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] potiuk commented on pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#issuecomment-1055680672
This is the last one that failed our `main`. I **think** I implemented in a mostly backwards-compatible way. The only doubt I have to the "IfNotPresent" - because default this parameter might be different if "latest" image is used. Not sure if it is worth complicating as this is only for deprecated usage/PodGenerator. @jedcunningham @kaxil @dimberman - let me know what you think.
--
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] jedcunningham commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r817015019
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,6 +178,11 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
+ if not image_pull_policy:
+ if image and image.endswith("latest"):
Review comment:
This probably also needs to handle images without tags (e.g. `debian`). Maybe:
```suggestion
if image and (image.endswith("latest") or ":" in 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#issuecomment-1055740331
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
--
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] jedcunningham commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r817039983
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
Ah! Yeah, that'd do it.
--
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] potiuk commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r816996009
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
I actually fixed it to follow 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #21905: Fix handling some None parameters in kubernetes 23 libs
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#issuecomment-1055741487
> Sorry for that @jedcunningham. I think I need to go to sleep. :)
Done
--
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] jedcunningham commented on pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#issuecomment-1055740202
All good 🍺
--
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] jedcunningham commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r817021787
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,6 +178,11 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
+ if not image_pull_policy:
+ if image and image.endswith("latest"):
Review comment:
Oh, yeah, oops 🤦♂️
--
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] jedcunningham merged pull request #21905: Fix handling some None parameters in kubernetes 23 libs
Posted by GitBox <gi...@apache.org>.
jedcunningham merged pull request #21905:
URL: https://github.com/apache/airflow/pull/21905
--
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] jedcunningham commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r817038009
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
I'm... confused... then. This works locally for me with 23.3.0
```
--- a/airflow/kubernetes/pod_generator_deprecated.py
+++ b/airflow/kubernetes/pod_generator_deprecated.py
@@ -178,7 +178,8 @@ class PodGenerator:
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ if image_pull_policy:
+ self.container.image_pull_policy = image_pull_policy
self.container.ports = ports or []
self.container.resources = resources
self.container.volume_mounts = volume_mounts or []
@@ -187,7 +188,8 @@ class PodGenerator:
self.spec = k8s.V1PodSpec(containers=[])
self.spec.security_context = security_context
self.spec.tolerations = tolerations
- self.spec.dns_policy = dnspolicy
+ if dnspolicy:
+ self.spec.dns_policy = dnspolicy
self.spec.scheduler_name = schedulername
self.spec.host_network = hostnetwork
self.spec.affinity = affinity
@@ -195,7 +197,8 @@ class PodGenerator:
self.spec.init_containers = init_containers
self.spec.volumes = volumes or []
self.spec.node_selector = node_selectors
- self.spec.restart_policy = restart_policy
+ if restart_policy:
+ self.spec.restart_policy = restart_policy
self.spec.priority_class_name = priority_class_name
self.spec.image_pull_secrets = []
```
--
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] potiuk commented on a change in pull request #21905: Add missing parameters for Kubernetes 23.3. library tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21905:
URL: https://github.com/apache/airflow/pull/21905#discussion_r817032785
##########
File path: airflow/kubernetes/pod_generator_deprecated.py
##########
@@ -178,7 +178,7 @@ def __init__(
self.container.command = cmds or []
self.container.args = args or []
- self.container.image_pull_policy = image_pull_policy
+ self.container.image_pull_policy = image_pull_policy if image_pull_policy else "IfNotPresent"
Review comment:
I Just tried it again with this (just to be sure) and it fails in 23.3.0 (it works with 22.* though):
```
if not image_pull_policy:
self.container.image_pull_policy = image_pull_policy
self.container.ports = ports or []
self.container.resources = resources
self.container.volume_mounts = volume_mounts or []
# Pod Spec
self.spec = k8s.V1PodSpec(containers=[])
self.spec.security_context = security_context
self.spec.tolerations = tolerations
if not dnspolicy:
self.spec.dns_policy = dnspolicy
self.spec.scheduler_name = schedulername
self.spec.host_network = hostnetwork
self.spec.affinity = affinity
self.spec.service_account_name = service_account_name
self.spec.init_containers = init_containers
self.spec.volumes = volumes or []
self.spec.node_selector = node_selectors
if not restart_policy:
self.spec.restart_policy = restart_policy
self.spec.priority_class_name = priority_class_name
```
--
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