From b4f3b42b603a2876fe2b3da2037020533fac8b45 Mon Sep 17 00:00:00 2001 From: Teddy Andrieux Date: Mon, 12 Oct 2026 15:10:17 +0000 Subject: [PATCH 1/3] fix(providers): keep machines on the image they booted on An image name is resolved again on every update, to the latest image of that name. Once the cloud publishes a newer one, any update of a live platform replaced every machine, the bastion included, whatever it was about. Toggling `offline` was enough to trigger it. The image is now only read when a machine is created. This is also a prerequisite for recreating a single node of a live platform, which goes through such an update. --- DESIGN.md | 11 +++++++++++ providers/aws.py | 5 +++++ providers/openstack.py | 5 +++++ 3 files changed, 21 insertions(+) diff --git a/DESIGN.md b/DESIGN.md index 248eb49f..a0356e14 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -17,6 +17,17 @@ The provider is picked by the `provider` configuration key, which keeps a stack configuration portable from one cloud to another as long as the values it uses exist on both. +### A machine keeps its image + +The image a machine boots on is read once, when the machine is created. An +image name is looked up again on every update, and it answers with the latest +image of that name. Following it would replace every machine of a live +platform as soon as the cloud publishes a newer one, whatever the update was +about. Changing `instance_image`, `bastion_image`, `restore_snapshot` or +`instance_image_file` on a live platform therefore leaves its machines alone. +Only the machines created afterwards boot on the new image, and destroying the +platform is the way to have all of them on it. + ### Provider differences The interface hides most of the differences, a few of them remain visible from diff --git a/providers/aws.py b/providers/aws.py index cf9bd80f..d60b40a2 100644 --- a/providers/aws.py +++ b/providers/aws.py @@ -96,6 +96,11 @@ def create_instance( else None ), tags=tags, + # NOTE: An image name is looked up again on every update, and it + # answers with the latest AMI of that name. Once a newer one is + # published, any update would replace every machine, whatever the + # update was about. + opts=pulumi.ResourceOptions(ignore_changes=["ami"]), ) def create_key_pair( diff --git a/providers/openstack.py b/providers/openstack.py index a8364c9c..634e556f 100644 --- a/providers/openstack.py +++ b/providers/openstack.py @@ -168,6 +168,11 @@ def create_instance( create=INSTANCE_TIMEOUT, update=INSTANCE_TIMEOUT, ), + # NOTE: An image name is looked up again on every update, and + # it answers with the latest image of that name. Once the cloud + # publishes a newer one, any update would replace every + # machine, whatever the update was about. + ignore_changes=["image_id"], ), ) From 753edb1a4227c89a164e897579a07e3b9dc97712 Mon Sep 17 00:00:00 2001 From: Teddy Andrieux Date: Mon, 12 Oct 2026 15:16:23 +0000 Subject: [PATCH 2/3] feat(action): update a live platform The action could only spawn a platform or destroy it. Any change, even switching it offline, meant spawning another one and losing what ran on the first. `update` takes the configuration the platform was last deployed with, merges the keys it is handed and brings the platform up again. It writes the SSH files back from the stack first. This lets it run from another job than the spawn, where the key the update waits through is missing. An end to end test switches a platform offline on each provider. It checks that the platform is cut off and that no machine was replaced. --- .github/workflows/e2e-tests.yaml | 17 ++++ .github/workflows/update-test.yaml | 129 +++++++++++++++++++++++++++++ CONTRIBUTING.md | 4 +- action.yaml | 70 +++++++++++----- docs/GITHUB_ACTION.md | 35 +++++++- tests/check_offline.sh | 17 ++++ tests/check_options.sh | 8 +- tools/merge_config.py | 66 +++++++++++++++ 8 files changed, 314 insertions(+), 32 deletions(-) create mode 100644 .github/workflows/update-test.yaml create mode 100755 tests/check_offline.sh create mode 100755 tools/merge_config.py diff --git a/.github/workflows/e2e-tests.yaml b/.github/workflows/e2e-tests.yaml index 1551d5ab..0df35230 100644 --- a/.github/workflows/e2e-tests.yaml +++ b/.github/workflows/e2e-tests.yaml @@ -56,6 +56,23 @@ jobs: checks: ${{ matrix.checks }} secrets: inherit + update: + name: ${{ matrix.name }} + strategy: + fail-fast: false + matrix: + include: + - name: update-aws + provider: aws + - name: update-openstack + provider: openstack + uses: ./.github/workflows/update-test.yaml + with: + name: ${{ matrix.name }} + stack: "${{ github.workflow }}-${{ github.run_number }}-${{ matrix.name }}" + provider: ${{ matrix.provider }} + secrets: inherit + snapshot: uses: ./.github/workflows/snapshot-test.yaml with: diff --git a/.github/workflows/update-test.yaml b/.github/workflows/update-test.yaml new file mode 100644 index 00000000..929a8e20 --- /dev/null +++ b/.github/workflows/update-test.yaml @@ -0,0 +1,129 @@ +name: "Update Test" + +on: + workflow_call: + inputs: + name: + description: "What the platform is called, in the job names" + required: true + type: string + stack: + description: "Name of the stack to spawn and update" + required: true + type: string + provider: + description: "Cloud provider to spawn the platform on" + required: true + type: string + +permissions: + id-token: write + actions: read + contents: read + +# NOTE: Read by the OpenStack provider straight from the environment, the way +# the AWS one reads what the credentials step leaves behind. They are harmless +# to a platform spawned on AWS, which never looks. The password cannot be set +# here: a workflow level environment cannot read secrets. +env: + OS_AUTH_URL: ${{ vars.OS_AUTH_URL }} + OS_USERNAME: ${{ vars.OS_USERNAME }} + OS_PROJECT_ID: ${{ vars.OS_PROJECT_ID }} + OS_PROJECT_NAME: ${{ vars.OS_PROJECT_NAME }} + # The variable holds the region, the tooling reads it under another name + OS_REGION_NAME: ${{ vars.OS_REGION }} + # Not configured anywhere, being the same for every OVH project + OS_USER_DOMAIN_NAME: "Default" + OS_PROJECT_DOMAIN_ID: "default" + OS_IDENTITY_API_VERSION: "3" + +jobs: + # Change a live platform rather than spawn one with the change already + # made. It updates the same platform several times in a row, which is why + # it is not a platform test like the others. + test: + name: "${{ inputs.name }} test" + runs-on: ubuntu-24.04 + env: + OS_PASSWORD: ${{ secrets.OS_PASSWORD }} + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + # NOTE: Needed whatever the provider, the stacks living in an S3 bucket + - name: Configure AWS credentials + uses: aws-actions/configure-aws-credentials@e1253824e5c10ff9df46874f81ed3ec929e19cfd # v6.3.0 + with: + 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 + + - name: Spawn the platform + id: spawn + uses: ./ + with: + action: spawn + artifacts_user: ${{ secrets.ARTIFACTS_USER }} + artifacts_password: ${{ secrets.ARTIFACTS_PASSWORD }} + stack_name: ${{ inputs.stack }} + configuration: |- + provider: ${{ inputs.provider }} + instance_image: rocky-9 + instance_count: 2 + authorized_cidrs: ${{ format('[{0},"__my_ip__"]', join(fromJSON(vars.AUTHORIZED_CIDRS), ',')) }} + + - name: Check the platform starts online + run: ssh -F ssh_config node-1 'timeout 15 curl -sS -o /dev/null https://1.1.1.1' + + - name: Take the platform offline + id: offline + uses: ./ + with: + action: update + stack_name: ${{ inputs.stack }} + configuration: |- + offline: true + + - name: Check the platform is offline + run: ./tests/check_offline.sh + + # NOTE: The whole point of switching a live platform, rather than + # spawning another one offline, is to keep what runs on it + - name: Check no machine was replaced + env: + BASTION_BEFORE: ${{ steps.spawn.outputs.bastion }} + BASTION_AFTER: ${{ steps.offline.outputs.bastion }} + NODES_BEFORE: ${{ steps.spawn.outputs.nodes }} + NODES_AFTER: ${{ steps.offline.outputs.nodes }} + run: |- + before=$(jq -cS '{bastion: .id}' <<< "$BASTION_BEFORE") + before+=$(jq -cS 'map_values(.id)' <<< "$NODES_BEFORE") + after=$(jq -cS '{bastion: .id}' <<< "$BASTION_AFTER") + after+=$(jq -cS 'map_values(.id)' <<< "$NODES_AFTER") + echo "before: $before" + echo "after: $after" + [ "$before" == "$after" ] + + destroy: + name: "${{ inputs.name }} destroy" + needs: test + if: always() + runs-on: ubuntu-24.04 + env: + OS_PASSWORD: ${{ secrets.OS_PASSWORD }} + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Configure AWS credentials + uses: aws-actions/configure-aws-credentials@e1253824e5c10ff9df46874f81ed3ec929e19cfd # v6.3.0 + with: + 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 + + - name: Destroy platform + uses: ./ + with: + action: destroy + stack_name: ${{ inputs.stack }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f413c5c5..fce280c3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -33,7 +33,9 @@ destroys them. Two more platforms go up beside them on OpenStack: one asking for everything at once, offline with extra networks and extra volumes, and one that is snapshotted and spawned back from its images, carrying a file written before the snapshot to prove it came back whole. That one takes its images -away afterwards, nothing else ever would. +away afterwards, nothing else ever would. One last platform goes up on each +provider and is changed while it runs, through the `update` action: it is +switched offline, and none of its machines may be replaced on the way. Each of them is a workflow of its own carrying its spawn, its checks and its destroy, so a platform goes away as soon as its own test is done rather than diff --git a/action.yaml b/action.yaml index 2f5faf5e..2d371991 100644 --- a/action.yaml +++ b/action.yaml @@ -3,7 +3,7 @@ description: Spawn and destroy platforms inputs: action: - description: "The action to perform (setup, list, spawn, snapshot, delete-snapshot or destroy)" + description: "The action to perform (setup, list, spawn, update, snapshot, delete-snapshot or destroy)" required: true default: "spawn" @@ -39,7 +39,7 @@ inputs: # Spawn specific configuration: - description: "The configuration for the Pulumi stack (yaml format - only for spawn action)" + description: "The configuration for the Pulumi stack (yaml format - for spawn, or the keys to change for update)" required: false default: "" @@ -57,10 +57,10 @@ inputs: outputs: bastion: - description: "The bastion output (only for spawn action)" + description: "The bastion output (only for spawn and update actions)" value: ${{ steps.spawn_outputs.outputs.bastion || '' }} nodes: - description: "The nodes output (only for spawn action)" + description: "The nodes output (only for spawn and update actions)" value: ${{ steps.spawn_outputs.outputs.nodes || '' }} stacks_list: description: "The list of stacks older than the given age (only for list action)" @@ -75,24 +75,24 @@ runs: ACTION: ${{ inputs.action }} run: | case "$ACTION" in - setup|list|spawn|snapshot|delete-snapshot|destroy) ;; + setup|list|spawn|update|snapshot|delete-snapshot|destroy) ;; *) - echo "Invalid action: $ACTION. Must be 'setup', 'list', 'spawn', 'snapshot', 'delete-snapshot' or 'destroy'." + echo "Invalid action: $ACTION. Must be 'setup', 'list', 'spawn', 'update', 'snapshot', 'delete-snapshot' or 'destroy'." exit 1 ;; esac - - name: "Check argument for spawn" - if: ${{ inputs.action == 'spawn' }} + - name: "Check argument for spawn and update" + if: ${{ inputs.action == 'spawn' || inputs.action == 'update' }} shell: bash env: CONFIGURATION: ${{ inputs.configuration }} run: | if [ "$CONFIGURATION" == "" ]; then - echo "Configuration must be provided for spawn action." + echo "Configuration must be provided for spawn and update actions." exit 1 fi - name: "Check stack argument" - if: ${{ inputs.action == 'destroy' || inputs.action == 'spawn' || inputs.action == 'snapshot' }} + if: ${{ inputs.action == 'destroy' || inputs.action == 'spawn' || inputs.action == 'update' || inputs.action == 'snapshot' }} shell: bash env: STACK_NAME_INPUT: ${{ inputs.stack_name }} @@ -249,16 +249,42 @@ runs: pulumi $PULUMI_DEFAULT_ARGS config set --type bool ssh_private_key_create true fi pulumi $PULUMI_DEFAULT_ARGS config set --type bool disable_auto_stop true + # }}} + + # Update {{{ + - name: Merge the configuration + if: ${{ inputs.action == 'update' }} + shell: bash + working-directory: ${{ github.action_path }} + # NOTE: Started from the configuration the platform was last deployed + # with rather than from whatever file is lying here. The job that + # spawned it has one, any other job has none, and both have to update + # the very same platform. + env: + CONFIGURATION: ${{ inputs.configuration }} + run: |- + pulumi $PULUMI_DEFAULT_ARGS stack select "$STACK_NAME" + pulumi $PULUMI_DEFAULT_ARGS config refresh --force + uv run tools/merge_config.py "$PULUMI_STACK_FILE" <<< "$CONFIGURATION" + - name: Bring the SSH files back + if: ${{ inputs.action == 'update' }} + shell: bash + working-directory: ${{ github.action_path }} + # NOTE: The update waits for every machine it replaces to answer SSH, + # with the key the spawn wrote. Only the job that spawned has that key. + run: |- + uv run tools/get_ssh_config.py --stack "$STACK_NAME" + # }}} - # Preview and spawn + # Preview and bring up {{{ - name: Preview - if: ${{ inputs.action == 'spawn' }} + if: ${{ inputs.action == 'spawn' || inputs.action == 'update' }} shell: bash working-directory: ${{ github.action_path }} run: |- pulumi $PULUMI_DEFAULT_ARGS preview - - name: Spawn - if: ${{ inputs.action == 'spawn' }} + - name: Bring the platform up + if: ${{ inputs.action == 'spawn' || inputs.action == 'update' }} shell: bash working-directory: ${{ github.action_path }} env: @@ -268,23 +294,23 @@ runs: # to do the first time. The provider already retries a call that never # connects, this catches what it answers and then cannot deliver. # - # The stack is not cancelled in between. It was created a few steps ago - # so nothing else can hold it, and cancelling an update that has just - # failed abandons whatever was in flight, which is how a port outlives - # the stack that made it and keeps an address nobody can take back. + # The stack is not cancelled in between. Cancelling an update that has + # just failed abandons whatever was in flight, which is how a port + # outlives the stack that made it and keeps an address nobody can take + # back. run: |- for attempt in $(seq 1 "$MAX_ATTEMPTS"); do if pulumi $PULUMI_DEFAULT_ARGS up --yes --skip-preview; then exit 0 fi - echo "Spawn failed [$attempt/$MAX_ATTEMPTS]" + echo "Update failed [$attempt/$MAX_ATTEMPTS]" uv run --group snapshot tools/clean_orphans.py || true done exit 1 # Add symlink to ssh_config to workspace - name: Copy SSH config - if: ${{ inputs.action == 'spawn' }} + if: ${{ inputs.action == 'spawn' || inputs.action == 'update' }} shell: bash working-directory: ${{ github.action_path }} env: @@ -328,7 +354,7 @@ runs: # Add step summary - name: Step Summary - if: ${{ inputs.action == 'spawn' }} + if: ${{ inputs.action == 'spawn' || inputs.action == 'update' }} shell: bash working-directory: ${{ github.action_path }} run: |- @@ -358,7 +384,7 @@ runs: # Add outputs - name: Set outputs - if: ${{ inputs.action == 'spawn' }} + if: ${{ inputs.action == 'spawn' || inputs.action == 'update' }} id: spawn_outputs shell: bash working-directory: ${{ github.action_path }} diff --git a/docs/GITHUB_ACTION.md b/docs/GITHUB_ACTION.md index 91825978..8b96a8d3 100644 --- a/docs/GITHUB_ACTION.md +++ b/docs/GITHUB_ACTION.md @@ -20,8 +20,8 @@ Whatever it is asked to do, it takes: | Input | Default | What it is | |-------|---------|------------| -| `action` | `spawn` | What to do: `spawn`, `snapshot`, `delete-snapshot`, `destroy` or `list` | -| `max_attempts` | `3` | How many times a spawn or a destroy is tried before giving up, since a cloud is not always reachable on the first go | +| `action` | `spawn` | What to do: `spawn`, `update`, `snapshot`, `delete-snapshot`, `destroy` or `list` | +| `max_attempts` | `3` | How many times a spawn, an update or a destroy is tried before giving up, since a cloud is not always reachable on the first go | | `store_to_s3` | `true` | Whether the state of the stack goes to the S3 bucket a garbage collection reads | The credentials of the cloud are not inputs. They go in the environment of the @@ -45,6 +45,37 @@ job, and they are the ones It answers with `bastion` and `nodes`, what the platform reports of its machines, as JSON. +## Updating + +| Input | Required | What it is | +|-------|----------|------------| +| `stack_name` | yes | The stack holding the platform to change | +| `configuration` | yes | The keys to change, as YAML. Every other key keeps the value the platform was last deployed with | + +```yaml +- uses: scality/platform-spawner@main + with: + action: update + stack_name: ${{ github.run_id }} + configuration: | + offline: true +``` + +It answers with `bastion` and `nodes`, like a spawn does. A machine is only +replaced when the change asks for it: switching `offline` leaves every one of +them running. The image of a machine is never one of those changes, see +[DESIGN.md](../DESIGN.md#a-machine-keeps-its-image). + +It works from any job, not only the one that spawned. The SSH files are +written back from the stack, the way +[troubleshooting](TROUBLESHOOTING.md#the-ssh-files-are-gone) does by hand. + +> **Note** +> `__my_ip__` is worked out again on every update, as the address of the +> runner doing it. An update run from another runner than the spawn moves the +> access over to that runner, and the first one loses it. Listing the +> addresses to keep in `authorized_cidrs` is what keeps them. + ## Destroying | Input | Required | What it is | diff --git a/tests/check_offline.sh b/tests/check_offline.sh new file mode 100755 index 00000000..6f5592da --- /dev/null +++ b/tests/check_offline.sh @@ -0,0 +1,17 @@ +#! /usr/bin/env bash +# +# Check that a platform is offline: the nodes cannot reach the internet, and +# the bastion still can. +# +# Run from wherever the generated ssh_config is, which is where the platform +# was spawned from. + +set -euo pipefail + +echo "The platform is offline" +if ssh -F ssh_config node-1 'timeout 15 curl -sS -o /dev/null https://1.1.1.1'; then + echo "node-1 reached the internet, the platform is not offline" >&2 + exit 1 +fi +# the very same reach, from the machine that is meant to have it +ssh -F ssh_config bastion 'timeout 15 curl -sS -o /dev/null https://1.1.1.1' diff --git a/tests/check_options.sh b/tests/check_options.sh index 648a6581..1380115f 100755 --- a/tests/check_options.sh +++ b/tests/check_options.sh @@ -30,10 +30,4 @@ echo " node-1 carries $disks disk(s)" # the one it boots on, plus the two asked for [ "$disks" -eq 3 ] -echo "The platform is offline" -if ssh -F ssh_config node-1 'timeout 15 curl -sS -o /dev/null https://1.1.1.1'; then - echo "node-1 reached the internet, the platform is not offline" >&2 - exit 1 -fi -# the very same reach, from the machine that is meant to have it -ssh -F ssh_config bastion 'timeout 15 curl -sS -o /dev/null https://1.1.1.1' +"$(dirname "$0")"/check_offline.sh diff --git a/tools/merge_config.py b/tools/merge_config.py new file mode 100755 index 00000000..04837e3e --- /dev/null +++ b/tools/merge_config.py @@ -0,0 +1,66 @@ +#! /usr/bin/env python3 +""" +Merge configuration keys, read as YAML on stdin, into the file of a stack. + +A live platform is updated on the configuration it was last deployed with, +plus whatever it is handed. The keys go straight into the file rather than +through `pulumi config set`. That command takes a string, sets a list or an +object one leaf at a time, and has no way to say that a list is now empty. +""" + +import argparse +import pathlib +import sys + +import yaml + +PROJECT_FILE = pathlib.Path(__file__).resolve().parent.parent / "Pulumi.yaml" +NOT_A_MAPPING = "The configuration must be a mapping of keys to values." + + +def __main__() -> int: + """Merge the keys read on stdin into the file of a stack.""" + args = _parse_args() + + keys = yaml.safe_load(sys.stdin) or {} + if not isinstance(keys, dict): + print(NOT_A_MAPPING, file=sys.stderr) + return 1 + + project = yaml.safe_load(PROJECT_FILE.read_text(encoding="utf-8"))["name"] + content = {} + if args.stack_file.exists(): + content = yaml.safe_load(args.stack_file.read_text(encoding="utf-8")) or {} + config = content.get("config") or {} + + merged = {} + for key, value in keys.items(): + # NOTE: A key of the project can be spelled with or without its + # namespace, and Pulumi refuses a file holding both spellings. The + # one it writes itself is the namespaced one, so that is the one kept. + name = key.removeprefix(f"{project}:") + if ":" not in name: + config.pop(name, None) + name = f"{project}:{name}" + + merged[name] = value + + config.update(merged) + print(yaml.safe_dump(merged, sort_keys=False), end="") + + content["config"] = config + args.stack_file.write_text(yaml.safe_dump(content, sort_keys=False), encoding="utf-8") + + return 0 + + +def _parse_args() -> argparse.Namespace: + """Read the command line.""" + parser = argparse.ArgumentParser(description=__doc__.strip().splitlines()[0]) + parser.add_argument("stack_file", type=pathlib.Path, help="file of the stack to merge into") + + return parser.parse_args() + + +if __name__ == "__main__": + sys.exit(__main__()) From 8488e61fa159323a77c2129575f9e517646a12e2 Mon Sep 17 00:00:00 2001 From: Teddy Andrieux Date: Mon, 12 Oct 2026 15:19:25 +0000 Subject: [PATCH 3/3] feat: take a single node away from a live platform and bring it back Testing how a product survives the loss of a server, and its replacement, needs that one machine gone while the others keep running. Lowering `instance_count` only ever removes the last node, and spawning again loses everything. `absent_nodes` names the nodes to leave out. Such a node keeps its interfaces, and with them its addresses. Its instance and volumes go away. Taking it out of the list brings it back blank, where it was. The SSH files now file a host key under the machine rather than its address. A node brought back at its old address would otherwise clash with the key of the one it replaces, here and on the bastion. The bastion is handed its config again whenever the machines change. A snapshot of a platform missing a node in the middle of its numbering is refused, since it could never be spawned back. The update end to end test takes node-2 away and brings it back. It checks that the node is a new, blank machine at its old addresses and that nothing else moved. --- .github/workflows/update-test.yaml | 107 +++++++++++++++++++++++++++++ CONTRIBUTING.md | 6 +- Pulumi.yaml | 6 ++ README.md | 1 + __main__.py | 34 ++++++++- docs/PLATFORM.md | 25 +++++++ docs/SNAPSHOTS.md | 6 ++ tools/generate_snapshot.py | 10 +++ tools/get_ssh_config.py | 5 +- tools/ssh_config.py | 19 +++++ 10 files changed, 215 insertions(+), 4 deletions(-) diff --git a/.github/workflows/update-test.yaml b/.github/workflows/update-test.yaml index 929a8e20..c62af761 100644 --- a/.github/workflows/update-test.yaml +++ b/.github/workflows/update-test.yaml @@ -71,6 +71,15 @@ jobs: instance_image: rocky-9 instance_count: 2 authorized_cidrs: ${{ format('[{0},"__my_ip__"]', join(fromJSON(vars.AUTHORIZED_CIDRS), ',')) }} + extra_volumes: + - size: 10 + + # NOTE: Run before node-2 goes away, not only once it is back. Reaching + # the nodes records their host keys, here and on the bastion. Without + # them, node-2 would come back with no old key to clash with, and the + # host key check further down would prove nothing. + - name: Check the platform + run: ./tests/check_platform.sh - name: Check the platform starts online run: ssh -F ssh_config node-1 'timeout 15 curl -sS -o /dev/null https://1.1.1.1' @@ -104,6 +113,104 @@ jobs: echo "after: $after" [ "$before" == "$after" ] + - name: Leave something behind on node-2 + env: + MARKER: ${{ github.run_id }} + run: ssh -F ssh_config node-2 "echo '$MARKER' > ~/update-marker" + + - name: Take node-2 away + id: away + uses: ./ + with: + action: update + stack_name: ${{ inputs.stack }} + configuration: |- + absent_nodes: [node-2] + + - name: Check node-2 is gone, and only node-2 + env: + NODES_BEFORE: ${{ steps.offline.outputs.nodes }} + NODES_AFTER: ${{ steps.away.outputs.nodes }} + run: |- + expected=$(jq -cS 'del(."node-2") | map_values(.id)' <<< "$NODES_BEFORE") + found=$(jq -cS 'map_values(.id)' <<< "$NODES_AFTER") + echo "expected: $expected" + echo "found: $found" + if [ "$found" != "$expected" ]; then + echo "The nodes are not those of the platform minus node-2" >&2 + exit 1 + fi + if grep -q '^Host node-2$' ssh_config; then + echo "node-2 is still in ssh_config" >&2 + exit 1 + fi + + - name: Bring node-2 back + id: back + uses: ./ + with: + action: update + stack_name: ${{ inputs.stack }} + configuration: |- + absent_nodes: [] + + - name: Check node-2 came back as a new machine, where it was + env: + NODES_BEFORE: ${{ steps.offline.outputs.nodes }} + NODES_AFTER: ${{ steps.back.outputs.nodes }} + run: |- + others_before=$(jq -cS 'del(."node-2") | map_values(.id)' <<< "$NODES_BEFORE") + others_after=$(jq -cS 'del(."node-2") | map_values(.id)' <<< "$NODES_AFTER") + if [ "$others_before" != "$others_after" ]; then + echo "Another node was replaced: $others_before became $others_after" >&2 + exit 1 + fi + + id_before=$(jq -r '."node-2".id' <<< "$NODES_BEFORE") + id_after=$(jq -r '."node-2".id' <<< "$NODES_AFTER") + echo "node-2 was $id_before, is $id_after" + if [ "$id_before" == "$id_after" ]; then + echo "node-2 is the machine it was" >&2 + exit 1 + fi + + ips_before=$(jq -cS '."node-2".private_ips' <<< "$NODES_BEFORE") + ips_after=$(jq -cS '."node-2".private_ips' <<< "$NODES_AFTER") + echo "node-2 sits at $ips_after" + if [ "$ips_before" != "$ips_after" ]; then + echo "node-2 moved from $ips_before" >&2 + exit 1 + fi + + state=$(ssh -F ssh_config node-2 '[ -e ~/update-marker ] && echo kept || echo blank') + echo "node-2 came back $state" + if [ "$state" != "blank" ]; then + echo "node-2 kept what was left on it" >&2 + exit 1 + fi + + disks=$(ssh -F ssh_config node-2 'lsblk --nodeps --noheadings --output TYPE | grep -c disk') + echo "node-2 carries $disks disk(s)" + # the one it boots on, plus the one asked for + [ "$disks" -eq 2 ] + + # NOTE: StrictHostKeyChecking lets a changed host key through, but turns + # forwarding off for the connection. Reaching the node proves nothing, + # only the warning tells. + - name: Check the host key of node-2 is not taken for a changed one + run: |- + here=$(ssh -F ssh_config node-2 true 2>&1) + bastion=$(ssh -F ssh_config bastion 'ssh -F ssh_config node-2 true' 2>&1) + for out in "$here" "$bastion"; do + if grep -q "HOST IDENTIFICATION HAS CHANGED" <<< "$out"; then + echo "$out" >&2 + exit 1 + fi + done + + - name: Check the platform again + run: ./tests/check_platform.sh + destroy: name: "${{ inputs.name }} destroy" needs: test diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fce280c3..cbb79185 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -34,8 +34,10 @@ for everything at once, offline with extra networks and extra volumes, and one that is snapshotted and spawned back from its images, carrying a file written before the snapshot to prove it came back whole. That one takes its images away afterwards, nothing else ever would. One last platform goes up on each -provider and is changed while it runs, through the `update` action: it is -switched offline, and none of its machines may be replaced on the way. +provider and is changed while it runs, through the `update` action. It is +switched offline, and none of its machines may be replaced on the way. Then +one of its nodes is taken away and brought back, blank and at the addresses +it had, while every other machine is left alone. Each of them is a workflow of its own carrying its spawn, its checks and its destroy, so a platform goes away as soon as its own test is done rather than diff --git a/Pulumi.yaml b/Pulumi.yaml index 2a05661f..042d3e81 100644 --- a/Pulumi.yaml +++ b/Pulumi.yaml @@ -76,6 +76,12 @@ config: type: integer description: "Number of instances to create" default: 3 + absent_nodes: + type: array + items: + type: string + description: "Nodes to leave out, keeping their addresses for when they come back (e.g. node-2)" + default: [] instance_flavor: type: string description: "Flavor of the instance, a portable size or a name from the cloud catalog" diff --git a/README.md b/README.md index 1e1a5d8f..e7f0f642 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,7 @@ A platform is described by the configuration of its stack. Set a key with | instance_image_file | Image file to upload and boot the instances on, instead of instance_image | string | `` | no | | instance_image_file_format | Format of the uploaded image file | string | `qcow2` | no | | instance_count | Number of instances to create | integer | `3` | no | +| absent_nodes | Nodes to leave out, keeping their addresses for when they come back (e.g. node-2) | array | `[]` | no | | instance_flavor | Flavor of the instance, a portable size or a name from the cloud catalog | string | `medium` | no | | instance_ssh_user | SSH user for the instances (defaults to the one the image is known to use) | string | `` | no | | instance_root_disk_size | Root disk size for the instance in GiB (AWS only, set by the flavor on OpenStack) | integer | `50` | no | diff --git a/__main__.py b/__main__.py index 7e06dafe..381efa3c 100644 --- a/__main__.py +++ b/__main__.py @@ -19,6 +19,7 @@ DUPLICATE_EXTRA_NETWORK = "Several extra networks are named {names}." INSTANCE_IMAGE_MISSING = "One of instance_image or instance_image_file must be set." SNAPSHOT_EXCLUDES_IMAGE_FILE = "restore_snapshot cannot be used with instance_image_file." +UNKNOWN_ABSENT_NODES = "absent_nodes names nodes the platform does not have: {names}." # Fallback for the images we know, anything else has to be configured SSH_USERS = { "rocky-8": "rocky", @@ -79,6 +80,7 @@ def __main__() -> None: instance_count = config.require_int("instance_count") instance_images = _instance_images(config, provider, instance_count) + absent_nodes = _absent_nodes(config, instance_count) instance_flavor = config.require("instance_flavor") ssh_info = { @@ -222,12 +224,16 @@ def __main__() -> None: ) instances.append(bastion) ssh_info["bastion"] = { + "id": bastion.id, "ip": bastion_public_iface.public_ip, "user": _ssh_user(config, "bastion"), } nodes_info = {} for node_index in range(1, instance_count + 1): + # NOTE: The interfaces of an absent node are kept. They hold its + # addresses, which is what brings it back exactly where it was rather + # than wherever the cloud would put a newcomer. cp_iface = provider.create_interface( subnet=control_plane_subnet, subnet_name="control-plane", @@ -245,6 +251,8 @@ def __main__() -> None: extra_ifaces = _create_extra_interfaces( provider, extra_subnets, f"node-{node_index}", 100 + node_index, [internal_sg] ) + if f"node-{node_index}" in absent_nodes: + continue node = provider.create_instance( name=f"node-{node_index}", @@ -270,6 +278,7 @@ def __main__() -> None: }, } ssh_info["nodes"][f"node-{node_index}"] = { + "id": node.id, "ip": cp_iface.ip, "user": _ssh_user(config, "instance"), } @@ -284,7 +293,7 @@ def __main__() -> None: ) booted = _wait_for_boot(ssh_config_ready, ["bastion", *nodes_info], instances) - _equip_bastion(ssh_config_ready, bastion_key, booted) + _equip_bastion(ssh_config_ready, bastion_key, booted, instances) _clean_up_ssh_files(ssh_config_file) @@ -331,6 +340,25 @@ def _prepare_ssh_key( return ssh_key_name +def _absent_nodes(config: pulumi.Config, instance_count: int) -> set[str]: + """ + Return the nodes to leave out of the platform. + + NOTE: Named rather than counted. Lowering `instance_count` only ever + takes the last node away, and the node worth taking away is whichever + one a test needs gone. A name the platform does not have is refused: it + is far more likely a typo than a wish, and ignoring it would leave the + node it meant running. + """ + absent = set(config.require_object("absent_nodes")) + unknown = absent - {f"node-{index}" for index in range(1, instance_count + 1)} + if unknown: + message = UNKNOWN_ABSENT_NODES.format(names=", ".join(sorted(unknown))) + raise ValueError(message) + + return absent + + def _parse_cidrs(cidrs: list[str]) -> list[pulumi.Output[str]]: """Mainly used to replace the `__my_ip__` placeholder.""" @@ -573,6 +601,7 @@ def _equip_bastion( config_file: pulumi.Output[str], bastion_key: pulumi_tls.PrivateKey, booted: pulumi_command.local.Command, + instances: list[pulumi.CustomResource], ) -> None: """ Hand the bastion its key and a config, so it reaches the nodes on its own. @@ -592,6 +621,9 @@ def _equip_bastion( ) ), environment={"BASTION_PRIVATE_KEY": bastion_key.private_key_openssh}, + # Handed again whenever a machine changes. The config names the + # nodes, and a replaced bastion starts with neither key nor config. + triggers=[instance.id for instance in instances], opts=pulumi.ResourceOptions(depends_on=[booted]), ) diff --git a/docs/PLATFORM.md b/docs/PLATFORM.md index d1146ae2..dd965b4a 100644 --- a/docs/PLATFORM.md +++ b/docs/PLATFORM.md @@ -43,6 +43,31 @@ offline and has no pool of its own to reach. The nodes run the workloads. `instance_count` says how many there are, three by default. +### Taking a node away + +`absent_nodes` names the nodes to leave out of a live platform, the way a +server dies under a cluster. Taking it out of the list brings the node back, +as a new machine: + +```bash +pulumi config set --path 'absent_nodes[0]' node-2 +pulumi up # node-2 is gone, every other machine is left alone +pulumi config rm absent_nodes +pulumi up # node-2 is back, blank, at the addresses it had +``` + +The node keeps its interfaces while it is away, and with them its addresses. +That is what brings it back exactly where it was, with nothing else on the +platform to tell. Everything else goes with it: its root disk and its extra +volumes come back blank. An absent node is also left out of the SSH files and +of what the platform reports of its nodes. + +The bastion cannot be taken away. It holds the way in and the clock of the +platform, and is not what a platform is spawned to test. + +From a workflow, the same goes through the +[`update` action](GITHUB_ACTION.md#updating). + ## Networks | Network | CIDR | What it is | diff --git a/docs/SNAPSHOTS.md b/docs/SNAPSHOTS.md index cd3ff3ea..40a23c0d 100644 --- a/docs/SNAPSHOTS.md +++ b/docs/SNAPSHOTS.md @@ -18,6 +18,12 @@ Every instance is stopped, captured as `--node-`, then started back. The bastion is left out, and so are the extra volumes. Only the instances themselves are captured. +A platform with a node [taken away](PLATFORM.md#taking-a-node-away) in the +middle of its numbering is refused. Instance N is spawned back from the image +of node N, so the missing image would leave the snapshot impossible to spawn +back. Taking the last nodes away is fine: the snapshot is then spawned back +with fewer instances. + ## Spawn a platform back from it ```bash diff --git a/tools/generate_snapshot.py b/tools/generate_snapshot.py index 4b345417..b496c952 100755 --- a/tools/generate_snapshot.py +++ b/tools/generate_snapshot.py @@ -27,6 +27,7 @@ UNFINISHED_IMAGE_STATUSES = frozenset({"", "queued", "saving"}) STOPPED = "SHUTOFF" RUNNING = "ACTIVE" +NODE_ABSENT = "{names} absent, a snapshot taken now could never be spawned back." def __main__() -> int: @@ -42,6 +43,15 @@ def __main__() -> int: print("No instance to snapshot", file=sys.stderr) return 1 + # NOTE: Instance N is spawned back from the image taken of node N, from + # node-1 on. A hole in the numbering means a node is absent. Its image + # would be missing, and the snapshot could never be spawned back. + missing = {f"node-{index}" for index in range(1, len(instances) + 1)} - instances.keys() + if missing: + names = ", ".join(sorted(missing)) + print(NODE_ABSENT.format(names=names), file=sys.stderr) + return 1 + snapshots = { instance_id: f"{product}-{args.name}-{name}" for name, instance_id in instances.items() } diff --git a/tools/get_ssh_config.py b/tools/get_ssh_config.py index 9c5416a0..eaab2234 100755 --- a/tools/get_ssh_config.py +++ b/tools/get_ssh_config.py @@ -36,7 +36,10 @@ def __main__() -> int: return 1 ssh_info = outputs["ssh_info"].value if "ssh_info" in outputs else {} - if not ssh_info.get("nodes"): + # NOTE: Only a missing list of nodes is refused, an empty one is not. + # Every node of a platform can be absent at once, and its bastion is + # still worth reaching. + if "nodes" not in ssh_info: print(NO_SSH_INFO.format(stack=stack_name), file=sys.stderr) return 1 diff --git a/tools/ssh_config.py b/tools/ssh_config.py index 9576a481..25aa88f0 100644 --- a/tools/ssh_config.py +++ b/tools/ssh_config.py @@ -46,6 +46,22 @@ def common_options(config_path: pathlib.Path) -> list[str]: ] +def host_key_alias(host: str, info: dict) -> list[str]: + """ + Return the option recording the host key of a machine under its id. + + NOTE: Under its id rather than under its address. A node taken away and + brought back sits at the address it had, with a new host key. Recorded by + address, that key would clash with the one of the machine it replaces. + Recorded by id, it gets an entry of its own, and a key changing under the + same machine is still caught. + """ + if not info.get("id"): + return [] + + return [f" HostKeyAlias {host}-{info['id']}"] + + def generate_bastion(ssh_info: dict, path: pathlib.Path) -> None: """ Write the config the bastion uses to reach the nodes. @@ -63,6 +79,7 @@ def generate_bastion(ssh_info: dict, path: pathlib.Path) -> None: if info.get("user"): config_lines.append(f" User {info['user']}") config_lines.append(f" IdentityFile ~/{BASTION_KEY}") + config_lines.extend(host_key_alias(host, info)) config_lines.append(" IdentitiesOnly yes") config_lines.append(" StrictHostKeyChecking no") config_lines.append(" ServerAliveInterval 15") @@ -83,6 +100,7 @@ def generate(ssh_info: dict, path: pathlib.Path) -> str: config_lines.append(f" User {ssh_info['bastion']['user']}") if ssh_info.get("key"): config_lines.append(f" IdentityFile {ssh_info['key']}") + config_lines.extend(host_key_alias("bastion", ssh_info["bastion"])) config_lines.extend(options) config_lines.append("") @@ -96,6 +114,7 @@ def generate(ssh_info: dict, path: pathlib.Path) -> str: config_lines.append(f" User {info['user']}") if ssh_info.get("key"): config_lines.append(f" IdentityFile {ssh_info['key']}") + config_lines.extend(host_key_alias(host, info)) config_lines.extend(options) config_lines.append("")