This is an automated email from the ASF dual-hosted git repository.

potiuk pushed a commit to branch v3-4-test
in repository https://gitbox.apache.org/repos/asf/airflow.git


The following commit(s) were added to refs/heads/v3-4-test by this push:
     new f5b20a98fe3 [v3-4-test] Stop the cdxgen server containers when SBOM 
generation ends (#75613) (#75614)
f5b20a98fe3 is described below

commit f5b20a98fe34a9237522ae9e47cfac6c8d1b3fed
Author: github-actions[bot] 
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Sun Oct 25 19:22:10 2026 +0100

    [v3-4-test] Stop the cdxgen server containers when SBOM generation ends 
(#75613) (#75614)
    
    The servers were stopped by terminating their `docker run` process, but
    `docker run -t` does not forward signals to the container, so every server
    kept running, and holding its memory, for the rest of the CI job. Since the
    canary moved to larger self-hosted runners it starts one server per CPU, and
    the clone of airflow-site that follows was killed (exit code 137).
    
    A server's child process also inherited the exit handlers of the servers
    forked before it, so a server that exited early would stop the others.
    (cherry picked from commit 05f55513e99059c8263cabd2ddd0322b63739656)
    
    
    Generated-by: Claude Opus 5
    
    Co-authored-by: Jarek Potiuk <[email protected]>
---
 dev/breeze/src/airflow_breeze/utils/cdxgen.py | 74 ++++++++++++++++-----------
 dev/breeze/tests/test_cdxgen.py               | 73 ++++++++++++++++++++++++++
 2 files changed, 117 insertions(+), 30 deletions(-)

diff --git a/dev/breeze/src/airflow_breeze/utils/cdxgen.py 
b/dev/breeze/src/airflow_breeze/utils/cdxgen.py
index 04a03084c0a..705cbfcba54 100644
--- a/dev/breeze/src/airflow_breeze/utils/cdxgen.py
+++ b/dev/breeze/src/airflow_breeze/utils/cdxgen.py
@@ -21,7 +21,6 @@ import atexit
 import multiprocessing
 import os
 import signal
-import sys
 import time
 from abc import abstractmethod
 from collections.abc import Generator
@@ -80,41 +79,56 @@ def start_cdxgen_servers(application_root_path: Path, 
run_in_parallel: bool, par
         print("::endgroup::")
 
 
+def remove_cdxgen_server(name: str) -> None:
+    # `docker run -t` does not forward signals to the container, so 
terminating the `docker run`
+    # process leaves the server running and holding its memory for the rest of 
the CI job.
+    run_command(["docker", "rm", "--force", name], check=False, 
capture_output=True, quiet=True)
+
+
 def fork_cdxgen_server(application_root_path, port=9090):
+    name = f"airflow-cdxgen-server-{port}"
+    remove_cdxgen_server(name)
     pid = os.fork()
     if pid:
-        # Parent process - send signal to process group of the child process
+        # Parent process - send signal to process group of the child process. 
atexit runs the
+        # handlers in reverse, so the container is removed after its `docker 
run` is terminated.
+        atexit.register(remove_cdxgen_server, name)
         atexit.register(os.killpg, pid, signal.SIGTERM)
         # Give the server child process some time to start
     else:
-        # Check if we are not a group leader already (We should not be)
-        if os.getpid() != os.getsid(0):
-            # and create a new process group where we are the leader
-            os.setpgid(0, 0)
-        run_command(
-            [
-                "docker",
-                "run",
-                "--init",
-                "--rm",
-                "-p",
-                f"{port}:{port}",
-                "-v",
-                "/tmp:/tmp",
-                "-v",
-                f"{application_root_path}:/app",
-                "-t",
-                "ghcr.io/cyclonedx/cdxgen",
-                "--server",
-                "--server-host",
-                "0.0.0.0",
-                "--server-port",
-                str(port),
-            ],
-            check=True,
-        )
-        # we should get here when the server gets terminated
-        sys.exit(0)
+        try:
+            # Check if we are not a group leader already (We should not be)
+            if os.getpid() != os.getsid(0):
+                # and create a new process group where we are the leader
+                os.setpgid(0, 0)
+            run_command(
+                [
+                    "docker",
+                    "run",
+                    "--init",
+                    "--rm",
+                    "--name",
+                    name,
+                    "-p",
+                    f"{port}:{port}",
+                    "-v",
+                    "/tmp:/tmp",
+                    "-v",
+                    f"{application_root_path}:/app",
+                    "-t",
+                    "ghcr.io/cyclonedx/cdxgen",
+                    "--server",
+                    "--server-host",
+                    "0.0.0.0",
+                    "--server-port",
+                    str(port),
+                ],
+                check=True,
+            )
+        finally:
+            # We get here when the server gets terminated. The child inherited 
the atexit handlers
+            # of the servers forked before it, which would stop those servers, 
so they are skipped.
+            os._exit(0)
 
 
 def get_port_mapping(x):
diff --git a/dev/breeze/tests/test_cdxgen.py b/dev/breeze/tests/test_cdxgen.py
new file mode 100644
index 00000000000..c497b2eb0bf
--- /dev/null
+++ b/dev/breeze/tests/test_cdxgen.py
@@ -0,0 +1,73 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import signal
+from unittest import mock
+
+import pytest
+
+from airflow_breeze.utils import cdxgen
+
+RM_SERVER = mock.call(
+    ["docker", "rm", "--force", "airflow-cdxgen-server-8081"], check=False, 
capture_output=True, quiet=True
+)
+
+
[email protected](cdxgen.atexit, "register", autospec=True)
[email protected](cdxgen, "run_command", autospec=True)
[email protected](cdxgen.os, "fork", autospec=True, return_value=1234)
+def test_parent_removes_the_server_container_after_terminating_docker_run(
+    mock_fork, mock_run_command, mock_register
+):
+    cdxgen.fork_cdxgen_server("/app", port=8081)
+
+    # A container left by an earlier run would hold the name and the port.
+    assert mock_run_command.call_args_list == [RM_SERVER]
+    # atexit calls the handlers in reverse order of registration.
+    assert mock_register.call_args_list == [
+        mock.call(cdxgen.remove_cdxgen_server, "airflow-cdxgen-server-8081"),
+        mock.call(cdxgen.os.killpg, 1234, signal.SIGTERM),
+    ]
+
+
+class ChildExit(Exception):
+    pass
+
+
[email protected](
+    "docker_run_error",
+    [pytest.param(None, id="server-terminated"), 
pytest.param(OSError("failed"), id="server-failed")],
+)
[email protected](cdxgen.os, "_exit", autospec=True, side_effect=ChildExit)
[email protected](cdxgen.os, "setpgid", autospec=True)
[email protected](cdxgen, "run_command", autospec=True)
[email protected](cdxgen.os, "fork", autospec=True, return_value=0)
+def test_child_runs_the_server_container_and_skips_inherited_exit_handlers(
+    mock_fork, mock_run_command, mock_setpgid, mock_exit, docker_run_error
+):
+    mock_run_command.side_effect = [mock.DEFAULT, docker_run_error or 
mock.DEFAULT]
+
+    # os._exit, unlike sys.exit, does not run the atexit handlers inherited 
from the parent,
+    # which would stop the servers forked earlier.
+    with pytest.raises(ChildExit):
+        cdxgen.fork_cdxgen_server("/app", port=8081)
+
+    mock_exit.assert_called_once_with(0)
+    docker_run = mock_run_command.call_args_list[-1].args[0]
+    assert docker_run[:2] == ["docker", "run"]
+    assert docker_run[docker_run.index("--name") + 1] == 
"airflow-cdxgen-server-8081"

Reply via email to