davlum commented on a change in pull request #6230: [AIRFLOW-5413] Allow K8S
worker pod to be configured from JSON/YAML file
URL: https://github.com/apache/airflow/pull/6230#discussion_r374995058
##########
File path: airflow/kubernetes/pod_generator.py
##########
@@ -301,36 +310,14 @@ def from_obj(obj) -> Optional[k8s.V1Pod]:
limits=limits
)
- pod_spec_generator = PodGenerator(
- image=namespaced.get('image'),
- envs=namespaced.get('env'),
- cmds=namespaced.get('cmds'),
- args=namespaced.get('args'),
- labels=namespaced.get('labels'),
- node_selectors=namespaced.get('node_selectors'),
- name=namespaced.get('name'),
- ports=namespaced.get('ports'),
- volumes=namespaced.get('volumes'),
- volume_mounts=namespaced.get('volume_mounts'),
- namespace=namespaced.get('namespace'),
- image_pull_policy=namespaced.get('image_pull_policy'),
- restart_policy=namespaced.get('restart_policy'),
- image_pull_secrets=namespaced.get('image_pull_secrets'),
- init_containers=namespaced.get('init_containers'),
- service_account_name=namespaced.get('service_account_name'),
- resources=resources,
- annotations=namespaced.get('annotations'),
- affinity=namespaced.get('affinity'),
- hostnetwork=namespaced.get('hostnetwork'),
- tolerations=namespaced.get('tolerations'),
- security_context=namespaced.get('security_context'),
- configmaps=namespaced.get('configmaps'),
- dnspolicy=namespaced.get('dnspolicy'),
- schedulername=namespaced.get('schedulername'),
- pod=namespaced.get('pod'),
- extract_xcom=namespaced.get('extract_xcom'),
- )
+ pod_args = list(inspect.signature(PodGenerator).parameters)
+ def reducer(d: dict, arg: str):
+ return {**d, arg: namespaced.get(arg)}
+
+ pod_kwargs = reduce(reducer, pod_args, {})
Review comment:
So the reason for this is a bit convoluted. Essentially the
`executor_config` argument has for some time [accepted
securityContext](https://github.com/apache/airflow/blob/ea93bb6f0ef71446539ad144c5abea0a85c64b57/airflow/example_dags/example_kubernetes_executor_config.py#L90)
as an argument, but actually does nothing with it. In fact, when that argument
is properly passed (using `security_context`), running as user 1000 actually
breaks ci, because I assume user 1000 doesn't have the appropriate permissions,
or necessarily exist. Because of this the arguments passed need to be actually
checked to see if they exist in the `PodGenerator` object. I agree the current
approach is too clever. We could actually error if `securityContext` is passed,
which would be a breaking change, or pop `securityContext` from the dict as a
weird edge case. As I mentioned earlier, later on we should deprecate passing
the dictionary object, and pass the PodGenerator directly as that would skip
over a lot of this messiness
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
