Compare commits

...

22 Commits

Author SHA1 Message Date
Bryn Mailer 290ec05aab feat(ebs): record an unpublish obligation on force detach and terminate (#1504)
* feat(ebs): record an unpublish obligation on force detach and terminate

A force detach, or a terminate whose unmount did not complete, clears a
volume's attachment while the node that served it may still export it.
The request now records an UnpublishObligation naming that publish in the
new spinifex-ebs-unpublish-obligations bucket before the clear, and
returns without waiting on the node.

The node-local unpublishObligationReaper settles its own node's
obligations under the mounter's per-volume lock, through the existing
UnpublishVolume client: it seals a surviving volume, drops a deleting or
absent one unsealed, and closes without a provider call when a newer
attachment or a newer publish on the node owns the export. A cluster-wide
pass closes the obligations of nodes removed from the cluster, and
spx admin volumes obligations lists the open ones.

The force-complete terminate and a stopped-instance terminate after a
failed seal record the same obligation; a terminate whose unmount
completed records none. ForceDetachVolume now answers with the attachment
in state detaching, as AWS answers a detach.

* fix(ebs): refuse a publish while the obligation reaper seals the volume

A publish waited on the volume lock while the unpublish obligation reaper
held it through a seal, so AttachVolume waited on work that scales with
the volume. A publish now refuses at once with ErrVolumeSettling, which
AttachVolume answers as IncorrectState, and the reaper takes the lock only
when nobody holds or awaits it.

* fix(ebs): refuse a publish behind any seal of the volume, a stop's included

A stop's or detach's sealing unpublish now marks the volume lock sealing,
so an attach after a force detach answers IncorrectState instead of
waiting out the seal. A waiting seal lets waiting publishes go first.

* fix(ebs): close a superseded unpublish obligation past a newer publish here

After a force detach and an attach again on the same node, the reaper
returned on the newer publish before it read the volume, so the old
obligation stayed open until the new attachment ended.

* fix(vm): recover a wedged terminate without waiting on its stalled unmount

The unmount records every volume as owed before it starts, so recovery
keeps that record rather than wait for the request lock the stalled
seal holds.
2026-11-04 15:59:41 +10:00
Julian Sommer e73eb1ce16 fix(ecs): refuse CreateCapacityProvider autoScalingGroupProvider enum and range breaks with AWS's ClientException (#1579) 2026-11-04 16:52:10 +11:00
Julian Sommer 700a72c4af fix(ecs): refuse ListServices, ListTaskDefinitions and ListTasks enum values outside AWS's sets with InvalidParameterException (#1577) 2026-11-04 16:40:18 +11:00
Julian Sommer 6a7c4cdb02 chore: update README 2026-11-04 15:32:49 +10:00
Julian Sommer 06f521969a fix(ecs): refuse Describe* include values outside the model's enum with AWS's error (#1576) 2026-11-04 16:24:05 +11:00
Julian Sommer fc57de83b4 fix(ecr): refuse imageIds, imageTag, acceptedMediaTypes and repositoryNames outside the model's bounds (#1575) 2026-11-04 16:12:08 +11:00
Julian Sommer 2f98f53ad1 fix: report AWS's per-protocol target group attributes and return the full set from ModifyTargetGroupAttributes (#1574) 2026-11-04 16:11:43 +11:00
Julian Sommer 2caebe352e fix: match ICMP type and code in security group ACLs (#1573) 2026-11-04 16:11:05 +11:00
Julian Sommer 5401921e3e fix(iam): authorize UpdateUser on the new user name as well as the existing user (#1572) 2026-11-04 16:10:48 +11:00
Josh Siviour ee6a661eeb ci: keep private design prompts out of the public PR template (#1571) 2026-11-04 14:21:27 +10:00
Julian Sommer 171e004c14 fix(ui): bump seroval to 1.7.6 and minimatch to 11.0.3 (#1570)
Resolves GHSA-7rv5-mrxv-xcr9, GHSA-wp97-x4hv-qvhp, GHSA-97r2-qh9m-474h,
GHSA-9wx9-5pv9-3349, GHSA-f86p-h4j2-7qgj (seroval) and GHSA-g3hv-75wg-m249
(minimatch, dev-only via eslint, forced with a pnpm override).
2026-11-04 14:47:34 +11:00
Julian Sommer 8c7c4bb772 test: run the viperblock provider's self-contained tests in parallel (#1569)
The conformance and NBD client suites, the fence, lease, dirty-marker,
abandon, takeover and stale-mount tests each stand up their own embedded
JetStream and Config, so they now overlap instead of running back to back.
Tests that touch package or process-wide state stay serial.
2026-11-04 14:45:31 +11:00
Julian Sommer ed3f6a3185 test: overlap the admin package's timer-bound waits with its serial tests (#1567)
The six account-teardown tests and the unanswered network-test case each
wait about 5s on a production timer. Calling t.Parallel() first queued all
seven through the parallel slots; starting the teardown or network test
before t.Parallel() lets the wait run during the serial phase instead.

Also generate 1024-bit RSA keys in TestMain and shorten the test-only
context in TestQueryMetaNodeStatus_Unreachable.

go test -coverprofile at GOMAXPROCS=4: 16.9s -> 7.1-8.0s.
2026-11-04 14:45:15 +11:00
Julian Sommer b333775a62 test: start accountlifecycle's silent-node fan-out tests first (#1568)
The four tests that wait out the fixed 3s describe fan-out deadline for a
silent node (9s, 6s, 6s, 6s) were queued behind the package's short
parallel tests, two of them as subtests of a table test. Hoist the two
slow table cases into top-level tests and move all four into a file that
sorts first, so they take parallel slots before the short tests do. The
table body becomes a shared helper; no case or assertion changes.
2026-11-04 14:44:53 +11:00
Josh Siviour 16a4e0d96b feat(ec2): report instances of an account that are not fully torn down (#1565)
* feat(ec2): report instances of an account that are not fully torn down

* fix(ec2): report a terminated instance held only in the primary store
2026-11-04 13:36:20 +10:00
Julian Sommer 535d9d6751 chore: run sqs jetstream message store tests in parallel (#1566)
Each test stands up its own embedded JetStream server, so top-level tests
and independent subtests now call t.Parallel. Tests that redeclare the
process-wide cluster size or capture the default logger stay sequential.
Package wall time drops from ~23.5s to ~4.4s locally.
2026-11-04 14:32:56 +11:00
Julian Sommer d18c08df93 chore: move mulga-design-check to bash 2026-11-04 13:07:35 +10:00
Julian Sommer 53eb4131bf chore: del CODEOWNERS 2026-11-04 13:02:12 +10:00
Julian Sommer 121aa5a17b chore: move sibling ref resolver scripts into .github/scripts 2026-11-04 13:01:47 +10:00
Julian Sommer 78d27af118 test: run the admin package's timeout-bound tests in parallel (#1564)
Six account-delete tests each wait one 5s teardown drain poll, and the
network-test and storage-status tests wait out fixed 5s and 2s timeouts.
Each uses its own embedded NATS, so they run concurrently: the package
drops from about 37s to 8s.
2026-11-04 12:48:38 +11:00
Julian Sommer 3f2fff3542 chore: suppress preflight build script output 2026-11-04 11:41:09 +10:00
Julian Sommer 769bb8014a test: stub the /dev/kvm probe so QEMU argv tests run on hosts without KVM 2026-11-04 11:37:01 +10:00
142 changed files with 4341 additions and 549 deletions
-15
View File
@@ -1,15 +0,0 @@
# CODEOWNERS — GitHub PR review enforcement
#
# Format: <path pattern> <owner> [<owner> …]
# Order matters — last matching pattern wins.
#
# Set the matching branch protection rule on `dev` and `main` to
# "Require review from Code Owners" or this file is advisory only.
# Network layer is governed by its invariant tests.
# Changes here must respect those clauses
# see spinifex/domains/network/invariants/
# mulga docs/development/proposals/region-local-architecture/0006-spinifex-network-layer-contract.md (proposal, not accepted)
/spinifex/domains/network/ @joshsiv-mulga
/spinifex/runtime/roles/vpcd/ @joshsiv-mulga
/spinifex/domains/network/invariants/ @joshsiv-mulga
-14
View File
@@ -6,20 +6,6 @@
<!-- The notable changes, as bullets. -->
## Account-scoped lifecycle impact
<!-- Complete this section only when the PR changes account-scoped resource lifecycle, account termination, resource cleanup, or a lifecycle boundary. Otherwise write: N/A. -->
- Lifecycle owner and resource family:
- Owner-local ordering, recovery, retained-state, or force impact:
- Cross-domain dependency or ownership impact:
- Evidence added or rechecked:
- ADR-0008 lifecycle register / ADR-0001 migration-record / bead updates:
## Mulga design
<!-- One line, so a reviewer knows whether the design records in the private mulga repository need to change. Write either "Updated: <mulga PR or bead id>" or "Not needed: <reason>". This is a reminder, not a gate. -->
## Community Contribution License
- [ ] All community contributions in this pull request are licensed to Mulga under the terms of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). By creating this pull request I represent that I have the right to license these contributions to Mulga under the Apache License 2.0.
@@ -1,87 +0,0 @@
#!/bin/sh
# hypervisor-disk-preflight_test.sh — drives hypervisor-disk-preflight.sh against
# a stub ssh that answers for each hypervisor from a canned file, so every
# verdict is checked without a real host.
set -eu
SCRIPT_DIR=$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd)
PREFLIGHT="$SCRIPT_DIR/hypervisor-disk-preflight.sh"
TMP=$(mktemp -d)
trap 'rm -rf "$TMP"' EXIT
fails=0
pass() { echo " ok - $1"; }
fail() { echo " FAIL - $1"; fails=$((fails + 1)); }
# The stub answers "<root-kb> <pool-kb> <path>" from $FAKE/<host>, or exits 255
# when that host has no file, the way ssh does for a host it cannot reach.
mkdir -p "$TMP/bin" "$TMP/hosts"
cat > "$TMP/bin/ssh" <<'STUB'
#!/bin/sh
for a in "$@"; do case $a in *@*) host=${a#*@} ;; esac; done
cat >/dev/null
if [ -f "$FAKE/$host" ]; then cat "$FAKE/$host"; exit 0; fi
echo "ssh: Could not resolve hostname $host" >&2
exit 255
STUB
chmod +x "$TMP/bin/ssh"
PATH="$TMP/bin:$PATH"
FAKE="$TMP/hosts"
export PATH FAKE
cat > "$TMP/env.tfvars" <<'TFVARS'
nodes = [
{ name = "wattle", wan_ip = "192.0.2.1", storage_pool = "drive1-pool", base_image = "/x.qcow2" },
# { name = "retired", wan_ip = "192.0.2.9", storage_pool = "drive1-pool", base_image = "/x.qcow2" },
{ name = "banksia", wan_ip = "192.0.2.2", storage_pool = "nvme1-pool", base_image = "/x.qcow2" },
]
TFVARS
GiB=1048576
plenty="$((50 * GiB)) $((200 * GiB)) /mnt/drive1/pool"
# run <label> <want-rc> <want-text>
run() {
rc=0
out=$(bash "$PREFLIGHT" "$TMP/env.tfvars" 2>&1) || rc=$?
if [ "$rc" -ne "$2" ]; then
fail "$1: exit $rc, want $2"
echo "$out" | sed 's/^/ /'
return
fi
case $out in
*"$3"*) pass "$1" ;;
*) fail "$1: output lacks '$3'"; echo "$out" | sed 's/^/ /' ;;
esac
}
echo "-- hypervisor-disk-preflight.sh"
echo "$plenty" > "$FAKE/wattle"
echo "$plenty" > "$FAKE/banksia"
run "every host has room" 0 "banksia: root fs 51200 MiB free"
echo "0 $((200 * GiB)) /mnt/drive1/pool" > "$FAKE/wattle"
run "a full root filesystem fails fast" 1 "::error::hypervisor wattle has 0 MiB free on the filesystem holding /etc/libvirt/qemu"
echo "$plenty" > "$FAKE/wattle"
echo "$((50 * GiB)) $((3 * GiB)) /mnt/nvme1/pool" > "$FAKE/banksia"
run "a nearly full pool fails fast" 1 "::error::hypervisor banksia has 3 GiB free in storage pool nvme1-pool"
echo "$plenty" > "$FAKE/banksia"
rm -f "$FAKE/wattle"
run "an unreachable host is reported, not failed" 0 "::warning::hypervisor disk preflight: cannot reach wattle"
echo "$plenty" > "$FAKE/wattle"
echo "$((50 * GiB)) - -" > "$FAKE/banksia"
run "a pool libvirt does not know is reported, not failed" 0 "pool nvme1-pool (-) not found"
echo "0 0 /x" > "$FAKE/retired"
echo "$plenty" > "$FAKE/banksia"
run "a commented-out node is never checked" 0 "wattle: root fs"
if [ "$fails" -ne 0 ]; then
echo "$fails failure(s)"
exit 1
fi
echo " hypervisor-disk-preflight ok"
+40
View File
@@ -0,0 +1,40 @@
#!/usr/bin/env bash
# Warn when a PR body has no filled "Mulga design" line. Reads PR_BODY and
# never fails the job: the check is a prompt, not a gate.
set -euo pipefail
shopt -s nocasematch
section_re='^#{1,6}[[:space:]]*Mulga design[[:space:]]*$'
line_re='^[[:space:]]*(Updated|Not needed)[[:space:]]*:[[:space:]]*(.*)$'
placeholder_re='<[^<>]*>'
filled() {
local body=$1 line value kept inside=0 pre rest
body=${body//$'\r'/}
while [[ $body == *'<!--'*'-->'* ]]; do
pre=${body%%'<!--'*}
rest=${body#*'<!--'}
body=$pre${rest#*'-->'}
done
while IFS= read -r line; do
if [[ $line =~ $section_re ]]; then
inside=1
elif ((inside)) && [[ ${line#"${line%%[![:space:]]*}"} == '#'* ]]; then
return 1
elif ((inside)) && [[ $line =~ $line_re ]]; then
value=${BASH_REMATCH[2]} kept=''
while [[ $value =~ $placeholder_re ]]; do
kept+=${value%%"${BASH_REMATCH[0]}"*}
value=${value#*"${BASH_REMATCH[0]}"}
done
[[ $kept$value == *[![:space:]]* ]] && return 0
fi
done <<<"$body"
return 1
}
if filled "${PR_BODY:-}"; then
echo "The Mulga design line is filled in."
else
echo '::warning title=Mulga design::Add a "## Mulga design" section with one line: "Updated: <mulga PR>" or "Not needed: <reason>".'
fi
-36
View File
@@ -1,36 +0,0 @@
#!/usr/bin/env python3
"""Warn when a PR body has no filled "Mulga design" line. Reads PR_BODY; never fails the job."""
import os
import re
COMMENT_RE = re.compile(r"<!--.*?-->", re.S)
SECTION_RE = re.compile(r"^#{1,6}\s*Mulga design\s*$", re.I)
LINE_RE = re.compile(r"^\s*(Updated|Not needed)\s*:\s*(.*?)\s*$", re.I)
PLACEHOLDER_RE = re.compile(r"<[^<>]*>")
def filled(body):
inside = False
for line in COMMENT_RE.sub("", body or "").splitlines():
if SECTION_RE.match(line):
inside = True
elif inside and line.lstrip().startswith("#"):
return False
elif inside:
m = LINE_RE.match(line)
if m and PLACEHOLDER_RE.sub("", m.group(2)).strip():
return True
return False
def main():
if filled(os.environ.get("PR_BODY", "")):
print("The Mulga design line is filled in.")
else:
print('::warning title=Mulga design::Add a line under "## Mulga design": "Updated: <mulga PR or bead id>" or "Not needed: <reason>".')
return 0
if __name__ == "__main__":
raise SystemExit(main())
@@ -1,23 +0,0 @@
import unittest
from mulga_design_check import filled
class FilledTest(unittest.TestCase):
def test_accepts_either_form(self):
self.assertTrue(filled("## Mulga design\nUpdated: mulga#275"))
self.assertTrue(filled("## Mulga design\n\nnot needed: internal refactor\n## Next"))
def test_rejects_unanswered(self):
self.assertFalse(filled(""))
self.assertFalse(filled("## Mulga design\n<!-- Updated: x -->"))
self.assertFalse(filled("## Mulga design\nUpdated: <mulga PR or bead id>"))
self.assertFalse(filled("## Mulga design\nUpdated:"))
def test_scoped_to_the_section(self):
self.assertFalse(filled("Updated: mulga#1"))
self.assertFalse(filled("## Mulga design\n## Other\nUpdated: mulga#1"))
if __name__ == "__main__":
unittest.main()
-47
View File
@@ -1,47 +0,0 @@
#!/bin/sh
# wait-for-memory_test.sh — drives wait-for-memory.sh against a fake meminfo
# that a background writer changes mid-wait, so each verdict is checked without
# a loaded host.
set -eu
SCRIPT_DIR=$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd)
WAIT="$SCRIPT_DIR/wait-for-memory.sh"
TMP=$(mktemp -d)
trap 'rm -rf "$TMP"' EXIT
fails=0
pass() { echo " ok - $1"; }
fail() { echo " FAIL - $1"; fails=$((fails + 1)); }
meminfo() { printf 'MemTotal: 65536000 kB\nMemAvailable: %s kB\n' "$(( $1 * 1024 ))" > "$TMP/meminfo"; }
MEMINFO="$TMP/meminfo"
MEM_WAIT_POLL_S=1
export MEMINFO MEM_WAIT_POLL_S
meminfo 30000
if MEM_WAIT_TIMEOUT_S=5 bash "$WAIT" 26624 > "$TMP/out" 2>&1; then
pass "enough memory returns at once"
else
fail "enough memory returns at once: $(cat "$TMP/out")"
fi
meminfo 21577
( sleep 2; meminfo 30000 ) &
if MEM_WAIT_TIMEOUT_S=20 bash "$WAIT" 26624 > "$TMP/out" 2>&1 && grep -q waiting "$TMP/out"; then
pass "memory freed mid-wait is admitted"
else
fail "memory freed mid-wait is admitted: $(cat "$TMP/out")"
fi
wait
meminfo 21577
if MEM_WAIT_TIMEOUT_S=2 bash "$WAIT" 26624 > "$TMP/out" 2>&1; then
fail "memory that never frees fails: exited 0"
elif grep -q '::error::need 26624 MiB available, have 21577 MiB' "$TMP/out" && grep -q 'RSS' "$TMP/out"; then
pass "memory that never frees fails and names the biggest processes"
else
fail "memory that never frees fails and names the biggest processes: $(cat "$TMP/out")"
fi
[ "$fails" -eq 0 ] || { echo "wait-for-memory: $fails failed"; exit 1; }
echo "wait-for-memory: all passed"
+4 -4
View File
@@ -1017,7 +1017,7 @@ jobs:
id: siblings
env:
BRANCH: ${{ env.GIT_BRANCH }}
run: bash .github/resolve-sibling-refs.sh
run: bash .github/scripts/resolve-sibling-refs.sh
working-directory: mulga/spinifex
- uses: actions/checkout@v7.0.1
@@ -1275,7 +1275,7 @@ jobs:
id: siblings
env:
BRANCH: ${{ env.GIT_BRANCH }}
run: bash .github/resolve-sibling-refs.sh
run: bash .github/scripts/resolve-sibling-refs.sh
working-directory: mulga/spinifex
- uses: actions/checkout@v7.0.1
@@ -1423,7 +1423,7 @@ jobs:
id: siblings
env:
BRANCH: ${{ github.ref_name }}
run: bash .github/resolve-sibling-refs.sh
run: bash .github/scripts/resolve-sibling-refs.sh
working-directory: spinifex
- name: Checkout Predastore
@@ -1547,7 +1547,7 @@ jobs:
id: siblings
env:
BRANCH: ${{ github.ref_name }}
run: bash .github/resolve-sibling-refs.sh
run: bash .github/scripts/resolve-sibling-refs.sh
working-directory: spinifex
- name: Checkout Predastore
+4 -5
View File
@@ -2,7 +2,8 @@ name: Mulga design
# Reminds the author to say whether the Mulga design records need to change.
# It reads only the pull request body, uses no secrets, and warns without ever
# failing: the check is a prompt, not a gate.
# failing: the check is a prompt, not a gate. Fork PRs are skipped because their
# authors cannot see the design records.
on:
pull_request:
@@ -16,7 +17,7 @@ permissions:
jobs:
mulga_design:
name: Mulga design line
if: ${{ !endsWith(github.actor, '[bot]') }}
if: ${{ !endsWith(github.actor, '[bot]') && github.event.pull_request.head.repo.full_name == github.repository }}
runs-on: ubuntu-26.04
steps:
- name: Checkout Code
@@ -28,6 +29,4 @@ jobs:
env:
PR_BODY: ${{ github.event.pull_request.body }}
working-directory: .github/scripts
run: |
python3 -m unittest -q test_mulga_design_check
python3 mulga_design_check.py
run: bash mulga-design-check.sh
+1 -1
View File
@@ -113,7 +113,7 @@ jobs:
id: siblings
env:
BRANCH: ${{ env.GIT_BRANCH }}
run: bash .github/resolve-sibling-refs.sh
run: bash .github/scripts/resolve-sibling-refs.sh
working-directory: spinifex
- uses: actions/checkout@v7.0.1
+1 -1
View File
@@ -84,7 +84,7 @@ jobs:
- name: Resolve sibling release tags from go.mod
id: siblings
run: bash spinifex/.github/resolve-release-sibling-refs.sh spinifex/go.mod
run: bash spinifex/.github/scripts/resolve-release-sibling-refs.sh spinifex/go.mod
- name: Checkout Viperblock
uses: actions/checkout@v7.0.1
+3 -3
View File
@@ -18,7 +18,7 @@ jobs:
- name: Resolve sibling release tags from go.mod
id: siblings
run: bash spinifex/.github/resolve-release-sibling-refs.sh spinifex/go.mod
run: bash spinifex/.github/scripts/resolve-release-sibling-refs.sh spinifex/go.mod
- name: Checkout Predastore
uses: actions/checkout@v7.0.1
@@ -85,7 +85,7 @@ jobs:
- name: Resolve sibling release tags from go.mod
id: siblings
run: bash spinifex/.github/resolve-release-sibling-refs.sh spinifex/go.mod
run: bash spinifex/.github/scripts/resolve-release-sibling-refs.sh spinifex/go.mod
- name: Checkout Predastore
uses: actions/checkout@v7.0.1
@@ -335,7 +335,7 @@ jobs:
- name: Resolve sibling release tags from go.mod
id: siblings
run: bash spinifex/.github/resolve-release-sibling-refs.sh spinifex/go.mod
run: bash spinifex/.github/scripts/resolve-release-sibling-refs.sh spinifex/go.mod
- name: Checkout Predastore
uses: actions/checkout@v7.0.1
+1 -1
View File
@@ -34,7 +34,7 @@ jobs:
id: siblings
env:
BRANCH: ${{ github.head_ref || github.ref_name }}
run: bash .github/resolve-sibling-refs.sh
run: bash .github/scripts/resolve-sibling-refs.sh
working-directory: spinifex
- name: Checkout Viperblock
+3 -3
View File
@@ -36,7 +36,7 @@ jobs:
id: siblings
env:
BRANCH: ${{ github.head_ref || github.ref_name }}
run: bash .github/resolve-sibling-refs.sh
run: bash .github/scripts/resolve-sibling-refs.sh
working-directory: spinifex
- name: Checkout Viperblock
@@ -166,7 +166,7 @@ jobs:
id: siblings
env:
BRANCH: ${{ github.head_ref || github.ref_name }}
run: bash .github/resolve-sibling-refs.sh
run: bash .github/scripts/resolve-sibling-refs.sh
working-directory: spinifex
- name: Checkout Viperblock
@@ -268,7 +268,7 @@ jobs:
id: siblings
env:
BRANCH: ${{ github.head_ref || github.ref_name }}
run: bash .github/resolve-sibling-refs.sh
run: bash .github/scripts/resolve-sibling-refs.sh
working-directory: spinifex
- name: Checkout Viperblock
+20 -18
View File
@@ -42,11 +42,15 @@ ifdef QUIET
_COVQ = 2>&1 | { grep -Ev '^\s*(ok|PASS|\?|=== RUN|--- PASS:)\s' | grep -v 'coverage: 0\.0%' || true; }; exit $${PIPESTATUS[0]}
_RACEQ = 2>&1 | { grep -Ev '^\s*(ok|PASS|\?|=== RUN|--- PASS:)\s' || true; }; exit $${PIPESTATUS[0]}
_SECQ = >
_HDR = :
_RUNSH = sh "$$t" >"$$log" 2>&1 || { echo "-- $$t"; cat "$$log"; exit 1; }
else
_Q =
_COVQ =
_RACEQ =
_SECQ = 2>&1 | tee
_HDR = echo -e
_RUNSH = echo "-- $$t"; sh "$$t" || exit 1
endif
build: go_build build-installer build-lb-agent generate-aws-model-coverage
@@ -240,27 +244,25 @@ preflight:
NODE_SCRIPTS := scripts/setup.sh scripts/setup-ovn.sh scripts/install-node.sh scripts/smoke-test.sh scripts/node-reset.sh scripts/uninstall-spx.sh scripts/lib/distro.sh scripts/lib/stage-sqs-store-microvm.sh
test-build-scripts:
@echo -e "\n....Running build/scripts/**/*_test.sh...."
@for t in $$(find build/scripts -name '*_test.sh' | sort); do \
echo "-- $$t"; \
sh "$$t" || exit 1; \
@$(_HDR) "\n....Running build/scripts/**/*_test.sh...."
@log=$$(mktemp); trap 'rm -f "$$log"' EXIT; for t in $$(find build/scripts -name '*_test.sh' | sort); do \
$(_RUNSH); \
done
@echo -e "\n....Running scripts/*_test.sh, scripts/lib/*_test.sh and scripts/appliance/*_test.sh...."
@for t in $$( (find scripts -maxdepth 1 -name '*_test.sh'; find scripts/lib scripts/appliance -name '*_test.sh') | sort); do \
echo "-- $$t"; \
sh "$$t" || exit 1; \
@$(_HDR) "\n....Running scripts/*_test.sh, scripts/lib/*_test.sh and scripts/appliance/*_test.sh...."
@log=$$(mktemp); trap 'rm -f "$$log"' EXIT; for t in $$( (find scripts -maxdepth 1 -name '*_test.sh'; find scripts/lib scripts/appliance -name '*_test.sh') | sort); do \
$(_RUNSH); \
done
@echo -e "\n....Running .github/scripts/*_test.sh...."
@for t in $$(find .github/scripts -maxdepth 1 -name '*_test.sh' | sort); do \
echo "-- $$t"; \
sh "$$t" || exit 1; \
@$(_HDR) "\n....Running .github/scripts/*_test.sh...."
@log=$$(mktemp); trap 'rm -f "$$log"' EXIT; for t in $$(find .github/scripts -maxdepth 1 -name '*_test.sh' | sort); do \
$(_RUNSH); \
done
shellcheck -S warning .github/scripts/hypervisor-disk-preflight.sh .github/scripts/hypervisor-disk-preflight_test.sh \
.github/scripts/wait-for-memory.sh .github/scripts/wait-for-memory_test.sh
@echo -e "\n....Running shellcheck over build/scripts/**/*.sh + the node scripts...."
shellcheck -S warning $$(find build/scripts -name '*.sh' | sort) $(NODE_SCRIPTS)
shellcheck -S warning -x scripts/build-oracle-appliance.sh $$(find scripts/appliance -name '*.sh' | sort)
@echo " test-build-scripts ok"
$(_Q)shellcheck -S warning .github/scripts/hypervisor-disk-preflight.sh \
.github/scripts/wait-for-memory.sh \
.github/scripts/mulga-design-check.sh
@$(_HDR) "\n....Running shellcheck over build/scripts/**/*.sh + the node scripts...."
$(_Q)shellcheck -S warning $$(find build/scripts -name '*.sh' | sort) $(NODE_SCRIPTS)
$(_Q)shellcheck -S warning -x scripts/build-oracle-appliance.sh $$(find scripts/appliance -name '*.sh' | sort)
@$(_HDR) " test-build-scripts ok"
# E2E harness unit tests. Build-tagged `e2e` so they're skipped by the
# default `go test ./spinifex/...`. Runs with mocked AWS clients — no
+2 -2
View File
@@ -70,8 +70,8 @@ The platform serves **405 AWS API operations** across the services below. See ex
| **RDS** | Databases | PostgreSQL and MariaDB, with automated backups | Available |
| **Route 53** | DNS | [Northstar](https://github.com/mulgadc/northstar): authoritative DNS with hosted zones | Available |
| **Bedrock** | AI deployment | vLLM for text generation and Hugging Face TEI for embeddings and reranking | Q4 2026 |
| **Lambda** | Serverless functions | | Q1 2027 |
| **SQS** | Message queues | | Q1 2027 |
| **Lambda** | Serverless functions | QEMU `microvm` execution environments | Q1 2027 |
| **SQS** | Message queues | A dedicated NATS JetStream store in a per-Region QEMU `microvm` guest | Q1 2027 |
Roadmap services ship under the same AWS API surface, so code written for AWS today works the moment they land. [Track what's shipped in the release notes](https://github.com/mulgadc/spinifex/releases).
+1 -1
View File
@@ -110,7 +110,7 @@ Three entry points, in the order they run:
### `iam` — everything IAM and STS
Policy evaluation against live requests rather than unit-level: a deny applying to the next request without a new credential, resource-scoped grants naming one key pair, `aws:SourceIp` seeing the real client address, assumed-role sessions carrying role policies, and revocation stopping one key alone.
Policy evaluation against live requests rather than unit-level: a deny applying to the next request without a new credential, resource-scoped grants naming one key pair, a user rename needing `iam:UpdateUser` on both the old and the new name, `aws:SourceIp` seeing the real client address, assumed-role sessions carrying role policies, and revocation stopping one key alone.
### `cert` — the cluster CA and TLS identity
@@ -0,0 +1,185 @@
// Package ebsobligation records the volume exports a detach left in place.
//
// A force detach, or a terminate whose unmount did not complete, clears a
// volume's attachment while the node that served it may still export it. The
// request cannot reach that node, so it records an UnpublishObligation here and
// returns; the named node settles it once it answers.
package ebsobligation
import (
"context"
"errors"
"fmt"
"regexp"
"strconv"
"time"
"github.com/mulgadc/spinifex/spinifex/foundation/state/kvstore"
"github.com/mulgadc/spinifex/spinifex/foundation/state/migrate"
"github.com/nats-io/nats.go"
"github.com/nats-io/nats.go/jetstream"
)
// Bucket holds one obligation per publish a cleared attachment left behind.
// It has no TTL: an obligation ends only when it is settled or closed.
const Bucket = "spinifex-ebs-unpublish-obligations"
// BucketVersion is the bucket schema version RunKV stamps.
const BucketVersion = 1
// SchemaVersion is the version of the UnpublishObligation record.
const SchemaVersion uint16 = 1
// UnpublishObligation names a publish a cleared attachment left on a node: the
// node, volume and handle, and the attachment it served. The attachment is what
// tells it apart from a later publish of the same volume on the same node.
type UnpublishObligation struct {
SchemaVersion uint16 `json:"schema_version"`
VolumeID string `json:"volume_id"`
AccountID string `json:"account_id"`
Handle string `json:"handle,omitempty"` // ProviderHandle when the attachment was cleared
NodeID string `json:"node_id"` // the node holding the publish
InstanceID string `json:"instance_id"` // the attachment that was cleared
AttachedAt time.Time `json:"attached_at"`
RecordedAt time.Time `json:"recorded_at"`
Attempts int `json:"attempts"`
Error string `json:"error,omitempty"`
}
// keyToken is one NATS subject token a key segment may be: the key is three
// tokens, so a segment holding a dot would split it and escape the filters.
var keyToken = regexp.MustCompile(`^[-_=a-zA-Z0-9]+$`)
// Key is the obligation's key, <node>.<volume-id>.<attached-at>, with the
// attachment time in Unix nanoseconds and an unknown time as 0.
func Key(o UnpublishObligation) (string, error) {
if !keyToken.MatchString(o.NodeID) {
return "", fmt.Errorf("unpublish obligation: node %q is not a valid key token", o.NodeID)
}
if !keyToken.MatchString(o.VolumeID) {
return "", fmt.Errorf("unpublish obligation: volume %q is not a valid key token", o.VolumeID)
}
var nanos int64
if !o.AttachedAt.IsZero() {
nanos = o.AttachedAt.UnixNano()
}
return o.NodeID + "." + o.VolumeID + "." + strconv.FormatInt(nanos, 10), nil
}
// allKeys matches every obligation key and not the bucket's version stamp,
// which is a single token.
const allKeys = "*.*.*"
func bucketConfig() kvstore.Config {
return kvstore.Config{
Name: Bucket,
Description: "EBS exports a cleared attachment left on a node, until that node unpublishes them",
History: 1,
OnOpen: func(ctx context.Context, kv jetstream.KeyValue) error {
return migrate.DefaultRegistry.RunKV(ctx, Bucket, kv, BucketVersion)
},
Missing: "EBS unpublish obligation bucket not initialized",
}
}
// Store reads and writes the obligation bucket.
type Store struct {
s *kvstore.Store[UnpublishObligation]
}
// NewStore returns a Store over js. The bucket is created on first use.
func NewStore(js jetstream.JetStream) *Store {
return &Store{s: kvstore.New[UnpublishObligation](js, bucketConfig())}
}
// Open creates or opens the bucket now, so a node that cannot reach it fails
// at startup rather than on its first force detach.
func (s *Store) Open(ctx context.Context) error {
_, err := s.s.KV(ctx)
return err
}
// Record writes o if no obligation with its key exists. An existing key is the
// same obligation written by an earlier attempt, so it counts as written.
func (s *Store) Record(ctx context.Context, o UnpublishObligation) error {
key, err := Key(o)
if err != nil {
return err
}
o.SchemaVersion = SchemaVersion
if _, err := s.s.Create(ctx, key, &o); err != nil && !errors.Is(err, kvstore.ErrExists) {
return fmt.Errorf("record unpublish obligation %s: %w", key, err)
}
return nil
}
// ListNode returns the open obligations naming node.
func (s *Store) ListNode(ctx context.Context, node string) ([]UnpublishObligation, error) {
if !keyToken.MatchString(node) {
return nil, fmt.Errorf("unpublish obligation: node %q is not a valid key token", node)
}
return s.list(ctx, node+".*.*")
}
// List returns every open obligation.
func (s *Store) List(ctx context.Context) ([]UnpublishObligation, error) {
return s.list(ctx, allKeys)
}
func (s *Store) list(ctx context.Context, filter string) ([]UnpublishObligation, error) {
items, _, err := s.s.Snapshot(ctx, filter)
if err != nil {
return nil, err
}
out := make([]UnpublishObligation, 0, len(items))
for _, item := range items {
out = append(out, item.Value)
}
return out, nil
}
// Close removes o. Removing one already gone is not an error.
func (s *Store) Close(ctx context.Context, o UnpublishObligation) error {
key, err := Key(o)
if err != nil {
return err
}
return s.s.Delete(ctx, key)
}
// RecordFailure counts a failed settlement on o and keeps its error, so an
// operator listing the obligations sees why it is still open.
func (s *Store) RecordFailure(ctx context.Context, o UnpublishObligation, cause error) error {
key, err := Key(o)
if err != nil {
return err
}
err = s.s.Mutate(ctx, key, func(cur *UnpublishObligation) (bool, error) {
cur.Attempts++
cur.Error = cause.Error()
return true, nil
})
if errors.Is(err, kvstore.ErrNotFound) {
return nil
}
return err
}
// ListOpen reads the open obligations for an operator tool without creating
// the bucket: a cluster that has never force-detached a volume has none, and
// that is not an error.
func ListOpen(ctx context.Context, nc *nats.Conn) ([]UnpublishObligation, error) {
js, err := jetstream.New(nc)
if err != nil {
return nil, fmt.Errorf("jetstream: %w", err)
}
if _, err := js.KeyValue(ctx, Bucket); errors.Is(err, jetstream.ErrBucketNotFound) {
return nil, nil
} else if err != nil {
return nil, fmt.Errorf("unpublish obligation bucket: %w", err)
}
cfg := bucketConfig()
cfg.AttachOnly = true
cfg.OnOpen = nil
return (&Store{s: kvstore.New[UnpublishObligation](js, cfg)}).List(ctx)
}
@@ -0,0 +1,141 @@
package ebsobligation_test
import (
"errors"
"testing"
"time"
testutil "github.com/mulgadc/spinifex/internal/testkit"
ebsobligation "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/obligation"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func newStore(t *testing.T) *ebsobligation.Store {
t.Helper()
_, _, js := testutil.StartTestJetStream(t)
store := ebsobligation.NewStore(js)
require.NoError(t, store.Open(t.Context()))
return store
}
func obligation(node, volumeID string, attachedAt time.Time) ebsobligation.UnpublishObligation {
return ebsobligation.UnpublishObligation{
VolumeID: volumeID, AccountID: "123456789012", NodeID: node,
InstanceID: "i-1", AttachedAt: attachedAt, RecordedAt: attachedAt.Add(time.Minute),
}
}
func TestKey_NamesNodeVolumeAndAttachment(t *testing.T) {
at := time.Unix(1_700_000_000, 5)
key, err := ebsobligation.Key(obligation("node-a", "vol-1", at))
require.NoError(t, err)
assert.Equal(t, "node-a.vol-1.1700000000000000005", key)
key, err = ebsobligation.Key(obligation("node-a", "vol-1", time.Time{}))
require.NoError(t, err)
assert.Equal(t, "node-a.vol-1.0", key, "an unknown attachment time is 0, not a huge negative")
}
// A segment holding a subject separator or wildcard would split the key or
// let one node's listing match another's.
func TestKey_RejectsSegmentsThatAreNotOneToken(t *testing.T) {
for _, o := range []ebsobligation.UnpublishObligation{
obligation("node.a", "vol-1", time.Time{}),
obligation("node-a", "vol.1", time.Time{}),
obligation("*", "vol-1", time.Time{}),
obligation("node-a", ">", time.Time{}),
obligation("", "vol-1", time.Time{}),
} {
_, err := ebsobligation.Key(o)
assert.Error(t, err, "%s/%s", o.NodeID, o.VolumeID)
}
}
func TestStore_RecordIsIdempotentPerAttachment(t *testing.T) {
store := newStore(t)
at := time.Unix(1_700_000_000, 0).UTC()
first := obligation("node-a", "vol-1", at)
first.Handle = "h1"
require.NoError(t, store.Record(t.Context(), first))
retry := first
retry.RecordedAt = at.Add(time.Hour)
require.NoError(t, store.Record(t.Context(), retry), "a retry of the same obligation is not an error")
require.NoError(t, store.Record(t.Context(), obligation("node-a", "vol-1", at.Add(time.Second))),
"a different attachment of the same volume is a different obligation")
open, err := store.List(t.Context())
require.NoError(t, err)
require.Len(t, open, 2)
for _, o := range open {
assert.Equal(t, ebsobligation.SchemaVersion, o.SchemaVersion)
if o.AttachedAt.Equal(at) {
assert.True(t, o.RecordedAt.Equal(at.Add(time.Minute)), "the first write is kept")
}
}
}
// The bucket's version stamp is a key in the same bucket; listing must not
// return it as an obligation.
func TestStore_ListSkipsTheVersionStampAndListNodeIsolatesNodes(t *testing.T) {
store := newStore(t)
at := time.Unix(1_700_000_000, 0).UTC()
require.NoError(t, store.Record(t.Context(), obligation("node-a", "vol-1", at)))
require.NoError(t, store.Record(t.Context(), obligation("node-a", "vol-2", at)))
require.NoError(t, store.Record(t.Context(), obligation("node-ab", "vol-3", at)))
all, err := store.List(t.Context())
require.NoError(t, err)
assert.Len(t, all, 3)
mine, err := store.ListNode(t.Context(), "node-a")
require.NoError(t, err)
volumes := []string{}
for _, o := range mine {
volumes = append(volumes, o.VolumeID)
}
assert.ElementsMatch(t, []string{"vol-1", "vol-2"}, volumes, "a node whose name prefixes another's sees only its own")
_, err = store.ListNode(t.Context(), "*")
assert.Error(t, err)
}
func TestStore_CloseAndRecordFailure(t *testing.T) {
store := newStore(t)
o := obligation("node-a", "vol-1", time.Unix(1_700_000_000, 0).UTC())
require.NoError(t, store.Record(t.Context(), o))
require.NoError(t, store.RecordFailure(t.Context(), o, errors.New("first")))
require.NoError(t, store.RecordFailure(t.Context(), o, errors.New("second")))
open, err := store.List(t.Context())
require.NoError(t, err)
require.Len(t, open, 1)
assert.Equal(t, 2, open[0].Attempts)
assert.Equal(t, "second", open[0].Error)
require.NoError(t, store.Close(t.Context(), o))
require.NoError(t, store.Close(t.Context(), o), "closing one already gone is not an error")
require.NoError(t, store.RecordFailure(t.Context(), o, errors.New("late")),
"a failure recorded after another sweep closed it does not resurrect it")
open, err = store.List(t.Context())
require.NoError(t, err)
assert.Empty(t, open)
}
func TestListOpen_MissingBucketIsEmptyAndNotCreated(t *testing.T) {
_, nc, js := testutil.StartTestJetStream(t)
open, err := ebsobligation.ListOpen(t.Context(), nc)
require.NoError(t, err)
assert.Empty(t, open)
_, err = js.KeyValue(t.Context(), ebsobligation.Bucket)
assert.Error(t, err, "an operator listing must not create the bucket")
store := ebsobligation.NewStore(js)
require.NoError(t, store.Open(t.Context()))
require.NoError(t, store.Record(t.Context(), obligation("node-a", "vol-1", time.Unix(1_700_000_000, 0).UTC())))
open, err = ebsobligation.ListOpen(t.Context(), nc)
require.NoError(t, err)
require.Len(t, open, 1)
assert.Equal(t, "vol-1", open[0].VolumeID)
}
+17 -3
View File
@@ -107,6 +107,7 @@ type StoppedInstanceStore interface {
LoadStoppedInstance(instanceID string) (*vm.VM, error)
ListStoppedInstances() ([]*vm.VM, error)
ListNonTerminatedInstances() ([]*vm.VM, error)
ListTerminatedInPrimary() ([]*vm.VM, error)
ListTerminatedInstances() ([]*vm.VM, error)
WriteStoppedInstance(instanceID string, instance *vm.VM) error
DeleteStoppedInstance(instanceID string) error
@@ -140,6 +141,17 @@ type InstanceTagWriter interface {
DeleteAllTags(ctx context.Context, accountID, resourceID string) error
}
// OwedUnpublish names the publish a terminate leaves in place when its unmount
// did not complete: the node still exporting the volume and the instance it
// served. The zero value means the unmount completed and nothing is owed.
type OwedUnpublish struct {
NodeID string
InstanceID string
}
// Owed reports whether a publish is left in place.
func (o OwedUnpublish) Owed() bool { return o.NodeID != "" }
// VolumeDeleter deletes EBS volumes. Implemented by domains/ec2/volume's
// VolumeServiceImpl (used by the daemon).
type VolumeDeleter interface {
@@ -148,14 +160,16 @@ type VolumeDeleter interface {
// DeleteVolumeOnTerminate deletes a DeleteOnTermination volume as part of
// an instance terminate: it clears any stale attachment (terminate
// implies detach) before deleting, so a still-attached boot volume left
// over from Stop is not rejected by DeleteVolume's in-use guard.
DeleteVolumeOnTerminate(ctx context.Context, volumeID, accountID string) error
// over from Stop is not rejected by DeleteVolume's in-use guard. When owed
// names a publish, an unpublish obligation is recorded before the clear.
DeleteVolumeOnTerminate(ctx context.Context, volumeID, accountID string, owed OwedUnpublish) error
// DetachVolumeOnTerminate clears a still-attached volume's attachment on
// terminate without deleting it, matching AWS semantics for a
// DeleteOnTermination=false volume: terminate still implies detach, it
// just leaves the volume behind as available rather than deleting it.
DetachVolumeOnTerminate(ctx context.Context, volumeID, accountID string) error
// When owed names a publish, an unpublish obligation is recorded first.
DetachVolumeOnTerminate(ctx context.Context, volumeID, accountID string, owed OwedUnpublish) error
}
// VolumeCreator creates the volumes a launch names beyond the root device, and
+66 -2
View File
@@ -1,6 +1,7 @@
package instance
import (
"cmp"
"context"
"encoding/base64"
"encoding/json"
@@ -3047,6 +3048,15 @@ func (s *InstanceServiceImpl) deleteInstanceVolumes(ctx context.Context, instanc
// stopped-path delete failure.
var lastErr error
// A volume whose seal failed at stop is still exported on the node that
// stopped it, so clearing its attachment owes that node an unpublish.
owedFor := func(volumeID string) OwedUnpublish {
if instance.LastNode == "" || !slices.Contains(instance.UnpublishOwed, volumeID) {
return OwedUnpublish{}
}
return OwedUnpublish{NodeID: instance.LastNode, InstanceID: instance.ID}
}
for _, ebsRequest := range instance.EBSRequests.Requests {
// Internal volumes (EFI) are always cleaned up via ebs.delete.
if ebsRequest.EFI {
@@ -3077,7 +3087,7 @@ func (s *InstanceServiceImpl) deleteInstanceVolumes(ctx context.Context, instanc
continue
}
// A volume already gone is the detach's own goal, not a failure.
if err := s.volumeDeleter.DetachVolumeOnTerminate(ctx, ebsRequest.Name, instance.AccountID); err != nil &&
if err := s.volumeDeleter.DetachVolumeOnTerminate(ctx, ebsRequest.Name, instance.AccountID, owedFor(ebsRequest.Name)); err != nil &&
!awserrors.IsNotFound(err) && !objectstore.IsNoSuchKeyError(err) {
slog.ErrorContext(ctx, "TerminateStoppedInstance: failed to detach volume", "name", ebsRequest.Name, "err", err)
lastErr = err
@@ -3096,7 +3106,7 @@ func (s *InstanceServiceImpl) deleteInstanceVolumes(ctx context.Context, instanc
// clears a Boot volume, runtime/daemon/vm_adapters.go) before deleting, so
// DeleteVolume's in-use guard does not reject an otherwise-legitimate
// delete. The resulting error is surfaced into lastErr, not swallowed.
if err := s.volumeDeleter.DeleteVolumeOnTerminate(ctx, ebsRequest.Name, instance.AccountID); err != nil {
if err := s.volumeDeleter.DeleteVolumeOnTerminate(ctx, ebsRequest.Name, instance.AccountID, owedFor(ebsRequest.Name)); err != nil {
slog.ErrorContext(ctx, "TerminateStoppedInstance: failed to delete volume", "name", ebsRequest.Name, "err", err)
lastErr = err
}
@@ -3905,3 +3915,57 @@ func (s *InstanceServiceImpl) AttachVolumeToStopped(ctx context.Context, input *
State: aws.String(ec2.VolumeAttachmentStateAttaching),
}, nil
}
// AccountTeardownOutstanding reports instances not terminated with every dependent done.
// Terminated records expire after 1h, so it is never sole evidence an account is clear.
// An unreadable record or one with no instance ID fails the read.
func (s *InstanceServiceImpl) AccountTeardownOutstanding(accountID string) ([]OutstandingInstance, error) {
if s.stoppedStore == nil {
return nil, errors.New("instance record store not configured")
}
live, err := s.stoppedStore.ListNonTerminatedInstances()
if err != nil {
return nil, fmt.Errorf("list non-terminated instances: %w", err)
}
primaryTerminated, err := s.stoppedStore.ListTerminatedInPrimary()
if err != nil {
return nil, fmt.Errorf("list terminated instances in primary store: %w", err)
}
terminated, err := s.stoppedStore.ListTerminatedInstances()
if err != nil {
return nil, fmt.Errorf("list terminated instances: %w", err)
}
// An instance can sit in both stores while a terminate is mid-flight. The
// non-terminated record is listed first and wins, so it is never reported
// as torn down on the strength of the terminated copy.
byID := map[string]OutstandingInstance{}
for _, store := range []struct {
name string
records []*vm.VM
}{{"non-terminated", live}, {"primary terminated", primaryTerminated}, {"terminated", terminated}} {
for _, v := range store.records {
if v == nil || v.ID == "" {
return nil, fmt.Errorf("%s instance store holds a record with no instance ID", store.name)
}
if !IsInstanceVisible(accountID, v.AccountID) {
continue
}
if v.Status == vm.StateTerminated && v.TeardownComplete() {
continue
}
if _, seen := byID[v.ID]; seen {
continue
}
byID[v.ID] = OutstandingInstance{
InstanceID: v.ID,
State: string(v.Status),
Teardown: maps.Clone(v.Teardown),
}
}
}
return slices.SortedFunc(maps.Values(byID), func(a, b OutstandingInstance) int {
return cmp.Compare(a.InstanceID, b.InstanceID)
}), nil
}
@@ -1734,6 +1734,7 @@ type fakeVolumeDeleter struct {
calls []string
deleted []string
detached []string
owed []OwedUnpublish
err error
}
@@ -1749,7 +1750,8 @@ func (f *fakeVolumeDeleter) DeleteVolume(_ context.Context, input *ec2.DeleteVol
// DeleteVolumeOnTerminate mirrors DeleteVolume's call/error bookkeeping; the
// stopped-instance terminate path calls this instead of DeleteVolume.
func (f *fakeVolumeDeleter) DeleteVolumeOnTerminate(_ context.Context, volumeID, _ string) error {
func (f *fakeVolumeDeleter) DeleteVolumeOnTerminate(_ context.Context, volumeID, _ string, owed OwedUnpublish) error {
f.owed = append(f.owed, owed)
f.calls = append(f.calls, volumeID)
if f.err != nil {
return f.err
@@ -1761,7 +1763,8 @@ func (f *fakeVolumeDeleter) DeleteVolumeOnTerminate(_ context.Context, volumeID,
// DetachVolumeOnTerminate mirrors DeleteVolumeOnTerminate's call/error
// bookkeeping but records into detached instead of deleted; the non-DoT
// Boot-volume branch calls this instead of deleting.
func (f *fakeVolumeDeleter) DetachVolumeOnTerminate(_ context.Context, volumeID, _ string) error {
func (f *fakeVolumeDeleter) DetachVolumeOnTerminate(_ context.Context, volumeID, _ string, owed OwedUnpublish) error {
f.owed = append(f.owed, owed)
f.calls = append(f.calls, volumeID)
if f.err != nil {
return f.err
@@ -2009,6 +2012,45 @@ func TestTerminateStoppedInstance_UserVolumeDeleted(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, []string{"vol-user-001"}, vd.deleted, "only DeleteOnTermination=true volumes deleted")
assert.Equal(t, []string{"vol-keep-001"}, vd.detached, "terminate implies detach even for a volume that survives it")
assert.Equal(t, []OwedUnpublish{{}, {}}, vd.owed, "a stop whose unmount completed owes no unpublish")
}
// A stop whose seal failed left those volumes exported on the node that
// stopped the instance, so terminate names that node for each of them alone.
func TestTerminateStoppedInstance_FailedSealOwesTheStoppingNodeAnUnpublish(t *testing.T) {
id := "i-seal-failed"
v := &vm.VM{ID: id, Status: vm.StateStopped, AccountID: "acc", LastNode: "node-a",
UnpublishOwed: []string{"vol-root-001", "vol-keep-001"}}
v.EBSRequests.Requests = []viperblocklegacyv1.EBSRequest{
{Name: "vol-root-001", Boot: true, DeleteOnTermination: true},
{Name: "vol-keep-001", DeleteOnTermination: false},
{Name: "vol-sealed-001", DeleteOnTermination: false},
}
store := &vmmock.StateStore{Stopped: map[string]*vm.VM{id: v}}
vd := &fakeVolumeDeleter{}
svc := &InstanceServiceImpl{stoppedStore: store, volumeDeleter: vd}
_, err := svc.TerminateStoppedInstance(context.Background(), &TerminateStoppedInstanceInput{InstanceID: id}, "acc")
require.NoError(t, err)
owed := OwedUnpublish{NodeID: "node-a", InstanceID: id}
assert.Equal(t, []string{"vol-root-001", "vol-keep-001", "vol-sealed-001"}, vd.calls)
assert.Equal(t, []OwedUnpublish{owed, owed, {}}, vd.owed)
}
// With no node recorded there is no node to name; the terminate owes nothing
// rather than naming an empty node.
func TestTerminateStoppedInstance_NoLastNodeOwesNothing(t *testing.T) {
id := "i-seal-failed-nonode"
v := &vm.VM{ID: id, Status: vm.StateStopped, AccountID: "acc", UnpublishOwed: []string{"vol-keep-001"}}
v.EBSRequests.Requests = []viperblocklegacyv1.EBSRequest{{Name: "vol-keep-001"}}
store := &vmmock.StateStore{Stopped: map[string]*vm.VM{id: v}}
vd := &fakeVolumeDeleter{}
svc := &InstanceServiceImpl{stoppedStore: store, volumeDeleter: vd}
_, err := svc.TerminateStoppedInstance(context.Background(), &TerminateStoppedInstanceInput{InstanceID: id}, "acc")
require.NoError(t, err)
assert.Equal(t, []OwedUnpublish{{}}, vd.owed)
}
// TestTerminateStoppedInstance_StampsTeardownVolumesDone locks the
@@ -0,0 +1,12 @@
package instance
// OutstandingInstance is an instance of an account that is not yet fully torn
// down, with the state its record carries.
type OutstandingInstance struct {
InstanceID string
// State is the record's lifecycle state, such as running or terminated.
State string
// Teardown is the per-dependent progress (done, pending or failed) the
// record carries. It is empty when no dependent has been recorded.
Teardown map[string]string
}
@@ -9,6 +9,7 @@ import (
testutil "github.com/mulgadc/spinifex/internal/testkit"
"github.com/mulgadc/spinifex/spinifex/bootstrap/config"
ebsmetadata "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata"
ec2instance "github.com/mulgadc/spinifex/spinifex/domains/ec2/instance"
ec2snapshot "github.com/mulgadc/spinifex/spinifex/domains/ec2/snapshot"
ec2volume "github.com/mulgadc/spinifex/spinifex/domains/ec2/volume"
awserrors "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
@@ -59,7 +60,7 @@ func TestCurrentBehaviour_AccountTermination_SnapshottedDeleteOnTerminationVolum
require.NoError(t, err)
require.NoError(t, volumes.UpdateVolumeState(accountID, volumeID, "in-use", instanceID, "/dev/sda1"))
err = volumes.DeleteVolumeOnTerminate(ctx, volumeID, accountID)
err = volumes.DeleteVolumeOnTerminate(ctx, volumeID, accountID, ec2instance.OwedUnpublish{})
assert.EqualError(t, err, awserrors.ErrorVolumeInUse, "the snapshot index refuses the terminate delete")
survivor, err := metadata.GetVolume(ctx, accountID, volumeID)
@@ -9,6 +9,7 @@ import (
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/aws/aws-sdk-go/service/s3"
"github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata"
ec2instance "github.com/mulgadc/spinifex/spinifex/domains/ec2/instance"
awserrors "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
ebsprovider "github.com/mulgadc/spinifex/spinifex/providers/ebs"
"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
@@ -194,7 +195,7 @@ func TestDeleteVolumeOnTerminate_DeleteReadFailureIsNotSuccess(t *testing.T) {
svc.store = failing
svc.metadata = ebsmetadata.NewStore(failing, "test-bucket")
err := svc.DeleteVolumeOnTerminate(context.Background(), volumeID, testVolAccountID)
err := svc.DeleteVolumeOnTerminate(context.Background(), volumeID, testVolAccountID, ec2instance.OwedUnpublish{})
require.Error(t, err, "a delete-read failure after a successful attachment clear must not be reported as success")
assert.False(t, awserrors.IsNotFound(err),
@@ -20,6 +20,7 @@ func newForceDetachService(t *testing.T) *VolumeServiceImpl {
t.Helper()
svc := newTestVolumeService("ap-southeast-2a")
svc.SetEBSProvider(ebsprovider.NewMemoryProvider(ebsprovider.Capabilities{}))
wireObligations(t, svc, &fakeNodeLocator{nodes: map[string]string{"i-stuck0000000000": "node-a"}}, "node-a", "node-b")
return svc
}
@@ -47,7 +48,11 @@ func TestForceDetachVolumeClearsTheAttachment(t *testing.T) {
// The previous instance is reported back: it is the only record of what
// the volume was torn away from.
assert.Equal(t, "i-stuck0000000000", aws.StringValue(attachment.InstanceId))
assert.Equal(t, "detached", aws.StringValue(attachment.State))
// The export is still up on the node that served it, so the attachment is
// reported mid-teardown, as AWS reports a detach it has not finished.
assert.Equal(t, "detaching", aws.StringValue(attachment.State))
assert.Equal(t, "/dev/sdf", aws.StringValue(attachment.Device))
assert.False(t, aws.TimeValue(attachment.AttachTime).IsZero())
meta, err := svc.GetVolumeMetadata("000000000011", "vol-forcedetach1")
require.NoError(t, err)
@@ -112,4 +117,8 @@ func TestForceDetachVolumeOnAnUnattachedVolume(t *testing.T) {
require.NoError(t, err)
assert.Empty(t, aws.StringValue(attachment.InstanceId))
assert.Equal(t, "detached", aws.StringValue(attachment.State))
open, err := svc.obligations.List(context.Background())
require.NoError(t, err)
assert.Empty(t, open, "an unattached volume has no publish to undo")
}
+55 -8
View File
@@ -20,6 +20,7 @@ import (
"github.com/mulgadc/spinifex/spinifex/bootstrap/config"
ebsencryption "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/encryption"
"github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata"
ebsobligation "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/obligation"
ebspolicy "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/policy"
ec2instance "github.com/mulgadc/spinifex/spinifex/domains/ec2/instance"
"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
@@ -59,6 +60,12 @@ type VolumeServiceImpl struct {
snapshotKV jetstream.KeyValue
provider ebsprovider.EBSProvider
metadata *ebsmetadata.Store
// The unpublish obligations a cleared attachment records, and what names
// the node holding the publish. Unset, a detach that leaves one fails.
obligations *ebsobligation.Store
instanceNodes InstanceNodeLocator
members func() []string
}
// SetEBSProvider injects the provider boundary used by the control plane. It
@@ -1083,7 +1090,7 @@ func (s *VolumeServiceImpl) ModifyVolume(ctx context.Context, input *ec2.ModifyV
// terminating instance's QEMU has already been asked to shut down — so this
// is always a metadata-only clear, never a QMP call. Errors from either step
// are returned, not swallowed.
func (s *VolumeServiceImpl) DeleteVolumeOnTerminate(ctx context.Context, volumeID, accountID string) error {
func (s *VolumeServiceImpl) DeleteVolumeOnTerminate(ctx context.Context, volumeID, accountID string, owed ec2instance.OwedUnpublish) error {
// Clearing the attachment would overwrite a recorded deletion, so a volume
// already owed is left alone: the reclaimer owns it from here, and resetting
// it to available would let a second teardown run against the same data.
@@ -1091,6 +1098,9 @@ func (s *VolumeServiceImpl) DeleteVolumeOnTerminate(ctx context.Context, volumeI
if err == nil && meta.State == volumeStateDeleting {
return nil
}
if err := s.recordTerminateObligation(ctx, accountID, volumeID, owed); err != nil {
return err
}
if err := s.UpdateVolumeState(accountID, volumeID, "available", "", ""); err != nil {
// No metadata doc means the volume is already gone, per GetVolume's own
@@ -1110,7 +1120,10 @@ func (s *VolumeServiceImpl) DeleteVolumeOnTerminate(ctx context.Context, volumeI
// available rather than deleting it. Metadata-only, no QMP — same rationale
// as DeleteVolumeOnTerminate: there is no live QEMU to hot-unplug on either
// terminate path.
func (s *VolumeServiceImpl) DetachVolumeOnTerminate(_ context.Context, volumeID, accountID string) error {
func (s *VolumeServiceImpl) DetachVolumeOnTerminate(ctx context.Context, volumeID, accountID string, owed ec2instance.OwedUnpublish) error {
if err := s.recordTerminateObligation(ctx, accountID, volumeID, owed); err != nil {
return err
}
if err := s.UpdateVolumeState(accountID, volumeID, "available", "", ""); err != nil {
// No metadata doc means the volume is already gone, per GetVolume's own
// contract, so absence here is success, not a teardown failure.
@@ -1129,8 +1142,12 @@ func (s *VolumeServiceImpl) DetachVolumeOnTerminate(_ context.Context, volumeID,
// The ordinary DetachVolume routes to ec2.cmd.{instanceID} for the QMP unplug,
// so a volume attached to an instance whose host stopped answering can never be
// detached and therefore never deleted. This exists for that deadlock and for
// nothing else: it leaves a live guest holding a device the control plane no
// longer believes in, which is only safe once that guest is being destroyed.
// nothing else.
//
// That host still exports the volume, and the unpublish has to go to it, so
// it cannot be made here. The request records an unpublish obligation naming
// the publish before it clears the attachment, and returns; the node settles
// it once it answers. A failed record clears nothing.
func (s *VolumeServiceImpl) ForceDetachVolume(ctx context.Context, input *ec2.DetachVolumeInput, accountID string) (*ec2.VolumeAttachment, error) {
if input == nil || input.VolumeId == nil || *input.VolumeId == "" {
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
@@ -1154,18 +1171,48 @@ func (s *VolumeServiceImpl) ForceDetachVolume(ctx context.Context, input *ec2.De
}
previous := meta.AttachedInstance
if previous != "" {
nodes, err := s.publishingNodes(previous)
if err != nil {
slog.ErrorContext(ctx, "ForceDetachVolume: cannot name the node holding the publish, clearing nothing",
"volumeId", volumeID, "instanceId", previous, "err", err)
return nil, errors.New(awserrors.ErrorInternalError)
}
if err := s.recordObligations(ctx, meta, nodes); err != nil {
slog.ErrorContext(ctx, "ForceDetachVolume: failed to record the unpublish obligation, clearing nothing",
"volumeId", volumeID, "instanceId", previous, "err", err)
return nil, errors.New(awserrors.ErrorInternalError)
}
}
if err := s.UpdateVolumeState(accountID, volumeID, "available", "", ""); err != nil {
slog.ErrorContext(ctx, "ForceDetachVolume: failed to clear attachment", "volumeId", volumeID, "err", err)
return nil, errors.New(awserrors.ErrorServerInternal)
}
slog.WarnContext(ctx, "ForceDetachVolume: attachment cleared in the control plane only",
if previous == "" {
return &ec2.VolumeAttachment{
VolumeId: aws.String(volumeID),
State: aws.String("detached"),
}, nil
}
slog.WarnContext(ctx, "ForceDetachVolume: attachment cleared, unpublish left to the node holding it",
"volumeId", volumeID, "previousInstance", previous, "accountId", accountID)
return &ec2.VolumeAttachment{
// AWS answers a detach with the attachment it is tearing down, in state
// detaching; the export here goes when the node settles the obligation.
attachment := &ec2.VolumeAttachment{
VolumeId: aws.String(volumeID),
InstanceId: aws.String(previous),
State: aws.String("detached"),
}, nil
State: aws.String("detaching"),
}
if meta.DeviceName != "" {
attachment.Device = aws.String(meta.DeviceName)
}
if !meta.AttachedAt.IsZero() {
attachment.AttachTime = aws.Time(meta.AttachedAt)
}
return attachment, nil
}
// DeleteVolume records a durable obligation to destroy an EBS volume and
@@ -16,6 +16,7 @@ import (
"github.com/mulgadc/spinifex/internal/testkit/ebsfake"
"github.com/mulgadc/spinifex/spinifex/bootstrap/config"
"github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata"
ec2instance "github.com/mulgadc/spinifex/spinifex/domains/ec2/instance"
"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
awsfilters "github.com/mulgadc/spinifex/spinifex/foundation/aws/filters"
"github.com/mulgadc/spinifex/spinifex/providers/ebs"
@@ -1789,7 +1790,7 @@ func TestDeleteVolumeOnTerminate_ClearsAttachmentThenDeletes(t *testing.T) {
AttachedInstance: "i-stopped",
})
err := svc.DeleteVolumeOnTerminate(context.Background(), volumeID, testVolAccountID)
err := svc.DeleteVolumeOnTerminate(context.Background(), volumeID, testVolAccountID, ec2instance.OwedUnpublish{})
require.NoError(t, err, "terminate implies detach: a stale attachment must not block the terminate delete")
deleting, err := svc.GetVolumeMetadata(testVolAccountID, volumeID)
@@ -1816,7 +1817,7 @@ func TestDeleteVolumeOnTerminate_AlreadyGoneMetadataIsSuccess(t *testing.T) {
svc.snapshotKV = kv
// No metadata doc is ever written for this volume ID.
err := svc.DeleteVolumeOnTerminate(context.Background(), "vol-already-gone", testVolAccountID)
err := svc.DeleteVolumeOnTerminate(context.Background(), "vol-already-gone", testVolAccountID, ec2instance.OwedUnpublish{})
require.NoError(t, err, "a volume with no metadata document is already gone, not a teardown failure")
}
@@ -1829,7 +1830,7 @@ func TestDetachVolumeOnTerminate_AlreadyGoneMetadataIsSuccess(t *testing.T) {
svc := newTestVolumeServiceWithStore("ap-southeast-2a", store)
svc.snapshotKV = kv
err := svc.DetachVolumeOnTerminate(context.Background(), "vol-already-gone", testVolAccountID)
err := svc.DetachVolumeOnTerminate(context.Background(), "vol-already-gone", testVolAccountID, ec2instance.OwedUnpublish{})
require.NoError(t, err, "a volume with no metadata document is already gone, not a teardown failure")
}
@@ -1913,7 +1914,7 @@ func TestDeleteVolume_SecondDeleteIsAcceptedWithoutASecondTeardown(t *testing.T)
// DeleteOnTermination arriving for a volume already owed must not reset it
// to available, which would let a second teardown through.
require.NoError(t, svc.DeleteVolumeOnTerminate(context.Background(), volumeID, testVolAccountID))
require.NoError(t, svc.DeleteVolumeOnTerminate(context.Background(), volumeID, testVolAccountID, ec2instance.OwedUnpublish{}))
meta, err := svc.metadata.GetVolume(context.Background(), testVolAccountID, volumeID)
require.NoError(t, err)
assert.Equal(t, "deleting", meta.State)
@@ -1961,7 +1962,7 @@ func TestDeleteVolumeOnTerminate_SurfacesDeleteFailure(t *testing.T) {
_, err = kv.Put(t.Context(), volumeID, snapData)
require.NoError(t, err)
err = svc.DeleteVolumeOnTerminate(context.Background(), volumeID, testVolAccountID)
err = svc.DeleteVolumeOnTerminate(context.Background(), volumeID, testVolAccountID, ec2instance.OwedUnpublish{})
require.Error(t, err, "a DeleteVolume failure must be surfaced, not swallowed")
assert.Contains(t, err.Error(), awserrors.ErrorVolumeInUse)
@@ -0,0 +1,365 @@
package volume
import (
"context"
"errors"
"fmt"
"log/slog"
"sync"
"time"
ebsmetadata "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata"
ebsobligation "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/obligation"
ec2instance "github.com/mulgadc/spinifex/spinifex/domains/ec2/instance"
"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
"github.com/mulgadc/spinifex/spinifex/runtime/compute/vm"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric"
)
// InstanceNodeLocator reports the node that last ran an instance. found is
// false when no record of the instance exists.
type InstanceNodeLocator interface {
InstanceLastNode(instanceID string) (node string, found bool, err error)
}
// SetUnpublishObligations wires the obligation store a cleared attachment
// records into, the locator that names the node holding the publish, and the
// cluster members to name when the instance has no record.
func (s *VolumeServiceImpl) SetUnpublishObligations(store *ebsobligation.Store, locator InstanceNodeLocator, members func() []string) {
s.obligations = store
s.instanceNodes = locator
s.members = members
}
// publishingNodes names the node holding instanceID's publish. With no record
// of the instance there is no node to name, so every member is named and each
// settles its own.
func (s *VolumeServiceImpl) publishingNodes(instanceID string) ([]string, error) {
if s.instanceNodes == nil {
return nil, errors.New("no instance node locator configured")
}
node, found, err := s.instanceNodes.InstanceLastNode(instanceID)
if err != nil {
return nil, fmt.Errorf("read instance %s: %w", instanceID, err)
}
if found && node != "" {
return []string{node}, nil
}
var members []string
if s.members != nil {
members = s.members()
}
if len(members) == 0 {
return nil, fmt.Errorf("instance %s has no record and no cluster member is known", instanceID)
}
return members, nil
}
// recordObligations writes one obligation per node for the attachment meta
// still carries. Every write must land before the caller clears it.
func (s *VolumeServiceImpl) recordObligations(ctx context.Context, meta ebsmetadata.Volume, nodes []string) error {
if s.obligations == nil {
return errors.New("no unpublish obligation store configured")
}
now := time.Now()
for _, node := range nodes {
if err := s.obligations.Record(ctx, ebsobligation.UnpublishObligation{
VolumeID: meta.VolumeID,
AccountID: meta.TenantID,
Handle: meta.ProviderHandle,
NodeID: node,
InstanceID: meta.AttachedInstance,
AttachedAt: meta.AttachedAt,
RecordedAt: now,
}); err != nil {
return err
}
}
return nil
}
// recordTerminateObligation records the obligation for a terminate that clears
// owed's attachment with its publish still in place. A volume no longer
// attached to that instance has nothing of it left to clear.
func (s *VolumeServiceImpl) recordTerminateObligation(ctx context.Context, accountID, volumeID string, owed ec2instance.OwedUnpublish) error {
if !owed.Owed() {
return nil
}
meta, err := s.metadata.GetVolume(ctx, accountID, volumeID)
if objectstore.IsNoSuchKeyError(err) {
return nil
}
if err != nil {
return fmt.Errorf("read volume %s before recording its unpublish obligation: %w", volumeID, err)
}
if meta.AttachedInstance != owed.InstanceID {
return nil
}
if err := s.recordObligations(ctx, meta, []string{owed.NodeID}); err != nil {
return fmt.Errorf("record unpublish obligation for %s: %w", volumeID, err)
}
return nil
}
// ObligationUnpublisher is this node's mounter as the obligation reaper drives
// it. Its lock is the one every publish and unpublish on this node takes.
type ObligationUnpublisher interface {
// HoldVolume takes volumeID's lock without waiting. false means a publish
// or unpublish of it is in flight here, and the sweep comes back later.
HoldVolume(volumeID string) (VolumeHold, bool)
}
// VolumeHold is one volume's publish lock on this node.
type VolumeHold interface {
// PublishedSince reports whether this process published the volume after
// t and has not unpublished it since.
PublishedSince(t time.Time) bool
// Unpublish tears down this node's export of the volume. A volume the
// provider holds no export of counts as done.
Unpublish(ctx context.Context, accountID, handle string, noSeal bool) error
Release()
}
// obligationAction is what settling an obligation does with the volume as it
// is now.
type obligationAction int
const (
// obligationWait leaves the obligation open: the attachment it names has
// not been cleared yet, or a publish here is newer than it.
obligationWait obligationAction = iota
// obligationSuperseded closes it without a provider call: a newer
// attachment owns the export.
obligationSuperseded
// obligationSeal unpublishes with a seal: nobody else writes the volume,
// so the copy here is the last owner's and its writes are kept.
obligationSeal
// obligationNoSeal unpublishes without sealing a volume being deleted.
obligationNoSeal
)
func (a obligationAction) String() string {
switch a {
case obligationSuperseded:
return "superseded"
case obligationSeal:
return "seal"
case obligationNoSeal:
return "no-seal"
default:
return "wait"
}
}
// decideObligation is the settlement policy over the volume's current record;
// meta is nil when the volume has none. It never unpublishes an attachment the
// volume still carries.
func decideObligation(o ebsobligation.UnpublishObligation, meta *ebsmetadata.Volume) obligationAction {
if meta == nil || meta.State == volumeStateDeleting {
return obligationNoSeal
}
if meta.AttachedInstance != "" {
if meta.AttachedInstance != o.InstanceID || meta.AttachedAt.After(o.AttachedAt) {
return obligationSuperseded
}
return obligationWait
}
return obligationSeal
}
// obligationSince is the time a publish on this node must follow to be newer
// than the one an obligation undoes.
func obligationSince(o ebsobligation.UnpublishObligation) time.Time {
if o.AttachedAt.IsZero() {
return o.RecordedAt
}
return o.AttachedAt
}
// unpublishObligationReaper settles the obligations naming its node through
// that node's mounter, once the node answers.
type unpublishObligationReaper struct {
svc *VolumeServiceImpl
node string
unpublisher ObligationUnpublisher
}
var _ vm.Reaper = (*unpublishObligationReaper)(nil)
// NewUnpublishObligationReaper builds the reaper that settles node's
// obligations through unpublisher, which must be the node's own mounter.
func (s *VolumeServiceImpl) NewUnpublishObligationReaper(node string, unpublisher ObligationUnpublisher) vm.Reaper {
return &unpublishObligationReaper{svc: s, node: node, unpublisher: unpublisher}
}
func (r *unpublishObligationReaper) Class() string { return "ebs-unpublish-obligation" }
// Scope is node-local: an obligation is settled only by the node it names,
// because only that node can tear its own export down.
func (r *unpublishObligationReaper) Scope() vm.ReaperScope { return vm.ScopeNodeLocal }
// Sweep settles what it can and reports how many obligations it closed. A
// failure is recorded on the obligation and retried on the next sweep.
func (r *unpublishObligationReaper) Sweep(ctx context.Context) (int, error) {
if r.svc.obligations == nil || r.unpublisher == nil {
return 0, nil
}
open, err := r.svc.obligations.ListNode(ctx, r.node)
if err != nil {
return 0, err
}
closed := 0
for _, o := range open {
if err := ctx.Err(); err != nil {
return closed, err
}
done, err := r.settle(ctx, o)
if err != nil {
slog.WarnContext(ctx, "ebs-unpublish-obligation: settlement failed, retrying next sweep",
"volumeId", o.VolumeID, "node", o.NodeID, "instanceId", o.InstanceID, "attempts", o.Attempts+1, "err", err)
if recErr := r.svc.obligations.RecordFailure(ctx, o, err); recErr != nil {
slog.WarnContext(ctx, "ebs-unpublish-obligation: could not record the failure",
"volumeId", o.VolumeID, "node", o.NodeID, "err", recErr)
}
continue
}
if done {
closed++
}
}
return closed, nil
}
// settle acts on one obligation under the volume's lock, so no publish on this
// node falls between reading the volume and unpublishing it.
func (r *unpublishObligationReaper) settle(ctx context.Context, o ebsobligation.UnpublishObligation) (bool, error) {
hold, ok := r.unpublisher.HoldVolume(o.VolumeID)
if !ok {
return false, nil
}
defer hold.Release()
var current *ebsmetadata.Volume
meta, err := r.svc.metadata.GetVolume(ctx, o.AccountID, o.VolumeID)
switch {
case err == nil:
current = &meta
case objectstore.IsNoSuchKeyError(err):
default:
return false, fmt.Errorf("read volume %s: %w", o.VolumeID, err)
}
action := decideObligation(o, current)
// A newer publish here may own the export, so only a superseded obligation,
// which sends nothing, closes past it.
if action != obligationSuperseded && hold.PublishedSince(obligationSince(o)) {
return false, nil
}
switch action {
case obligationWait:
return false, nil
case obligationSuperseded:
slog.WarnContext(ctx, "ebs-unpublish-obligation: a newer attachment owns the volume, closing without unpublishing",
"volumeId", o.VolumeID, "node", o.NodeID, "instanceId", o.InstanceID, "attachedInstance", current.AttachedInstance)
case obligationSeal, obligationNoSeal:
if err := hold.Unpublish(ctx, o.AccountID, o.Handle, action == obligationNoSeal); err != nil {
return false, err
}
slog.InfoContext(ctx, "ebs-unpublish-obligation: unpublished the export a cleared attachment left",
"volumeId", o.VolumeID, "node", o.NodeID, "instanceId", o.InstanceID, "noSeal", action == obligationNoSeal)
}
if err := r.svc.obligations.Close(ctx, o); err != nil {
return false, fmt.Errorf("close obligation: %w", err)
}
recordObligationClosed(o.NodeID, action.String())
return true, nil
}
// NodeDeparted reports whether node has left the cluster, rather than being
// down, stale or partitioned, which must not count.
type NodeDeparted func(ctx context.Context, node string) (bool, error)
// unpublishObligationCloser closes, without a provider call, the obligations of
// nodes removed from the cluster: such a node holds nothing a member can reach.
type unpublishObligationCloser struct {
svc *VolumeServiceImpl
departed NodeDeparted
}
var _ vm.Reaper = (*unpublishObligationCloser)(nil)
// NewUnpublishObligationCloser builds the cluster-wide pass that closes the
// obligations of nodes departed reports as gone.
func (s *VolumeServiceImpl) NewUnpublishObligationCloser(departed NodeDeparted) vm.Reaper {
return &unpublishObligationCloser{svc: s, departed: departed}
}
func (c *unpublishObligationCloser) Class() string { return "ebs-unpublish-obligation-close" }
// Scope is cluster-wide: membership is the cluster's, and one pass is enough.
func (c *unpublishObligationCloser) Scope() vm.ReaperScope { return vm.ScopeClusterWide }
func (c *unpublishObligationCloser) Sweep(ctx context.Context) (int, error) {
if c.svc.obligations == nil || c.departed == nil {
return 0, nil
}
open, err := c.svc.obligations.List(ctx)
if err != nil {
return 0, err
}
gone := map[string]bool{}
closed := 0
for _, o := range open {
departed, seen := gone[o.NodeID]
if !seen {
departed, err = c.departed(ctx, o.NodeID)
if err != nil {
slog.WarnContext(ctx, "ebs-unpublish-obligation: cannot tell whether a node left the cluster, keeping its obligations",
"node", o.NodeID, "err", err)
departed = false
}
gone[o.NodeID] = departed
}
if !departed {
continue
}
if err := c.svc.obligations.Close(ctx, o); err != nil {
slog.WarnContext(ctx, "ebs-unpublish-obligation: failed to close a departed node's obligation",
"volumeId", o.VolumeID, "node", o.NodeID, "err", err)
continue
}
slog.WarnContext(ctx, "ebs-unpublish-obligation: closed for a node removed from the cluster",
"volumeId", o.VolumeID, "node", o.NodeID, "instanceId", o.InstanceID)
recordObligationClosed(o.NodeID, "departed")
closed++
}
return closed, nil
}
const obligationMeterName = "github.com/mulgadc/spinifex/spinifex/domains/ec2/volume/obligation"
var (
obligationClosedOnce sync.Once
obligationClosedCounter metric.Int64Counter
)
// recordObligationClosed counts one closed obligation by the node it named
// and how it closed.
func recordObligationClosed(node, outcome string) {
obligationClosedOnce.Do(func() {
var err error
obligationClosedCounter, err = otel.Meter(obligationMeterName).Int64Counter("mulga.ebs.unpublish_obligation.closed",
metric.WithDescription("Unpublish obligations closed, by the node they named and how they closed."),
metric.WithUnit("{obligation}"))
if err != nil {
otel.Handle(err)
}
})
if obligationClosedCounter == nil {
return
}
obligationClosedCounter.Add(context.Background(), 1, metric.WithAttributes(
attribute.String("node", node), attribute.String("outcome", outcome)))
}
@@ -0,0 +1,596 @@
package volume
//test:in-package — the obligation reaper and its policy are unexported, and
// seeding a volume's metadata document is how an attachment is set up.
import (
"context"
"errors"
"sync"
"testing"
"time"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
testutil "github.com/mulgadc/spinifex/internal/testkit"
ebsmetadata "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata"
ebsobligation "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/obligation"
ec2instance "github.com/mulgadc/spinifex/spinifex/domains/ec2/instance"
awserrors "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
ebsprovider "github.com/mulgadc/spinifex/spinifex/providers/ebs"
"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
"github.com/nats-io/nats.go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
type fakeNodeLocator struct {
nodes map[string]string
err error
}
var _ InstanceNodeLocator = (*fakeNodeLocator)(nil)
func (f *fakeNodeLocator) InstanceLastNode(instanceID string) (string, bool, error) {
if f.err != nil {
return "", false, f.err
}
node, found := f.nodes[instanceID]
return node, found, nil
}
// wireObligations gives svc an obligation bucket on a JetStream of its own and
// returns the connection under it, so a test can cut it.
func wireObligations(t *testing.T, svc *VolumeServiceImpl, locator InstanceNodeLocator, members ...string) *nats.Conn {
t.Helper()
_, nc, js := testutil.StartTestJetStream(t)
store := ebsobligation.NewStore(js)
require.NoError(t, store.Open(t.Context()))
svc.SetUnpublishObligations(store, locator, func() []string { return members })
return nc
}
type unpublishCall struct {
volumeID, accountID, handle string
noSeal bool
}
// fakeUnpublisher stands in for a node's mounter: busy volumes refuse the
// hold, publishedAt is the last publish this process made of each volume.
type fakeUnpublisher struct {
mu sync.Mutex
busy map[string]bool
publishedAt map[string]time.Time
err error
calls []unpublishCall
held int
}
var _ ObligationUnpublisher = (*fakeUnpublisher)(nil)
func (f *fakeUnpublisher) HoldVolume(volumeID string) (VolumeHold, bool) {
f.mu.Lock()
defer f.mu.Unlock()
if f.busy[volumeID] {
return nil, false
}
f.held++
return &fakeHold{f: f, volumeID: volumeID}, true
}
func (f *fakeUnpublisher) unpublishes() []unpublishCall {
f.mu.Lock()
defer f.mu.Unlock()
return append([]unpublishCall(nil), f.calls...)
}
type fakeHold struct {
f *fakeUnpublisher
volumeID string
}
var _ VolumeHold = (*fakeHold)(nil)
func (h *fakeHold) PublishedSince(t time.Time) bool {
h.f.mu.Lock()
defer h.f.mu.Unlock()
at, ok := h.f.publishedAt[h.volumeID]
return ok && at.After(t)
}
func (h *fakeHold) Unpublish(_ context.Context, accountID, handle string, noSeal bool) error {
h.f.mu.Lock()
defer h.f.mu.Unlock()
h.f.calls = append(h.f.calls, unpublishCall{volumeID: h.volumeID, accountID: accountID, handle: handle, noSeal: noSeal})
return h.f.err
}
func (h *fakeHold) Release() {
h.f.mu.Lock()
defer h.f.mu.Unlock()
h.f.held--
}
// putAttachment writes volumeID's document with an explicit attachment, so a
// test controls AttachedAt rather than racing the clock.
func putAttachment(t *testing.T, svc *VolumeServiceImpl, volumeID, state, instanceID string, attachedAt time.Time) ebsmetadata.Volume {
t.Helper()
meta := ebsmetadata.Volume{
VolumeID: volumeID, TenantID: testVolAccountID, CapacityGiB: 1,
State: state, AvailabilityZone: "ap-southeast-2a", VolumeType: "gp3",
AttachedInstance: instanceID, AttachedAt: attachedAt, ProviderHandle: "handle-" + volumeID,
}
if instanceID != "" {
meta.DeviceName = "/dev/sdf"
}
require.NoError(t, svc.metadata.PutVolume(context.Background(), meta))
return meta
}
func readVolume(t *testing.T, svc *VolumeServiceImpl, volumeID string) ebsmetadata.Volume {
t.Helper()
meta, err := svc.metadata.GetVolume(context.Background(), testVolAccountID, volumeID)
require.NoError(t, err)
return meta
}
func openObligations(t *testing.T, svc *VolumeServiceImpl) []ebsobligation.UnpublishObligation {
t.Helper()
open, err := svc.obligations.List(context.Background())
require.NoError(t, err)
return open
}
func forceDetach(t *testing.T, svc *VolumeServiceImpl, volumeID string) (*ec2.VolumeAttachment, error) {
t.Helper()
return svc.ForceDetachVolume(context.Background(), &ec2.DetachVolumeInput{
VolumeId: aws.String(volumeID), Force: aws.Bool(true),
}, testVolAccountID)
}
// --- ForceDetachVolume records the obligation ---
func TestForceDetach_RecordsOneObligationNamingThePublish(t *testing.T) {
svc := newTestVolumeService("ap-southeast-2a")
wireObligations(t, svc, &fakeNodeLocator{nodes: map[string]string{"i-old": "node-a"}}, "node-a", "node-b")
attachedAt := time.Unix(1_700_000_000, 0).UTC()
putAttachment(t, svc, "vol-fd-once", "in-use", "i-old", attachedAt)
_, err := forceDetach(t, svc, "vol-fd-once")
require.NoError(t, err)
open := openObligations(t, svc)
require.Len(t, open, 1, "only the node that last ran the instance is named")
o := open[0]
assert.Equal(t, "vol-fd-once", o.VolumeID)
assert.Equal(t, testVolAccountID, o.AccountID)
assert.Equal(t, "handle-vol-fd-once", o.Handle)
assert.Equal(t, "node-a", o.NodeID)
assert.Equal(t, "i-old", o.InstanceID)
assert.True(t, attachedAt.Equal(o.AttachedAt), "the obligation names the attachment it undoes")
assert.Equal(t, ebsobligation.SchemaVersion, o.SchemaVersion)
}
// A caller retrying a force detach whose clear failed writes the same key, so
// the retry leaves one obligation, not two.
func TestForceDetach_RetryLeavesOneObligation(t *testing.T) {
svc := newTestVolumeService("ap-southeast-2a")
wireObligations(t, svc, &fakeNodeLocator{nodes: map[string]string{"i-old": "node-a"}}, "node-a")
meta := putAttachment(t, svc, "vol-fd-retry", "in-use", "i-old", time.Unix(1_700_000_000, 0).UTC())
require.NoError(t, svc.recordObligations(context.Background(), meta, []string{"node-a"}))
_, err := forceDetach(t, svc, "vol-fd-retry")
require.NoError(t, err)
assert.Len(t, openObligations(t, svc), 1)
}
// With no record of the instance there is no node to name, so every member is
// named and each one settles its own.
func TestForceDetach_UnknownInstanceNamesEveryMember(t *testing.T) {
svc := newTestVolumeService("ap-southeast-2a")
wireObligations(t, svc, &fakeNodeLocator{}, "node-a", "node-b", "node-c")
putAttachment(t, svc, "vol-fd-every", "in-use", "i-gone", time.Unix(1_700_000_000, 0).UTC())
_, err := forceDetach(t, svc, "vol-fd-every")
require.NoError(t, err)
nodes := []string{}
for _, o := range openObligations(t, svc) {
nodes = append(nodes, o.NodeID)
}
assert.ElementsMatch(t, []string{"node-a", "node-b", "node-c"}, nodes)
}
// The attachment is the only record of where the export is. If the obligation
// cannot be written, clearing it would forget an export, so nothing is cleared.
func TestForceDetach_ObligationWriteFailureClearsNothing(t *testing.T) {
svc := newTestVolumeService("ap-southeast-2a")
nc := wireObligations(t, svc, &fakeNodeLocator{nodes: map[string]string{"i-old": "node-a"}}, "node-a")
putAttachment(t, svc, "vol-fd-nowrite", "in-use", "i-old", time.Unix(1_700_000_000, 0).UTC())
nc.Close()
_, err := forceDetach(t, svc, "vol-fd-nowrite")
require.Error(t, err)
assert.Equal(t, awserrors.ErrorInternalError, err.Error())
meta := readVolume(t, svc, "vol-fd-nowrite")
assert.Equal(t, "in-use", meta.State)
assert.Equal(t, "i-old", meta.AttachedInstance)
}
func TestForceDetach_UnnameableNodeClearsNothing(t *testing.T) {
for name, locator := range map[string]InstanceNodeLocator{
"locator fails": &fakeNodeLocator{err: errors.New("instance bucket unreachable")},
"no record and no members": &fakeNodeLocator{},
"no locator configured": nil,
} {
t.Run(name, func(t *testing.T) {
svc := newTestVolumeService("ap-southeast-2a")
wireObligations(t, svc, locator)
putAttachment(t, svc, "vol-fd-noname", "in-use", "i-old", time.Unix(1_700_000_000, 0).UTC())
_, err := forceDetach(t, svc, "vol-fd-noname")
require.Error(t, err)
assert.Equal(t, awserrors.ErrorInternalError, err.Error())
assert.Equal(t, "i-old", readVolume(t, svc, "vol-fd-noname").AttachedInstance)
assert.Empty(t, openObligations(t, svc))
})
}
}
// providerCallCounter fails the test's expectations if the request path makes
// any provider call: an unpublish waits on a node that may never answer.
type providerCallCounter struct {
ebsprovider.EBSProvider
mu sync.Mutex
unpublish int
publish int
}
func (p *providerCallCounter) UnpublishVolume(ctx context.Context, req ebsprovider.UnpublishVolumeRequest) (*ebsprovider.UnpublishedVolume, error) {
p.mu.Lock()
p.unpublish++
p.mu.Unlock()
<-ctx.Done()
return nil, ctx.Err()
}
func (p *providerCallCounter) PublishVolume(ctx context.Context, req ebsprovider.PublishVolumeRequest) (*ebsprovider.PublishedVolume, error) {
p.mu.Lock()
p.publish++
p.mu.Unlock()
<-ctx.Done()
return nil, ctx.Err()
}
// TestS8_ForceDetachRecordsTheObligationAndReturns pins that a force detach
// never waits on the node holding the export: it decides its errors, records
// the obligation and returns, and the unpublish is left to that node's reaper.
func TestS8_ForceDetachRecordsTheObligationAndReturns(t *testing.T) {
svc := newTestVolumeService("ap-southeast-2a")
provider := &providerCallCounter{EBSProvider: ebsprovider.NewMemoryProvider(ebsprovider.Capabilities{})}
svc.SetEBSProvider(provider)
wireObligations(t, svc, &fakeNodeLocator{nodes: map[string]string{"i-wedged": "node-unreachable"}}, "node-unreachable")
putAttachment(t, svc, "vol-s8-forcedetach", "in-use", "i-wedged", time.Unix(1_700_000_000, 0).UTC())
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
attachment, err := svc.ForceDetachVolume(ctx, &ec2.DetachVolumeInput{
VolumeId: aws.String("vol-s8-forcedetach"), Force: aws.Bool(true),
}, testVolAccountID)
require.NoError(t, err)
require.NoError(t, ctx.Err(), "the request returned on its own, not on the deadline")
assert.Equal(t, "detaching", aws.StringValue(attachment.State))
provider.mu.Lock()
assert.Zero(t, provider.unpublish, "the request path makes no unpublish call")
assert.Zero(t, provider.publish)
provider.mu.Unlock()
open := openObligations(t, svc)
require.Len(t, open, 1, "the unpublish the request did not wait for is recorded")
assert.Equal(t, "node-unreachable", open[0].NodeID)
assert.Empty(t, readVolume(t, svc, "vol-s8-forcedetach").AttachedInstance)
}
// --- Terminate paths ---
func TestTerminate_OwedUnpublishRecordsOneObligationBeforeTheClear(t *testing.T) {
for name, terminate := range map[string]func(*VolumeServiceImpl, string, ec2instance.OwedUnpublish) error{
"detach": func(svc *VolumeServiceImpl, id string, owed ec2instance.OwedUnpublish) error {
return svc.DetachVolumeOnTerminate(context.Background(), id, testVolAccountID, owed)
},
"delete": func(svc *VolumeServiceImpl, id string, owed ec2instance.OwedUnpublish) error {
return svc.DeleteVolumeOnTerminate(context.Background(), id, testVolAccountID, owed)
},
} {
t.Run(name, func(t *testing.T) {
svc := newTestVolumeService("ap-southeast-2a")
svc.snapshotKV = setupTestVolumeKV(t)
wireObligations(t, svc, &fakeNodeLocator{}, "node-a")
seedTestProviderVolume(t, svc, "vol-term-owed", 1)
putAttachment(t, svc, "vol-term-owed", "in-use", "i-term", time.Unix(1_700_000_000, 0).UTC())
require.NoError(t, terminate(svc, "vol-term-owed", ec2instance.OwedUnpublish{NodeID: "node-a", InstanceID: "i-term"}))
open := openObligations(t, svc)
require.Len(t, open, 1)
assert.Equal(t, "node-a", open[0].NodeID)
assert.Equal(t, "i-term", open[0].InstanceID)
})
}
}
// A terminate whose unmount completed owes nothing, and an attachment that is
// already some other instance's is not this terminate's to undo.
func TestTerminate_NothingOwedRecordsNoObligation(t *testing.T) {
for name, tc := range map[string]struct {
attachedTo string
owed ec2instance.OwedUnpublish
}{
"unmount completed": {attachedTo: "i-term", owed: ec2instance.OwedUnpublish{}},
"attached to another": {attachedTo: "i-other", owed: ec2instance.OwedUnpublish{NodeID: "node-a", InstanceID: "i-term"}},
} {
t.Run(name, func(t *testing.T) {
svc := newTestVolumeService("ap-southeast-2a")
wireObligations(t, svc, &fakeNodeLocator{}, "node-a")
putAttachment(t, svc, "vol-term-none", "in-use", tc.attachedTo, time.Unix(1_700_000_000, 0).UTC())
require.NoError(t, svc.DetachVolumeOnTerminate(context.Background(), "vol-term-none", testVolAccountID, tc.owed))
assert.Empty(t, openObligations(t, svc))
})
}
}
func TestTerminate_ObligationWriteFailureClearsNothing(t *testing.T) {
svc := newTestVolumeService("ap-southeast-2a")
nc := wireObligations(t, svc, &fakeNodeLocator{}, "node-a")
putAttachment(t, svc, "vol-term-nowrite", "in-use", "i-term", time.Unix(1_700_000_000, 0).UTC())
nc.Close()
err := svc.DetachVolumeOnTerminate(context.Background(), "vol-term-nowrite", testVolAccountID,
ec2instance.OwedUnpublish{NodeID: "node-a", InstanceID: "i-term"})
require.Error(t, err)
assert.Equal(t, "i-term", readVolume(t, svc, "vol-term-nowrite").AttachedInstance)
}
// --- The reaper ---
func recordObligation(t *testing.T, svc *VolumeServiceImpl, node, volumeID, instanceID string, attachedAt time.Time) ebsobligation.UnpublishObligation {
t.Helper()
o := ebsobligation.UnpublishObligation{
VolumeID: volumeID, AccountID: testVolAccountID, Handle: "handle-" + volumeID,
NodeID: node, InstanceID: instanceID, AttachedAt: attachedAt, RecordedAt: attachedAt.Add(time.Minute),
}
require.NoError(t, svc.obligations.Record(context.Background(), o))
return o
}
func sweepObligations(t *testing.T, svc *VolumeServiceImpl, node string, unpublisher ObligationUnpublisher) int {
t.Helper()
closed, err := svc.NewUnpublishObligationReaper(node, unpublisher).Sweep(context.Background())
require.NoError(t, err)
return closed
}
// TestS7_ObligationReaperNeverUnpublishesACurrentAttachment pins that the
// reaper, which runs with no customer asking, never tears down an export a
// live attachment uses and never removes or rewrites the volume.
func TestS7_ObligationReaperNeverUnpublishesACurrentAttachment(t *testing.T) {
cleared := time.Unix(1_700_000_000, 0).UTC()
for name, tc := range map[string]struct {
state string
attachedTo string
attachedAt time.Time
publishedAt time.Time // a publish this process made, zero if none
stillOpen bool
}{
// The volume moved to another instance, possibly on this very node.
"attached to another instance": {state: "in-use", attachedTo: "i-new", attachedAt: cleared.Add(time.Hour)},
// The same instance was attached again after the clear.
"same instance attached again": {state: "in-use", attachedTo: "i-old", attachedAt: cleared.Add(time.Hour)},
// The clear has not landed: the obligation was written first.
"attachment not yet cleared": {state: "in-use", attachedTo: "i-old", attachedAt: cleared, stillOpen: true},
// A hot attach published here and has not yet written in-use.
"newer publish here, record not yet in-use": {state: "available", publishedAt: cleared.Add(time.Hour), stillOpen: true},
// Force detach, then attach again on this node: the new publish is here.
"attached again here after a newer publish": {state: "in-use", attachedTo: "i-new", attachedAt: cleared.Add(time.Hour), publishedAt: cleared.Add(time.Hour)},
"same instance attached again here": {state: "in-use", attachedTo: "i-old", attachedAt: cleared.Add(time.Hour), publishedAt: cleared.Add(time.Hour)},
} {
t.Run(name, func(t *testing.T) {
svc := newTestVolumeService("ap-southeast-2a")
wireObligations(t, svc, &fakeNodeLocator{}, "node-a")
before := putAttachment(t, svc, "vol-s7-current", tc.state, tc.attachedTo, tc.attachedAt)
recordObligation(t, svc, "node-a", "vol-s7-current", "i-old", cleared)
unpublisher := &fakeUnpublisher{publishedAt: map[string]time.Time{}}
if !tc.publishedAt.IsZero() {
unpublisher.publishedAt["vol-s7-current"] = tc.publishedAt
}
sweepObligations(t, svc, "node-a", unpublisher)
assert.Empty(t, unpublisher.unpublishes(), "a current attachment's export must never be unpublished")
assert.Zero(t, unpublisher.held, "every hold is released")
after := readVolume(t, svc, "vol-s7-current")
assert.Equal(t, before.State, after.State, "the reaper never rewrites the volume")
assert.Equal(t, before.AttachedInstance, after.AttachedInstance)
if tc.stillOpen {
assert.Len(t, openObligations(t, svc), 1, "the obligation waits for its attachment to be gone")
} else {
assert.Empty(t, openObligations(t, svc), "a superseded obligation closes without a provider call")
}
})
}
}
// TestS7_ObligationReaperSealsLiveDataAndDropsOnlyDeletingVolumes pins that
// the reaper keeps a surviving volume's writes and only drops them unsealed
// for a volume that is being deleted or is gone.
func TestS7_ObligationReaperSealsLiveDataAndDropsOnlyDeletingVolumes(t *testing.T) {
cleared := time.Unix(1_700_000_000, 0).UTC()
for name, tc := range map[string]struct {
state string // empty for no document
noSeal bool
}{
"available keeps its writes": {state: "available", noSeal: false},
"deleting is dropped": {state: volumeStateDeleting, noSeal: true},
"absent is dropped": {state: "", noSeal: true},
} {
t.Run(name, func(t *testing.T) {
svc := newTestVolumeService("ap-southeast-2a")
wireObligations(t, svc, &fakeNodeLocator{}, "node-a")
if tc.state != "" {
putAttachment(t, svc, "vol-s7-settle", tc.state, "", time.Time{})
}
recordObligation(t, svc, "node-a", "vol-s7-settle", "i-old", cleared)
unpublisher := &fakeUnpublisher{}
assert.Equal(t, 1, sweepObligations(t, svc, "node-a", unpublisher))
assert.Equal(t, []unpublishCall{{volumeID: "vol-s7-settle", accountID: testVolAccountID, handle: "handle-vol-s7-settle", noSeal: tc.noSeal}},
unpublisher.unpublishes())
assert.Empty(t, openObligations(t, svc))
if tc.state != "" {
assert.Equal(t, tc.state, readVolume(t, svc, "vol-s7-settle").State, "the reaper never rewrites the volume")
}
})
}
}
// TestS7_ObligationReaperNeverTreatsAnUnreadableVolumeAsGone pins that a read
// failing for a reason other than absence leaves the obligation open: taking
// it for a deleted volume would drop live writes unsealed.
func TestS7_ObligationReaperNeverTreatsAnUnreadableVolumeAsGone(t *testing.T) {
memory := objectstore.NewMemoryObjectStore()
svc := newTestVolumeServiceWithStore("ap-southeast-2a", memory)
wireObligations(t, svc, &fakeNodeLocator{}, "node-a")
putAttachment(t, svc, "vol-s7-unread", "available", "", time.Time{})
recordObligation(t, svc, "node-a", "vol-s7-unread", "i-old", time.Unix(1_700_000_000, 0).UTC())
failing := &getFailingStore{MemoryObjectStore: memory, failKey: metadataKey(t, "vol-s7-unread")}
svc.metadata = ebsmetadata.NewStore(failing, "test-bucket")
unpublisher := &fakeUnpublisher{}
assert.Zero(t, sweepObligations(t, svc, "node-a", unpublisher))
assert.Empty(t, unpublisher.unpublishes())
open := openObligations(t, svc)
require.Len(t, open, 1)
assert.Equal(t, 1, open[0].Attempts, "the failure is counted where an operator can see it")
assert.Contains(t, open[0].Error, "reconstruction failed")
}
// End to end through the service: a force detach followed by a new attachment
// of the same volume on the same node must leave that new export alone.
func TestS7_ForceDetachThenReattachOnTheSameNodeKeepsTheNewExport(t *testing.T) {
svc := newTestVolumeService("ap-southeast-2a")
wireObligations(t, svc, &fakeNodeLocator{nodes: map[string]string{"i-old": "node-a", "i-new": "node-a"}}, "node-a")
putAttachment(t, svc, "vol-s7-reattach", "in-use", "i-old", time.Now().Add(-time.Hour).UTC())
_, err := forceDetach(t, svc, "vol-s7-reattach")
require.NoError(t, err)
require.NoError(t, svc.UpdateVolumeState(testVolAccountID, "vol-s7-reattach", "in-use", "i-new", "/dev/sdg"))
unpublisher := &fakeUnpublisher{}
assert.Equal(t, 1, sweepObligations(t, svc, "node-a", unpublisher))
assert.Empty(t, unpublisher.unpublishes(), "the new attachment's export must survive the old obligation")
assert.Equal(t, "i-new", readVolume(t, svc, "vol-s7-reattach").AttachedInstance)
assert.Empty(t, openObligations(t, svc))
}
func TestObligationReaper_SettlesOnlyItsOwnNode(t *testing.T) {
svc := newTestVolumeService("ap-southeast-2a")
wireObligations(t, svc, &fakeNodeLocator{}, "node-a", "node-b")
putAttachment(t, svc, "vol-own", "available", "", time.Time{})
recordObligation(t, svc, "node-b", "vol-own", "i-old", time.Unix(1_700_000_000, 0).UTC())
unpublisher := &fakeUnpublisher{}
assert.Zero(t, sweepObligations(t, svc, "node-a", unpublisher))
assert.Empty(t, unpublisher.unpublishes(), "only the node holding the export can tear it down")
assert.Len(t, openObligations(t, svc), 1)
}
// A publish or unpublish of the volume in flight on this node holds its lock;
// the sweep skips it rather than waiting behind a possibly wedged call.
func TestObligationReaper_SkipsABusyVolume(t *testing.T) {
svc := newTestVolumeService("ap-southeast-2a")
wireObligations(t, svc, &fakeNodeLocator{}, "node-a")
putAttachment(t, svc, "vol-busy", "available", "", time.Time{})
recordObligation(t, svc, "node-a", "vol-busy", "i-old", time.Unix(1_700_000_000, 0).UTC())
unpublisher := &fakeUnpublisher{busy: map[string]bool{"vol-busy": true}}
assert.Zero(t, sweepObligations(t, svc, "node-a", unpublisher))
assert.Empty(t, unpublisher.unpublishes())
open := openObligations(t, svc)
require.Len(t, open, 1)
assert.Zero(t, open[0].Attempts, "a busy volume is not a failure")
}
func TestObligationReaper_FailedUnpublishStaysOpenAndCounts(t *testing.T) {
svc := newTestVolumeService("ap-southeast-2a")
wireObligations(t, svc, &fakeNodeLocator{}, "node-a")
putAttachment(t, svc, "vol-fail", "available", "", time.Time{})
recordObligation(t, svc, "node-a", "vol-fail", "i-old", time.Unix(1_700_000_000, 0).UTC())
unpublisher := &fakeUnpublisher{err: errors.New("seal refused")}
assert.Zero(t, sweepObligations(t, svc, "node-a", unpublisher))
assert.Zero(t, sweepObligations(t, svc, "node-a", unpublisher))
open := openObligations(t, svc)
require.Len(t, open, 1)
assert.Equal(t, 2, open[0].Attempts)
assert.Equal(t, "seal refused", open[0].Error)
assert.Zero(t, unpublisher.held, "a failed unpublish still releases the hold")
}
// An obligation with no attachment time falls back to when it was recorded, so
// a publish here after the clear still counts as newer.
func TestObligationSince_FallsBackToRecordedAt(t *testing.T) {
recorded := time.Unix(1_700_000_000, 0)
assert.Equal(t, recorded, obligationSince(ebsobligation.UnpublishObligation{RecordedAt: recorded}))
attached := recorded.Add(-time.Hour)
assert.Equal(t, attached, obligationSince(ebsobligation.UnpublishObligation{AttachedAt: attached, RecordedAt: recorded}))
}
// --- The cluster-wide close ---
func TestObligationCloser_ClosesOnlyDepartedNodes(t *testing.T) {
svc := newTestVolumeService("ap-southeast-2a")
wireObligations(t, svc, &fakeNodeLocator{}, "node-a")
at := time.Unix(1_700_000_000, 0).UTC()
recordObligation(t, svc, "node-gone", "vol-c1", "i-1", at)
recordObligation(t, svc, "node-gone", "vol-c2", "i-2", at)
recordObligation(t, svc, "node-down", "vol-c3", "i-3", at)
recordObligation(t, svc, "node-unknown", "vol-c4", "i-4", at)
asked := map[string]int{}
departed := func(_ context.Context, node string) (bool, error) {
asked[node]++
switch node {
case "node-gone":
return true, nil
case "node-unknown":
return false, errors.New("heartbeat bucket unreachable")
default:
return false, nil
}
}
closed, err := svc.NewUnpublishObligationCloser(departed).Sweep(context.Background())
require.NoError(t, err)
assert.Equal(t, 2, closed)
remaining := []string{}
for _, o := range openObligations(t, svc) {
remaining = append(remaining, o.NodeID)
}
assert.ElementsMatch(t, []string{"node-down", "node-unknown"}, remaining,
"a node that is down, or whose membership cannot be read, keeps its obligations")
assert.Equal(t, 1, asked["node-gone"], "membership is asked once per node per sweep")
}
@@ -33,19 +33,19 @@ func BatchDeleteImage(ctx context.Context, deleter ImageDeleter, accountID strin
if err := ValidateRepositoryScope(req.RepositoryName, req.RegistryID, accountID); err != nil {
return nil, err
}
if len(req.ImageIDs) > maxImageBatch {
return nil, MaxItemsError("imageIds", maxImageBatch)
if err := validateImageIDs(req.ImageIDs, true); err != nil {
return nil, err
}
deleted := make([]*ecr.ImageIdentifier, 0, len(req.ImageIDs))
failures := make([]*ecr.ImageFailure, 0)
for _, imageID := range req.ImageIDs {
if imageID.ImageDigest == "" && imageID.ImageTag == "" {
if imageID.ImageDigest == "" && imageID.ImageTag == nil {
failures = append(failures, imageFailure(imageID, ecr.ImageFailureCodeMissingDigestAndTag, "no digest or tag specified"))
continue
}
digest, err := deleter.DeleteImage(ctx, accountID, req.RepositoryName, imageID.ImageTag, imageID.ImageDigest)
digest, err := deleter.DeleteImage(ctx, accountID, req.RepositoryName, imageID.tag(), imageID.ImageDigest)
if errors.Is(err, ecrregistry.ErrImageNotFound) {
failures = append(failures, imageFailure(imageID, ecr.ImageFailureCodeImageNotFound, ImageNotFoundReason))
continue
@@ -56,8 +56,8 @@ func BatchDeleteImage(ctx context.Context, deleter ImageDeleter, accountID strin
}
deletedImage := &ecr.ImageIdentifier{ImageDigest: aws.String(digest)}
if imageID.ImageTag != "" {
deletedImage.ImageTag = aws.String(imageID.ImageTag)
if imageID.ImageTag != nil {
deletedImage.ImageTag = imageID.ImageTag
}
deleted = append(deleted, deletedImage)
}
+12 -7
View File
@@ -36,8 +36,13 @@ func BatchGetImage(ctx context.Context, reader ManifestReader, accountID string,
if err := ValidateRepositoryScope(req.RepositoryName, req.RegistryID, accountID); err != nil {
return nil, err
}
if len(req.ImageIDs) > maxImageBatch {
return nil, MaxItemsError("imageIds", maxImageBatch)
if err := validateImageIDs(req.ImageIDs, true); err != nil {
return nil, err
}
if req.AcceptedMediaTypes != nil {
if err := validateListLength("acceptedMediaTypes", len(req.AcceptedMediaTypes), 1, maxMediaTypeListItems); err != nil {
return nil, err
}
}
images := make([]*ecr.Image, 0, len(req.ImageIDs))
@@ -45,7 +50,7 @@ func BatchGetImage(ctx context.Context, reader ManifestReader, accountID string,
for _, imageID := range req.ImageIDs {
reference := imageID.ImageDigest
if reference == "" {
reference = imageID.ImageTag
reference = imageID.tag()
}
if reference == "" {
failures = append(failures, imageFailure(imageID, ecr.ImageFailureCodeMissingDigestAndTag, "no digest or tag specified"))
@@ -69,8 +74,8 @@ func BatchGetImage(ctx context.Context, reader ManifestReader, accountID string,
ImageManifest: aws.String(string(manifest)),
ImageManifestMediaType: aws.String(mediaType),
}
if imageID.ImageTag != "" {
image.ImageId.ImageTag = aws.String(imageID.ImageTag)
if imageID.ImageTag != nil {
image.ImageId.ImageTag = imageID.ImageTag
}
images = append(images, image)
}
@@ -87,8 +92,8 @@ func imageFailure(imageID imageIdentifier, code, reason string) *ecr.ImageFailur
if imageID.ImageDigest != "" {
failure.ImageId.ImageDigest = aws.String(imageID.ImageDigest)
}
if imageID.ImageTag != "" {
failure.ImageId.ImageTag = aws.String(imageID.ImageTag)
if imageID.ImageTag != nil {
failure.ImageId.ImageTag = imageID.ImageTag
}
return failure
}
+13 -5
View File
@@ -10,10 +10,15 @@ import (
ecrregistry "github.com/mulgadc/spinifex/spinifex/domains/ecr/registry"
)
// imageIdentifier is the AWS JSON 1.1 {imageDigest, imageTag} pair.
// imageIdentifier is the AWS JSON 1.1 {imageDigest, imageTag} pair. ImageTag
// is a pointer so an empty tag can be told apart from an omitted one.
type imageIdentifier struct {
ImageDigest string `json:"imageDigest"`
ImageTag string `json:"imageTag"`
ImageDigest string `json:"imageDigest"`
ImageTag *string `json:"imageTag"`
}
func (id imageIdentifier) tag() string {
return aws.StringValue(id.ImageTag)
}
type describeImagesRequest struct {
@@ -36,6 +41,9 @@ func DescribeImages(ctx context.Context, catalog ImageCatalog, accountID string,
if err := ValidateRepositoryScope(req.RepositoryName, req.RegistryID, accountID); err != nil {
return nil, err
}
if err := validateImageIDs(req.ImageIDs, false); err != nil {
return nil, err
}
if err := req.Filter.validate(); err != nil {
return nil, err
}
@@ -53,7 +61,7 @@ func DescribeImages(ctx context.Context, catalog ImageCatalog, accountID string,
if image.ImageDigest != "" && image.ImageDigest == record.Digest {
return true
}
if image.ImageTag != "" && slices.Contains(record.Tags, image.ImageTag) {
if image.ImageTag != nil && slices.Contains(record.Tags, *image.ImageTag) {
return true
}
}
@@ -82,7 +90,7 @@ func DescribeImages(ctx context.Context, catalog ImageCatalog, accountID string,
}
if len(req.ImageIDs) > 0 && len(details) == 0 {
first := req.ImageIDs[0]
return nil, ImageNotFoundError(accountID, req.RepositoryName, first.ImageDigest, first.ImageTag)
return nil, ImageNotFoundError(accountID, req.RepositoryName, first.ImageDigest, first.tag())
}
return &ecr.DescribeImagesOutput{ImageDetails: details}, nil
@@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"errors"
"fmt"
"log/slog"
"github.com/aws/aws-sdk-go/service/ecr"
@@ -40,6 +41,13 @@ func DescribeRepositories(ctx context.Context, store RepositoryStore, endpoint R
return nil, errors.New(awserrors.ErrorAccessDenied)
}
switch {
case req.RepositoryNames != nil && len(req.RepositoryNames) == 0:
return nil, ConstraintError("RepositoryNameList", "Must not be empty")
case len(req.RepositoryNames) > maxRepositoryNameList:
return nil, ConstraintError("RepositoryNameList", fmt.Sprintf("Must have length less than or equal to %d", maxRepositoryNameList))
}
names := req.RepositoryNames
if len(names) == 0 {
var err error
@@ -0,0 +1,92 @@
package awsapi
import (
"context"
"strings"
"testing"
"github.com/mulgadc/spinifex/spinifex/domains/ecr"
"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// The messages below are AWS's, captured from real ECR refusals.
const (
awsImageIDsNull = "Invalid parameter at 'imageIds' failed to satisfy constraint: 'Member must not be null'"
awsImageIDsEmpty = "Invalid parameter at 'imageIds' failed to satisfy constraint: 'Member must have length greater than or equal to 1'"
awsImageIDsTooMany = "Invalid parameter at 'imageIds' failed to satisfy constraint: 'Member must have length less than or equal to 100'"
awsImageTagEmpty = "Invalid parameter at 'imageIds.2.member.imageTag' failed to satisfy constraint: 'Member must have length greater than or equal to 1'"
awsImageTagTooLong = "Invalid parameter at 'imageIds.1.member.imageTag' failed to satisfy constraint: 'Member must have length less than or equal to 300'"
awsMediaTypesEmpty = "Invalid parameter at 'acceptedMediaTypes' failed to satisfy constraint: 'Member must have length greater than or equal to 1'"
awsMediaTypesTooMny = "Invalid parameter at 'acceptedMediaTypes' failed to satisfy constraint: 'Member must have length less than or equal to 100'"
)
func requireECRInvalidParameter(t *testing.T, err error, want string) {
t.Helper()
require.Error(t, err)
code, message, ok := awserrors.ResolveErrorDetail(err)
require.True(t, ok)
assert.Equal(t, awserrors.ErrorECRInvalidParameter, code)
assert.Equal(t, want, message)
}
func imageIDCases() map[string]struct{ ids, want string } {
long := strings.Repeat("a", 301)
return map[string]struct{ ids, want string }{
"omitted": {"", awsImageIDsNull},
"empty": {`,"imageIds":[]`, awsImageIDsEmpty},
"too many": {`,"imageIds":[` + repeatImageIdentifier(101) + `]`, awsImageIDsTooMany},
"empty tag": {`,"imageIds":[{"imageTag":"v1"},{"imageDigest":"sha256:x","imageTag":""}]`, awsImageTagEmpty},
"tag too long": {`,"imageIds":[{"imageTag":"` + long + `"}]`, awsImageTagTooLong},
}
}
func TestBatchImageActions_RefuseImageIDsOutsideModel(t *testing.T) {
for name, tc := range imageIDCases() {
body := []byte(`{"repositoryName":"team/app"` + tc.ids + `}`)
t.Run("BatchGetImage "+name, func(t *testing.T) {
_, err := BatchGetImage(context.Background(), fakeManifestReader{}, "123456789012", body)
requireECRInvalidParameter(t, err, tc.want)
})
t.Run("BatchDeleteImage "+name, func(t *testing.T) {
_, err := BatchDeleteImage(context.Background(), fakeImageDeleter{}, "123456789012", body)
requireECRInvalidParameter(t, err, tc.want)
})
}
}
func TestBatchGetImage_RefusesAcceptedMediaTypesOutsideModel(t *testing.T) {
many := strings.TrimSuffix(strings.Repeat(`"m",`, 101), ",")
for want, types := range map[string]string{awsMediaTypesEmpty: `[]`, awsMediaTypesTooMny: `[` + many + `]`} {
body := []byte(`{"repositoryName":"team/app","imageIds":[{"imageTag":"v1"}],"acceptedMediaTypes":` + types + `}`)
_, err := BatchGetImage(context.Background(), fakeManifestReader{}, "123456789012", body)
requireECRInvalidParameter(t, err, want)
}
}
// DescribeImages makes imageIds optional, so only an omitted list lists all.
func TestDescribeImages_RefusesImageIDsOutsideModel(t *testing.T) {
for name, tc := range imageIDCases() {
if tc.want == awsImageIDsNull {
continue
}
t.Run(name, func(t *testing.T) {
_, err := DescribeImages(context.Background(), nil, "123456789012", []byte(`{"repositoryName":"team/app"`+tc.ids+`}`))
requireECRInvalidParameter(t, err, tc.want)
})
}
}
func TestDescribeRepositories_RefusesRepositoryNamesOutsideModel(t *testing.T) {
store := ecr.NewNATSMetaStore(newRepositoryActionTestConn(t))
many := strings.TrimSuffix(strings.Repeat(`"team/app",`, 101), ",")
cases := map[string]string{
`[]`: "Invalid parameter at 'RepositoryNameList' failed to satisfy constraint: 'Must not be empty'",
`[` + many + `]`: "Invalid parameter at 'RepositoryNameList' failed to satisfy constraint: 'Must have length less than or equal to 100'",
}
for names, want := range cases {
_, err := DescribeRepositories(context.Background(), store, repositoryActionEndpoint(), repositoryActionTestAccount, []byte(`{"repositoryNames":`+names+`}`))
requireECRInvalidParameter(t, err, want)
}
}
+41
View File
@@ -18,6 +18,11 @@ const (
minLifecyclePolicyTextLength = 100
maxLifecyclePolicyTextLength = 30720
maxRepositoryPolicyTextLength = 10240
minImageTagLength = 1
maxImageTagLength = 300
maxMediaTypeListItems = 100
maxRepositoryNameList = 100
)
// ValidateRepositoryName returns InvalidParameterException naming the first
@@ -52,6 +57,42 @@ func MaxItemsError(field string, limit int) error {
return ConstraintError(field, fmt.Sprintf("Member must have length less than or equal to %d", limit))
}
// validateListLength refuses a list field whose member count falls outside
// [minLen, maxLen], the bounds of field's model shape.
func validateListLength(field string, n, minLen, maxLen int) error {
if n < minLen {
return ConstraintError(field, fmt.Sprintf("Member must have length greater than or equal to %d", minLen))
}
if n > maxLen {
return MaxItemsError(field, maxLen)
}
return nil
}
// validateImageIDs checks imageIds against ImageIdentifierList: 1-100 members,
// each tag 1-300 characters. An omitted list is refused only when required.
func validateImageIDs(ids []imageIdentifier, required bool) error {
if ids == nil {
if required {
return RequiredParameterError("imageIds")
}
return nil
}
if err := validateListLength("imageIds", len(ids), 1, maxImageBatch); err != nil {
return err
}
for i, id := range ids {
if id.ImageTag == nil {
continue
}
field := fmt.Sprintf("imageIds.%d.member.imageTag", i+1)
if err := ValidateTextLength(field, *id.ImageTag, minImageTagLength, maxImageTagLength); err != nil {
return err
}
}
return nil
}
// ValidateTextLength refuses text whose length in characters falls outside
// [minLen, maxLen], the bounds of field's model shape.
func ValidateTextLength(field, text string, minLen, maxLen int) error {
+27
View File
@@ -76,6 +76,9 @@ func DescribeClusters(ctx context.Context, nc *nats.Conn, accountID string, body
if err := envelope.UnmarshalJSONBody(body, input); err != nil {
return nil, err
}
if err := checkClustersInclude(input.Include); err != nil {
return nil, err
}
return ecsdomain.NewNATSECSService(nc).DescribeClusters(ctx, input, accountID)
}
@@ -134,6 +137,9 @@ func ListTaskDefinitions(ctx context.Context, nc *nats.Conn, accountID string, b
if err := envelope.UnmarshalJSONBody(body, input); err != nil {
return nil, err
}
if err := checkListTaskDefinitions(input); err != nil {
return nil, err
}
return ecsdomain.NewNATSECSService(nc).ListTaskDefinitions(ctx, input, accountID)
}
@@ -190,6 +196,9 @@ func DescribeContainerInstances(ctx context.Context, nc *nats.Conn, accountID st
if err := envelope.UnmarshalJSONBody(body, input); err != nil {
return nil, err
}
if err := checkContainerInstancesInclude(input.Include); err != nil {
return nil, err
}
return ecsdomain.NewNATSECSService(nc).DescribeContainerInstances(ctx, input, accountID)
}
@@ -313,6 +322,9 @@ func DescribeTasks(ctx context.Context, nc *nats.Conn, accountID string, body []
if err := envelope.UnmarshalJSONBody(body, input); err != nil {
return nil, err
}
if err := checkTasksInclude(input.Include); err != nil {
return nil, err
}
return ecsdomain.NewNATSECSService(nc).DescribeTasks(ctx, input, accountID)
}
@@ -323,6 +335,9 @@ func ListTasks(ctx context.Context, nc *nats.Conn, accountID string, body []byte
if err := envelope.UnmarshalJSONBody(body, input); err != nil {
return nil, err
}
if err := checkListTasks(input); err != nil {
return nil, err
}
return ecsdomain.NewNATSECSService(nc).ListTasks(ctx, input, accountID)
}
@@ -391,6 +406,9 @@ func DescribeServices(ctx context.Context, nc *nats.Conn, accountID string, body
if err := envelope.UnmarshalJSONBody(body, input); err != nil {
return nil, err
}
if err := checkServicesInclude(input.Include); err != nil {
return nil, err
}
return ecsdomain.NewNATSECSService(nc).DescribeServices(ctx, input, accountID)
}
@@ -401,6 +419,9 @@ func ListServices(ctx context.Context, nc *nats.Conn, accountID string, body []b
if err := envelope.UnmarshalJSONBody(body, input); err != nil {
return nil, err
}
if err := checkListServices(body, input); err != nil {
return nil, err
}
return ecsdomain.NewNATSECSService(nc).ListServices(ctx, input, accountID)
}
@@ -493,6 +514,9 @@ func CreateCapacityProvider(ctx context.Context, nc *nats.Conn, accountID string
if err := envelope.UnmarshalJSONBody(body, input); err != nil {
return nil, err
}
if err := checkAutoScalingGroupProvider(input.AutoScalingGroupProvider); err != nil {
return nil, err
}
return ecsdomain.NewNATSECSService(nc).CreateCapacityProvider(ctx, input, accountID)
}
@@ -504,6 +528,9 @@ func DescribeCapacityProviders(ctx context.Context, nc *nats.Conn, accountID str
if err := envelope.UnmarshalJSONBody(body, input); err != nil {
return nil, err
}
if err := checkCapacityProvidersInclude(input.Include); err != nil {
return nil, err
}
return ecsdomain.NewNATSECSService(nc).DescribeCapacityProviders(ctx, input, accountID)
}
@@ -0,0 +1,59 @@
package awsapi
import (
"github.com/aws/aws-sdk-go/service/ecs"
"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
)
// Messages AWS answers, as ClientException, for each autoScalingGroupProvider
// break, before it looks up the Auto Scaling group.
const (
msgManagedDraining = "The specified managed draining setting is invalid. Specify ENABLED or DISABLED and try again."
msgManagedTerminationProtection = "The specified managed termination protection setting is invalid. Specify ENABLED or DISABLED and try again."
msgManagedScalingStatus = "The specified managed scaling status setting is invalid. Specify ENABLED or DISABLED and try again."
msgTargetCapacity = "The managed scaling target capacity value is invalid. Specify an integer greater than 0 and less than or equal to 100 and try again."
msgMinimumScalingStepSize = "The specified minimum scaling step size is invalid. Specify an integer between 1 and 10000 and try again."
msgMaximumScalingStepSize = "The specified maximum scaling step size is invalid. Specify an integer between 1 and 10000 and try again."
msgInstanceWarmupPeriod = "The managed scaling instance warmup period value is invalid. Specify an integer greater than or equal to 0 and less than or equal to 9999 and try again."
)
func validEnabledDisabled(v *string) bool {
return v == nil || *v == "ENABLED" || *v == "DISABLED"
}
func validRange(v *int64, lo, hi int64) bool {
return v == nil || (*v >= lo && *v <= hi)
}
// checkAutoScalingGroupProvider refuses an enum or range break inside
// autoScalingGroupProvider. AWS refuses an instanceWarmupPeriod of 10000 although
// the model allows it, so the upper bound here is 9999.
func checkAutoScalingGroupProvider(p *ecs.AutoScalingGroupProvider) error {
if p == nil {
return nil
}
fail := func(msg string) error { return awserrors.Errorf(awserrors.ErrorECSClientException, "%s", msg) }
if !validEnabledDisabled(p.ManagedDraining) {
return fail(msgManagedDraining)
}
if !validEnabledDisabled(p.ManagedTerminationProtection) {
return fail(msgManagedTerminationProtection)
}
ms := p.ManagedScaling
if ms == nil {
return nil
}
switch {
case !validEnabledDisabled(ms.Status):
return fail(msgManagedScalingStatus)
case !validRange(ms.TargetCapacity, 1, 100):
return fail(msgTargetCapacity)
case !validRange(ms.MinimumScalingStepSize, 1, 10000):
return fail(msgMinimumScalingStepSize)
case !validRange(ms.MaximumScalingStepSize, 1, 10000):
return fail(msgMaximumScalingStepSize)
case !validRange(ms.InstanceWarmupPeriod, 0, 9999):
return fail(msgInstanceWarmupPeriod)
}
return nil
}
@@ -0,0 +1,51 @@
package awsapi
import (
"context"
"testing"
"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// Each break answers AWS's ClientException and message before dispatch. A valid
// request reaches dispatch, which fails here only because nc is nil.
func TestCreateCapacityProviderAutoScalingGroupProvider(t *testing.T) {
body := func(asg string) []byte {
return []byte(`{"name":"cp","autoScalingGroupProvider":{"autoScalingGroupArn":"arn:aws:autoscaling:x"` + asg + `}}`)
}
refused := []struct {
name, asg, msg string
}{
{"managedDraining", `,"managedDraining":"BOGUS"`, msgManagedDraining},
{"managedTerminationProtection", `,"managedTerminationProtection":"enabled"`, msgManagedTerminationProtection},
{"status", `,"managedScaling":{"status":"BOGUS"}`, msgManagedScalingStatus},
{"targetCapacity/0", `,"managedScaling":{"targetCapacity":0}`, msgTargetCapacity},
{"targetCapacity/101", `,"managedScaling":{"targetCapacity":101}`, msgTargetCapacity},
{"minimumScalingStepSize/0", `,"managedScaling":{"minimumScalingStepSize":0}`, msgMinimumScalingStepSize},
{"maximumScalingStepSize/10001", `,"managedScaling":{"maximumScalingStepSize":10001}`, msgMaximumScalingStepSize},
{"instanceWarmupPeriod/-1", `,"managedScaling":{"instanceWarmupPeriod":-1}`, msgInstanceWarmupPeriod},
{"instanceWarmupPeriod/10000", `,"managedScaling":{"instanceWarmupPeriod":10000}`, msgInstanceWarmupPeriod},
}
for _, tc := range refused {
t.Run(tc.name, func(t *testing.T) {
_, err := CreateCapacityProvider(context.Background(), nil, "123456789012", body(tc.asg), nil)
code, msg, ok := awserrors.ResolveErrorDetail(err)
require.True(t, ok, "err = %v", err)
assert.Equal(t, awserrors.ErrorECSClientException, code)
assert.Equal(t, tc.msg, msg)
})
}
valid := []string{
``,
`,"managedDraining":"ENABLED","managedTerminationProtection":"DISABLED","managedScaling":{"status":"ENABLED","targetCapacity":1,"minimumScalingStepSize":1,"maximumScalingStepSize":10000,"instanceWarmupPeriod":0}`,
`,"managedDraining":"DISABLED","managedTerminationProtection":"ENABLED","managedScaling":{"status":"DISABLED","targetCapacity":100,"minimumScalingStepSize":10000,"maximumScalingStepSize":1,"instanceWarmupPeriod":9999}`,
}
for _, asg := range valid {
_, err := CreateCapacityProvider(context.Background(), nil, "123456789012", body(asg), nil)
_, _, refusedErr := awserrors.ResolveErrorDetail(err)
assert.False(t, refusedErr, "asg %q: err = %v", asg, err)
}
}
@@ -0,0 +1,68 @@
package awsapi
import (
"slices"
"strings"
"github.com/aws/aws-sdk-go/aws"
"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
)
// Include values each Describe* operation accepts, in the order AWS lists them
// in its refusal. Taken from the ECS model rather than the SDK's *_Values().
var (
clusterIncludeValues = []string{"ATTACHMENTS", "CONFIGURATIONS", "SETTINGS", "STATISTICS", "TAGS"}
tagsIncludeValues = []string{"TAGS"}
containerInstanceIncludeValues = []string{"TAGS", "CONTAINER_INSTANCE_HEALTH"}
)
// firstInvalidInclude returns the first include value outside allowed.
func firstInvalidInclude(include []*string, allowed []string) (string, bool) {
for _, v := range include {
if s := aws.StringValue(v); !slices.Contains(allowed, s) {
return s, true
}
}
return "", false
}
func includeOneOf(allowed []string) string {
return "[" + strings.Join(allowed, ",") + "]"
}
func checkClustersInclude(include []*string) error {
if _, bad := firstInvalidInclude(include, clusterIncludeValues); bad {
return awserrors.Errorf(awserrors.ErrorECSInvalidParameter, "include should be one of %s", includeOneOf(clusterIncludeValues))
}
return nil
}
func checkServicesInclude(include []*string) error {
if _, bad := firstInvalidInclude(include, tagsIncludeValues); bad {
return awserrors.Errorf(awserrors.ErrorECSInvalidParameter, "include should be one of %s", includeOneOf(tagsIncludeValues))
}
return nil
}
func checkTasksInclude(include []*string) error {
if _, bad := firstInvalidInclude(include, tagsIncludeValues); bad {
return awserrors.Errorf(awserrors.ErrorECSInvalidParameter, "include contains an invalid value, should be one of %s", includeOneOf(tagsIncludeValues))
}
return nil
}
func checkContainerInstancesInclude(include []*string) error {
if _, bad := firstInvalidInclude(include, containerInstanceIncludeValues); bad {
return awserrors.Errorf(awserrors.ErrorECSInvalidParameter, "include should be one of %s", includeOneOf(containerInstanceIncludeValues))
}
return nil
}
// checkCapacityProvidersInclude answers ClientException, as AWS does for this
// operation alone, naming the refused value.
func checkCapacityProvidersInclude(include []*string) error {
if v, bad := firstInvalidInclude(include, tagsIncludeValues); bad {
return awserrors.Errorf(awserrors.ErrorECSClientException, "%s is not a valid value for include. The value must be one of [%s]", v, includeOneOf(tagsIncludeValues))
}
return nil
}
@@ -0,0 +1,53 @@
package awsapi
import (
"context"
"testing"
"github.com/nats-io/nats.go"
"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
type describeHandler func(context.Context, *nats.Conn, string, []byte, PassRoleChecker) (any, error)
// A refused include answers before dispatch with AWS's code and message. A
// valid one reaches dispatch, which fails here only because nc is nil.
func TestDescribeInclude(t *testing.T) {
cases := []struct {
name string
handler describeHandler
valid []string
wantCode string
wantMsg string
}{
{"DescribeClusters", DescribeClusters, []string{"ATTACHMENTS", "CONFIGURATIONS", "SETTINGS", "STATISTICS", "TAGS"},
awserrors.ErrorECSInvalidParameter, "include should be one of [ATTACHMENTS,CONFIGURATIONS,SETTINGS,STATISTICS,TAGS]"},
{"DescribeServices", DescribeServices, []string{"TAGS"},
awserrors.ErrorECSInvalidParameter, "include should be one of [TAGS]"},
{"DescribeTasks", DescribeTasks, []string{"TAGS"},
awserrors.ErrorECSInvalidParameter, "include contains an invalid value, should be one of [TAGS]"},
{"DescribeContainerInstances", DescribeContainerInstances, []string{"TAGS", "CONTAINER_INSTANCE_HEALTH"},
awserrors.ErrorECSInvalidParameter, "include should be one of [TAGS,CONTAINER_INSTANCE_HEALTH]"},
{"DescribeCapacityProviders", DescribeCapacityProviders, []string{"TAGS"},
awserrors.ErrorECSClientException, "BOGUS is not a valid value for include. The value must be one of [[TAGS]]"},
}
for _, tc := range cases {
t.Run(tc.name+"/invalid", func(t *testing.T) {
_, err := tc.handler(context.Background(), nil, "123456789012", []byte(`{"include":["TAGS","BOGUS"]}`), nil)
code, msg, ok := awserrors.ResolveErrorDetail(err)
require.True(t, ok, "err = %v", err)
assert.Equal(t, tc.wantCode, code)
assert.Equal(t, tc.wantMsg, msg)
})
for _, v := range tc.valid {
t.Run(tc.name+"/"+v, func(t *testing.T) {
_, err := tc.handler(context.Background(), nil, "123456789012", []byte(`{"include":["`+v+`"]}`), nil)
_, _, refused := awserrors.ResolveErrorDetail(err)
assert.False(t, refused, "err = %v", err)
})
}
}
}
@@ -0,0 +1,62 @@
package awsapi
import (
"encoding/json"
"slices"
"strings"
"github.com/aws/aws-sdk-go/service/ecs"
"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
)
// Enum sets the List* operations accept, in the order AWS lists them in its
// refusal. launchType also accepts FMI, which AWS takes but the model omits.
var (
listLaunchTypes = []string{"EC2", "FARGATE", "EXTERNAL", "FMI", "MANAGED_INSTANCES"}
listSchedulingStrategies = []string{"REPLICA", "DAEMON"}
listResourceManagementTypes = []string{"CUSTOMER", "ECS"}
listSortOrders = []string{"ASC", "DESC"}
listTaskDefinitionStatuses = []string{"ACTIVE", "INACTIVE", "DELETE_IN_PROGRESS"}
listDesiredStatuses = []string{"RUNNING", "PENDING", "STOPPED"}
)
// checkListEnum refuses a set value outside allowed, naming the member as
// AWS's message does. An omitted value is left to the operation's default.
func checkListEnum(v *string, label string, allowed []string) error {
if v == nil || slices.Contains(allowed, *v) {
return nil
}
return awserrors.Errorf(awserrors.ErrorECSInvalidParameter, "%s should be one of [%s]", label, strings.Join(allowed, ","))
}
func checkListServices(body []byte, input *ecs.ListServicesInput) error {
if err := checkListEnum(input.LaunchType, "launch type", listLaunchTypes); err != nil {
return err
}
if err := checkListEnum(input.SchedulingStrategy, "Scheduling strategy", listSchedulingStrategies); err != nil {
return err
}
// The pinned SDK's ListServicesInput predates resourceManagementType, so it
// is read off the raw body; a body that fails to parse was already refused.
var raw struct {
ResourceManagementType *string `json:"resourceManagementType"`
}
if len(body) > 0 {
_ = json.Unmarshal(body, &raw)
}
return checkListEnum(raw.ResourceManagementType, "Resource management type", listResourceManagementTypes)
}
func checkListTaskDefinitions(input *ecs.ListTaskDefinitionsInput) error {
if err := checkListEnum(input.Sort, "Sort", listSortOrders); err != nil {
return err
}
return checkListEnum(input.Status, "Status", listTaskDefinitionStatuses)
}
func checkListTasks(input *ecs.ListTasksInput) error {
if err := checkListEnum(input.DesiredStatus, "Desired Status", listDesiredStatuses); err != nil {
return err
}
return checkListEnum(input.LaunchType, "launch type", listLaunchTypes)
}
@@ -0,0 +1,53 @@
package awsapi
import (
"context"
"testing"
"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// A value outside a List* enum is refused before dispatch with AWS's message.
// A valid one reaches dispatch, which fails here only because nc is nil.
func TestListEnums(t *testing.T) {
cases := []struct {
name string
handler describeHandler
member string
valid []string
wantMsg string
}{
{"ListServices", ListServices, "launchType", []string{"EC2", "FARGATE", "EXTERNAL", "FMI", "MANAGED_INSTANCES"},
"launch type should be one of [EC2,FARGATE,EXTERNAL,FMI,MANAGED_INSTANCES]"},
{"ListServices", ListServices, "schedulingStrategy", []string{"REPLICA", "DAEMON"},
"Scheduling strategy should be one of [REPLICA,DAEMON]"},
{"ListServices", ListServices, "resourceManagementType", []string{"CUSTOMER", "ECS"},
"Resource management type should be one of [CUSTOMER,ECS]"},
{"ListTaskDefinitions", ListTaskDefinitions, "sort", []string{"ASC", "DESC"},
"Sort should be one of [ASC,DESC]"},
{"ListTaskDefinitions", ListTaskDefinitions, "status", []string{"ACTIVE", "INACTIVE", "DELETE_IN_PROGRESS"},
"Status should be one of [ACTIVE,INACTIVE,DELETE_IN_PROGRESS]"},
{"ListTasks", ListTasks, "desiredStatus", []string{"RUNNING", "PENDING", "STOPPED"},
"Desired Status should be one of [RUNNING,PENDING,STOPPED]"},
{"ListTasks", ListTasks, "launchType", []string{"EC2", "FARGATE", "EXTERNAL", "FMI", "MANAGED_INSTANCES"},
"launch type should be one of [EC2,FARGATE,EXTERNAL,FMI,MANAGED_INSTANCES]"},
}
for _, tc := range cases {
t.Run(tc.name+"/"+tc.member+"/invalid", func(t *testing.T) {
_, err := tc.handler(context.Background(), nil, "123456789012", []byte(`{"`+tc.member+`":"BOGUS"}`), nil)
code, msg, ok := awserrors.ResolveErrorDetail(err)
require.True(t, ok, "err = %v", err)
assert.Equal(t, awserrors.ErrorECSInvalidParameter, code)
assert.Equal(t, tc.wantMsg, msg)
})
for _, v := range tc.valid {
t.Run(tc.name+"/"+tc.member+"/"+v, func(t *testing.T) {
_, err := tc.handler(context.Background(), nil, "123456789012", []byte(`{"`+tc.member+`":"`+v+`"}`), nil)
_, _, refused := awserrors.ResolveErrorDetail(err)
assert.False(t, refused, "err = %v", err)
})
}
}
}
@@ -32,8 +32,12 @@ type attrResource[R any] struct {
save func(*R) error
accountIDOf func(*R) string
defaults func(*R) map[string]string
attrsOf func(*R) map[string]string
setAttrs func(*R, map[string]string)
// accepts overrides the known-key check, which otherwise uses defaults.
accepts func(key string) bool
// unknownKey builds the error for a rejected key; nil means a bare ValidationError.
unknownKey func(key string) error
attrsOf func(*R) map[string]string
setAttrs func(*R, map[string]string)
// validate rejects a bad value for a known key before anything is stored.
validate func(key, value string) error
// afterChange runs once the changed keys are persisted; its error fails the call.
@@ -57,29 +61,36 @@ func (r attrResource[R]) load() (*R, error) {
// modifyResourceAttributes validates and merges submitted attributes, persisting only
// on change. rawCount is pre-nil-filter count, used to distinguish "nothing sent"
// from "everything sent was invalid".
func modifyResourceAttributes[R any](res attrResource[R], pairs []attrPair, rawCount int) ([]attrPair, error) {
// from "everything sent was invalid". It returns the submitted pairs and the full merged set.
func modifyResourceAttributes[R any](res attrResource[R], pairs []attrPair, rawCount int) (submitted, full []attrPair, err error) {
if res.arn == "" {
return nil, errors.New(awserrors.ErrorMissingParameter)
return nil, nil, errors.New(awserrors.ErrorMissingParameter)
}
rec, err := res.load()
if err != nil {
return nil, err
return nil, nil, err
}
known := res.defaults(rec)
accepts := res.accepts
if accepts == nil {
known := res.defaults(rec)
accepts = func(k string) bool { _, ok := known[k]; return ok }
}
attrs := res.attrsOf(rec)
submitted := make([]attrPair, 0, len(pairs))
submitted = make([]attrPair, 0, len(pairs))
var changed []string
for _, p := range pairs {
if _, ok := known[p.Key]; !ok {
if !accepts(p.Key) {
slog.Warn(res.opName+": rejecting unknown attribute key", "arn", res.arn, "key", p.Key)
return nil, errors.New(awserrors.ErrorValidationError)
if res.unknownKey != nil {
return nil, nil, res.unknownKey(p.Key)
}
return nil, nil, errors.New(awserrors.ErrorValidationError)
}
if res.validate != nil {
if err := res.validate(p.Key, p.Value); err != nil {
slog.Warn(res.opName+": rejecting attribute value", "arn", res.arn, "key", p.Key)
return nil, err
return nil, nil, err
}
}
}
@@ -102,7 +113,7 @@ func modifyResourceAttributes[R any](res attrResource[R], pairs []attrPair, rawC
// landed write.
if rawCount > 0 && len(submitted) == 0 {
slog.Warn(res.opName+": all submitted attributes were invalid", "arn", res.arn, "submitted_count", rawCount)
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
return nil, nil, errors.New(awserrors.ErrorInvalidParameterValue)
}
// Skip the NATS/KV write when nothing changed. Terraform re-applies the same
@@ -112,16 +123,16 @@ func modifyResourceAttributes[R any](res attrResource[R], pairs []attrPair, rawC
res.setAttrs(rec, attrs)
if err := res.save(rec); err != nil {
slog.Error(res.opName+": failed to persist", "arn", res.arn, "err", err)
return nil, errors.New(awserrors.ErrorServerInternal)
return nil, nil, errors.New(awserrors.ErrorServerInternal)
}
if res.afterChange != nil {
if err := res.afterChange(rec, changed); err != nil {
return nil, err
return nil, nil, err
}
}
}
return submitted, nil
return submitted, mergedAttributes(res, rec), nil
}
// describeResourceAttributes returns the record's attributes merged over the
@@ -136,6 +147,11 @@ func describeResourceAttributes[R any](res attrResource[R]) ([]attrPair, error)
return nil, err
}
return mergedAttributes(res, rec), nil
}
// mergedAttributes returns rec's stored attributes over its defaults, sorted by key.
func mergedAttributes[R any](res attrResource[R], rec *R) []attrPair {
merged := res.defaults(rec)
maps.Copy(merged, res.attrsOf(rec))
@@ -144,7 +160,7 @@ func describeResourceAttributes[R any](res attrResource[R]) ([]attrPair, error)
for _, k := range keys {
out = append(out, attrPair{Key: k, Value: merged[k]})
}
return out, nil
return out
}
// sdkAttrsIn converts an SDK attribute slice into internal pairs, dropping nil
@@ -196,9 +212,15 @@ func (s *ELBv2ServiceImpl) tgAttrResource(ctx context.Context, arn, accountID, o
return err
},
accountIDOf: func(r *TargetGroupRecord) string { return r.AccountID },
defaults: func(*TargetGroupRecord) map[string]string { return DefaultTargetGroupAttributes() },
attrsOf: func(r *TargetGroupRecord) map[string]string { return r.Attributes },
setAttrs: func(r *TargetGroupRecord, m map[string]string) { r.Attributes = m },
defaults: func(r *TargetGroupRecord) map[string]string {
return DefaultTargetGroupAttributes(r.Protocol, r.TargetType)
},
accepts: func(key string) bool { return targetGroupAttributeKeys[key] },
unknownKey: func(key string) error {
return awserrors.Errorf(awserrors.ErrorValidationError, "Target group attribute key '%s' is not recognized", key)
},
attrsOf: func(r *TargetGroupRecord) map[string]string { return r.Attributes },
setAttrs: func(r *TargetGroupRecord, m map[string]string) { r.Attributes = m },
}
}
@@ -299,11 +321,11 @@ func (s *ELBv2ServiceImpl) ModifyTargetGroupAttributes(ctx context.Context, inpu
const op = "ModifyTargetGroupAttributes"
arn := aws.StringValue(input.TargetGroupArn)
pairs, raw := sdkAttrsIn(input.Attributes, tgAttrKeyVal, arn, op)
submitted, err := modifyResourceAttributes(s.tgAttrResource(ctx, arn, accountID, op), pairs, raw)
_, full, err := modifyResourceAttributes(s.tgAttrResource(ctx, arn, accountID, op), pairs, raw)
if err != nil {
return nil, err
}
return &elbv2.ModifyTargetGroupAttributesOutput{Attributes: sdkAttrsOut(submitted, mkTGAttr)}, nil
return &elbv2.ModifyTargetGroupAttributesOutput{Attributes: sdkAttrsOut(full, mkTGAttr)}, nil
}
func (s *ELBv2ServiceImpl) DescribeTargetGroupAttributes(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, accountID string) (*elbv2.DescribeTargetGroupAttributesOutput, error) {
@@ -326,7 +348,7 @@ func (s *ELBv2ServiceImpl) ModifyLoadBalancerAttributes(ctx context.Context, inp
const op = "ModifyLoadBalancerAttributes"
arn := aws.StringValue(input.LoadBalancerArn)
pairs, raw := sdkAttrsIn(input.Attributes, lbAttrKeyVal, arn, op)
submitted, err := modifyResourceAttributes(s.lbAttrResource(ctx, arn, accountID, op), pairs, raw)
submitted, _, err := modifyResourceAttributes(s.lbAttrResource(ctx, arn, accountID, op), pairs, raw)
if err != nil {
return nil, err
}
+136 -46
View File
@@ -5,7 +5,9 @@ import (
"encoding/xml"
"fmt"
awsidentifiers "github.com/mulgadc/spinifex/spinifex/foundation/aws/identifiers"
"maps"
"sort"
"strconv"
"strings"
"sync"
"testing"
@@ -2747,65 +2749,143 @@ func TestUpdateStoredConfig_MissingTargetGroup(t *testing.T) {
// --- Attribute tests ---
// TestDefaultTargetGroupAttributes_CoversTerraformKeys guards the full
// TargetGroupAttribute key set from the ELBv2 API reference. A missing key
// fails the whole ModifyTargetGroupAttributes call with ValidationError.
func TestDefaultTargetGroupAttributes_CoversTerraformKeys(t *testing.T) {
t.Parallel()
attrs := DefaultTargetGroupAttributes()
expected := map[string]string{
var (
wantHTTPTGAttrs = map[string]string{
"deregistration_delay.timeout_seconds": "300",
"stickiness.enabled": "false",
"stickiness.type": "lb_cookie",
"stickiness.lb_cookie.duration_seconds": "86400",
"load_balancing.algorithm.anomaly_mitigation": "off",
"load_balancing.algorithm.type": "round_robin",
"load_balancing.cross_zone.enabled": "use_load_balancer_configuration",
"slow_start.duration_seconds": "0",
"stickiness.app_cookie.cookie_name": "",
"stickiness.app_cookie.duration_seconds": "86400",
"load_balancing.cross_zone.enabled": "use_load_balancer_configuration",
"load_balancing.algorithm.type": "round_robin",
"load_balancing.algorithm.anomaly_mitigation": "off",
"slow_start.duration_seconds": "0",
"lambda.multi_value_headers.enabled": "false",
"stickiness.enabled": "false",
"stickiness.lb_cookie.duration_seconds": "86400",
"stickiness.type": "lb_cookie",
"target_group_health.dns_failover.minimum_healthy_targets.count": "1",
"target_group_health.dns_failover.minimum_healthy_targets.percentage": "off",
"target_group_health.unhealthy_state_routing.minimum_healthy_targets.count": "1",
"target_group_health.unhealthy_state_routing.minimum_healthy_targets.percentage": "off",
"proxy_protocol_v2.enabled": "false",
}
wantTCPTGAttrs = map[string]string{
"deregistration_delay.connection_termination.enabled": "false",
"preserve_client_ip.enabled": "false",
"deregistration_delay.timeout_seconds": "300",
"load_balancing.cross_zone.enabled": "use_load_balancer_configuration",
"proxy_protocol_v2.enabled": "false",
"stickiness.enabled": "false",
"stickiness.type": "source_ip",
"target_group_health.dns_failover.minimum_healthy_targets.count": "1",
"target_group_health.dns_failover.minimum_healthy_targets.percentage": "off",
"target_group_health.unhealthy_state_routing.minimum_healthy_targets.count": "1",
"target_group_health.unhealthy_state_routing.minimum_healthy_targets.percentage": "off",
"target_health_state.unhealthy.connection_termination.enabled": "true",
"target_health_state.unhealthy.draining_interval_seconds": "0",
"target_failover.on_deregistration": "no_rebalance",
"target_failover.on_unhealthy": "no_rebalance",
}
)
for k, v := range expected {
got, ok := attrs[k]
assert.True(t, ok, "target group default attributes missing key %q — terraform will hit ValidationError", k)
assert.Equal(t, v, got, "target group default value mismatch for key %s", k)
// wantTGAttrs returns the attribute set AWS reports for a new target group.
func wantTGAttrs(protocol, targetType string) map[string]string {
if protocol == "HTTP" {
return maps.Clone(wantHTTPTGAttrs)
}
m := maps.Clone(wantTCPTGAttrs)
m["preserve_client_ip.enabled"] = strconv.FormatBool(targetType == "instance")
return m
}
func tgAttrMap(attrs []*elbv2.TargetGroupAttribute) map[string]string {
m := make(map[string]string, len(attrs))
for _, a := range attrs {
m[*a.Key] = *a.Value
}
return m
}
var tgAttrCases = []struct{ protocol, targetType string }{
{"HTTP", "instance"}, {"HTTP", "ip"}, {"TCP", "instance"}, {"TCP", "ip"},
}
func createAttrTG(t *testing.T, svc *ELBv2ServiceImpl, protocol, targetType string) *string {
t.Helper()
out, err := svc.CreateTargetGroup(context.Background(), &elbv2.CreateTargetGroupInput{
Name: aws.String("tg-attr-" + strings.ToLower(protocol) + "-" + targetType),
Protocol: aws.String(protocol),
Port: aws.Int64(80),
TargetType: aws.String(targetType),
}, testAccountID)
require.NoError(t, err)
return out.TargetGroups[0].TargetGroupArn
}
func TestDescribeTargetGroupAttributes_Defaults(t *testing.T) {
t.Parallel()
svc := setupTestService(t)
tgOut, err := svc.CreateTargetGroup(context.Background(), &elbv2.CreateTargetGroupInput{Name: aws.String("tg-attr-defaults")}, testAccountID)
require.NoError(t, err)
for _, tc := range tgAttrCases {
t.Run(tc.protocol+"/"+tc.targetType, func(t *testing.T) {
t.Parallel()
svc := setupTestService(t)
arn := createAttrTG(t, svc, tc.protocol, tc.targetType)
out, err := svc.DescribeTargetGroupAttributes(context.Background(), &elbv2.DescribeTargetGroupAttributesInput{
TargetGroupArn: tgOut.TargetGroups[0].TargetGroupArn,
out, err := svc.DescribeTargetGroupAttributes(context.Background(), &elbv2.DescribeTargetGroupAttributesInput{
TargetGroupArn: arn,
}, testAccountID)
require.NoError(t, err)
assert.Equal(t, wantTGAttrs(tc.protocol, tc.targetType), tgAttrMap(out.Attributes))
})
}
}
func TestModifyTargetGroupAttributes_ReturnsFullSet(t *testing.T) {
t.Parallel()
for _, tc := range tgAttrCases {
t.Run(tc.protocol+"/"+tc.targetType, func(t *testing.T) {
t.Parallel()
svc := setupTestService(t)
arn := createAttrTG(t, svc, tc.protocol, tc.targetType)
out, err := svc.ModifyTargetGroupAttributes(context.Background(), &elbv2.ModifyTargetGroupAttributesInput{
TargetGroupArn: arn,
Attributes: []*elbv2.TargetGroupAttribute{
{Key: aws.String("deregistration_delay.timeout_seconds"), Value: aws.String("30")},
},
}, testAccountID)
require.NoError(t, err)
want := wantTGAttrs(tc.protocol, tc.targetType)
want["deregistration_delay.timeout_seconds"] = "30"
assert.Equal(t, want, tgAttrMap(out.Attributes))
})
}
}
// The load balancer controller writes NLB keys on every target group it manages.
func TestModifyTargetGroupAttributes_AcceptsOtherProtocolKeys(t *testing.T) {
t.Parallel()
svc := setupTestService(t)
arn := createAttrTG(t, svc, "HTTP", "instance")
_, err := svc.ModifyTargetGroupAttributes(context.Background(), &elbv2.ModifyTargetGroupAttributesInput{
TargetGroupArn: arn,
Attributes: []*elbv2.TargetGroupAttribute{
{Key: aws.String("proxy_protocol_v2.enabled"), Value: aws.String("false")},
{Key: aws.String("target_failover.on_unhealthy"), Value: aws.String("no_rebalance")},
{Key: aws.String("target_failover.on_deregistration"), Value: aws.String("no_rebalance")},
{Key: aws.String("lambda.multi_value_headers.enabled"), Value: aws.String("false")},
},
}, testAccountID)
require.NoError(t, err)
}
defaults := DefaultTargetGroupAttributes()
assert.Len(t, out.Attributes, len(defaults))
attrMap := make(map[string]string)
for _, a := range out.Attributes {
attrMap[*a.Key] = *a.Value
}
for k, v := range defaults {
assert.Equal(t, v, attrMap[k], "default mismatch for key %s", k)
}
func TestModifyTargetGroupAttributes_UnknownKeyMessage(t *testing.T) {
t.Parallel()
svc := setupTestService(t)
arn := createAttrTG(t, svc, "TCP", "ip")
_, err := svc.ModifyTargetGroupAttributes(context.Background(), &elbv2.ModifyTargetGroupAttributesInput{
TargetGroupArn: arn,
Attributes: []*elbv2.TargetGroupAttribute{{Key: aws.String("bogus.key"), Value: aws.String("1")}},
}, testAccountID)
require.Error(t, err)
code, msg, _ := awserrors.ResolveErrorDetail(err)
assert.Equal(t, awserrors.ErrorValidationError, code)
assert.Equal(t, "Target group attribute key 'bogus.key' is not recognized", msg)
}
func TestDescribeLoadBalancerAttributes_ALBDefaults(t *testing.T) {
@@ -3025,6 +3105,14 @@ func echoMap(pairs []echoPair) map[string]string {
return m
}
// modifyLen is the number of pairs k's Modify returns after n were submitted.
func (k attrKind) modifyLen(n int) int {
if k.modifySetLen > 0 {
return k.modifySetLen
}
return n
}
// attrArnPtr returns nil for an empty ARN so the MissingArn cases exercise the
// real "no ARN supplied" path rather than an empty-string ARN.
func attrArnPtr(arn string) *string {
@@ -3046,6 +3134,7 @@ type attrKind struct {
defaultKey string // a default present after create
defaultVal string
unknownKey string // a key the handler must reject
modifySetLen int // size of the full set Modify returns; 0 when it echoes the submitted pairs
create func(t *testing.T, svc *ELBv2ServiceImpl, name string) string
modify func(svc *ELBv2ServiceImpl, arn, accountID string, attrs []rawAttr) ([]echoPair, error)
@@ -3062,7 +3151,8 @@ func attrKinds() []attrKind {
keyA: "deregistration_delay.timeout_seconds", valA: "45", valA2: "30",
keyB: "stickiness.enabled", valB: "true",
defaultKey: "stickiness.type", defaultVal: "lb_cookie",
unknownKey: "stickness.enabled", // typo of "stickiness"
unknownKey: "stickness.enabled", // typo of "stickiness"
modifySetLen: len(wantHTTPTGAttrs),
create: func(t *testing.T, svc *ELBv2ServiceImpl, name string) string {
out, err := svc.CreateTargetGroup(context.Background(), &elbv2.CreateTargetGroupInput{Name: aws.String(name)}, testAccountID)
require.NoError(t, err)
@@ -3179,7 +3269,7 @@ func TestAttributeRoundTrip(t *testing.T) {
// pass a length check.
echoed, err := k.modify(svc, arn, testAccountID, kvAttrs([2]string{k.keyA, k.valA}, [2]string{k.keyB, k.valB}))
require.NoError(t, err)
require.Len(t, echoed, 2)
require.Len(t, echoed, k.modifyLen(2))
em := echoMap(echoed)
assert.Equal(t, k.valA, em[k.keyA])
assert.Equal(t, k.valB, em[k.keyB])
@@ -3298,10 +3388,9 @@ func TestAttributeSkipsInvalidEntries(t *testing.T) {
kvAttr(k.keyB, k.valB),
})
require.NoError(t, err)
// Only the one valid attribute should be returned.
require.Len(t, echoed, 1)
assert.Equal(t, k.keyB, echoed[0].Key)
assert.Equal(t, k.valB, echoed[0].Val)
// The invalid elements are dropped and the valid one applied.
require.Len(t, echoed, k.modifyLen(1))
assert.Equal(t, k.valB, echoMap(echoed)[k.keyB])
// The valid attribute should have been persisted.
desc, err := k.describe(svc, arn, testAccountID)
@@ -3387,7 +3476,7 @@ func TestAttributeNoopSkipsPersist(t *testing.T) {
// Second modify with identical values — must skip the Put.
echoed, err := k.modify(svc, arn, testAccountID, kvAttrs([2]string{k.keyA, k.valA}))
require.NoError(t, err)
require.Len(t, echoed, 1)
require.Len(t, echoed, k.modifyLen(1))
assert.Equal(t, revBefore, k.revision(t, svc, arn), "identical modify must not increment KV revision")
// Empty attribute list is also a no-op.
@@ -3410,7 +3499,8 @@ func TestAttributeRejectsUnknownKey(t *testing.T) {
arn := k.create(t, svc, "attr-unknown-key")
_, err := k.modify(svc, arn, testAccountID, kvAttrs([2]string{k.unknownKey, "true"}))
assert.EqualError(t, err, awserrors.ErrorValidationError)
code, _ := awserrors.ResolveErrorCode(err)
assert.Equal(t, awserrors.ErrorValidationError, code)
// The rejected key must not have been persisted.
desc, err := k.describe(svc, arn, testAccountID)
+55 -22
View File
@@ -342,9 +342,18 @@ func DefaultLoadBalancerAttributes(lbType string) map[string]string {
}
}
// DefaultTargetGroupAttributes returns the default attribute set for target groups.
func DefaultTargetGroupAttributes() map[string]string {
return maps.Clone(targetGroupAttributeDefaults)
// DefaultTargetGroupAttributes returns the attributes AWS reports for a new
// target group of this protocol and target type. HTTPS uses the HTTP set and
// every other protocol the TCP set.
func DefaultTargetGroupAttributes(protocol, targetType string) map[string]string {
if protocol == ProtocolHTTP || protocol == ProtocolHTTPS {
return maps.Clone(httpTargetGroupAttributeDefaults)
}
attrs := maps.Clone(tcpTargetGroupAttributeDefaults)
if targetType == TargetTypeInstance {
attrs["preserve_client_ip.enabled"] = "true"
}
return attrs
}
// Default attribute sets are package-level vars; callers receive a clone so
@@ -395,32 +404,56 @@ var (
"secondary_ips.auto_assigned.per_subnet": "0",
}
targetGroupAttributeDefaults = map[string]string{
// HTTP and HTTPS target groups, as AWS reports them for instance and ip targets.
httpTargetGroupAttributeDefaults = map[string]string{
"deregistration_delay.timeout_seconds": "300",
"stickiness.enabled": "false",
"stickiness.type": "lb_cookie",
"stickiness.lb_cookie.duration_seconds": "86400",
"load_balancing.cross_zone.enabled": "use_load_balancer_configuration",
"load_balancing.algorithm.type": "round_robin",
"slow_start.duration_seconds": "0",
"load_balancing.algorithm.anomaly_mitigation": "off",
"load_balancing.algorithm.type": "round_robin",
"load_balancing.cross_zone.enabled": "use_load_balancer_configuration",
"slow_start.duration_seconds": "0",
"stickiness.app_cookie.cookie_name": "",
"stickiness.app_cookie.duration_seconds": "86400",
"lambda.multi_value_headers.enabled": "false",
"stickiness.enabled": "false",
"stickiness.lb_cookie.duration_seconds": "86400",
"stickiness.type": "lb_cookie",
"target_group_health.dns_failover.minimum_healthy_targets.count": "1",
"target_group_health.dns_failover.minimum_healthy_targets.percentage": "off",
"target_group_health.unhealthy_state_routing.minimum_healthy_targets.count": "1",
"target_group_health.unhealthy_state_routing.minimum_healthy_targets.percentage": "off",
// NLB target-group defaults. The AWS Load Balancer Controller always
// writes these on NLB target groups (e.g. ingress-nginx); without them as
// known keys the modify is rejected and LBC loops on the Service forever.
"proxy_protocol_v2.enabled": "false",
"deregistration_delay.connection_termination.enabled": "false",
"preserve_client_ip.enabled": "false",
"target_health_state.unhealthy.connection_termination.enabled": "true",
"target_health_state.unhealthy.draining_interval_seconds": "0",
// GWLB-only; AWS requires both to hold the same value.
"target_failover.on_deregistration": "no_rebalance",
"target_failover.on_unhealthy": "no_rebalance",
}
// TCP-family target groups; preserve_client_ip.enabled is set by target type.
tcpTargetGroupAttributeDefaults = map[string]string{
"deregistration_delay.connection_termination.enabled": "false",
"deregistration_delay.timeout_seconds": "300",
"load_balancing.cross_zone.enabled": "use_load_balancer_configuration",
"preserve_client_ip.enabled": "false",
"proxy_protocol_v2.enabled": "false",
"stickiness.enabled": "false",
"stickiness.type": "source_ip",
"target_group_health.dns_failover.minimum_healthy_targets.count": "1",
"target_group_health.dns_failover.minimum_healthy_targets.percentage": "off",
"target_group_health.unhealthy_state_routing.minimum_healthy_targets.count": "1",
"target_group_health.unhealthy_state_routing.minimum_healthy_targets.percentage": "off",
"target_health_state.unhealthy.connection_termination.enabled": "true",
"target_health_state.unhealthy.draining_interval_seconds": "0",
}
// Keys ModifyTargetGroupAttributes accepts on any target group. The load
// balancer controller writes the NLB keys on every NLB group, so this is the
// union of the protocol sets plus the lambda and GWLB keys.
targetGroupAttributeKeys = func() map[string]bool {
keys := map[string]bool{
"lambda.multi_value_headers.enabled": true,
"target_failover.on_deregistration": true,
"target_failover.on_unhealthy": true,
}
for k := range httpTargetGroupAttributeDefaults {
keys[k] = true
}
for k := range tcpTargetGroupAttributeDefaults {
keys[k] = true
}
return keys
}()
)
+11
View File
@@ -167,6 +167,17 @@ func ResourceARNs(action, accountID string, input any, svc iamdomain.IAMService)
if resource == "" {
resource = anyResource
}
if action == "UpdateUser" && resource != anyResource {
newName, err := stringField(input, "NewUserName")
if err != nil {
return nil, err
}
if newName != "" {
// AWS also authorizes the rename target, at the user's current path.
prefix := resource[:strings.LastIndex(resource, "/")+1]
return []string{resource, prefix + newName}, nil
}
}
return []string{resource}, nil
}
@@ -32,6 +32,7 @@ type iamAuthzPolicyService struct {
canonicalErr error
deleteRoleErr error
deleted []string
renamed []string
}
func (s *iamAuthzPolicyService) CanonicalResourceARN(_ string, _ arn.IAMResourceType, name string) (string, error) {
@@ -53,6 +54,11 @@ func (s *iamAuthzPolicyService) DeleteRole(_ string, input *iam.DeleteRoleInput)
return &iam.DeleteRoleOutput{}, nil
}
func (s *iamAuthzPolicyService) UpdateUser(_ string, input *iam.UpdateUserInput) (*iam.UpdateUserOutput, error) {
s.renamed = append(s.renamed, *input.UserName+"->"+*input.NewUserName)
return &iam.UpdateUserOutput{}, nil
}
// iamGateway pairs a caller holding statements with the service they act on.
type iamGateway struct {
h *gwtest.Harness
@@ -249,3 +255,34 @@ func TestAddRoleToInstanceProfile_MissingRoleStaysValidationFault(t *testing.T)
err := dispatchIAM(t, gw, "Action=AddRoleToInstanceProfile&InstanceProfileName=web")
require.EqualError(t, err, awserrors.ErrorMissingParameter)
}
// TestIAMRequest_UpdateUserRenameAuthorizesSourceAndTarget pins AWS's rule that
// a rename needs iam:UpdateUser on both the user and user/<NewUserName>, the
// target taking the user's current path.
func TestIAMRequest_UpdateUserRenameAuthorizesSourceAndTarget(t *testing.T) {
users := map[string]string{"alice": "arn:aws:iam::123456789012:user/teams/alice"}
const (
source = "arn:aws:iam::*:user/teams/alice"
target = "arn:aws:iam::*:user/teams/bob"
body = "Action=UpdateUser&UserName=alice&NewUserName=bob"
)
t.Run("source-only grant is denied", func(t *testing.T) {
gw, svc := iamScopedGateway(users, statement("Allow", "iam:UpdateUser", source))
assertDenied(t, dispatchIAM(t, gw, body))
assert.Empty(t, svc.renamed)
})
t.Run("target-only grant is denied", func(t *testing.T) {
gw, svc := iamScopedGateway(users, statement("Allow", "iam:UpdateUser", target))
assertDenied(t, dispatchIAM(t, gw, body))
assert.Empty(t, svc.renamed)
})
t.Run("grant on both succeeds", func(t *testing.T) {
gw, svc := iamScopedGateway(users, statement("Allow", "iam:UpdateUser", source),
statement("Allow", "iam:UpdateUser", target))
require.NoError(t, dispatchIAM(t, gw, body))
assert.Equal(t, []string{"alice->bob"}, svc.renamed)
})
}
@@ -281,7 +281,7 @@ func TestRegistration_UpdateUser(t *testing.T) {
})
require.NoError(t, err)
require.Len(t, calls, 1)
assert.Equal(t, []string{"arn:aws:iam::" + testAccountID + ":user/alice"}, calls[0].resources)
assert.Equal(t, []string{"arn:aws:iam::" + testAccountID + ":user/alice", "arn:aws:iam::" + testAccountID + ":user/alicia"}, calls[0].resources)
assert.Contains(t, w.Body.String(), "UpdateUserResponse")
require.NotNil(t, svc.updateUser)
assert.Equal(t, "alicia", *svc.updateUser.NewUserName)
+11 -3
View File
@@ -47,8 +47,8 @@ const denyACLSeverity = "info"
// MUST be validated upstream; values are interpolated verbatim.
type Rule struct {
IPProtocol string // "tcp", "udp", "icmp", or "-1" (all)
FromPort int64
ToPort int64
FromPort int64 // ICMP type for "icmp", -1 for any
ToPort int64 // ICMP code for "icmp", -1 for any
CIDR string
SourceSG string
}
@@ -181,7 +181,15 @@ func appendProtocolMatch(parts []string, rule Rule) ([]string, error) {
case "udp":
return appendPortMatch(parts, "udp", rule.FromPort, rule.ToPort), nil
case "icmp":
return append(parts, "icmp4"), nil
// AWS reads an ICMP rule's FromPort as the type and ToPort as the code.
parts = append(parts, "icmp4")
if rule.FromPort != -1 {
parts = append(parts, fmt.Sprintf("icmp4.type == %d", rule.FromPort))
}
if rule.ToPort != -1 {
parts = append(parts, fmt.Sprintf("icmp4.code == %d", rule.ToPort))
}
return parts, nil
case "-1", "":
return parts, nil
default:
@@ -4,6 +4,7 @@ import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestACL_TCPPortFromCIDR(t *testing.T) {
@@ -45,6 +46,8 @@ func TestACL_PortRange(t *testing.T) {
func TestACL_ICMP(t *testing.T) {
match, err := BuildIngressACLMatch("sg_test", Rule{
IPProtocol: "icmp",
FromPort: -1,
ToPort: -1,
CIDR: "0.0.0.0/0",
})
assert.NoError(t, err)
@@ -53,6 +56,37 @@ func TestACL_ICMP(t *testing.T) {
assert.NotContains(t, match, "udp.dst")
}
// An ICMP rule opens only the type and code it names, -1 meaning any; the
// console's ping rule (type 8, code -1) must not open echo reply or redirects.
func TestACL_ICMPTypeAndCode(t *testing.T) {
cases := []struct {
name string
fromPort int64
toPort int64
want, wantNone []string
}{
{"echo request any code", 8, -1, []string{"icmp4", "icmp4.type == 8"}, []string{"icmp4.code"}},
{"type and code", 3, 4, []string{"icmp4.type == 3", "icmp4.code == 4"}, nil},
{"echo reply", 0, 0, []string{"icmp4.type == 0", "icmp4.code == 0"}, nil},
{"all icmp", -1, -1, []string{"icmp4"}, []string{"icmp4.type", "icmp4.code"}},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
rule := Rule{IPProtocol: "icmp", FromPort: tc.fromPort, ToPort: tc.toPort, CIDR: "0.0.0.0/0"}
for _, build := range []func(string, Rule) (string, error){BuildIngressACLMatch, BuildEgressACLMatch} {
match, err := build("sg_test", rule)
require.NoError(t, err)
for _, w := range tc.want {
assert.Contains(t, match, w)
}
for _, n := range tc.wantNone {
assert.NotContains(t, match, n)
}
}
})
}
}
func TestACL_AllProtocols(t *testing.T) {
match, err := BuildIngressACLMatch("sg_test", Rule{
IPProtocol: "-1",
@@ -27,6 +27,7 @@ func md5Hex(b []byte) string {
}
func TestSendWithAttributesStoresVersionedRecordAndReceivesThem(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", nil)
in := input("orders", "hello")
@@ -56,6 +57,7 @@ func TestSendWithAttributesStoresVersionedRecordAndReceivesThem(t *testing.T) {
}
func TestSendWithoutAttributesKeepsPlainRecord(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", nil)
res := e.send(t, "orders", "plain", nil)
@@ -70,6 +72,7 @@ func TestSendWithoutAttributesKeepsPlainRecord(t *testing.T) {
// A record as an earlier release wrote it, appended directly, still reads.
func TestReceiveReadsEarlierPlainRecords(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", nil)
enq := strconv.FormatInt(fixedNow.UnixMilli(), 10)
@@ -32,6 +32,7 @@ func messageStreamRequests(subjects []string, stream string) (leaderGets, infos
// for bodies: one leader message get per stored body through the bound, with
// no direct get and no consumer on the message stream.
func TestBodyEnumerationIsLeaderServed(t *testing.T) {
t.Parallel()
srv, js := startServer(t, t.TempDir())
s, err := sqsjs.New(context.Background(), js, testConfig)
require.NoError(t, err)
@@ -85,6 +86,7 @@ func TestBodyEnumerationIsLeaderServed(t *testing.T) {
// and neighbouring namespaces in stream order, and stops at the first page's
// bound even when later bodies exist.
func TestScanStepsOverGapsInOrderWithinTheBound(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
ctx := context.Background()
a, b := ns(0, 0, "gap-a"), ns(0, 0, "gap-b")
@@ -119,6 +121,7 @@ func TestScanStepsOverGapsInOrderWithinTheBound(t *testing.T) {
}
func TestBodyEnumerationCancelled(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
appendTo(t, s, ns(0, 0, "cancel"), "b")
ctx, cancel := context.WithCancel(context.Background())
@@ -47,6 +47,7 @@ func claimPos(pos message.Position, vt time.Duration) message.ClaimInput {
}
func TestClaimLifecycleOnJetStream(t *testing.T) {
t.Parallel()
s := claimStore(t)
ctx := context.Background()
pos := appendAt(t, s, fixedNow, fixedNow.Add(10*time.Second))
@@ -80,6 +81,7 @@ func TestClaimLifecycleOnJetStream(t *testing.T) {
// Competing receivers on one message: exactly one claim is acknowledged.
func TestClaimCompetingReceiversOnJetStream(t *testing.T) {
t.Parallel()
s := claimStore(t)
ctx := context.Background()
for _, prior := range []bool{false, true} {
@@ -140,6 +142,7 @@ func (l *lostClaimReply) PutState(ctx context.Context, ns message.Namespace, pos
}
func TestClaimAppliedButLostReplyOnJetStream(t *testing.T) {
t.Parallel()
s := claimStore(t)
ctx := context.Background()
pos := appendAt(t, s, fixedNow, fixedNow)
@@ -208,6 +211,7 @@ func (h *hookedStore) PutState(ctx context.Context, ns message.Namespace, pos me
// tombstone between the claim's body read and its state read leaves the
// record absent again; the claim must not resurrect the deleted message.
func TestClaimDoesNotResurrectRemovedMessage(t *testing.T) {
t.Parallel()
s := claimStore(t)
ctx := context.Background()
pos := appendAt(t, s, fixedNow, fixedNow)
@@ -255,6 +259,7 @@ func (c *steppingClock) claimer(s message.Store) *message.Claimer {
// A slow body or state read does not spend the visibility: the deadline is
// measured from the claim attempt, after both reads.
func TestClaimDeadlineFromAttemptNotRequestStart(t *testing.T) {
t.Parallel()
s := claimStore(t)
ctx := context.Background()
pos := appendAt(t, s, fixedNow, fixedNow)
@@ -36,12 +36,14 @@ func r1Harness(t *testing.T) conformance.Harness {
}
func TestR1Conformance(t *testing.T) {
t.Parallel()
conformance.Run(t, r1Harness)
}
// TestR1SharedInstanceConformance runs every case on one long-lived store,
// so each must pass beside other queues' data on the shared streams.
func TestR1SharedInstanceConformance(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
conformance.Run(t, func(*testing.T) conformance.Harness {
return conformance.Harness{Store: s, MessageShards: testConfig.MessageStreams, StateShards: testConfig.StateBuckets}
@@ -50,6 +50,7 @@ func (e receiveEnv) state(t *testing.T, q queue.Record, h string) (map[string]an
}
func TestDeleteCurrentHandleWritesTombstone(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", nil)
m := e.receiveOne(t, "orders")
@@ -73,6 +74,7 @@ func TestDeleteCurrentHandleWritesTombstone(t *testing.T) {
// An older handle succeeds without deleting the newer receive.
func TestDeleteOlderHandleKeepsNewerReceive(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", nil)
first := e.receiveOne(t, "orders")
@@ -94,6 +96,7 @@ func TestDeleteOlderHandleKeepsNewerReceive(t *testing.T) {
}
func TestDeleteIsIdempotent(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", nil)
m := e.receiveOne(t, "orders")
@@ -113,6 +116,7 @@ func TestDeleteIsIdempotent(t *testing.T) {
// A delete racing a re-receive either tombstones before it, so the receive
// gets nothing, or loses to it and leaves the newer receive in flight.
func TestDeleteRacingReReceive(t *testing.T) {
t.Parallel()
for range 10 {
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", nil)
@@ -139,6 +143,7 @@ func TestDeleteRacingReReceive(t *testing.T) {
}
func TestDeleteRefusesInvalidHandles(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", nil)
e.create(t, acct, "audit", nil)
@@ -170,7 +175,9 @@ func TestDeleteRefusesInvalidHandles(t *testing.T) {
}
func TestDeleteUnsettledOutcomes(t *testing.T) {
t.Parallel()
t.Run("lost reply, tombstone applied", func(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", nil)
m := e.receiveOne(t, "orders")
@@ -180,6 +187,7 @@ func TestDeleteUnsettledOutcomes(t *testing.T) {
assert.Equal(t, message.StatusDeleted, st["s"])
})
t.Run("lost reply, nothing applied", func(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", nil)
m := e.receiveOne(t, "orders")
@@ -190,6 +198,7 @@ func TestDeleteUnsettledOutcomes(t *testing.T) {
assert.Equal(t, before, after)
})
t.Run("state read fails", func(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
e.create(t, acct, "orders", nil)
m := e.receiveOne(t, "orders")
@@ -203,6 +212,7 @@ func TestDeleteUnsettledOutcomes(t *testing.T) {
"older than key": `{"v":1,"s":"inflight","c":1,"g":0,"d":1,"r":1,"f":1}`,
} {
t.Run(name, func(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", nil)
m := e.receiveOne(t, "orders")
@@ -222,6 +232,7 @@ func TestDeleteUnsettledOutcomes(t *testing.T) {
// Neither handles nor bodies appear in errors or logs on any refusal.
func TestDeleteKeepsHandlesOutOfErrorsAndLogs(t *testing.T) {
// Not parallel: it captures the process-wide default logger.
var logs bytes.Buffer
prev := slog.Default()
slog.SetDefault(slog.New(slog.NewJSONHandler(&logs, &slog.HandlerOptions{Level: slog.LevelDebug})))
@@ -33,6 +33,7 @@ func emptied(t *testing.T, f *feed) {
// TestFeedCancelledBeforeOpenCreatesNothing: Feed with an ended ctx returns
// its error and creates no consumer.
func TestFeedCancelledBeforeOpenCreatesNothing(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
ctx, cancel := context.WithCancel(context.Background())
cancel()
@@ -51,6 +52,7 @@ func TestFeedCancelledBeforeOpenCreatesNothing(t *testing.T) {
// TestFeedStartCancelledDuringCreate: a first open whose create finishes after
// ctx ended releases that consumer and returns ctx's error.
func TestFeedStartCancelledDuringCreate(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
f := openWedgeable(t, s)
emptied(t, f)
@@ -79,6 +81,7 @@ func TestFeedStartCancelledDuringCreate(t *testing.T) {
// after ctx ended releases that consumer, returns ctx's error and no change;
// the stored change is delivered by the next Next.
func TestFeedRecoveryCancelledDuringCreate(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
f := openWedgeable(t, s)
ctx, cancel := context.WithCancel(context.Background())
@@ -102,6 +105,7 @@ func TestFeedRecoveryCancelledDuringCreate(t *testing.T) {
// during a recovery's create, Next returns ErrFeedClosed and Close succeeds
// only once the late consumer is deleted.
func TestFeedRecoveryCloseWinsOverCancel(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
f := openWedgeable(t, s)
ctx, cancel := context.WithCancel(context.Background())
@@ -120,6 +124,7 @@ func TestFeedRecoveryCloseWinsOverCancel(t *testing.T) {
// TestFeedConsumerHasInactiveBackstop: every feed consumer carries the
// server-side inactive threshold that removes one an ambiguous create left.
func TestFeedConsumerHasInactiveBackstop(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
f := openWedgeable(t, s)
info, err := f.consumer.Info(context.Background())
@@ -144,6 +149,7 @@ func (c cancelThenFail) OrderedConsumer(context.Context, jsapi.OrderedConsumerCo
// TestFeedFailedOpenAfterCancelReportsCancellation: an open that fails after
// ctx ended still reports ctx's error, alongside the create failure.
func TestFeedFailedOpenAfterCancelReportsCancellation(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
f := openWedgeable(t, s)
emptied(t, f)
@@ -58,6 +58,7 @@ func returnsWithin(ch <-chan error, d time.Duration) (error, bool) {
// TestFeedConcurrentCloseWaitsForDelete: a second Close does not report
// success while the first Close's delete is still in progress.
func TestFeedConcurrentCloseWaitsForDelete(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
f := openWedgeable(t, s)
d := controlDeletes(f, true)
@@ -76,6 +77,7 @@ func TestFeedConcurrentCloseWaitsForDelete(t *testing.T) {
// TestFeedCloseWaitsForRecoveryRelease: Close waits for the release of a
// consumer a recovery has detached.
func TestFeedCloseWaitsForRecoveryRelease(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
ns := message.Namespace{Queue: "q"}
appendAt(t, s, ns)
@@ -104,6 +106,7 @@ func TestFeedCloseWaitsForRecoveryRelease(t *testing.T) {
// TestFeedCloseRetriesFailedDelete: a failed delete is an error, not success,
// and a later Close retries it.
func TestFeedCloseRetriesFailedDelete(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
f := openWedgeable(t, s)
d := controlDeletes(f, false)
@@ -118,6 +121,7 @@ func TestFeedCloseRetriesFailedDelete(t *testing.T) {
// TestFeedCloseRetriesFailedRecoveryRelease: a recovery's failed delete is
// retried by Close rather than forgotten.
func TestFeedCloseRetriesFailedRecoveryRelease(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
ns := message.Namespace{Queue: "q"}
appendAt(t, s, ns)
@@ -139,6 +143,7 @@ func TestFeedCloseRetriesFailedRecoveryRelease(t *testing.T) {
// TestFeedDeliveryClaimedBeforeCloseIsReturned: Close beginning after Next has
// claimed a change does not withdraw it, and the feed then reports closed.
func TestFeedDeliveryClaimedBeforeCloseIsReturned(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
ns := message.Namespace{Queue: "q"}
first := appendAt(t, s, ns)
@@ -176,6 +181,7 @@ func (l lostReply) OrderedConsumer(_ context.Context, cfg jsapi.OrderedConsumerC
}
func TestFeedFailedCreateLeavesNoConsumer(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
f := openWedgeable(t, s)
require.NoError(t, f.discard(f.detachForTest()))
@@ -188,6 +194,7 @@ func TestFeedFailedCreateLeavesNoConsumer(t *testing.T) {
// TestFeedCallerDeadlineDoesNotCutCreateShort: real creates under caller
// deadlines that expire mid-request complete, so none lands unowned.
func TestFeedCallerDeadlineDoesNotCutCreateShort(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
f := openWedgeable(t, s)
require.NoError(t, f.discard(f.detachForTest()))
@@ -205,6 +212,7 @@ func TestFeedCallerDeadlineDoesNotCutCreateShort(t *testing.T) {
// finds nothing, Close deletes the name again, catching a create that landed
// after that first delete.
func TestFeedCloseRetriesAbandonedCreate(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
f := openWedgeable(t, s)
require.NoError(t, f.discard(f.detachForTest()))
@@ -65,6 +65,7 @@ func recoverInto(ctx context.Context, t *testing.T, s *Store, f *feed) (*blockin
}
func TestFeedNextAfterCloseCreatesNoConsumer(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
appendAt(t, s, message.Namespace{Queue: "q"})
f := openWedgeable(t, s)
@@ -81,6 +82,7 @@ func TestFeedNextAfterCloseCreatesNoConsumer(t *testing.T) {
// TestFeedCloseInterruptsBlockedNext: a Next waiting on a consumer with
// nothing to deliver returns message.ErrFeedClosed promptly once Close runs.
func TestFeedCloseInterruptsBlockedNext(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
f := openWedgeable(t, s)
f.idleCheck = time.Minute
@@ -103,6 +105,7 @@ func TestFeedCloseInterruptsBlockedNext(t *testing.T) {
// TestFeedCloseConcurrentWithNext races Close against active Next calls and
// recoveries; run under -race. Every round ends closed with no consumer.
func TestFeedCloseConcurrentWithNext(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
ns := message.Namespace{Queue: "q"}
for range 3 {
@@ -151,6 +154,7 @@ func (s *stallingStream) GetMsg(ctx context.Context, _ uint64, _ ...jsapi.GetMsg
// TestFeedCloseInterruptsRecoveryProbe: Close ends a Next blocked in the
// idle check's stream read, not only one waiting on the consumer.
func TestFeedCloseInterruptsRecoveryProbe(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
f := openWedgeable(t, s)
stall := &stallingStream{Stream: f.stream, entered: make(chan struct{})}
@@ -176,6 +180,7 @@ func TestFeedCloseInterruptsRecoveryProbe(t *testing.T) {
// TestFeedCloseWaitsForInFlightOpen: Close waits for a reopen already
// creating a consumer, and that consumer is gone when Close returns nil.
func TestFeedCloseWaitsForInFlightOpen(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
f := openWedgeable(t, s)
b, next := recoverInto(context.Background(), t, s, f)
@@ -202,6 +207,7 @@ func TestFeedCloseWaitsForInFlightOpen(t *testing.T) {
// rather than success; a repeated Close waits again and succeeds once the
// late consumer has been deleted.
func TestFeedCloseReportsStuckOpen(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
f := openWedgeable(t, s)
f.closeWait = 200 * time.Millisecond
@@ -227,6 +233,7 @@ func TestFeedCloseReportsStuckOpen(t *testing.T) {
// TestFeedCloseReportsLateDeleteFailure: if the consumer a racing reopen
// created cannot be deleted, Close returns that error, not success.
func TestFeedCloseReportsLateDeleteFailure(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
f := openWedgeable(t, s)
b, next := recoverInto(context.Background(), t, s, f)
@@ -265,6 +272,7 @@ func (c *closeOnDelivery) Next(opts ...jsapi.NextOpt) (jsapi.Msg, error) {
// TestFeedWithholdsDeliveryRacingClose: a change received after Close began
// is not returned, and a feed from the last delivered cursor still gets it.
func TestFeedWithholdsDeliveryRacingClose(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
ns := message.Namespace{Queue: "q"}
first := appendAt(t, s, ns)
@@ -97,6 +97,7 @@ func consumerNames(t *testing.T, s *Store) []string {
// consumer that delivers nothing, Next replaces the consumer from the cursor
// and returns that change within one idle check plus the reopen.
func TestFeedRecoversFromWedgedPull(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
ns := message.Namespace{Queue: "q"}
first, second := appendAt(t, s, ns), appendAt(t, s, ns)
@@ -129,6 +130,7 @@ func TestFeedRecoversFromWedgedPull(t *testing.T) {
// TestFeedIdleWithNothingStoredKeepsConsumer: an idle feed caught up with the
// stream waits on its consumer and never rebuilds it.
func TestFeedIdleWithNothingStoredKeepsConsumer(t *testing.T) {
t.Parallel()
s, _ := feedStore(t)
ns := message.Namespace{Queue: "q"}
appendAt(t, s, ns)
@@ -288,6 +288,7 @@ func testR3FeedOpenAcrossRestart(t *testing.T) {
// TestFeedProbeStallSnapshot drives the probe's stall path: the snapshot runs
// once, sees live elapsed time, and finishes before expect returns.
func TestFeedProbeStallSnapshot(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
f, err := s.Feed(context.Background(), 0, 0)
require.NoError(t, err)
@@ -64,6 +64,7 @@ func guestStore(t *testing.T) (message.Store, *server.Options) {
// ADR-0011:S4: the guest's own config admits a full 1 MiB message with the
// metadata a send writes, while the application limit stays at 1 MiB.
func TestS4_GuestConfigAdmitsAFullMessage(t *testing.T) {
t.Parallel()
e := newSendEnv(t)
store, opts := guestStore(t)
assert.Equal(t, int32(2<<20), opts.MaxPayload)
@@ -92,6 +93,7 @@ func TestS4_GuestConfigAdmitsAFullMessage(t *testing.T) {
// ADR-0011:S4: a full 1 MiB message whose size is shared between ten
// attributes and the body still fits the guest's own payload limit.
func TestS4_GuestConfigAdmitsAFullMessageWithAttributes(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
store, _ := guestStore(t)
q := e.create(t, acct, "orders", nil)
@@ -15,6 +15,7 @@ import (
// TestKVRemovedMatchesKVGet pins the markers nats.go's KV Get treats as an
// absent key, including server-written TTL and purge markers.
func TestKVRemovedMatchesKVGet(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
name string
key string
@@ -63,6 +64,7 @@ func startJetStream(t *testing.T) jsapi.JetStream {
// captured before the pass. A later delete still removes a key: with
// history 1 it also removes the key's earlier record.
func TestLeaderStateKeysStopsAtTheBound(t *testing.T) {
t.Parallel()
js := startJetStream(t)
ctx := context.Background()
kv, err := js.CreateKeyValue(ctx, jsapi.KeyValueConfig{Bucket: "b", History: 1})
@@ -120,6 +122,7 @@ func (c *cancelOnRead) Options() jsapi.JetStreamOptions {
// TestLeaderStateKeysStopsWhenCancelled returns the context's error from a
// pass cancelled between records rather than a partial key set.
func TestLeaderStateKeysStopsWhenCancelled(t *testing.T) {
t.Parallel()
js := startJetStream(t)
kv, err := js.CreateKeyValue(context.Background(), jsapi.KeyValueConfig{Bucket: "c", History: 1})
require.NoError(t, err)
@@ -90,6 +90,7 @@ func goReceive(r *message.Receiver, ctx context.Context, in message.ReceiveInput
// A send from a separate sender wakes a waiting receiver without a recheck.
func TestLongPollWakesOnSend(t *testing.T) {
t.Parallel()
e := newLongPollEnv(t)
e.create(t, "orders", nil)
res := goReceive(e.receiver(nil, noRecheck), context.Background(), wait("orders", 10))
@@ -107,6 +108,7 @@ func TestLongPollWakesOnSend(t *testing.T) {
// An append to another queue on the same shard wakes nothing it can take,
// so the waiter keeps waiting and times out empty.
func TestLongPollIgnoresOtherQueues(t *testing.T) {
t.Parallel()
e := newLongPollEnv(t)
e.create(t, "orders", nil)
e.create(t, "other", nil)
@@ -121,6 +123,7 @@ func TestLongPollIgnoresOtherQueues(t *testing.T) {
}
func TestLongPollEmptyTimesOut(t *testing.T) {
t.Parallel()
e := newLongPollEnv(t)
e.create(t, "orders", nil)
start := time.Now()
@@ -135,6 +138,7 @@ func TestLongPollEmptyTimesOut(t *testing.T) {
// Two waiting receivers on separate instances: one send is taken once.
func TestLongPollCompetingReceivers(t *testing.T) {
t.Parallel()
e := newLongPollEnv(t)
e.create(t, "orders", nil)
a := goReceive(e.receiver(nil, noRecheck), context.Background(), wait("orders", 3))
@@ -153,6 +157,7 @@ func TestLongPollCompetingReceivers(t *testing.T) {
}
func TestLongPollStopsOnCancel(t *testing.T) {
t.Parallel()
e := newLongPollEnv(t)
e.create(t, "orders", nil)
ctx, cancel := context.WithCancel(context.Background())
@@ -200,6 +205,7 @@ func (f failingFeed) Next(ctx context.Context) (message.Change, error) {
// A wake feed that fails after opening ends the wait as unavailable, not
// as a successful empty receive.
func TestLongPollFeedFailureAfterOpen(t *testing.T) {
t.Parallel()
e := newLongPollEnv(t)
e.create(t, "orders", nil)
lose := make(chan struct{})
@@ -253,6 +259,7 @@ func (s *heldStore) Scan(ctx context.Context, shard message.Shard, from message.
// A scan held past the wait ends it on time: empty after a completed poll,
// unavailable when no poll ever completed. No wake consumer is left.
func TestLongPollHeldScan(t *testing.T) {
t.Parallel()
e := newLongPollEnv(t)
e.create(t, "orders", nil)
@@ -18,6 +18,7 @@ import (
// code, an ordered Messages iterator idling with stored messages pending
// after a full cluster restart. Opt-in: it is slow and probabilistic.
func TestNATSOrderedFeedStallRepro(t *testing.T) {
// Not parallel: it redeclares the process-wide cluster size.
rounds, _ := strconv.Atoi(os.Getenv("NATS_ORDERED_REPRO_ROUNDS"))
if rounds < 1 {
t.Skip("set NATS_ORDERED_REPRO_ROUNDS to run")
@@ -52,6 +52,7 @@ func externalStore(t *testing.T, js jsapi.JetStream) *sqsjs.Store {
}
func TestR3ExternalConformance(t *testing.T) {
// Not parallel: it redeclares the process-wide cluster size.
urls := os.Getenv(externalURLsEnv)
if urls == "" {
t.Skipf("set %s to a dedicated three-node SQS JetStream cluster", externalURLsEnv)
@@ -35,6 +35,7 @@ func collect(seqs *[]uint64, stopAt uint64) func(*jsapi.RawStreamMsg) (bool, err
// TestReadRangeStopsAtTheBound never passes fn a match beyond the bound,
// even when the bound itself is a removed position.
func TestReadRangeStopsAtTheBound(t *testing.T) {
t.Parallel()
js := rangeStream(t)
var seqs []uint64
last, done, err := readRange(context.Background(), js, "r", "r.>", 0, 3, collect(&seqs, 0))
@@ -47,6 +48,7 @@ func TestReadRangeStopsAtTheBound(t *testing.T) {
// TestReadRangeDoneWhenStoppedAtTheBound reports the pass complete when fn
// stops on the bound's own match, and incomplete when it stops earlier.
func TestReadRangeDoneWhenStoppedAtTheBound(t *testing.T) {
t.Parallel()
js := rangeStream(t)
var seqs []uint64
last, done, err := readRange(context.Background(), js, "r", "r.>", 0, 4, collect(&seqs, 4))
@@ -66,6 +68,7 @@ func TestReadRangeDoneWhenStoppedAtTheBound(t *testing.T) {
// TestReadRangeReturnsRequestErrors surfaces a failed leader read rather
// than reporting the pass complete.
func TestReadRangeReturnsRequestErrors(t *testing.T) {
t.Parallel()
js := rangeStream(t)
var seqs []uint64
_, _, err := readRange(context.Background(), js, "r", "r.>", 0, 1, collect(&seqs, 0))
@@ -73,6 +73,7 @@ func recv(name string, n int) message.ReceiveInput {
}
func TestReceiveReturnsSentMessageWithHandle(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", map[string]string{"VisibilityTimeout": "30"})
sent := e.send(t, "orders", "hello", nil)
@@ -111,6 +112,7 @@ func TestReceiveReturnsSentMessageWithHandle(t *testing.T) {
}
func TestReceiveMaxNumberOfMessages(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
e.create(t, acct, "orders", nil)
for i := range 15 {
@@ -154,6 +156,7 @@ func TestReceiveMaxNumberOfMessages(t *testing.T) {
}
func TestReceiveIsolatesQueues(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
e.create(t, acct, "orders", nil)
e.create(t, acct, "audit", nil)
@@ -175,6 +178,7 @@ func TestReceiveIsolatesQueues(t *testing.T) {
// A queue whose first visible message lies beyond one receive's scan budget
// is reached by the next receive, which resumes the pass.
func TestReceiveResumesBeyondScanBudget(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
e.create(t, acct, "orders", nil)
budget := message.ReceiveScanPage * message.ReceiveScanPages
@@ -199,6 +203,7 @@ func TestReceiveResumesBeyondScanBudget(t *testing.T) {
}
func TestReceiveCompetingConsumers(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
e.create(t, acct, "orders", nil)
for i := range 20 {
@@ -233,7 +238,9 @@ func TestReceiveCompetingConsumers(t *testing.T) {
// A scan failure or an unsettled claim is never an empty queue; a receive
// that claimed something still returns it.
func TestReceiveReportsUnsettledOutcomes(t *testing.T) {
t.Parallel()
t.Run("scan fails", func(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
e.create(t, acct, "orders", nil)
e.send(t, "orders", "hello", nil)
@@ -242,6 +249,7 @@ func TestReceiveReportsUnsettledOutcomes(t *testing.T) {
require.ErrorIs(t, err, message.ErrUnavailable)
})
t.Run("only claim is ambiguous", func(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
e.create(t, acct, "orders", nil)
e.send(t, "orders", "hello", nil)
@@ -251,6 +259,7 @@ func TestReceiveReportsUnsettledOutcomes(t *testing.T) {
assert.Empty(t, got)
})
t.Run("another claim succeeds", func(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
e.create(t, acct, "orders", nil)
e.send(t, "orders", "first", nil)
@@ -262,6 +271,7 @@ func TestReceiveReportsUnsettledOutcomes(t *testing.T) {
assert.Equal(t, second.MessageID, got[0].MessageID)
})
t.Run("newer record", func(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", nil)
sent := e.send(t, "orders", "hello", nil)
@@ -103,6 +103,7 @@ func input(name, body string) message.SendInput {
func ptr(n int) *int { return &n }
func TestSendAppendsBodyAndMetadata(t *testing.T) {
t.Parallel()
e := newSendEnv(t)
q := e.create(t, acct, "orders", nil)
@@ -138,11 +139,13 @@ func (f faultStore) Append(ctx context.Context, ns message.Namespace, in message
}
func TestSendLostAppendReplyIsNeverSuccess(t *testing.T) {
t.Parallel()
e := newSendEnv(t)
q := e.create(t, acct, "orders", nil)
errLost := errors.New("reply lost")
t.Run("applied but the reply was lost", func(t *testing.T) {
t.Parallel()
fs := faultStore{Store: e.store, append: func(ctx context.Context, ns message.Namespace, in message.AppendInput) (message.AppendResult, error) {
_, err := e.store.Append(ctx, ns, in)
require.NoError(t, err)
@@ -157,6 +160,7 @@ func TestSendLostAppendReplyIsNeverSuccess(t *testing.T) {
// Neither logs nor errors carry a body, whatever the outcome.
func TestSendNeverExposesTheBody(t *testing.T) {
// Not parallel: it captures the process-wide default logger.
var logs bytes.Buffer
prev := slog.Default()
slog.SetDefault(slog.New(slog.NewTextHandler(&logs, &slog.HandlerOptions{Level: slog.LevelDebug})))
@@ -31,6 +31,7 @@ func stateBucketRequests(subjects []string, bucket string) (leaderGets int, othe
// for state keys: one leader message get per record under the prefix, then
// one that finds none, with no direct get and no consumer on the bucket.
func TestStateEnumerationIsLeaderServed(t *testing.T) {
t.Parallel()
srv, js := startServer(t, t.TempDir())
s, err := sqsjs.New(context.Background(), js, testConfig)
require.NoError(t, err)
@@ -80,6 +81,7 @@ func TestStateEnumerationIsLeaderServed(t *testing.T) {
// TestStateEnumerationSkipsGapsAndOtherNamespaces counts through removed
// revisions and interleaved records of neighbouring namespaces.
func TestStateEnumerationSkipsGapsAndOtherNamespaces(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
ctx := context.Background()
q := ns(1, 0, "gap")
@@ -109,6 +111,7 @@ func TestStateEnumerationSkipsGapsAndOtherNamespaces(t *testing.T) {
// TestStateEnumerationMarkers drops a key at its delete or purge marker and
// counts it again when a later write follows.
func TestStateEnumerationMarkers(t *testing.T) {
t.Parallel()
s, js := newStore(t)
ctx := context.Background()
q := ns(0, 0, "mark")
@@ -130,6 +133,7 @@ func TestStateEnumerationMarkers(t *testing.T) {
}
func TestStateEnumerationCancelled(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
q := ns(0, 0, "cancel")
pos := appendTo(t, s, q, "b")
@@ -102,6 +102,7 @@ func (r *apiRecorder) drain(t *testing.T, client *nats.Conn) []string {
// to have returned once its test's cleanups finish, including one left
// blocked on a sentinel nobody drained.
func TestRecorderStopsWithItsTest(t *testing.T) {
// Not parallel: its final checks need both subtests to have finished first.
srv, js := startServer(t, t.TempDir())
var idle, stuck *apiRecorder
t.Run("drained", func(t *testing.T) {
@@ -130,6 +131,7 @@ func TestRecorderStopsWithItsTest(t *testing.T) {
// message get, which only the stream leader answers, never a direct get,
// which a follower may answer from state it has not yet applied.
func TestGetStateIsLeaderServed(t *testing.T) {
t.Parallel()
srv, js := startServer(t, t.TempDir())
s, err := sqsjs.New(context.Background(), js, testConfig)
require.NoError(t, err)
@@ -163,6 +165,7 @@ func TestGetStateIsLeaderServed(t *testing.T) {
// TestGetStateSemantics covers each way a record can be absent, and that a
// present one carries the value and revision its last write returned.
func TestGetStateSemantics(t *testing.T) {
t.Parallel()
s, js := newStore(t)
ctx := context.Background()
q := ns(1, 0, "sem")
@@ -207,6 +210,7 @@ func TestGetStateSemantics(t *testing.T) {
}
func TestGetStateErrors(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
q := ns(0, 0, "errs")
pos := appendTo(t, s, q, "b")
@@ -230,6 +234,7 @@ func TestGetStateErrors(t *testing.T) {
// TestGetStateHonoursAPIPrefix reads state through a JetStream context
// built for a domain and for an explicit prefix.
func TestGetStateHonoursAPIPrefix(t *testing.T) {
t.Parallel()
srv, err := server.NewServer(&server.Options{
Host: "127.0.0.1", Port: -1, JetStream: true, JetStreamDomain: "hub",
StoreDir: t.TempDir(), NoLog: true, NoSigs: true,
@@ -70,12 +70,14 @@ func streamNames(t *testing.T, js jsapi.JetStream) []string {
}
func TestDefaultConfigPerTopology(t *testing.T) {
t.Parallel()
assert.Equal(t, sqsjs.Config{MessageStreams: 16, StateBuckets: 8, DuplicateWindow: sqsjs.DefaultDuplicateWindow}, sqsjs.DefaultConfig(1))
assert.Equal(t, sqsjs.Config{MessageStreams: 8, StateBuckets: 8, DuplicateWindow: sqsjs.DefaultDuplicateWindow}, sqsjs.DefaultConfig(3))
require.NoError(t, sqsjs.DefaultConfig(1).Validate())
}
func TestConfigValidate(t *testing.T) {
t.Parallel()
for name, cfg := range map[string]sqsjs.Config{
"no streams": {StateBuckets: 1, DuplicateWindow: time.Minute},
"no buckets": {MessageStreams: 1, DuplicateWindow: time.Minute},
@@ -91,6 +93,7 @@ func TestConfigValidate(t *testing.T) {
// TestMessageStreamConfigPerReplicaCount covers both topologies' stream shape,
// including R3, which one embedded server cannot host.
func TestMessageStreamConfigPerReplicaCount(t *testing.T) {
t.Parallel()
for nodes, want := range map[int]int{1: 1, 3: 3} {
sc := testConfig.MessageStreamConfig(2, clustersize.ReplicasFor(nodes))
assert.Equal(t, want, sc.Replicas, "%d nodes", nodes)
@@ -105,6 +108,7 @@ func TestMessageStreamConfigPerReplicaCount(t *testing.T) {
}
func TestNamesAndKeys(t *testing.T) {
t.Parallel()
q := ns(5, 1, "q-1")
assert.Equal(t, "sqs.m.5.q-1", sqsjs.Subject(q))
assert.Equal(t, "q.5.q-1.42", sqsjs.StateKey(q, 42))
@@ -114,6 +118,7 @@ func TestNamesAndKeys(t *testing.T) {
// TestNewProvisionsOnlyTheFixedLayout holds ADR-0011:S9: no stream, bucket
// or consumer is created per queue.
func TestNewProvisionsOnlyTheFixedLayout(t *testing.T) {
t.Parallel()
s, js := newStore(t)
want := []string{"KV_sqs_state_0", "KV_sqs_state_1", "SQS_M_0", "SQS_M_1", "SQS_M_2"}
require.Equal(t, want, streamNames(t, js))
@@ -148,6 +153,7 @@ func TestNewProvisionsOnlyTheFixedLayout(t *testing.T) {
}
func TestAppendSubjectPerNamespace(t *testing.T) {
t.Parallel()
s, js := newStore(t)
ctx := context.Background()
a, b, otherShard := ns(1, 0, "qa"), ns(1, 1, "qb"), ns(2, 0, "qa")
@@ -179,6 +185,7 @@ func TestAppendSubjectPerNamespace(t *testing.T) {
}
func TestAppendRejectsBadInput(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
ctx := context.Background()
_, err := s.Append(ctx, ns(0, 0, "a.b"), message.AppendInput{})
@@ -196,6 +203,7 @@ func TestAppendRejectsBadInput(t *testing.T) {
// TestAllocationWithNoResourceFails: an allocation no configuration ever
// provisioned is an error, never a silent create or a not-found.
func TestAllocationWithNoResourceFails(t *testing.T) {
t.Parallel()
s, js := newStore(t)
ctx := context.Background()
_, err := s.Append(ctx, ns(7, 0, "q"), message.AppendInput{Body: []byte("m")})
@@ -209,6 +217,7 @@ func TestAllocationWithNoResourceFails(t *testing.T) {
}
func TestIdempotentAppend(t *testing.T) {
t.Parallel()
s, js := newStore(t)
ctx := context.Background()
q, neighbour := ns(0, 0, "qa"), ns(0, 1, "qb")
@@ -235,6 +244,7 @@ func TestIdempotentAppend(t *testing.T) {
// TestAppendSurvivesRestart: an acknowledged append is on disk, so a server
// started over the same directory still serves it at the same position.
func TestAppendSurvivesRestart(t *testing.T) {
t.Parallel()
dir := t.TempDir()
srv, js := startServer(t, dir)
ctx := context.Background()
@@ -266,6 +276,7 @@ func TestAppendSurvivesRestart(t *testing.T) {
}
func TestStateCompareAndSet(t *testing.T) {
t.Parallel()
s, js := newStore(t)
ctx := context.Background()
q := ns(2, 1, "q")
@@ -301,6 +312,7 @@ func TestStateCompareAndSet(t *testing.T) {
}
func TestStateIsolatedAcrossSharedBucket(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
ctx := context.Background()
// Same queue ID and state bucket, different message shards: a position
@@ -318,6 +330,7 @@ func TestStateIsolatedAcrossSharedBucket(t *testing.T) {
}
func TestConcurrentStateWrites(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
ctx := context.Background()
q := ns(1, 1, "q")
@@ -350,6 +363,7 @@ func TestConcurrentStateWrites(t *testing.T) {
// changed count applies to new queues, and an existing queue keeps the
// stream and bucket its record names.
func TestTopologyTransitionKeepsExistingAllocations(t *testing.T) {
t.Parallel()
_, js := startServer(t, t.TempDir())
ctx := context.Background()
before, err := sqsjs.New(ctx, js, sqsjs.Config{MessageStreams: 3, StateBuckets: 3, DuplicateWindow: time.Minute})
@@ -399,6 +413,7 @@ func TestTopologyTransitionKeepsExistingAllocations(t *testing.T) {
// are supported, so no other size reaches provisioning, which would
// otherwise create R5 on five nodes.
func TestNewRefusesUnsupportedClusterSize(t *testing.T) {
// Not parallel: it redeclares the process-wide cluster size.
_, js := startServer(t, t.TempDir())
for _, nodes := range []int{2, 4, 5} {
t.Run(fmt.Sprintf("%d nodes", nodes), func(t *testing.T) {
@@ -429,6 +444,7 @@ func requireUsage(t *testing.T, s *sqsjs.Store, q message.Namespace, want messag
}
func TestGet(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
ctx := context.Background()
a, b := ns(1, 0, "qa"), ns(1, 1, "qb")
@@ -451,6 +467,7 @@ func TestGet(t *testing.T) {
}
func TestDeleteIsNamespaceScopedAndIdempotent(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
ctx := context.Background()
a, b := ns(0, 0, "qa"), ns(0, 0, "qb")
@@ -470,6 +487,7 @@ func TestDeleteIsNamespaceScopedAndIdempotent(t *testing.T) {
}
func TestDeleteThroughIsSubjectScoped(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
ctx := context.Background()
a, b := ns(2, 1, "qa"), ns(2, 1, "qb")
@@ -505,6 +523,7 @@ func TestDeleteThroughIsSubjectScoped(t *testing.T) {
}
func TestCountExcludesDeletedStateAndNeighbours(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
ctx := context.Background()
// Same stream and state bucket, so only the subject and key prefix
@@ -538,6 +557,7 @@ func TestCountExcludesDeletedStateAndNeighbours(t *testing.T) {
}
func TestDeleteNamespaceRemovesOnlyItsData(t *testing.T) {
t.Parallel()
s, js := newStore(t)
ctx := context.Background()
a, b := ns(0, 1, "qa"), ns(0, 1, "qb")
@@ -567,6 +587,7 @@ func TestDeleteNamespaceRemovesOnlyItsData(t *testing.T) {
// TestUndeclaredAllocationCreatesNothing: every read and cleanup on an
// allocation no configuration provisioned fails and creates no resource.
func TestUndeclaredAllocationCreatesNothing(t *testing.T) {
t.Parallel()
s, js := newStore(t)
ctx := context.Background()
before := streamNames(t, js)
@@ -585,6 +606,7 @@ func TestUndeclaredAllocationCreatesNothing(t *testing.T) {
}
func TestLimits(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
assert.Equal(t, message.Limits{RetentionBackstop: message.MinRetentionBackstop, IdempotencyWindow: testConfig.DuplicateWindow}, s.Limits())
}
@@ -593,6 +615,7 @@ func TestLimits(t *testing.T) {
// real queue's message shard and queue ID but a state shard with no bucket
// must fail before it purges the queue's bodies.
func TestDeleteNamespaceWithUndeclaredStateShardDeletesNothing(t *testing.T) {
t.Parallel()
s, js := newStore(t)
ctx := context.Background()
valid := ns(1, 1, "q")
@@ -651,6 +674,7 @@ func consumerCount(t *testing.T, js jsapi.JetStream, stream string) int {
// with different state shards; each change names its complete namespace and
// only domain metadata.
func TestFeedReconstructsNamespaces(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
ctx := context.Background()
a, b, other := ns(1, 0, "qa"), ns(1, 1, "qb"), ns(2, 1, "qa")
@@ -671,6 +695,7 @@ func TestFeedReconstructsNamespaces(t *testing.T) {
// TestFeedDeliversNoBody: the feed is headers-only; a body never reaches it.
func TestFeedDeliversNoBody(t *testing.T) {
t.Parallel()
s, js := newStore(t)
appendTo(t, s, ns(0, 0, "q"), "body bytes")
f := openFeed(t, s, 0, 0)
@@ -692,6 +717,7 @@ func TestFeedDeliversNoBody(t *testing.T) {
}
func TestFeedOrderResumeAndDeletedOmission(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
ctx := context.Background()
q := ns(0, 1, "q")
@@ -710,6 +736,7 @@ func TestFeedOrderResumeAndDeletedOmission(t *testing.T) {
}
func TestFeedWakesOnLiveAppend(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
q := ns(2, 0, "q")
f := openFeed(t, s, 2, 0)
@@ -731,6 +758,7 @@ func TestFeedWakesOnLiveAppend(t *testing.T) {
}
func TestFeedNextHonoursContext(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
f := openFeed(t, s, 0, 0)
ctx, cancel := context.WithCancel(context.Background())
@@ -743,6 +771,7 @@ func TestFeedNextHonoursContext(t *testing.T) {
}
func TestFeedCloseLeavesNoConsumer(t *testing.T) {
t.Parallel()
s, js := newStore(t)
appendTo(t, s, ns(1, 0, "q"), "x")
f, err := s.Feed(context.Background(), 1, 0)
@@ -754,6 +783,7 @@ func TestFeedCloseLeavesNoConsumer(t *testing.T) {
}
func TestFeedRejectsMalformedStateShardHeader(t *testing.T) {
t.Parallel()
s, js := newStore(t)
ctx := context.Background()
q := ns(0, 0, "q")
@@ -779,6 +809,7 @@ func TestFeedRejectsMalformedStateShardHeader(t *testing.T) {
}
func TestFeedOpensRetainedStreamButCreatesNone(t *testing.T) {
t.Parallel()
dir := t.TempDir()
srv, js := startServer(t, dir)
ctx := context.Background()
@@ -804,6 +835,7 @@ func TestFeedOpensRetainedStreamButCreatesNone(t *testing.T) {
// TestFeedAfterMaximumCursorReplaysNothing: no position follows the maximum
// cursor, so the feed must not wrap to the start of the stream.
func TestFeedAfterMaximumCursorReplaysNothing(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
q := ns(0, 0, "q")
appendTo(t, s, q, "existing")
@@ -825,6 +857,7 @@ func namespaces(t *testing.T, s *sqsjs.Store, shard message.Shard) []message.Nam
// and a neighbouring shard. The store holds no queue records, so every row
// is rebuilt from stored data alone.
func TestNamespacesListsCompleteRows(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
ctx := context.Background()
both, stateOnly, bodyOnly, cleared := ns(1, 1, "both"), ns(1, 0, "state-only"), ns(1, 1, "body-only"), ns(1, 0, "cleared")
@@ -863,6 +896,7 @@ func TestNamespacesListsCompleteRows(t *testing.T) {
// bucket, a state-only queue on a historical bucket and a queue on a
// historical stream are still listed, and listing creates nothing.
func TestNamespacesFindsHistoricalBuckets(t *testing.T) {
t.Parallel()
dir := t.TempDir()
srv, js := startServer(t, dir)
ctx := context.Background()
@@ -894,8 +928,10 @@ func TestNamespacesFindsHistoricalBuckets(t *testing.T) {
// so bodies and state that disagree, or state split across buckets, is
// reported rather than resolved by guessing.
func TestNamespacesRefusesConflictingAllocations(t *testing.T) {
t.Parallel()
ctx := context.Background()
t.Run("BodyAndStateDisagree", func(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
p := appendTo(t, s, ns(1, 0, "q"), "m")
_, err := s.PutState(ctx, ns(1, 1, "q"), p, []byte("s"), 0)
@@ -904,6 +940,7 @@ func TestNamespacesRefusesConflictingAllocations(t *testing.T) {
require.ErrorIs(t, err, sqsjs.ErrCorruptRecord)
})
t.Run("StateInTwoBuckets", func(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
for shard := range message.Shard(2) {
_, err := s.PutState(ctx, ns(1, shard, "q"), 1, []byte("s"), 0)
@@ -913,6 +950,7 @@ func TestNamespacesRefusesConflictingAllocations(t *testing.T) {
require.ErrorIs(t, err, sqsjs.ErrCorruptRecord)
})
t.Run("FirstAndLastBodyDisagree", func(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
appendTo(t, s, ns(1, 0, "q"), "m")
appendTo(t, s, ns(1, 1, "q"), "m")
@@ -920,6 +958,7 @@ func TestNamespacesRefusesConflictingAllocations(t *testing.T) {
require.ErrorIs(t, err, sqsjs.ErrCorruptRecord)
})
t.Run("MalformedStateKey", func(t *testing.T) {
t.Parallel()
s, js := newStore(t)
kv, err := js.KeyValue(ctx, sqsjs.BucketName(0))
require.NoError(t, err)
@@ -933,6 +972,7 @@ func TestNamespacesRefusesConflictingAllocations(t *testing.T) {
// TestNamespacesRefusesAMiddleBodyWithAnotherStateShard: every body is
// checked, not only the first and last.
func TestNamespacesRefusesAMiddleBodyWithAnotherStateShard(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
appendTo(t, s, ns(1, 0, "q"), "first")
appendTo(t, s, ns(1, 1, "q"), "middle")
@@ -943,6 +983,7 @@ func TestNamespacesRefusesAMiddleBodyWithAnotherStateShard(t *testing.T) {
// TestNamespacesLeavesNoConsumer: the enumeration pass removes its consumer.
func TestNamespacesLeavesNoConsumer(t *testing.T) {
t.Parallel()
s, js := newStore(t)
for i := range 600 {
appendTo(t, s, ns(1, 0, fmt.Sprintf("q%d", i%3)), "m")
@@ -959,6 +1000,7 @@ func TestNamespacesLeavesNoConsumer(t *testing.T) {
// configuration's shard might, would let any replica answer with a stale
// body. The store refuses the read rather than serve one.
func TestGetRefusesDirectGetStream(t *testing.T) {
t.Parallel()
ctx := context.Background()
_, js := startServer(t, t.TempDir())
s, err := sqsjs.New(ctx, js, testConfig)
@@ -56,6 +56,7 @@ func requireState(t *testing.T, s message.Store, m swept, want bool) {
}
func TestSweepDeletesOnlyTombstonedBodies(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
ctx := context.Background()
q, neighbour, other := ns(1, 1, "q"), ns(1, 0, "n"), ns(2, 1, "q")
@@ -97,6 +98,7 @@ func (failStateDelete) DeleteState(context.Context, message.Namespace, message.P
}
func TestSweepCrashBetweenBodyAndStateDelete(t *testing.T) {
t.Parallel()
s, _ := newStore(t)
ctx := context.Background()
m := withState(t, s, ns(0, 1, "q"), "deleted")
@@ -24,6 +24,7 @@ func vis(name, handle string, seconds int) message.ChangeVisibilityInput {
}
func TestChangeVisibilityExtendsDeadline(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", map[string]string{"VisibilityTimeout": "30"})
m := e.receiveOne(t, "orders")
@@ -51,6 +52,7 @@ func TestChangeVisibilityExtendsDeadline(t *testing.T) {
// The cap is the issuing receive's time plus 12 hours, inclusive, and does
// not reset when the deadline is extended.
func TestChangeVisibilityTwelveHourCap(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", map[string]string{"VisibilityTimeout": "30"})
m := e.receiveOne(t, "orders")
@@ -75,6 +77,7 @@ func TestChangeVisibilityTwelveHourCap(t *testing.T) {
}
func TestChangeVisibilityRefusesInvalidHandles(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", nil)
e.create(t, acct, "audit", nil)
@@ -96,6 +99,7 @@ func TestChangeVisibilityRefusesInvalidHandles(t *testing.T) {
// A delete landing between the read and the write wins; the change retries,
// finds the tombstone and reports not in flight.
func TestChangeVisibilityRacingDelete(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", nil)
m := e.receiveOne(t, "orders")
@@ -110,6 +114,7 @@ func TestChangeVisibilityRacingDelete(t *testing.T) {
// bounded retries are spent; the record ends at one of the requested
// deadlines with its generation intact.
func TestChangeVisibilityConcurrent(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", nil)
m := e.receiveOne(t, "orders")
@@ -141,7 +146,9 @@ func TestChangeVisibilityConcurrent(t *testing.T) {
}
func TestChangeVisibilityUnsettledOutcomes(t *testing.T) {
t.Parallel()
t.Run("lost reply, applied", func(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
e.create(t, acct, "orders", nil)
m := e.receiveOne(t, "orders")
@@ -149,6 +156,7 @@ func TestChangeVisibilityUnsettledOutcomes(t *testing.T) {
require.ErrorIs(t, e.changer(f).ChangeVisibility(context.Background(), vis("orders", m.ReceiptHandle, 60)), message.ErrUnavailable)
})
t.Run("lost reply, not applied, not resent", func(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", nil)
m := e.receiveOne(t, "orders")
@@ -160,6 +168,7 @@ func TestChangeVisibilityUnsettledOutcomes(t *testing.T) {
assert.Equal(t, before, after)
})
t.Run("state read fails", func(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
e.create(t, acct, "orders", nil)
m := e.receiveOne(t, "orders")
@@ -173,6 +182,7 @@ func TestChangeVisibilityUnsettledOutcomes(t *testing.T) {
"incomplete": `{"v":1,"s":"inflight","c":1,"g":1}`,
} {
t.Run(name, func(t *testing.T) {
t.Parallel()
e := newReceiveEnv(t)
q := e.create(t, acct, "orders", nil)
m := e.receiveOne(t, "orders")
@@ -0,0 +1,89 @@
package accountlifecycle
//test:in-package — the volume reaper and its fake cluster are unexported, and
// these drive its force path against a node that never answers.
// Every test here waits out the fixed describe fan-out deadline a silent node
// costs, two or three times over. This file sorts first so they take parallel
// slots before the package's short tests queue ahead of them.
import (
"testing"
"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// A lone node's NotFound says nothing about the node that stayed silent, so
// the holder is not proven gone and the volume keeps its attachment.
func TestVolumeReaperForceRetainsAVolumeWhenALoneNodeAnswersNotFoundWhileAnotherIsSilent(t *testing.T) {
t.Parallel()
assertForceRetainsTheHeldVolume(t, func(cluster *fakeCluster) {
cluster.fail("ec2.DescribeInstances", awserrors.ErrorInvalidInstanceIDNotFound)
}, []string{fakeClusterNode, "node-2"})
}
// A terminated copy is not proof either while the node that may still run the
// holder has not answered.
func TestVolumeReaperForceRetainsAVolumeWhenATerminatedCopyIsFoundWhileTheHolderNodeIsSilent(t *testing.T) {
t.Parallel()
assertForceRetainsTheHeldVolume(t, func(cluster *fakeCluster) {
cluster.reply("ec2.DescribeTerminatedInstances", instancesIn("terminated", "i-held"))
}, []string{fakeClusterNode, "node-2"})
}
// The holder lives on node-2, which never answers, while node-1 answers every
// fan-out twice. A count of replies reads that as complete and the holder as
// gone; only judging by responder identity sees that node-2 is missing.
func TestVolumeReaperForceRetainsAVolumeWhenOneNodeAnswersTwiceAndTheHolderNodeIsSilent(t *testing.T) {
t.Parallel()
cluster := newFakeCluster(t)
volume := newHeldVolume(cluster, "vol-held", "i-on-silent-node")
cluster.echoAsNode(t, "ec2.DescribeInstances", fakeClusterNode)
reaper := newVolumeReaper(cluster.nc, fakeClusterNode, "node-2")
err := reaper.Delete(silentClusterCtx(t), "000000000042", Resource{Kind: "volume", ID: "vol-held"}, true)
require.Error(t, err)
assert.Contains(t, err.Error(), "vol-held")
assert.Contains(t, err.Error(), "i-on-silent-node")
deleted, holder, detaches := volume.state()
assert.False(t, deleted, "a holder on a silent node is not proven gone")
assert.Equal(t, "i-on-silent-node", holder)
assert.Zero(t, detaches)
assert.NotContains(t, cluster.called(), forceDetachSubject)
}
func TestForcedTeardownKeepsTheAccountWhenOneNodeAnswersTwiceAndTheHolderNodeIsSilent(t *testing.T) {
t.Parallel()
cluster := newFakeCluster(t)
volume := newHeldVolume(cluster, "vol-held", "i-on-silent-node")
cluster.echoAsNode(t, "ec2.DescribeInstances", fakeClusterNode)
accounts := newFakeAccounts()
engine := quietEngine(accounts, newVolumeReaper(cluster.nc, fakeClusterNode, "node-2"))
req := request()
req.Force = true
result, err := engine.Teardown(silentClusterCtx(t), req)
require.ErrorIs(t, err, ErrResourcesStuck)
require.Equal(t, 1, result.StuckCount())
var stuck Stuck
for _, stage := range result.Stages {
if len(stage.Stuck) > 0 {
stuck = stage.Stuck[0]
}
}
assert.Equal(t, "vol-held", stuck.Resource.ID)
assert.Contains(t, stuck.Reason, "vol-held")
assert.Contains(t, stuck.Reason, "i-on-silent-node")
assert.False(t, result.AccountDeleted)
assert.False(t, accounts.deleted)
assert.Equal(t, AccountStatusTerminating, accounts.account.Status)
deleted, holder, detaches := volume.state()
assert.False(t, deleted)
assert.Equal(t, "i-on-silent-node", holder)
assert.Zero(t, detaches)
}
@@ -365,19 +365,12 @@ func TestVolumeReaperForceRetainsAVolumeALiveInstanceHolds(t *testing.T) {
// the volume and its attachment exactly as they were.
func TestVolumeReaperForceRetainsAVolumeWhenTheHolderCannotBeProvenGone(t *testing.T) {
t.Parallel()
twoNodes := []string{fakeClusterNode, "node-2"}
cases := []struct {
name string
setup func(cluster *fakeCluster)
nodes []string
}{
{"configured node set unknown", func(*fakeCluster) {}, []string{}},
{"lone node answers NotFound while another is silent", func(cluster *fakeCluster) {
cluster.fail("ec2.DescribeInstances", awserrors.ErrorInvalidInstanceIDNotFound)
}, twoNodes},
{"terminated copy found while the holder node is silent", func(cluster *fakeCluster) {
cluster.reply("ec2.DescribeTerminatedInstances", instancesIn("terminated", "i-held"))
}, twoNodes},
{"node describe fails", func(cluster *fakeCluster) {
cluster.fail("ec2.DescribeInstances", awserrors.ErrorServerInternal)
}, nil},
@@ -398,27 +391,34 @@ func TestVolumeReaperForceRetainsAVolumeWhenTheHolderCannotBeProvenGone(t *testi
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
cluster := newFakeCluster(t)
volume := newHeldVolume(cluster, "vol-held", "i-held")
tc.setup(cluster)
nodes := tc.nodes
if nodes == nil {
nodes = []string{fakeClusterNode}
}
reaper := newVolumeReaper(cluster.nc, nodes...)
err := reaper.Delete(silentClusterCtx(t), "000000000042", Resource{Kind: "volume", ID: "vol-held"}, true)
require.Error(t, err)
assert.Contains(t, err.Error(), "vol-held")
deleted, holder, detaches := volume.state()
assert.False(t, deleted)
assert.Equal(t, "i-held", holder)
assert.Zero(t, detaches)
assertForceRetainsTheHeldVolume(t, tc.setup, nodes)
})
}
}
// assertForceRetainsTheHeldVolume force-deletes a volume i-held holds and
// asserts the volume and its attachment survive untouched.
func assertForceRetainsTheHeldVolume(t *testing.T, setup func(cluster *fakeCluster), nodes []string) {
t.Helper()
cluster := newFakeCluster(t)
volume := newHeldVolume(cluster, "vol-held", "i-held")
setup(cluster)
reaper := newVolumeReaper(cluster.nc, nodes...)
err := reaper.Delete(silentClusterCtx(t), "000000000042", Resource{Kind: "volume", ID: "vol-held"}, true)
require.Error(t, err)
assert.Contains(t, err.Error(), "vol-held")
deleted, holder, detaches := volume.state()
assert.False(t, deleted)
assert.Equal(t, "i-held", holder)
assert.Zero(t, detaches)
}
// The deadlock force exists for: the instance is gone but its volume record
// still names it, so the ordinary delete refuses forever. Once the holder is
// proven absent the attachment is cleared and the volume deleted.
@@ -935,58 +935,3 @@ func TestEC2ReapersRemoveInterfacesBeforeSubnets(t *testing.T) {
func newVolumeReaper(nc *nats.Conn, nodeIDs ...string) *volumeReaper {
return &volumeReaper{nc: nc, nodeIDs: nodeIDs}
}
// The holder lives on node-2, which never answers, while node-1 answers every
// fan-out twice. A count of replies reads that as complete and the holder as
// gone; only judging by responder identity sees that node-2 is missing.
func TestVolumeReaperForceRetainsAVolumeWhenOneNodeAnswersTwiceAndTheHolderNodeIsSilent(t *testing.T) {
t.Parallel()
cluster := newFakeCluster(t)
volume := newHeldVolume(cluster, "vol-held", "i-on-silent-node")
cluster.echoAsNode(t, "ec2.DescribeInstances", fakeClusterNode)
reaper := newVolumeReaper(cluster.nc, fakeClusterNode, "node-2")
err := reaper.Delete(silentClusterCtx(t), "000000000042", Resource{Kind: "volume", ID: "vol-held"}, true)
require.Error(t, err)
assert.Contains(t, err.Error(), "vol-held")
assert.Contains(t, err.Error(), "i-on-silent-node")
deleted, holder, detaches := volume.state()
assert.False(t, deleted, "a holder on a silent node is not proven gone")
assert.Equal(t, "i-on-silent-node", holder)
assert.Zero(t, detaches)
assert.NotContains(t, cluster.called(), forceDetachSubject)
}
func TestForcedTeardownKeepsTheAccountWhenOneNodeAnswersTwiceAndTheHolderNodeIsSilent(t *testing.T) {
t.Parallel()
cluster := newFakeCluster(t)
volume := newHeldVolume(cluster, "vol-held", "i-on-silent-node")
cluster.echoAsNode(t, "ec2.DescribeInstances", fakeClusterNode)
accounts := newFakeAccounts()
engine := quietEngine(accounts, newVolumeReaper(cluster.nc, fakeClusterNode, "node-2"))
req := request()
req.Force = true
result, err := engine.Teardown(silentClusterCtx(t), req)
require.ErrorIs(t, err, ErrResourcesStuck)
require.Equal(t, 1, result.StuckCount())
var stuck Stuck
for _, stage := range result.Stages {
if len(stage.Stuck) > 0 {
stuck = stage.Stuck[0]
}
}
assert.Equal(t, "vol-held", stuck.Resource.ID)
assert.Contains(t, stuck.Reason, "vol-held")
assert.Contains(t, stuck.Reason, "i-on-silent-node")
assert.False(t, result.AccountDeleted)
assert.False(t, accounts.deleted)
assert.Equal(t, AccountStatusTerminating, accounts.account.Status)
deleted, holder, detaches := volume.state()
assert.False(t, deleted)
assert.Equal(t, "i-on-silent-node", holder)
assert.Zero(t, detaches)
}
@@ -65,6 +65,8 @@ func TestNonFinalizingDeleteEndsExercisedAndALaterDeleteTakesItOver(t *testing.T
_, err = gw.deleteAccount(t.Context(), deleteAccountBody(t, account.AccountID, "tenant@example.com", deleteClientToken))
require.NoError(t, err)
// Parallel only once the teardown is running, so its drain poll overlaps the serial tests.
t.Parallel()
done := awaitState(t, gw, account.AccountID, operatorv1.DeletionStateCompleted)
assert.Equal(t, exercised.OperationID, done.OperationID)
@@ -39,6 +39,8 @@ func TestDeletionRecordCarriesTheIAMQuiescence(t *testing.T) {
_, err := gw.deleteAccount(t.Context(),
deleteAccountBody(t, account.AccountID, "tenant@example.com", deleteClientToken))
require.NoError(t, err)
// Parallel only once the teardown is running, so its drain poll overlaps the serial tests.
t.Parallel()
require.Eventually(t, func() bool {
return describeDeletion(t, gw, account.AccountID).State == operatorv1.DeletionStateCompleted
}, 30*time.Second, 100*time.Millisecond)
@@ -271,7 +271,6 @@ func TestAdminDeleteAccountDryRunChangesNothing(t *testing.T) {
// and holding the call open for it would make every timeout-and-retry start a
// second one.
func TestAdminDeleteAccountReturnsBeforeTheTeardownFinishes(t *testing.T) {
t.Parallel()
gw := newDeleteAccountGateway(t)
account := newTenantAccount(t, gw, "tenant@example.com")
@@ -284,6 +283,8 @@ func TestAdminDeleteAccountReturnsBeforeTheTeardownFinishes(t *testing.T) {
assert.NotEmpty(t, response.DeletionID)
assert.Equal(t, account.AccountID, response.AccountID)
assert.Equal(t, operatorv1.DeletionStateRunning, response.State)
// Parallel only once the teardown is running, so its drain poll overlaps the serial tests.
t.Parallel()
// Progress is polled, so the record has to reach a terminal state on its own.
require.Eventually(t, func() bool {
@@ -303,7 +304,6 @@ func TestAdminDeleteAccountReturnsBeforeTheTeardownFinishes(t *testing.T) {
// The record outlives the account it describes, so a retry that arrives after
// the teardown finished is answered from it rather than as a missing account.
func TestAdminDeleteAccountReplaysAfterTheAccountIsGone(t *testing.T) {
t.Parallel()
gw := newDeleteAccountGateway(t)
account := newTenantAccount(t, gw, "tenant@example.com")
body := deleteAccountBody(t, account.AccountID, "tenant@example.com", deleteClientToken)
@@ -311,6 +311,8 @@ func TestAdminDeleteAccountReplaysAfterTheAccountIsGone(t *testing.T) {
first, err := gw.deleteAccount(t.Context(), body)
require.NoError(t, err)
started := first.(*DeleteAccountResponse)
// Parallel only once the teardown is running, so its drain poll overlaps the serial tests.
t.Parallel()
require.Eventually(t, func() bool {
return describeDeletion(t, gw, account.AccountID).State == operatorv1.DeletionStateCompleted
@@ -83,6 +83,8 @@ func TestDeletionResumerFinishesAnAbandonedJobWithoutARequest(t *testing.T) {
storeDeletionJob(t, gw, abandonedJob(account.AccountID, "tenant@example.com"))
require.Equal(t, 1, gw.resumeDeletions(t.Context(), time.Now().UTC()))
// Parallel only once the teardown is running, so its drain poll overlaps the serial tests.
t.Parallel()
require.Eventually(t, func() bool {
return describeDeletion(t, gw, account.AccountID).State == operatorv1.DeletionStateCompleted
@@ -113,6 +115,8 @@ func TestConcurrentResumersProduceOneTakeover(t *testing.T) {
wg.Wait()
assert.Equal(t, int32(1), resumed.Load())
// Parallel only once the teardown is running, so its drain poll overlaps the serial tests.
t.Parallel()
require.Eventually(t, func() bool {
return describeDeletion(t, gw, account.AccountID).State == operatorv1.DeletionStateCompleted
}, 30*time.Second, 100*time.Millisecond)
+4 -4
View File
@@ -5,10 +5,10 @@ import (
"testing"
)
// TestMain lowers the RSA key size for the whole admin test package. 2048-bit
// keys are still valid and generate several times faster than the 4096-bit
// production default, which dominates the admin suite runtime.
// TestMain lowers the RSA key size for the whole admin test package. 1024 bits
// is the smallest size crypto/rsa generates, and it runs the same certificate
// code as the 4096-bit production default at a fraction of the CPU.
func TestMain(m *testing.M) {
certKeyBits = 2048
certKeyBits = 1024
os.Exit(m.Run())
}
@@ -205,6 +205,8 @@ func TestNetworkTestFailsARunWhoseServerNeverAnswers(t *testing.T) {
started, err := d.StartNetworkTest(ctx, map[string]string{"Plane": "lan", "Source": "a", "Target": "b", "DurationSeconds": "5"})
require.NoError(t, err)
// Parallel only once the test is running, so its serve timeout overlaps the serial tests.
t.Parallel()
var done NetworkTest
require.Eventually(t, func() bool {
+3 -1
View File
@@ -16,6 +16,7 @@ import (
)
func TestGetStorageStatus_Success(t *testing.T) {
t.Parallel()
_, nc := startEmbeddedNATS(t)
// Mock daemon returning storage config
@@ -98,8 +99,9 @@ func TestApplyMetaStatus_NoLeaderStaysHealthy(t *testing.T) {
}
func TestQueryMetaNodeStatus_Unreachable(t *testing.T) {
t.Parallel()
out := &MetaNodeStatus{ID: 1, Host: "127.0.0.1", Port: 1} // nothing listens here
ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond)
ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
defer cancel()
queryMetaNodeStatus(ctx, out, nil, "127.0.0.1", 1)
+53
View File
@@ -31,6 +31,7 @@ import (
"github.com/mulgadc/bluebottle/pkg/safecast"
"github.com/mulgadc/spinifex/spinifex/bootstrap/config"
"github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata"
ebsobligation "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/obligation"
ec2image "github.com/mulgadc/spinifex/spinifex/domains/ec2/image"
"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
ec2vpc "github.com/mulgadc/spinifex/spinifex/domains/ec2/vpc"
@@ -249,6 +250,18 @@ the backend. This is the list of volumes where that would cost something.`,
Run: runVolumesUnsealedCmd,
}
var volumesObligationsCmd = &cobra.Command{
Use: "obligations",
Short: "Report volume exports a cleared attachment left on a node",
Long: `List the open unpublish obligations: exports a force detach, or a terminate
whose unmount did not complete, left on the node that held the attachment.
The node named settles each one once it answers. An obligation stays open
while its node is down; it closes without the node only once the node has
been removed from the cluster. This command only reports.`,
Run: runVolumesObligationsCmd,
}
var accountCmd = &cobra.Command{
Use: "account",
Short: "Manage Spinifex accounts",
@@ -353,6 +366,7 @@ func init() {
adminCmd.AddCommand(volumesCmd)
volumesCmd.AddCommand(volumesOrphansCmd)
volumesCmd.AddCommand(volumesUnsealedCmd)
volumesCmd.AddCommand(volumesObligationsCmd)
adminCmd.AddCommand(accountCmd)
accountCmd.AddCommand(accountCreateCmd)
@@ -4158,6 +4172,45 @@ func printChecksumError(w io.Writer, imageFile, imageName string, image imagecat
fmt.Fprintf(w, " spx admin images import --name %s --force\n", imageName)
}
func runVolumesObligationsCmd(_ *cobra.Command, _ []string) {
_, nc, err := loadConfigAndConnect()
if err != nil {
fmt.Fprintf(os.Stderr, "Could not connect to the cluster: %v\n", err)
os.Exit(1)
}
defer nc.Close()
open, err := ebsobligation.ListOpen(context.Background(), nc)
if err != nil {
fmt.Fprintln(os.Stderr, "Could not read the unpublish obligations:", err)
os.Exit(1)
}
printUnpublishObligations(os.Stdout, open, time.Now())
}
// printUnpublishObligations writes the open obligations, oldest first, with
// the node each waits on and its age.
func printUnpublishObligations(w io.Writer, open []ebsobligation.UnpublishObligation, now time.Time) {
if len(open) == 0 {
fmt.Fprintln(w, "No open unpublish obligations: no node holds an export of a cleared attachment.")
return
}
sort.Slice(open, func(i, j int) bool { return open[i].RecordedAt.Before(open[j].RecordedAt) })
fmt.Fprintf(w, "%d open unpublish obligation(s):\n\n", len(open))
for _, o := range open {
fmt.Fprintf(w, " %s\n", o.VolumeID)
fmt.Fprintf(w, " node: %s\n", o.NodeID)
fmt.Fprintf(w, " instance: %s\n", o.InstanceID)
fmt.Fprintf(w, " age: %s\n", now.Sub(o.RecordedAt).Truncate(time.Second))
if o.Attempts > 0 {
fmt.Fprintf(w, " attempts: %d\n", o.Attempts)
}
if o.Error != "" {
fmt.Fprintf(w, " error: %s\n", o.Error)
}
}
}
func runVolumesUnsealedCmd(_ *cobra.Command, _ []string) {
_, nc, err := loadConfigAndConnect()
if err != nil {
@@ -0,0 +1,37 @@
package cli
import (
"bytes"
"strings"
"testing"
"time"
ebsobligation "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/obligation"
"github.com/stretchr/testify/assert"
)
func TestPrintUnpublishObligations_OldestFirstWithFailures(t *testing.T) {
now := time.Unix(1_700_010_000, 0)
var out bytes.Buffer
printUnpublishObligations(&out, []ebsobligation.UnpublishObligation{
{VolumeID: "vol-newer", NodeID: "node-b", InstanceID: "i-2", RecordedAt: now.Add(-90 * time.Second)},
{VolumeID: "vol-older", NodeID: "node-a", InstanceID: "i-1", RecordedAt: now.Add(-2 * time.Hour),
Attempts: 3, Error: "seal refused"},
}, now)
text := out.String()
assert.Contains(t, text, "2 open unpublish obligation(s)")
assert.Less(t, strings.Index(text, "vol-older"), strings.Index(text, "vol-newer"), "the longest-open obligation is listed first")
assert.Contains(t, text, "node: node-a")
assert.Contains(t, text, "instance: i-1")
assert.Contains(t, text, "age: 2h0m0s")
assert.Contains(t, text, "attempts: 3")
assert.Contains(t, text, "error: seal refused")
assert.Equal(t, 1, strings.Count(text, "attempts:"), "an obligation that never failed shows no attempts")
}
func TestPrintUnpublishObligations_None(t *testing.T) {
var out bytes.Buffer
printUnpublishObligations(&out, nil, time.Now())
assert.Contains(t, out.String(), "No open unpublish obligations")
}
+1
View File
@@ -24,6 +24,7 @@ Single source of truth for the current schema version of every config file Spini
| `spinifex-cluster-config` | `1` | `daemon.ClusterConfigBucketVersion` |
| `spinifex-terminated-instances` | `3` | `daemon.TerminatedInstanceBucketVersion` |
| `spinifex-uefi-stores` | `1` | `uefistore.BucketVersion` |
| `spinifex-ebs-unpublish-obligations` | `1` | `ebsobligation.BucketVersion` |
## Registered migrations
@@ -173,6 +173,8 @@ func runPrefix() string {
// TestViperblockdConformance judges the provider handlers by the same suite
// MemoryProvider and qemunbdd answer to.
func TestViperblockdConformance(t *testing.T) {
t.Parallel()
h := inProcessProvider(t)
conformance.RunSuiteWithConfig(t, h.newProvider, conformance.SuiteConfig{
NamePrefix: runPrefix(),
@@ -188,6 +190,8 @@ func TestViperblockdConformance(t *testing.T) {
// cannot: whether the nbdkit export viperblockd publishes is usable by libnbd,
// which knows nothing about this codebase.
func TestViperblockdNBDClient(t *testing.T) {
t.Parallel()
h := inProcessProvider(t)
conformance.RunNBDClientSuiteWithConfig(t, h.newProvider, conformance.NBDClientConfig{
NodeID: h.nodeID,
@@ -18,6 +18,8 @@ import (
// local copy over the backend, so a leftover here answers describe as if the
// volume still existed after another node deleted it.
func TestProviderHandlers_CreateVolume_LeavesNoLocalState(t *testing.T) {
t.Parallel()
fixture := testpredastore.Start(t)
_, natsURL := setupEmbeddedNATS(t)
@@ -24,6 +24,8 @@ import (
// that is gone. Returning its URI is what makes a volume permanently
// unstartable: every caller then waits for a socket nothing will create.
func TestMountEntryIsStaleWhenSocketIsGone(t *testing.T) {
t.Parallel()
socket := filepath.Join(t.TempDir(), "nbd.sock")
stale, why := mountEntryIsStale(MountedVolume{Name: "vol-1", Socket: socket, PID: os.Getpid()})
@@ -38,6 +40,8 @@ func TestMountEntryIsStaleWhenSocketIsGone(t *testing.T) {
// The pid is the second piece of positive evidence, for an export whose socket
// file outlived the process that owned it.
func TestMountEntryIsStaleWhenProcessIsGone(t *testing.T) {
t.Parallel()
socket := filepath.Join(t.TempDir(), "nbd.sock")
require.NoError(t, os.WriteFile(socket, nil, 0o600))
@@ -53,6 +57,8 @@ func TestMountEntryIsStaleWhenProcessIsGone(t *testing.T) {
// releaseStaleMount must drop the entry so the remount behind it can proceed,
// and must be safe to call for a volume that is not mounted at all.
func TestReleaseStaleMountDropsTheEntry(t *testing.T) {
t.Parallel()
cfg := &Config{MountedVolumes: []MountedVolume{{Name: "vol-1"}, {Name: "vol-2"}}}
require.NoError(t, releaseStaleMount(t.Context(), cfg, "vol-1"))
@@ -91,6 +97,8 @@ func orphanedConfig(t *testing.T, volumeName string, pid int) *Config {
// went, so a remount beside it is a second writer. It has to be stopped, and
// its copy sealed as any unmount would, before the volume is opened again.
func TestMountVolume_SocketGoneProcessAliveIsStoppedBeforeRemount(t *testing.T) {
t.Parallel()
const volumeName = "vol-orphanalive1"
cfg := orphanedConfig(t, volumeName, 0)
pid := cfg.MountedVolumes[0].PID
@@ -114,6 +122,8 @@ func TestMountVolume_SocketGoneProcessAliveIsStoppedBeforeRemount(t *testing.T)
// An nbdkit that will not stop is still a writer, so the publish is refused as
// retry-later and the entry kept for the next attempt to retry the stop.
func TestMountVolume_SocketGoneProcessUnstoppableIsUnavailable(t *testing.T) {
t.Parallel()
const volumeName = "vol-orphanstuck1"
cfg := orphanedConfig(t, volumeName, zombiePID(t))
cfg.exportStopWait = 50 * time.Millisecond
@@ -16,6 +16,8 @@ import (
// wrong with this node — so the export would be renewed indefinitely and the
// instance would be stopped here and unable to start anywhere.
func TestVolumeAbandon_GivesUpTheExportAndTheLease(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-abandoned"
@@ -39,6 +41,8 @@ func TestVolumeAbandon_GivesUpTheExportAndTheLease(t *testing.T) {
// out the TTL. Here the node still holds it and hands it back, so the winner can
// open the volume on its next attempt.
func TestVolumeAbandon_ReleasesTheKeySoTheNewOwnerNeedNotWaitOutTheTTL(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-abandonreleases"
@@ -57,6 +61,8 @@ func TestVolumeAbandon_ReleasesTheKeySoTheNewOwnerNeedNotWaitOutTheTTL(t *testin
// The one thing it must never do, for the fence's reason: this node's block map
// is behind the node that took the volume, and sealing publishes it over theirs.
func TestVolumeAbandon_DoesNotSeal(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-abandonnoseal"
@@ -75,6 +81,8 @@ func TestVolumeAbandon_DoesNotSeal(t *testing.T) {
// Reporting success on a kill that did not take would tell the caller a writer
// stopped that is still running, which is worse than refusing.
func TestVolumeAbandon_KillFailureIsReportedAndLeavesTheVolumeMounted(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-abandonkillfails"
@@ -100,6 +108,8 @@ func TestVolumeAbandon_KillFailureIsReportedAndLeavesTheVolumeMounted(t *testing
// most of them were never exported here — so this is not a failure, and treating
// it as one would make every recovery look broken.
func TestVolumeAbandon_NotExportedHereIsNotAnError(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
cfg := &Config{
@@ -117,6 +127,8 @@ func TestVolumeAbandon_NotExportedHereIsNotAnError(t *testing.T) {
// it over when it opens the volume, so clearing it here would erase the only
// note that writes were lost.
func TestVolumeAbandon_LeavesTheDirtyMarkerToTheNewOwner(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-abandonkeepsmarker"
@@ -31,6 +31,8 @@ func newTestDirty(t *testing.T, natsURL, owner string) *volumeDirty {
// built for. A node killed mid-write never reaches its seal, so a marker written
// only when a seal fails is absent in exactly the case it exists to describe.
func TestVolumeDirty_MarkedOnOpenNotOnSealFailure(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-dirtymarkedonopen"
@@ -53,6 +55,8 @@ func TestVolumeDirty_MarkedOnOpenNotOnSealFailure(t *testing.T) {
// that node has failed its window, so refusing here would leave the instance
// unable to run anywhere rather than running with a slightly older copy.
func TestVolumeDirty_DoesNotPinAVolumeToAGoneNode(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-dirtytakeoverallowed"
@@ -75,6 +79,8 @@ func TestVolumeDirty_DoesNotPinAVolumeToAGoneNode(t *testing.T) {
// takeover silently loses whatever node-a never sealed, so the marker has to
// carry who held it and why for the operator who reads it afterwards.
func TestVolumeDirty_TakeoverNamesThePreviousHolder(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-dirtytakeoverrecorded"
@@ -102,6 +108,8 @@ func TestVolumeDirty_TakeoverNamesThePreviousHolder(t *testing.T) {
// about itself. Reopening a volume this node already holds is the ordinary
// restart path, not a lost-writes event.
func TestVolumeDirty_OwnerReopeningIsNotATakeover(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-dirtyownerreopens"
@@ -125,6 +133,8 @@ func TestVolumeDirty_OwnerReopeningIsNotATakeover(t *testing.T) {
// one-way door: once the writes reach the backend the volume is clean again and
// carries no placement preference.
func TestVolumeDirty_ClearedAfterASuccessfulSeal(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-dirtyclearsonseal"
@@ -141,6 +151,8 @@ func TestVolumeDirty_ClearedAfterASuccessfulSeal(t *testing.T) {
// TestVolumeDirty_ClearIsIdempotent covers the ordinary path, where every
// successful unmount clears a marker that may already be gone.
func TestVolumeDirty_ClearIsIdempotent(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
dirty := newTestDirty(t, natsURL, "node-a")
@@ -153,6 +165,8 @@ func TestVolumeDirty_ClearIsIdempotent(t *testing.T) {
// state to seal, and its seal says nothing about the copy that moved on —
// clearing there would erase the record that the winner's writes are unsealed.
func TestVolumeDirty_ClearDeclinesAMarkerAnotherNodeOwns(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-dirtyclearnotours"
@@ -173,6 +187,8 @@ func TestVolumeDirty_ClearDeclinesAMarkerAnotherNodeOwns(t *testing.T) {
// node reopening a volume it already held. The stale export's clear must not
// remove the marker the newer one wrote.
func TestVolumeDirty_ClearDeclinesAnOlderGenerationOfTheSameNode(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-dirtyclearoldgen"
@@ -191,6 +207,8 @@ func TestVolumeDirty_ClearDeclinesAnOlderGenerationOfTheSameNode(t *testing.T) {
// renaming itself as the current copy. Placement reads this marker, so an
// overwrite here would point a later takeover at the stale node.
func TestVolumeDirty_MarkRefusesToOverwriteALaterGeneration(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-dirtymarkstale"
@@ -212,6 +230,8 @@ func TestVolumeDirty_MarkRefusesToOverwriteALaterGeneration(t *testing.T) {
// nothing records, so a later takeover could not tell they existed — and the
// lease has to go back, or the volume is stranded on a node not writing it.
func TestVolumeDirty_OpenFailsWhenTheMarkerCannotBeWritten(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-dirtymarkunwritable"
@@ -239,6 +259,8 @@ func TestVolumeDirty_OpenFailsWhenTheMarkerCannotBeWritten(t *testing.T) {
// every mount because the bucket is missing is a worse failure than the one
// being described.
func TestVolumeDirty_NoStoreDoesNotBlockMounts(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
cfg := &Config{leases: newTestLeases(t, natsURL, "node-b")}
@@ -252,6 +274,8 @@ func TestVolumeDirty_NoStoreDoesNotBlockMounts(t *testing.T) {
// 'spx admin volumes unsealed', which is the only way to see a volume whose
// writes are stranded on a node that is not coming back.
func TestVolumeDirty_ListReportsTheHolder(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
nc, err := nats.Connect(natsURL)
@@ -272,6 +296,8 @@ func TestVolumeDirty_ListReportsTheHolder(t *testing.T) {
// A node answers "do you hold unsealed writes" for exactly the volumes whose
// marker names it, over the subject a start on another node reaches it by.
func TestHoldsUnsealedWrites_AnswersOverTheNodeSubject(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
nc, err := nats.Connect(natsURL)
require.NoError(t, err)
@@ -299,6 +325,8 @@ func TestHoldsUnsealedWrites_AnswersOverTheNodeSubject(t *testing.T) {
// A node that cannot read its markers must say so rather than answer none,
// because the answer decides whether another node may open the volume.
func TestHoldsUnsealedWrites_UnreadableMarkerIsAnError(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
dirty := newTestDirty(t, natsURL, "node-a")
cfg := &Config{NodeName: "node-a", dirty: dirty}
@@ -73,6 +73,8 @@ func hangingFenceConfig(t *testing.T, natsURL, volumeName string) (*Config, *han
// before the server can re-grant the entry, so nbdkit has to be dead inside that
// margin even when the read naming the winner never comes back.
func TestS4_FenceStopsTheWriterBeforeReadingTheHolder(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-fencekillfirst"
@@ -105,6 +107,8 @@ func TestS4_FenceStopsTheWriterBeforeReadingTheHolder(t *testing.T) {
// after the kill. The holder read only names the winner, and the daemon's notice
// waits behind it, so a store that never answers must not hold the guest up.
func TestS4_FenceHolderReadIsBounded(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-fencereadbounded"
@@ -170,6 +174,8 @@ func noticeConfig(t *testing.T, natsURL, volumeName string) (*Config, *nats.Conn
// its disk, and a node's NATS restarting mid-fence leaves nobody subscribed for
// the first send. It has to arrive once the daemon is back, however late.
func TestS4_FenceNoticeSurvivesAMissedDelivery(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-fencenoticelate"
@@ -213,6 +219,8 @@ func TestS4_FenceNoticeSurvivesAMissedDelivery(t *testing.T) {
// exported here again it belongs to a newer mount, and a notice that finally
// lands then would stop the guest that mount is serving.
func TestS4_StaleFenceNoticeIsDropped(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-fencenoticestale"
@@ -240,6 +248,8 @@ func TestS4_StaleFenceNoticeIsDropped(t *testing.T) {
// TestS4_FenceNoticeStopsWithTheConnection keeps a shutting-down viperblockd from
// leaving a notice goroutine spinning on a connection that cannot carry it.
func TestS4_FenceNoticeStopsWithTheConnection(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-fencenoticeclosed"
@@ -261,6 +271,8 @@ func TestS4_FenceNoticeStopsWithTheConnection(t *testing.T) {
// TestFenceNotice_UnavailableDaemonIsAskedAgain keeps a refusal that says
// "not now" from ending delivery: the guest is still running on a dead disk.
func TestFenceNotice_UnavailableDaemonIsAskedAgain(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-fencenoticeunavailable"
@@ -299,6 +311,8 @@ func TestFenceNotice_UnavailableDaemonIsAskedAgain(t *testing.T) {
// TestFenceNotice_UnsendableNoticeIsNotRetried stops a notice that can never
// be addressed from spinning until shutdown on a goroutine nobody watches.
func TestFenceNotice_UnsendableNoticeIsNotRetried(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-fencenoticeunsendable"
@@ -318,6 +332,8 @@ func TestFenceNotice_UnsendableNoticeIsNotRetried(t *testing.T) {
// TestFenceNotice_RefusedNoticeIsNotRepeated keeps a notice the daemon heard and
// could not act on from being resent forever: a refusal is an answer.
func TestFenceNotice_RefusedNoticeIsNotRepeated(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-fencenoticerefused"
@@ -87,6 +87,8 @@ func withLocalState(t *testing.T, cfg *Config, volumeName string) {
// only way to stop being one without a backend that can refuse is to give up
// the export.
func TestVolumeFence_TearsDownWhenAnotherNodeHoldsTheLease(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-fencelost"
@@ -114,6 +116,8 @@ func TestVolumeFence_TearsDownWhenAnotherNodeHoldsTheLease(t *testing.T) {
// it is proof this node cannot tell. Stopping one guest is the cheaper error
// than leaving two nodes writing one volume.
func TestVolumeFence_FencesAnEntryNobodyHolds(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-fencelapsed"
@@ -134,6 +138,8 @@ func TestVolumeFence_FencesAnEntryNobodyHolds(t *testing.T) {
// not take would tell the cluster a writer stopped that is still running, which
// is worse than not fencing at all.
func TestVolumeFence_KillFailureLeavesTheVolumeMounted(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-fencekillfails"
@@ -168,6 +174,8 @@ func TestVolumeFence_KillFailureLeavesTheVolumeMounted(t *testing.T) {
// the lease is held by a node that is no longer writing and the volume cannot
// start anywhere.
func TestVolumeFence_CompletesOnceTheWriterFinallyExits(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-fencekilllate"
@@ -205,6 +213,8 @@ func TestVolumeFence_CompletesOnceTheWriterFinallyExits(t *testing.T) {
// rather than ignored. Nothing then says what became of the writer, so treating
// the PID's absence as an exit would release the lease on a guess.
func TestVolumeFence_DoesNotCompleteForAWriterItCannotSee(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-fencekillrefused"
@@ -230,6 +240,8 @@ func TestVolumeFence_DoesNotCompleteForAWriterItCannotSee(t *testing.T) {
// never do. This node's copy is the stale one, so sealing it would publish an
// older state over the winner's — the corruption the fence exists to prevent.
func TestVolumeFence_DoesNotSealOnTheWayOut(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-fencenoseal"
@@ -247,6 +259,8 @@ func TestVolumeFence_DoesNotSealOnTheWayOut(t *testing.T) {
// reads afterwards. The winner takes the marker over when it opens the volume,
// so a fencing node clearing it would erase the only note that writes were lost.
func TestVolumeFence_LeavesTheDirtyMarkerToTheWinner(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-fencekeepsmarker"
@@ -263,6 +277,8 @@ func TestVolumeFence_LeavesTheDirtyMarkerToTheWinner(t *testing.T) {
// TestVolumeFence_AlreadyUnmountedIsNotAnError covers the ordinary race, where
// a volume is unmounted normally between losing its lease and the fence running.
func TestVolumeFence_AlreadyUnmountedIsNotAnError(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
cfg := &Config{
@@ -280,6 +296,8 @@ func TestVolumeFence_AlreadyUnmountedIsNotAnError(t *testing.T) {
// would publish a state torn between two writers. The entry has to stay
// mounted so a retry re-attempts both.
func TestUnmountVolume_RefusesToSealUnderALiveWriter(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-unmountkillfails"
@@ -110,6 +110,8 @@ func stopRenewals(lease *volumeLease) {
// when it applies the create, so validity measured from a late reply outlives
// the entry and lets a holder keep writing after a peer has taken the volume.
func TestVolumeLease_SlowAcquireAckDoesNotExtendValidity(t *testing.T) {
t.Parallel()
leases, kv := newSlowAckLeases(t, "node-a")
kv.createDelay = 30 * time.Second
kv.failUpdates = true // the generation publish never lands, so only the create proves the lease
@@ -126,6 +128,8 @@ func TestVolumeLease_SlowAcquireAckDoesNotExtendValidity(t *testing.T) {
// whole validity window cannot authorize an open, because the server may
// already have expired the entry and granted it to a peer.
func TestVolumeLease_AcquireAckedAfterValidityIsRefused(t *testing.T) {
t.Parallel()
leases, kv := newSlowAckLeases(t, "node-a")
kv.createDelay = volumeLeaseValidity + time.Second
kv.failUpdates = true
@@ -144,6 +148,8 @@ func TestVolumeLease_AcquireAckedAfterValidityIsRefused(t *testing.T) {
// conditioned on the create's revision, so its success proves the entry is
// still ours as of when it was sent, however late the create's reply was.
func TestVolumeLease_GenerationPublishRenewsALateAcquisition(t *testing.T) {
t.Parallel()
leases, kv := newSlowAckLeases(t, "node-a")
kv.createDelay = volumeLeaseValidity + time.Second
@@ -159,6 +165,8 @@ func TestVolumeLease_GenerationPublishRenewsALateAcquisition(t *testing.T) {
// TestVolumeLease_SlowRenewalAckDoesNotExtendValidity is the renewal half of
// the slow-reply overlap.
func TestVolumeLease_SlowRenewalAckDoesNotExtendValidity(t *testing.T) {
t.Parallel()
leases, kv := newSlowAckLeases(t, "node-a")
lease, err := leases.acquire(t.Context(), "vol-slowrenewalack")
@@ -176,6 +184,8 @@ func TestVolumeLease_SlowRenewalAckDoesNotExtendValidity(t *testing.T) {
// export is already being fenced and a peer may hold the volume, so a renewal
// that lands afterwards must not make the holder valid again.
func TestVolumeLease_RenewalSucceedingAfterSurrenderDoesNotReviveTheLease(t *testing.T) {
t.Parallel()
leases, kv := newSlowAckLeases(t, "node-a")
lease, err := leases.acquire(t.Context(), "vol-renewalaftersurrender")
@@ -206,6 +216,8 @@ func TestVolumeLease_RenewalSucceedingAfterSurrenderDoesNotReviveTheLease(t *tes
// interval equals the margin, so waiting for the next tick to notice a lapse
// can reach the server's expiry and overlap the next holder.
func TestVolumeLease_LapseIsFencedAtTheDeadlineNotTheNextCheck(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
leases := newTestLeases(t, natsURL, "node-a")
leases.checkEvery = time.Hour
@@ -237,7 +249,11 @@ func TestVolumeLease_LapseIsFencedAtTheDeadlineNotTheNextCheck(t *testing.T) {
// fence releases it, and handing it to another open in between would
// authorize writes the lease no longer covers.
func TestVolumeLease_LostOrLapsedLeaseIsNotShared(t *testing.T) {
t.Parallel()
t.Run("lost", func(t *testing.T) {
t.Parallel()
leases, _ := newSlowAckLeases(t, "node-a")
lease, err := leases.acquire(t.Context(), "vol-sharelost")
require.NoError(t, err)
@@ -252,6 +268,8 @@ func TestVolumeLease_LostOrLapsedLeaseIsNotShared(t *testing.T) {
assert.Equal(t, 1, lease.refs)
})
t.Run("lapsed", func(t *testing.T) {
t.Parallel()
leases, kv := newSlowAckLeases(t, "node-a")
lease, err := leases.acquire(t.Context(), "vol-sharelapsed")
require.NoError(t, err)
@@ -272,6 +290,8 @@ func TestVolumeLease_LostOrLapsedLeaseIsNotShared(t *testing.T) {
// deadline timer has run. The holder must give the volume up rather than let
// the new confirmation hide the lapse from the timer.
func TestVolumeLease_RenewalAckedAfterTheDeadlineDoesNotRevive(t *testing.T) {
t.Parallel()
leases, kv := newSlowAckLeases(t, "node-a")
var fences []leaseLossKind
var fenceMu sync.Mutex
@@ -324,6 +344,8 @@ func TestVolumeLease_RenewalAckedAfterTheDeadlineDoesNotRevive(t *testing.T) {
// TestVolumeLease_RenewalAckedJustBeforeTheDeadlineKeepsTheLease is the
// boundary: a reply that lands while validity still holds is a real renewal.
func TestVolumeLease_RenewalAckedJustBeforeTheDeadlineKeepsTheLease(t *testing.T) {
t.Parallel()
leases, kv := newSlowAckLeases(t, "node-a")
lease, err := leases.acquire(t.Context(), "vol-renewalbeforedeadline")
require.NoError(t, err)
@@ -34,6 +34,8 @@ func newTestLeases(t *testing.T, natsURL, owner string) *volumeLeases {
// engines on one encrypted volume issue overlapping AES-GCM nonces, so the
// second claimant losing is the whole point of the lease.
func TestVolumeLease_SecondNodeIsRefused(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
first := newTestLeases(t, natsURL, "node-a")
@@ -53,6 +55,8 @@ func TestVolumeLease_SecondNodeIsRefused(t *testing.T) {
// one-way door: a volume detached from one node has to be attachable on the
// next, which is the ordinary EBS lifecycle.
func TestVolumeLease_ReleasedVolumeIsClaimableAgain(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
first := newTestLeases(t, natsURL, "node-a")
@@ -72,6 +76,8 @@ func TestVolumeLease_ReleasedVolumeIsClaimableAgain(t *testing.T) {
// opens a detached engine while the mount that is being torn down still holds
// the lease. Refusing itself there would fail every seal.
func TestVolumeLease_RepeatAcquireOnOneNodeShares(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
leases := newTestLeases(t, natsURL, "node-a")
@@ -98,6 +104,8 @@ func TestVolumeLease_RepeatAcquireOnOneNodeShares(t *testing.T) {
// nbdkit exports outlive the daemon, so recovery has to re-adopt them; waiting
// out the TTL would leave live exports untracked.
func TestVolumeLease_NodeReclaimsItsOwnStaleEntry(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-leaserestart01"
@@ -119,6 +127,8 @@ func TestVolumeLease_NodeReclaimsItsOwnStaleEntry(t *testing.T) {
// A daemon that cannot reach JetStream cannot establish that it is the only
// opener, and opening blind is what the lease exists to prevent.
func TestVolumeLease_OpenRefusesWithoutALeaseStore(t *testing.T) {
t.Parallel()
cfg := &Config{NodeName: "test-node"}
lease, err := cfg.acquireVolumeLease(t.Context(), "vol-nostore00000001")
@@ -130,6 +140,8 @@ func TestVolumeLease_OpenRefusesWithoutALeaseStore(t *testing.T) {
// snapshot request that lands on a node without the volume mounted must not
// open a second engine over storage another node is writing.
func TestVolumeLease_UnmountedSnapshotIsRefusedByTheHolder(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
const volumeName = "vol-snapshotexcl01"
@@ -152,6 +164,8 @@ func TestVolumeLease_UnmountedSnapshotIsRefusedByTheHolder(t *testing.T) {
// fail-closed default on the snapshot path specifically. A daemon that cannot
// reach JetStream cannot establish that it is the only opener.
func TestVolumeLease_UnmountedSnapshotRefusesWithoutALeaseStore(t *testing.T) {
t.Parallel()
cfg := &Config{NodeName: "node-a", BaseDir: t.TempDir()}
snapshot, err := snapshotVolumeEngine(t.Context(), cfg, "vol-snapshotnostor", "snap-nostore00000001")
@@ -230,11 +244,15 @@ func leaseEntry(t *testing.T, owner string, generation uint64) []byte {
// has been taken, must mark the lease lost rather than shrug it off as
// transient and keep renewing over whoever now holds the volume.
func TestVolumeLease_RenewalConflictLosesTheLeaseToARealPeer(t *testing.T) {
t.Parallel()
for name, code := range map[string]jetstream.ErrorCode{
"single replica": jetstream.JSErrCodeStreamWrongLastSequence,
"replicated": jetstream.JSErrCodeStreamWrongLastSequenceConstant,
} {
t.Run(name, func(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
leases := newTestLeases(t, natsURL, "node-a")
@@ -265,6 +283,8 @@ func TestVolumeLease_RenewalConflictLosesTheLeaseToARealPeer(t *testing.T) {
// ten-second JetStream stall on prod destroyed four running guests this way,
// each fenced with winner=unknown against a peer that never existed.
func TestVolumeLease_RenewalConflictOnThisNodesOwnWriteIsNotATakeover(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
leases := newTestLeases(t, natsURL, "node-a")
@@ -310,6 +330,8 @@ func TestVolumeLease_RenewalConflictOnThisNodesOwnWriteIsNotATakeover(t *testing
// reclaimed the volume through takeOver. The owner matches and the lease is
// still superseded, so the generation is what decides it.
func TestVolumeLease_RenewalConflictOnAnOlderGenerationIsATakeover(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
leases := newTestLeases(t, natsURL, "node-a")
@@ -336,6 +358,8 @@ func TestVolumeLease_RenewalConflictOnAnOlderGenerationIsATakeover(t *testing.T)
// nothing, so it must not fence — and it must not silently extend the lease
// either. Local validity is the bound that depends on reading nothing.
func TestVolumeLease_RenewalConflictWithAnUnreadableEntryKeepsTheLease(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
leases := newTestLeases(t, natsURL, "node-a")
@@ -364,6 +388,8 @@ func TestVolumeLease_RenewalConflictWithAnUnreadableEntryKeepsTheLease(t *testin
// gone rather than changed. There is nothing to re-read and nothing to adopt,
// and the server may already have granted it to somebody else.
func TestVolumeLease_RenewalAgainstAMissingEntryIsATakeover(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
leases := newTestLeases(t, natsURL, "node-a")
@@ -395,6 +421,8 @@ func TestVolumeLease_RenewalAgainstAMissingEntryIsATakeover(t *testing.T) {
// rule, that an unnameable winner never fences, would remove the second and
// leave a partitioned node writing for as long as it stayed partitioned.
func TestVolumeLease_FencingRule(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
leases := newTestLeases(t, natsURL, "node-a")
@@ -427,6 +455,8 @@ func TestVolumeLease_FencingRule(t *testing.T) {
// cannot address another volume's entry: "." and ">" are JetStream subject
// tokens, and a name carrying them would claim or read the wrong key.
func TestVolumeLease_RejectsUnsafeKeys(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
leases := newTestLeases(t, natsURL, "node-a")
@@ -441,6 +471,8 @@ func TestVolumeLease_RejectsUnsafeKeys(t *testing.T) {
// the server may grant the entry to somebody else, and it has to get at least
// one check in between giving up on renewal and reaching that point.
func TestVolumeLease_ValidityIsBoundedBelowTheServerTTL(t *testing.T) {
t.Parallel()
require.Less(t, volumeLeaseValidity, volumeLeaseTTL,
"a holder that stops only when the server expires the entry has already overlapped its successor")
require.Less(t, volumeLeaseRenewInterval, volumeLeaseValidity,
@@ -456,6 +488,8 @@ func TestVolumeLease_ValidityIsBoundedBelowTheServerTTL(t *testing.T) {
// outage begins, so what it survives is validity minus that interval, and that
// has to cover the whole unwritable window rather than one stalled write.
func TestVolumeLease_SurvivesTheOutageTheHostProduces(t *testing.T) {
t.Parallel()
tolerated := volumeLeaseValidity - volumeLeaseRenewInterval
require.GreaterOrEqual(t, tolerated, jetstreamOutageBound,
"a lease that cannot outlast the outages these hosts produce surrenders volumes no peer has taken")
@@ -473,6 +507,8 @@ func TestVolumeLease_SurvivesTheOutageTheHostProduces(t *testing.T) {
// has to give the volume up on its own clock, and report the reason as a stall
// rather than as a peer taking it — nobody has necessarily taken it yet.
func TestVolumeLease_UnconfirmedRenewalSurrendersTheVolume(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
leases := newTestLeases(t, natsURL, "node-a")
@@ -556,6 +592,8 @@ func (k *blockingKV) updateInFlight() bool {
// a device stall, and the holder has to stop writing on validity regardless —
// so the surrender has to land while that renewal is still inside JetStream.
func TestVolumeLease_ValidityIsEnforcedWhileARenewalIsBlocked(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
leases := newTestLeases(t, natsURL, "node-a")
@@ -609,6 +647,8 @@ func TestVolumeLease_ValidityIsEnforcedWhileARenewalIsBlocked(t *testing.T) {
// after. The entry may already belong to another node, so deleting it on the
// way out would evict a live holder and hand the volume to a third opener.
func TestVolumeLease_SurrenderedLeaseIsNotDeletedOnRelease(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
leases := newTestLeases(t, natsURL, "node-a")
@@ -633,6 +673,8 @@ func TestVolumeLease_SurrenderedLeaseIsNotDeletedOnRelease(t *testing.T) {
// into a second engine on one encrypted volume, and treating a timeout as
// permanent strands a guest across a cluster restart, which is what happened.
func TestLeaseStoreUnavailable(t *testing.T) {
t.Parallel()
tests := []struct {
name string
err error
@@ -655,6 +697,8 @@ func TestLeaseStoreUnavailable(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
assert.Equal(t, tt.want, leaseStoreUnavailable(tt.err))
})
}
@@ -665,6 +709,8 @@ func TestLeaseStoreUnavailable(t *testing.T) {
// ErrLeaseStoreUnavailable, because that is the sentinel mountErrRetryable
// looks for and the only reason the relaunch backoff engages.
func TestVolumeLease_UnreachableStoreIsRetryable(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
nc, err := nats.Connect(natsURL)
@@ -59,6 +59,8 @@ func requireMarkerStillNames(t *testing.T, cfg *Config, volume, owner string) {
// A node still heartbeating holds its unsealed writes in memory, and opening
// the volume from the backend checkpoint would silently discard them.
func TestVolumeTakeover_RefusedWhileTheMarkerOwnerIsLive(t *testing.T) {
t.Parallel()
const volume = "vol-takeoverlive"
cfg := takeoverFixture(t, volume, &fakeLiveness{seen: time.Now().Add(-5 * time.Second), found: true})
@@ -70,6 +72,8 @@ func TestVolumeTakeover_RefusedWhileTheMarkerOwnerIsLive(t *testing.T) {
}
func TestVolumeTakeover_AllowedOnceTheMarkerOwnerIsStale(t *testing.T) {
t.Parallel()
const volume = "vol-takeoverstale"
cfg := takeoverFixture(t, volume, &fakeLiveness{seen: time.Now().Add(-2 * nodeLivenessWindow), found: true})
@@ -83,6 +87,8 @@ func TestVolumeTakeover_AllowedOnceTheMarkerOwnerIsStale(t *testing.T) {
}
func TestVolumeTakeover_AllowedWhenTheMarkerOwnerHasNoHeartbeat(t *testing.T) {
t.Parallel()
const volume = "vol-takeoverunseen"
cfg := takeoverFixture(t, volume, &fakeLiveness{found: false})
@@ -91,6 +97,8 @@ func TestVolumeTakeover_AllowedWhenTheMarkerOwnerHasNoHeartbeat(t *testing.T) {
}
func TestVolumeTakeover_NoMarkerNeverAsksAboutLiveness(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
liveness := &fakeLiveness{err: errors.New("must not be asked")}
cfg := &Config{
@@ -106,6 +114,8 @@ func TestVolumeTakeover_NoMarkerNeverAsksAboutLiveness(t *testing.T) {
// An unreadable heartbeat is not evidence that the owner is gone.
func TestVolumeTakeover_RefusedWhenLivenessCannotBeRead(t *testing.T) {
t.Parallel()
const volume = "vol-takeovererror"
cfg := takeoverFixture(t, volume, &fakeLiveness{err: errors.New("kv unavailable")})
@@ -116,6 +126,8 @@ func TestVolumeTakeover_RefusedWhenLivenessCannotBeRead(t *testing.T) {
}
func TestVolumeTakeover_RefusedWhenNoLivenessStoreIsWired(t *testing.T) {
t.Parallel()
const volume = "vol-takeovernostore"
cfg := takeoverFixture(t, volume, nil)
@@ -128,6 +140,8 @@ func TestVolumeTakeover_RefusedWhenNoLivenessStoreIsWired(t *testing.T) {
// Refusing without a store is only safe if production always has one, or every
// takeover in the cluster would be refused.
func TestRegisterProviderSubjects_WiresALivenessStore(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
cfg := setupTestConfig(t, natsURL)
startProviderSubjects(t, cfg, natsURL)
@@ -138,6 +152,8 @@ func TestRegisterProviderSubjects_WiresALivenessStore(t *testing.T) {
// The daemon writes heartbeats under clusterv1.HeartbeatKey with a JSON record
// carrying more than the timestamp. The reader must date one it did not shape.
func TestHeartbeatLiveness_ReadsWhatTheDaemonWrites(t *testing.T) {
t.Parallel()
_, natsURL := setupEmbeddedNATS(t)
nc, err := nats.Connect(natsURL)
require.NoError(t, err)
@@ -152,7 +152,7 @@ func (m *Manager) HandleCrash(instance *VM, waitErr error) {
removeTelemetryArtifacts(instance)
if m.deps.VolumeMounter != nil {
if err := m.deps.VolumeMounter.Unmount(context.Background(), instance); err != nil {
if err := m.unmountVolumes(context.Background(), instance); err != nil {
slog.Error("Volume unmount failed during crash handling",
"instance", instance.ID, "err", err)
}
+46 -1
View File
@@ -1,6 +1,9 @@
package vm
import "errors"
import (
"errors"
"fmt"
)
// ErrInstanceNotFound is returned by manager methods that look up an
// instance by id when no entry exists in the running map.
@@ -16,6 +19,44 @@ var ErrInvalidTransition = errors.New("invalid state transition")
// because stopping viperblock next is what tears the volume's state.
var ErrVolumeSealFailed = errors.New("volume seal failed")
// VolumeUnpublishError is one volume an unmount left exported. A VolumeMounter
// reports each such volume with it, so the manager knows which publishes a
// later terminate leaves in place.
type VolumeUnpublishError struct {
VolumeID string
Err error
}
func (e *VolumeUnpublishError) Error() string {
return fmt.Sprintf("seal %s: %v", e.VolumeID, e.Err)
}
func (e *VolumeUnpublishError) Unwrap() error { return e.Err }
// volumesLeftPublished returns the volumes an unmount error leaves exported:
// one VolumeUnpublishError per volume, joined. An error that names none leaves
// the outcome of every volume unknown, so all of them are returned.
func volumesLeftPublished(err error, volumes []string) []string {
if err == nil {
return nil
}
parts := []error{err}
var joined interface{ Unwrap() []error }
if errors.As(err, &joined) {
parts = joined.Unwrap()
}
var named []string
for _, part := range parts {
if ve, ok := errors.AsType[*VolumeUnpublishError](part); ok {
named = append(named, ve.VolumeID)
}
}
if len(named) == 0 {
return append([]string(nil), volumes...)
}
return named
}
// ErrAttachmentLimitExceeded is returned by AttachVolume when the
// instance has no free /dev/sd[f-p] slot to assign.
var ErrAttachmentLimitExceeded = errors.New("attachment limit exceeded")
@@ -24,6 +65,10 @@ var ErrAttachmentLimitExceeded = errors.New("attachment limit exceeded")
// volumeID is not present in the instance's EBSRequests.
var ErrVolumeNotAttached = errors.New("volume not attached to instance")
// ErrVolumeSettling is returned by a mount while this node is still sealing
// the volume's export from an earlier attachment.
var ErrVolumeSettling = errors.New("volume is still being detached from an earlier attachment")
// ErrVolumeNotDetachable is returned by DetachVolume when the target
// volume is a boot or EFI volume that cannot be hot-unplugged.
var ErrVolumeNotDetachable = errors.New("volume cannot be detached")

Some files were not shown because too many files have changed in this diff Show More