From db0b3ac9acb780e8868ac915993aa0297e3025ed Mon Sep 17 00:00:00 2001 From: Teddy Andrieux Date: Tue, 6 Oct 2026 09:03:59 +0200 Subject: [PATCH 1/4] refactor(tools): make the orphan clean up about more than ports A cloud that answers the call doing the work and not the one waiting for it leaves more than ports behind, and the tool that sweeps after it is named and written as though ports were all there is. Give it the name of what it does and one function per thing it sweeps, so that the next kind of leftover is a function rather than a rewrite. Nothing it does changes. --- action.yaml | 4 +- docs/TROUBLESHOOTING.md | 2 +- ...clean_orphan_ports.py => clean_orphans.py} | 44 ++++++++++++------- 3 files changed, 32 insertions(+), 18 deletions(-) rename tools/{clean_orphan_ports.py => clean_orphans.py} (73%) diff --git a/action.yaml b/action.yaml index ecab4e05..63ab1b8a 100644 --- a/action.yaml +++ b/action.yaml @@ -268,7 +268,7 @@ runs: exit 0 fi echo "Spawn failed [$attempt/$MAX_ATTEMPTS]" - uv run --group snapshot tools/clean_orphan_ports.py || true + uv run --group snapshot tools/clean_orphans.py || true done exit 1 @@ -432,7 +432,7 @@ runs: exit 0 fi echo "Destroy failed [$attempt/$MAX_ATTEMPTS]" - uv run --group snapshot tools/clean_orphan_ports.py || true + uv run --group snapshot tools/clean_orphans.py || true done exit 1 - name: Remove the stack diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 4ecc7014..08f44002 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -27,7 +27,7 @@ destroying answers that the subnet still has an allocation, which leaves the platform running and unremovable. ```bash -uv run --group snapshot tools/clean_orphan_ports.py +uv run --group snapshot tools/clean_orphans.py ``` Only a port of ours, on a network of ours, that nothing is using and that the diff --git a/tools/clean_orphan_ports.py b/tools/clean_orphans.py similarity index 73% rename from tools/clean_orphan_ports.py rename to tools/clean_orphans.py index 856ed6f3..8232f88f 100755 --- a/tools/clean_orphan_ports.py +++ b/tools/clean_orphans.py @@ -1,18 +1,15 @@ #! /usr/bin/env python3 """ -Take away the ports a failed spawn left behind. +Take away what a failed spawn left behind on the cloud. -A cloud that goes down mid-spawn can answer the call that creates a port and -then not the one that waits for it. The port exists, the stack never learned -of it, and since every address here is fixed, it holds the one the next try -needs: spawning again answers `IpAddressAlreadyAllocated` for as long as it -is there, and destroying answers that the subnet still has an allocation. +A cloud that goes down mid-spawn can answer the call that does the work and +then not the one that waits for it. What was asked for exists, the stack +never learned of it, and the next attempt walks straight into it. -Only a port of ours, on a network of ours, that nothing is using and that the -stack does not know about. Anything a router, the DHCP agent or an instance -holds is left alone. +Only what belongs to the stack and what the stack does not know about is +taken away. Whatever the cloud itself holds is left alone. -NOTE: OpenStack only, being the only provider where we pin the addresses. +NOTE: OpenStack only, the cloud this has been seen on. """ import argparse @@ -34,7 +31,7 @@ def __main__() -> int: - """Delete the ports of a stack that the stack does not know about.""" + """Take away what the stack owns on the cloud and does not know about.""" args = _parse_args() workspace, stack_name = stack.open_stack(args.stack) @@ -44,14 +41,31 @@ def __main__() -> int: return 0 resources = workspace.export_stack(stack_name).deployment.get("resources", []) + cloud = openstack.connect() + + return _clean_ports(cloud, resources, stack_name) + + +def _clean_ports( + cloud: openstack.connection.Connection, + resources: list[dict], + stack_name: str, +) -> int: + """ + Delete the ports of a stack that the stack does not know about. + + Every address here is fixed, so a port left behind holds the one the next + attempt needs: spawning again answers `IpAddressAlreadyAllocated` for as + long as it is there, and destroying answers that the subnet still has an + allocation. + """ networks = {r["id"] for r in resources if r.get("type") == NETWORK_TYPE} known = {r["id"] for r in resources if r.get("type") == PORT_TYPE} if not networks: - print(f"Nothing to do, {stack_name} holds no network") + print(f"No port to look at, {stack_name} holds no network") return 0 - cloud = openstack.connect() - seen, orphans = _survey(cloud, networks, known) + seen, orphans = _survey_ports(cloud, networks, known) print(f"{len(seen)} port(s) on the {len(networks)} network(s) of {stack_name}:") for line in seen: print(f"\t{line}") @@ -78,7 +92,7 @@ def __main__() -> int: return 1 if failed else 0 -def _survey( +def _survey_ports( cloud: openstack.connection.Connection, networks: set[str], known: set[str], From 2c02fbaf53f2a3b8b8c6e2e455a79f1e04b8bec3 Mon Sep 17 00:00:00 2001 From: Teddy Andrieux Date: Tue, 6 Oct 2026 09:05:47 +0200 Subject: [PATCH 2/4] fix(tools): detach the volumes a timed out attach left behind OVH answered a volume attachment with a 504 after three minutes, having made the attachment all the same. Pulumi was told the call failed, so the stack never learned of it, and every attempt after that answered `Invalid volume: volume ... already attached`. Retrying does not get past it: the attachment the next attempt wants is the one already there. A destroy does not either, since a volume in use cannot be deleted. Detach what the stack owns and holds no attachment for, next to the ports already swept between two attempts. The volume comes back available and the attempt that follows makes the attachment itself, which is what lands it in the stack. Taking the attachment into the stack instead would ask the tool to write state that a spawn is about to write anyway. The wait is ours rather than the one `detach_volume` does, which ignores the timeout it is handed and would hold the job until the runner gives up on it. --- docs/TROUBLESHOOTING.md | 31 +++++---- tools/clean_orphans.py | 142 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 161 insertions(+), 12 deletions(-) diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 08f44002..da9d50c4 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -17,23 +17,32 @@ touched, so this works just as well on a platform someone else spawned. A platform running on a key of your own is reported as such. That key is yours to point at. -## A failed spawn left ports behind +## A failed spawn left something behind -A cloud that goes down mid-spawn can answer the call that creates a port and -then not the one that waits for it. The port exists and the stack never -learned of it. Every address here is fixed, so that port holds the one the -next attempt needs: spawning again answers `IpAddressAlreadyAllocated`, and -destroying answers that the subnet still has an allocation, which leaves the -platform running and unremovable. +A cloud that goes down mid-spawn can answer the call that does the work and +then not the one that waits for it. What was asked for exists, the stack never +learned of it, and the next attempt walks straight into it. ```bash uv run --group snapshot tools/clean_orphans.py ``` -Only a port of ours, on a network of ours, that nothing is using and that the -stack does not know about. Whatever a router, the DHCP agent or an instance -holds is left alone. The action runs it between two attempts, so a spawn or a -destroy that trips over one recovers on its own. +Two kinds of leftover get a platform stuck for good: + +- A port holds a fixed address, and every address here is fixed, so it holds + the one the next attempt needs. Spawning again answers + `IpAddressAlreadyAllocated`, and destroying answers that the subnet still + has an allocation. +- A volume attachment holds its volume. Attaching again answers that the + volume is already attached, and destroying cannot delete a volume that is in + use. The volume is detached rather than taken into the stack, so the next + attempt makes the attachment itself. + +Only what the stack owns and does not know about is touched: a port on a +network of ours, a volume of ours the stack holds no attachment for. Whatever +the cloud itself holds, its routers and its DHCP agents, is left alone. The +action runs it between two attempts, so a spawn or a destroy that trips over +one recovers on its own. ## Work against another state backend diff --git a/tools/clean_orphans.py b/tools/clean_orphans.py index 8232f88f..d06ebd5d 100755 --- a/tools/clean_orphans.py +++ b/tools/clean_orphans.py @@ -23,7 +23,11 @@ NETWORK_TYPE = "openstack:networking/network:Network" PORT_TYPE = "openstack:networking/port:Port" +VOLUME_TYPE = "openstack:blockstorage/volume:Volume" +VOLUME_ATTACH_TYPE = "openstack:compute/volumeAttach:VolumeAttach" OPENSTACK = "openstack" +AVAILABLE = "available" +VOLUME_ERROR = "error" # What the cloud owns on a network it hands us: the DHCP port, the router # interfaces, the floating IPs. Everything else on a network of ours is ours, # whether or not it says what it is attached to. @@ -43,7 +47,10 @@ def __main__() -> int: resources = workspace.export_stack(stack_name).deployment.get("resources", []) cloud = openstack.connect() - return _clean_ports(cloud, resources, stack_name) + failed = _clean_ports(cloud, resources, stack_name) + failed += _clean_volume_attachments(cloud, resources, stack_name, args.timeout) + + return 1 if failed else 0 def _clean_ports( @@ -121,10 +128,143 @@ def _survey_ports( return seen, orphans +def _clean_volume_attachments( + cloud: openstack.connection.Connection, + resources: list[dict], + stack_name: str, + timeout: int, +) -> int: + """ + Detach the volumes of a stack that the stack holds no attachment for. + + An attachment that went through unanswered holds its volume for good: + attaching again answers that the volume is already attached, which no + amount of retrying gets past, and destroying cannot delete a volume that + is in use. + + The volume is detached rather than taken into the stack, so that the next + attempt makes the attachment itself and the stack ends up knowing about + the one it runs on. + """ + volumes = {r["id"] for r in resources if r.get("type") == VOLUME_TYPE} + known = { + volume + for r in resources + if r.get("type") == VOLUME_ATTACH_TYPE + for volume in _attached_volumes(r) + } + if not volumes: + print(f"No attachment to look at, {stack_name} holds no volume") + return 0 + + seen, orphans = _survey_volumes(cloud, volumes, known) + print(f"{len(seen)} volume(s) of {stack_name}:") + for line in seen: + print(f"\t{line}") + + if not orphans: + return 0 + + print(f"{len(orphans)} to detach:") + detached, failed = [], [] + for volume in orphans: + name = volume.name or volume.id + try: + _detach(cloud, volume, timeout) + except openstack.exceptions.SDKException as err: + print(f"{name} could not be detached: {err}", file=sys.stderr) + failed.append(name) + else: + detached.append(f"{name} ({volume.id})") + + if detached: + step_summary.write(f"Volumes left attached by `{stack_name}`", detached) + + return 1 if failed else 0 + + +def _attached_volumes(resource: dict) -> set[str]: + """ + Say which volume an attachment of the stack holds. + + NOTE: Read twice, from the output and from the id, which carries the + instance and the volume it joins. Taking a free volume for an attached + one only leaves it where it is, where the other way around would pull a + volume out from under a running instance. + """ + outputs = resource.get("outputs") or {} + _, _, from_id = str(resource.get("id") or "").partition("/") + + return {volume for volume in (outputs.get("volumeId"), from_id) if volume} + + +def _survey_volumes( + cloud: openstack.connection.Connection, + volumes: set[str], + known: set[str], +) -> tuple[list[str], list]: + """ + Say what holds the volumes of a stack, and which of them are ours to free. + + NOTE: Every volume is reported, like every port is. A spawn that keeps + failing on one volume says nothing about the others, and whether they are + attached at all is what tells how far the last attempt got. + """ + seen, orphans = [], [] + for volume_id in sorted(volumes): + try: + volume = cloud.block_storage.get_volume(volume_id) + except openstack.exceptions.ResourceNotFound: + seen.append(f"{volume_id:<40} gone from the cloud") + continue + + servers = ", ".join(a.get("server_id", "?") for a in volume.attachments or []) + if volume_id in known: + held = f"in the stack, on {servers}" if servers else "in the stack, attached to nothing" + elif servers: + held = f"left attached to {servers}" + orphans.append(volume) + else: + held = volume.status + seen.append(f"{volume.name or volume_id:<40} {held}") + + return seen, orphans + + +def _detach( + cloud: openstack.connection.Connection, + volume: openstack.block_storage.v3.volume.Volume, + timeout: int, +) -> None: + """ + Take a volume off whatever holds it, and wait until it really is off. + + NOTE: Waiting on our own terms rather than through `detach_volume`, which + waits without a timeout whatever it is handed. A detach that never ends + would hold the run that called us for as long as the job is given. + """ + for attachment in volume.attachments or []: + server_id = attachment.get("server_id") + print(f"\t{volume.name or volume.id} off {server_id}") + cloud.compute.delete_volume_attachment( + server=server_id, + volume=volume.id, + ignore_missing=True, + ) + + cloud.block_storage.wait_for_status( + volume, + status=AVAILABLE, + failures=[VOLUME_ERROR], + wait=timeout, + ) + + def _parse_args() -> argparse.Namespace: """Read the command line.""" parser = argparse.ArgumentParser(description=__doc__.strip().splitlines()[0]) parser.add_argument("--stack", default=None, help="stack to clean up (default: current)") + parser.add_argument("--timeout", type=int, default=600, help="seconds to wait for a detach") return parser.parse_args() From b56363412ec9a90482d4edcc453bd89533d93882 Mon Sep 17 00:00:00 2001 From: Teddy Andrieux Date: Tue, 6 Oct 2026 10:50:52 +0200 Subject: [PATCH 3/4] feat(action): show the stacks a list found in the job summary A list hands its answer to whoever called it and leaves nothing of its own behind. The nightly collection shows an empty summary, and what it found has to be read out of the log of a step that only echoed it. Write the names under the age they were matched on, in the shape the tools already use for their own sections. A run that found nothing says so rather than showing a heading over nothing at all. --- action.yaml | 10 ++++++++++ docs/GITHUB_ACTION.md | 5 +++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/action.yaml b/action.yaml index 63ab1b8a..529de97e 100644 --- a/action.yaml +++ b/action.yaml @@ -205,6 +205,16 @@ runs: ) echo "STACKS_LIST=$stack_list" >> "$GITHUB_OUTPUT" + # NOTE: Written here rather than left to whoever called us. A list is + # the whole answer of this action, and a job that only hands it on has + # nothing of its own to show for the run. + { + echo "### Stacks older than ${AGE}h" + echo + jq -r 'if length == 0 then "None." else .[] | "- `\(.)`" end' <<< "$stack_list" + echo + } >> "$GITHUB_STEP_SUMMARY" + # Spawn {{{ # Prepare stack config diff --git a/docs/GITHUB_ACTION.md b/docs/GITHUB_ACTION.md index dbc7d808..91825978 100644 --- a/docs/GITHUB_ACTION.md +++ b/docs/GITHUB_ACTION.md @@ -80,8 +80,9 @@ to be given to both, or left out of both to take the name of the repository. |-------|----------|------------| | `age` | yes | An age in hours | -It answers with `stacks_list`, the stacks older than that age. This is what a -garbage collection runs on. +It answers with `stacks_list`, the stacks older than that age, and writes the +same names to the summary of the job. This is what a garbage collection runs +on. ## Details From 6d0d2ed8b8fc2fc0d0962899616df1dad7fbac69 Mon Sep 17 00:00:00 2001 From: Teddy Andrieux Date: Tue, 6 Oct 2026 18:30:22 +0200 Subject: [PATCH 4/4] ci: read the AWS account id from a secret The account id is a secret now rather than a repository variable, and every workflow still reads it where it no longer is. The role ARN comes out with nothing in the middle, which AWS refuses, so the credentials step fails before a job has done anything. Nothing else has to move. The id is only ever read inside a step, where a secret reads like a variable, and the reusable workflows already take what they need through `secrets: inherit`. --- .github/workflows/gc-cron.yaml | 4 ++-- .github/workflows/platform-test.yaml | 4 ++-- .github/workflows/snapshot-test.yaml | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/gc-cron.yaml b/.github/workflows/gc-cron.yaml index 72990a9a..971d37b4 100644 --- a/.github/workflows/gc-cron.yaml +++ b/.github/workflows/gc-cron.yaml @@ -23,7 +23,7 @@ jobs: - name: Configure AWS credentials uses: aws-actions/configure-aws-credentials@7474bc4690e29a8392af63c5b98e7449536d5c3a # v4.3.1 with: - role-to-assume: arn:aws:iam::${{ vars.AWS_MAIN_ACCOUNT_ID }}:role/scality-github-e2e-infrastructure + role-to-assume: arn:aws:iam::${{ secrets.AWS_MAIN_ACCOUNT_ID }}:role/scality-github-e2e-infrastructure role-session-name: GitHubActions-${{ github.run_id }} aws-region: eu-north-1 @@ -63,7 +63,7 @@ jobs: - name: Configure AWS credentials uses: aws-actions/configure-aws-credentials@7474bc4690e29a8392af63c5b98e7449536d5c3a # v4.3.1 with: - role-to-assume: arn:aws:iam::${{ vars.AWS_MAIN_ACCOUNT_ID }}:role/scality-github-e2e-infrastructure + role-to-assume: arn:aws:iam::${{ secrets.AWS_MAIN_ACCOUNT_ID }}:role/scality-github-e2e-infrastructure role-session-name: GitHubActions-${{ github.run_id }} aws-region: eu-north-1 diff --git a/.github/workflows/platform-test.yaml b/.github/workflows/platform-test.yaml index 6f5a53b2..dda0f40a 100644 --- a/.github/workflows/platform-test.yaml +++ b/.github/workflows/platform-test.yaml @@ -55,7 +55,7 @@ jobs: - name: Configure AWS credentials uses: aws-actions/configure-aws-credentials@7474bc4690e29a8392af63c5b98e7449536d5c3a # v4.3.1 with: - role-to-assume: arn:aws:iam::${{ vars.AWS_MAIN_ACCOUNT_ID }}:role/scality-github-e2e-infrastructure + role-to-assume: arn:aws:iam::${{ secrets.AWS_MAIN_ACCOUNT_ID }}:role/scality-github-e2e-infrastructure role-session-name: GitHubActions-${{ github.run_id }} aws-region: eu-north-1 @@ -87,7 +87,7 @@ jobs: - name: Configure AWS credentials uses: aws-actions/configure-aws-credentials@7474bc4690e29a8392af63c5b98e7449536d5c3a # v4.3.1 with: - role-to-assume: arn:aws:iam::${{ vars.AWS_MAIN_ACCOUNT_ID }}:role/scality-github-e2e-infrastructure + role-to-assume: arn:aws:iam::${{ secrets.AWS_MAIN_ACCOUNT_ID }}:role/scality-github-e2e-infrastructure role-session-name: GitHubActions-${{ github.run_id }} aws-region: eu-north-1 diff --git a/.github/workflows/snapshot-test.yaml b/.github/workflows/snapshot-test.yaml index 770a0569..f26a76ab 100644 --- a/.github/workflows/snapshot-test.yaml +++ b/.github/workflows/snapshot-test.yaml @@ -49,7 +49,7 @@ jobs: - name: Configure AWS credentials uses: aws-actions/configure-aws-credentials@7474bc4690e29a8392af63c5b98e7449536d5c3a # v4.3.1 with: - role-to-assume: arn:aws:iam::${{ vars.AWS_MAIN_ACCOUNT_ID }}:role/scality-github-e2e-infrastructure + role-to-assume: arn:aws:iam::${{ secrets.AWS_MAIN_ACCOUNT_ID }}:role/scality-github-e2e-infrastructure role-session-name: GitHubActions-${{ github.run_id }} aws-region: eu-north-1 @@ -141,7 +141,7 @@ jobs: - name: Configure AWS credentials uses: aws-actions/configure-aws-credentials@7474bc4690e29a8392af63c5b98e7449536d5c3a # v4.3.1 with: - role-to-assume: arn:aws:iam::${{ vars.AWS_MAIN_ACCOUNT_ID }}:role/scality-github-e2e-infrastructure + role-to-assume: arn:aws:iam::${{ secrets.AWS_MAIN_ACCOUNT_ID }}:role/scality-github-e2e-infrastructure role-session-name: GitHubActions-${{ github.run_id }} aws-region: eu-north-1