From dbda0f22421ea675d389ddd5490fa91107e02357 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 22 Sep 2025 15:40:02 +0200 Subject: [PATCH 1/2] fix: handle force flag for activation in workers-offline Signed-off-by: Alex --- src/aap_eda/api/views/activation.py | 87 ++++++++++++++++++------ tests/integration/api/test_activation.py | 64 +++++++++++++++++ 2 files changed, 132 insertions(+), 19 deletions(-) diff --git a/src/aap_eda/api/views/activation.py b/src/aap_eda/api/views/activation.py index e39fc8830..dbc1088a4 100644 --- a/src/aap_eda/api/views/activation.py +++ b/src/aap_eda/api/views/activation.py @@ -16,7 +16,6 @@ import redis from ansible_base.rbac.api.related import check_related_permissions from ansible_base.rbac.models import RoleDefinition -from django.conf import settings from django.db import transaction from django.forms import model_to_dict from django_filters import rest_framework as defaultfilters @@ -208,17 +207,30 @@ def partial_update(self, request, pk): None, description="The Activation has been deleted.", ), + status.HTTP_409_CONFLICT: OpenApiResponse( + None, + description="Activation blocked while Workers offline.", + ), } | RedisDependencyMixin.redis_unavailable_response(), + parameters=[ + OpenApiParameter( + name="force", + description="Force delete after worker node offline", + required=False, + type=bool, + ) + ], ) def destroy(self, request, *args, **kwargs): activation = self.get_object() + force_delete = str_to_bool( + request.query_params.get("force", "false"), + ) - if activation.status == ActivationStatus.WORKERS_OFFLINE: - raise api_exc.Conflict( - f"An Activation in state '{activation.status}' cannot be " - "deleted.", - ) + self._check_workers_offline_with_force( + activation, force_delete, "Deleted" + ) audit_log = logging_utils.generate_simple_audit_log( "Delete", @@ -434,14 +446,33 @@ def _start(self, request, activation: models.Activation) -> Response: None, description="Activation has been disabled.", ), + status.HTTP_409_CONFLICT: OpenApiResponse( + None, + description="Activation blocked while Workers offline.", + ), } | RedisDependencyMixin.redis_unavailable_response(), + parameters=[ + OpenApiParameter( + name="force", + description="Force disable after worker node offline", + required=False, + type=bool, + ) + ], ) @action(methods=["post"], detail=True, rbac_action=Action.DISABLE) def disable(self, request, pk): activation = self.get_object() self._check_deleting(activation) + force_disable = str_to_bool( + request.query_params.get("force", "false"), + ) + + self._check_workers_offline_with_force( + activation, force_disable, "Disabled" + ) if activation.is_enabled: # Redis must be available in order to perform the delete. @@ -509,19 +540,9 @@ def restart(self, request, pk): request.query_params.get("force", "false"), ) - if ( - settings.DEPLOYMENT_TYPE == "podman" - and activation.status == ActivationStatus.WORKERS_OFFLINE - and not force_restart - ): - # block the restart and return an error - raise api_exc.Conflict( - "An activation with an activation_status of 'Workers offline' " - "cannot be Restarted because this will leave an orphaned " - "container running on one of the 'activation-worker-node's. " - "If you want to force a restart, please add the " - "/?force=true query param." - ) + self._check_workers_offline_with_force( + activation, force_restart, "Restarted" + ) if not activation.is_enabled: raise api_exc.Forbidden( detail="Activation is disabled and cannot be run." @@ -616,6 +637,34 @@ def _check_deleting(self, activation): detail="Object is being deleted", code=409 ) + def _check_workers_offline_with_force( + self, activation, force_flag, operation_name + ): + """ + Check if activation is in WORKERS_OFFLINE status and handle force flag. + + Args: + activation: The activation object to check + force_flag: Boolean indicating if force operation is requested + operation_name: String name of the operation + (e.g., "Restarted", "Disabled", "Deleted") + + Raises: + api_exc.Conflict: If activation is WORKERS_OFFLINE and force flag + is False + """ + if ( + activation.status == ActivationStatus.WORKERS_OFFLINE + and not force_flag + ): + raise api_exc.Conflict( + f"An activation with an activation_status of " + f"'Workers offline' cannot be {operation_name} because this " + f"may leave an orphaned container running. " + f"If you want to force a {operation_name.lower()}, please " + f"add the /?force=true query param." + ) + @extend_schema_view( retrieve=extend_schema( diff --git a/tests/integration/api/test_activation.py b/tests/integration/api/test_activation.py index 3c6427a7b..3aa8c68db 100644 --- a/tests/integration/api/test_activation.py +++ b/tests/integration/api/test_activation.py @@ -836,6 +836,70 @@ def test_disable_activation_redis_unavailable( } +@pytest.mark.django_db +@pytest.mark.parametrize( + ("force_disable", "expected_response"), + [ + ( + "true", + status.HTTP_204_NO_CONTENT, + ), + ( + "false", + status.HTTP_409_CONFLICT, + ), + ], +) +@patch("aap_eda.api.serializers.activation.settings.DEPLOYMENT_TYPE", "podman") +def test_disable_activation_workers_offline( + force_disable, + expected_response, + default_activation: models.Activation, + admin_client: APIClient, + preseed_credential_types, +): + default_activation.status = enums.ActivationStatus.WORKERS_OFFLINE + default_activation.save(update_fields=["status"]) + + response = admin_client.post( + f"{api_url_v1}/activations/{default_activation.id}/disable/" + f"?force={force_disable}" + ) + assert response.status_code == expected_response + + +@pytest.mark.django_db +@pytest.mark.parametrize( + ("force_delete", "expected_response"), + [ + ( + "true", + status.HTTP_204_NO_CONTENT, + ), + ( + "false", + status.HTTP_409_CONFLICT, + ), + ], +) +@patch("aap_eda.api.serializers.activation.settings.DEPLOYMENT_TYPE", "podman") +def test_delete_activation_workers_offline( + force_delete, + expected_response, + default_activation: models.Activation, + admin_client: APIClient, + preseed_credential_types, +): + default_activation.status = enums.ActivationStatus.WORKERS_OFFLINE + default_activation.save(update_fields=["status"]) + + response = admin_client.delete( + f"{api_url_v1}/activations/{default_activation.id}/" + f"?force={force_delete}" + ) + assert response.status_code == expected_response + + @pytest.mark.django_db def test_list_activation_instances( default_activation: models.Activation, From 8d0deee0f49629b2b4418ca0a08082fdcce674da Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 22 Sep 2025 16:20:10 +0200 Subject: [PATCH 2/2] restore PODMAN deployment check Signed-off-by: Alex --- src/aap_eda/api/views/activation.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/aap_eda/api/views/activation.py b/src/aap_eda/api/views/activation.py index dbc1088a4..ad3246134 100644 --- a/src/aap_eda/api/views/activation.py +++ b/src/aap_eda/api/views/activation.py @@ -16,6 +16,7 @@ import redis from ansible_base.rbac.api.related import check_related_permissions from ansible_base.rbac.models import RoleDefinition +from django.conf import settings from django.db import transaction from django.forms import model_to_dict from django_filters import rest_framework as defaultfilters @@ -654,7 +655,8 @@ def _check_workers_offline_with_force( is False """ if ( - activation.status == ActivationStatus.WORKERS_OFFLINE + settings.DEPLOYMENT_TYPE == "podman" + and activation.status == ActivationStatus.WORKERS_OFFLINE and not force_flag ): raise api_exc.Conflict(