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