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/04/14 18:30:45 UTC

[GitHub] [airflow] jedcunningham commented on a change in pull request #15373: Require `name` with KubernetesPodOperator

jedcunningham commented on a change in pull request #15373:
URL: https://github.com/apache/airflow/pull/15373#discussion_r613488004



##########
File path: tests/providers/cncf/kubernetes/operators/test_kubernetes_pod.py
##########
@@ -234,6 +235,133 @@ def test_randomize_pod_name(self, mock_client, monitor_mock, start_mock):
         assert start_mock.call_args[0][0].metadata.name.startswith(name_base)
         assert start_mock.call_args[0][0].metadata.name != name_base
 
+    def test_pod_name_required(self):
+        with pytest.raises(AirflowException, match="`name` is required"):
+            KubernetesPodOperator(
+                namespace='default',
+                image="ubuntu:16.04",
+                cmds=["bash", "-cx"],
+                arguments=["echo 10"],
+                labels={"foo": "bar"},
+                task_id="task",
+                in_cluster=False,
+                do_xcom_push=False,
+                cluster_context='default',
+            )
+
+    @mock.patch("airflow.providers.cncf.kubernetes.utils.pod_launcher.PodLauncher.start_pod")
+    @mock.patch("airflow.providers.cncf.kubernetes.utils.pod_launcher.PodLauncher.monitor_pod")
+    @mock.patch("airflow.kubernetes.kube_client.get_kube_client")
+    def test_full_pod_spec(self, mock_client, monitor_mock, start_mock):
+        from airflow.utils.state import State
+
+        pod_spec = k8s.V1Pod(
+            metadata=k8s.V1ObjectMeta(name="hello", labels={"foo": "bar"}, namespace="mynamespace"),
+            spec=k8s.V1PodSpec(
+                containers=[
+                    k8s.V1Container(
+                        name="base",
+                        image="ubuntu:16.04",
+                        command=["something"],
+                    )
+                ]
+            ),
+        )
+
+        k = KubernetesPodOperator(
+            task_id="task",
+            in_cluster=False,
+            do_xcom_push=False,
+            cluster_context='default',
+            full_pod_spec=pod_spec,
+        )
+        monitor_mock.return_value = (State.SUCCESS, None)
+        context = self.create_context(k)
+        k.execute(context=context)
+
+        assert start_mock.call_args[0][0].metadata.name == pod_spec.metadata.name
+        assert start_mock.call_args[0][0].metadata.labels == pod_spec.metadata.labels
+        assert start_mock.call_args[0][0].metadata.namespace == pod_spec.metadata.namespace
+        assert start_mock.call_args[0][0].spec.containers[0].image == pod_spec.spec.containers[0].image
+        assert start_mock.call_args[0][0].spec.containers[0].command == pod_spec.spec.containers[0].command
+
+        # kwargs take precedence, however
+        start_mock.reset_mock()
+        image = "some.custom.image:andtag"
+        name_base = "world"
+        k = KubernetesPodOperator(
+            task_id="task",
+            in_cluster=False,
+            do_xcom_push=False,
+            cluster_context='default',
+            full_pod_spec=pod_spec,
+            name=name_base,
+            image=image,
+        )
+        context = self.create_context(k)
+        k.execute(context=context)
+
+        assert start_mock.call_args[0][0].metadata.name.startswith(name_base)
+        assert start_mock.call_args[0][0].metadata.name != name_base
+        assert start_mock.call_args[0][0].spec.containers[0].image == image
+
+    @mock.patch("airflow.providers.cncf.kubernetes.utils.pod_launcher.PodLauncher.start_pod")
+    @mock.patch("airflow.providers.cncf.kubernetes.utils.pod_launcher.PodLauncher.monitor_pod")
+    @mock.patch("airflow.kubernetes.kube_client.get_kube_client")
+    def test_pod_template_file(self, mock_client, monitor_mock, start_mock):
+        from airflow.utils.state import State

Review comment:
       I'm planning a follow up cleanup PR with more, don't want to muddy the waters of this review, so I'm following the same pattern as the existing tests 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