feat: added celery task to remove offline site if hide download is true (#2681)

* added celery task to remove offline siet if hide download is true

* fixed failing test

* addressed feedback

* added removal pipeline for hide_download=true

* removed redundant tests

* Revert "added celery task to remove offline siet if hide download is true"

This reverts commit e1092ec.

* adde cleanup stask in offline build gate step on failure

* removed redundant cleanup tasks

* fixed failing tests

* removed redundant code and docstring

* updated and removed redundant test

Author: umar8hassan

content_sync/pipelines/definitions/concourse/site_pipeline.py

@@ -860,15 +860,22 @@ def __init__(self, config: SitePipelineDefinitionConfig, **kwargs):
         resource_types.append(KeyvalResourceType())
         resources = SitePipelineResources(config=config)
         online_job = self.get_online_build_job(config=config)
-        offline_build_gate_put_step = TryStep(
-            try_=PutStep(
-                put=self._offline_build_gate_identifier,
-                timeout="1m",
-                attempts=3,
-                get_params={"strict": True},
-                no_get=True,
-            )
+
+        # Create the inner put step with error handlers
+        inner_put_step = PutStep(
+            put=self._offline_build_gate_identifier,
+            timeout="1m",
+            attempts=3,
+            get_params={"strict": True},
+            no_get=True,
         )
+        # Add cleanup step to the inner put step.
+        # This will trigger before TryStep suppresses the error
+        inner_put_step.on_error = self.get_offline_content_cleanup_step()
+
+        # Wrap in TryStep to prevent online job failure
+        offline_build_gate_put_step = TryStep(try_=inner_put_step)
+
         online_job.plan.append(offline_build_gate_put_step)
         offline_job = self.get_offline_build_job(config=config)
         offline_build_gate_get_step = add_error_handling(
@@ -928,6 +935,50 @@ def __init__(self, config: SitePipelineDefinitionConfig, **kwargs):
             **kwargs,
         )
 
+    def get_offline_content_cleanup_step(self) -> TaskStep:
+        """
+        Create a task step to remove offline content from S3 when offline build
+        gate fails
+
+        Returns:
+            TaskStep: A task step that removes content from the offline S3 bucket
+        """
+        # Use same variable naming as unpublished sites for consistency
+        minio_root_user = settings.AWS_ACCESS_KEY_ID
+        minio_root_password = settings.AWS_SECRET_ACCESS_KEY
+        cli_endpoint_url = get_cli_endpoint_url()
+
+        cleanup_task = TaskStep(
+            task="remove-offline-content-task",
+            timeout="5m",
+            attempts=3,
+            config=TaskConfig(
+                platform="linux",
+                image_resource=AWS_CLI_REGISTRY_IMAGE,
+                run=Command(
+                    path="sh",
+                    args=[
+                        "-exc",
+                        f"""
+                        echo "Removing offline content for site: ((site:url_path))"
+                        aws s3{cli_endpoint_url} rm s3://((site:web_bucket))/((site:url_path))/((site:short_id)).zip
+                        aws s3{cli_endpoint_url} rm s3://((site:web_bucket))/((site:url_path))/((site:short_id))-video.zip
+                        aws s3{cli_endpoint_url} rm s3://((site:offline_bucket))/((site:url_path))/ --recursive
+                        echo "Offline content cleanup completed"
+                        """,  # noqa: E501
+                    ],
+                ),
+            ),
+        )
+
+        if is_dev():
+            cleanup_task.params = {
+                "AWS_ACCESS_KEY_ID": minio_root_user,
+                "AWS_SECRET_ACCESS_KEY": minio_root_password,
+            }
+
+        return cleanup_task
+
     def get_online_build_job(self, config: SitePipelineDefinitionConfig):
         ocw_studio_webhook_started_step = OcwStudioWebhookStep(
             pipeline_name=config.vars["pipeline_name"],

content_sync/pipelines/definitions/concourse/site_pipeline_test.py

@@ -653,3 +653,147 @@ def assert_base_build_tasks(tasks: list[dict], offline: bool):  # noqa: FBT001
     assert f"--baseURL /{prefix.lstrip('/')}" in dummy_vars["hugo_args_online"]
     assert dummy_vars["hugo_args_offline"] == config.hugo_args_offline
     assert dummy_vars["prefix"] == expected_prefix
+
+
+@pytest.mark.parametrize("is_dev", [True, False])
+def test_offline_content_cleanup_step(website, settings, mocker, is_dev):
+    """
+    Test that the offline content cleanup step is correctly configured
+    """
+    # Setup
+    settings.AWS_ACCESS_KEY_ID = "test_access_key_id"
+    settings.AWS_SECRET_ACCESS_KEY = "test_secret_access_key"  # noqa: S105
+    mock_utils_is_dev = mocker.patch("content_sync.utils.is_dev")
+    mock_pipeline_is_dev = mocker.patch(
+        "content_sync.pipelines.definitions.concourse.site_pipeline.is_dev"
+    )
+    mock_main_utils_is_dev = mocker.patch("main.utils.is_dev")
+    mock_utils_is_dev.return_value = is_dev
+    mock_pipeline_is_dev.return_value = is_dev
+    mock_main_utils_is_dev.return_value = is_dev
+    cli_endpoint_url = f" --endpoint-url {DEV_ENDPOINT_URL}" if is_dev else ""
+
+    offline_bucket = "test-offline-bucket"
+    config = SitePipelineDefinitionConfig(
+        site=website,
+        pipeline_name="test",
+        instance_vars="",
+        site_content_branch="main",
+        static_api_url="https://test.example.com/",
+        storage_bucket="test-storage",
+        artifacts_bucket="test-artifacts",
+        web_bucket="test-web",
+        offline_bucket=offline_bucket,
+        resource_base_url="https://test.example.com/",
+        ocw_hugo_themes_branch="main",
+        ocw_hugo_projects_branch="main",
+    )
+
+    pipeline_definition = SitePipelineDefinition(config=config)
+    cleanup_step = pipeline_definition.get_offline_content_cleanup_step()
+
+    # Test the cleanup step configuration
+    assert cleanup_step.task == "remove-offline-content-task"
+    assert cleanup_step.timeout.root == "5m"
+    assert cleanup_step.attempts == 3
+
+    # Test the command configuration
+    assert cleanup_step.config.run.path == "sh"
+
+    # Check that the actual command structure matches expectations
+    actual_command = cleanup_step.config.run.args[1]
+
+    # Check that all three AWS S3 remove commands are present
+    assert (
+        f"aws s3{cli_endpoint_url} rm s3://((site:web_bucket))/((site:url_path))/((site:short_id)).zip"
+        in actual_command
+    )
+    assert (
+        f"aws s3{cli_endpoint_url} rm s3://((site:web_bucket))/((site:url_path))/((site:short_id))-video.zip"
+        in actual_command
+    )
+    assert (
+        f"aws s3{cli_endpoint_url} rm s3://((site:offline_bucket))/((site:url_path))/ --recursive"
+        in actual_command
+    )
+
+    # Check that the core components are present
+    assert (
+        'echo "Removing offline content for site: ((site:url_path))"' in actual_command
+    )
+    assert 'echo "Offline content cleanup completed"' in actual_command
+
+    assert cleanup_step.config.platform == "linux"
+    assert cleanup_step.config.image_resource == AWS_CLI_REGISTRY_IMAGE
+
+    # Test environment variables for dev
+    if is_dev:
+        assert cleanup_step.params["AWS_ACCESS_KEY_ID"] == "test_access_key_id"
+        assert cleanup_step.params["AWS_SECRET_ACCESS_KEY"] == "test_secret_access_key"  # noqa: S105
+    else:
+        assert not hasattr(cleanup_step, "params") or cleanup_step.params is None
+
+
+def test_offline_build_gate_cleanup_task(website, settings, mocker):
+    """
+    Test that the offline build gate put step has proper failure handling attached
+    """
+    # Setup
+    settings.AWS_ACCESS_KEY_ID = "test_access_key_id"
+    settings.AWS_SECRET_ACCESS_KEY = "test_secret_access_key"  # noqa: S105
+    mock_utils_is_dev = mocker.patch("content_sync.utils.is_dev")
+    mock_pipeline_is_dev = mocker.patch(
+        "content_sync.pipelines.definitions.concourse.site_pipeline.is_dev"
+    )
+    mock_main_utils_is_dev = mocker.patch("main.utils.is_dev")
+    mock_utils_is_dev.return_value = False
+    mock_pipeline_is_dev.return_value = False
+    mock_main_utils_is_dev.return_value = False
+
+    config = SitePipelineDefinitionConfig(
+        site=website,
+        pipeline_name="test",
+        instance_vars="",
+        site_content_branch="main",
+        static_api_url="https://test.example.com/",
+        storage_bucket="test-storage",
+        artifacts_bucket="test-artifacts",
+        web_bucket="test-web",
+        offline_bucket="test-offline-bucket",
+        resource_base_url="https://test.example.com/",
+        ocw_hugo_themes_branch="main",
+        ocw_hugo_projects_branch="main",
+    )
+
+    pipeline_definition = SitePipelineDefinition(config=config)
+    rendered_definition = json.loads(pipeline_definition.json(indent=2, by_alias=True))
+
+    # Find the online job
+    online_job = None
+    for job in rendered_definition["jobs"]:
+        if job["name"] == "online-site-job":
+            online_job = job
+            break
+
+    assert online_job is not None, "Online job should exist"
+
+    # Find the offline build gate put step (should be the last step in the online job)
+    gate_put_step = None
+    for step in online_job["plan"]:
+        if (
+            "try" in step
+            and "put" in step["try"]
+            and step["try"]["put"] == "offline-build-gate"
+        ):
+            gate_put_step = step["try"]  # Get the inner put step, not the try step
+            break
+
+    assert gate_put_step is not None, "Offline build gate put step should exist"
+
+    # Verify that failure handling is attached to the inner put step
+    assert "on_error" in gate_put_step
+
+    # Verify the failure handling is the cleanup task
+    assert gate_put_step["on_error"]["task"] == "remove-offline-content-task"
+    assert gate_put_step["on_error"]["timeout"] == "5m"
+    assert gate_put_step["on_error"]["attempts"] == 3