This is an automated email from the ASF dual-hosted git repository. eladkal pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push: new 22d583403b Fix AzureContainerInstanceOperator remove_on_error (#35212) 22d583403b is described below commit 22d583403b79356d7ff604e8f3a3bad924764029 Author: Daniel van der Ende <daniel.vandere...@gmail.com> AuthorDate: Wed Nov 8 19:04:47 2023 +0100 Fix AzureContainerInstanceOperator remove_on_error (#35212) The remove_on_error parameter of the AzureContainerInstanceOperator was not working as intended. When set to `False` it should only skip the removal of the Azure Container Group **if** the exit code is non-zero. For zero exit codes, the infra should be dropped. This was not happening; the Azure infra was always left in place. This commit fixes that behaviour. Co-authored-by: Elad Kalif <45845474+elad...@users.noreply.github.com> --- .../azure/operators/container_instances.py | 11 +++++------ .../operators/test_azure_container_instances.py | 23 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/airflow/providers/microsoft/azure/operators/container_instances.py b/airflow/providers/microsoft/azure/operators/container_instances.py index 30b4877d3e..dde105e022 100644 --- a/airflow/providers/microsoft/azure/operators/container_instances.py +++ b/airflow/providers/microsoft/azure/operators/container_instances.py @@ -278,12 +278,11 @@ class AzureContainerInstancesOperator(BaseOperator): self.on_kill() def on_kill(self) -> None: - if self.remove_on_error: - self.log.info("Deleting container group") - try: - self._ci_hook.delete(self.resource_group, self.name) - except Exception: - self.log.exception("Could not delete container group") + self.log.info("Deleting container group") + try: + self._ci_hook.delete(self.resource_group, self.name) + except Exception: + self.log.exception("Could not delete container group") def _monitor_logging(self, resource_group: str, name: str) -> int: last_state = None diff --git a/tests/providers/microsoft/azure/operators/test_azure_container_instances.py b/tests/providers/microsoft/azure/operators/test_azure_container_instances.py index a0b41e305a..6205ac6f5d 100644 --- a/tests/providers/microsoft/azure/operators/test_azure_container_instances.py +++ b/tests/providers/microsoft/azure/operators/test_azure_container_instances.py @@ -110,6 +110,7 @@ class TestACIOperator: image="container-image", region="region", task_id="task", + remove_on_error=False, ) aci.execute(None) @@ -151,6 +152,28 @@ class TestACIOperator: assert aci_mock.return_value.delete.call_count == 1 + @mock.patch("airflow.providers.microsoft.azure.operators.container_instances.AzureContainerInstanceHook") + def test_execute_with_failures_without_removal(self, aci_mock): + expected_cg = make_mock_container(state="Terminated", exit_code=1, detail_status="test") + aci_mock.return_value.get_state.return_value = expected_cg + + aci_mock.return_value.exists.return_value = False + + aci = AzureContainerInstancesOperator( + ci_conn_id=None, + registry_conn_id=None, + resource_group="resource-group", + name="container-name", + image="container-image", + region="region", + task_id="task", + remove_on_error=False, + ) + with pytest.raises(AirflowException): + aci.execute(None) + + assert aci_mock.return_value.delete.call_count == 0 + @mock.patch("airflow.providers.microsoft.azure.operators.container_instances.AzureContainerInstanceHook") def test_execute_with_tags(self, aci_mock): expected_cg = make_mock_container(state="Terminated", exit_code=0, detail_status="test")