mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-21 19:56:33 +00:00
d26e441007e327f3ecb392fee09986af7300a89f
9 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Josh Siviour
|
48749576c3 |
test: rebalance E2E into in-process integration tier + shared-guest live scenarios (#556)
* test: add in-process integration test tier Roughly half of the behavioural tests under tests/e2e/ assert control-plane API shapes, authz and error codes that need no live environment, yet they can only run against a tofu-provisioned environment on a scarce self-hosted runner. This adds the tier they can move down to. The real gateway router runs in-process: SetupRoutes() is a pure function of GatewayConfig, so it serves from an httptest server over embedded NATS JetStream with real IAM/STS services and genuine SigV4 verification. Only the daemon side of the wire is faked -- the ec2.*/ebs.* NATS subjects a live node would answer are stubbed per test. Nothing is provisioned and no fixed ports are bound, so the tier runs concurrently on a shared build host and is safe as a CI front gate ahead of the live suites. It is build-tagged and kept out of preflight, which stays unit-only. * test(integration): add IAM/STS client accessors and port the zero-stub authz tests Adds IAMClient/STSClient plus ClientsWithCreds/ClientsWithSessionCreds so a test can act as a principal other than the seeded root user, and stubs the node-discovery subject in StartGateway so call paths that trigger discovery resolve immediately instead of riding out the fan-out timeout. Ports the four enforcement tests that guard on ec2:DescribeRegions, a gateway-local action, so they need no daemon-side stub at all. * test(integration): port remaining IAM/STS authz tests from e2e Ports TestIAMUserCRUD, TestIAMAccessKeyLifecycle, TestIAMUserAuthentication, TestIAMPolicyCRUD, TestIAMPolicyAttachmentEnforcement, TestIAMPolicyLifecycle, TestIAMGroupsLifecycle, TestIAMRolesAndProfiles and TestSTSAssumeRoleAndGetCallerIdentity from tests/e2e/iam/ into the in-process integration tier. Each test now self-bootstraps its own users/policies/roles instead of relying on shared suite-level state, since every integration test gets its own gateway and embedded NATS. TestIAMRolesAndProfiles stubs ec2.IamProfileAssociation.describe because DeleteInstanceProfile always counts live associations over NATS regardless of which guard actually fires. TestIAMCleanup is not ported: it only swept a shared long-lived E2E environment, and every ordering/dependency assertion it made is already covered by TestIAMPolicyLifecycle, TestIAMGroupsLifecycle and TestIAMRolesAndProfiles's own delete-conflict guards. * test(e2e): remove IAM/STS authz tests migrated to the integration tier All 13 signed-request authz tests plus TestIAMCleanup now have faithful, mutation-proven equivalents in tests/integration/ — the gateway dispatches IAM/STS actions directly with no daemon hop, so the in-process tier is a faithful and far cheaper substitute. The iam/ e2e suite now runs only TestIAMInstanceProfileAssociation and TestIMDS. Drop the now fully-dead iam_helpers_test.go alongside the six deleted test files, and remove harness.IAMGroupARN/IAMDeleteGroupBestEffort, which had no remaining caller anywhere in the repo once iam_groups_test.go was gone. * feat(integration): daemon-lite harness for key/tags/routetable/vpc Adds DaemonLite, an in-process stand-in for a live spinifex daemon that subscribes the real key, tags, route table, and VPC/subnet/SG/IGW service implementations directly to their NATS subjects, backed by memory and embedded-JetStream stores. Ports TestKeyPairs, TestTagManagement, TestRouteTableValidation, and TestReplaceRouteConvergence from tests/e2e/single so the integration tier drives genuine daemon-side business logic instead of StubSubject canned replies. vpcd (OVN) is out of scope, so its "vpc.create-sg"/"vpc.delete-sg" acknowledgements are stubbed with a fixed success reply — the SG record itself is still written by the real service impl before that event fires, so the stub only unblocks the synchronous round-trip rather than replacing tested behavior. * ci(e2e): gate live E2E jobs behind the in-process integration tier Runs the in-process integration suite first, on the ci-single runner host rather than inside a provisioned VM, so a broken control plane is caught before either live job pays the Spinifex install. Both e2e_single and e2e_multi now require it, and e2e_status treats a skipped integration job as a failure so the gating status cannot go green on a gate that never ran. Drops the shadow-mode suite selector: targeted per-diff suite selection is not the direction being pursued, and leaving a non-functional shadow job in the gating workflow only invited it to be mistaken for a live gate. * test(e2e): remove single/ tests superseded by daemon-lite ports TestKeyPairs, TestTagManagement, TestRouteTableValidation, and TestReplaceRouteConvergence now have proven-equivalent ports in tests/integration/ (mutation testing confirmed parity). Delete the e2e originals and their runX helpers, along with keypair_test.go's and tags_test.go's file-local helpers left with no caller. generateImportPubKey is shared by other single/ tests so it moves to helpers_test.go instead of being deleted. Slims the manifest-lint baseline for the removed create-call entries. * ci: run the lb e2e suite on multinode only Both e2e jobs filter SUITES by exclusion, and neither excluded lb, so the suite ran twice per e2e run: 354s on single and 654s on multi. Balancing across instances on one node is not a deployment mode we support, and ELBv2 proper requires two AZs, so the single-node run was redundant. Drops ~354s from the single-node job, which is the gate critical path. * test(integration): port 3 of 7 ECR tests to the in-process tier Wires the gateway's ECR control plane (repository/policy/lifecycle metadata over an in-process daemon-lite NATS subscriber, blob and manifest bytes via a memory object store, JWT issuer/verifier keyed off the existing IAM master key) into the integration harness, and ports TestECRControlPlane, TestECRGetLoginPassword, and TestECRIAMAuthorization_AssumedRolePullOnly from tests/e2e/ecr — each proven equivalent by mutation (evaluatePrincipalPolicy forced to allow, and the repo-describe path forced to not-found) before its E2E original was removed. The other 4 authorization tests (PullOnly, ReadOnly, DetachPolicyDeniesImmediately, DeactivatedKeyReturns401) stay on the E2E tier: AttachUserPolicy lacks the AWS-managed-ARN existence-check bypass that AttachRolePolicy/AttachGroupPolicy already have, so attaching AmazonEC2ContainerRegistryPullOnly/ReadOnly straight to a user fails NoSuchEntity in both tiers alike. Left untouched pending a fix to that shared IAM code. * fix(iam): accept AWS-managed policy ARNs on AttachUserPolicy AttachUserPolicy called getPolicyByARN unconditionally, so attaching an AWS-managed ARN to a user returned NoSuchEntity. AttachRolePolicy and AttachGroupPolicy already skip that lookup for AWS-managed ARNs and store them opaquely; the user path was missed when that guard was introduced. ListAttachedUserPolicies had the mirror of the same gap: it resolved every ARN through getPolicyByARN and silently dropped the ones it could not find, so a managed ARN would disappear from the listing rather than round-trip. Evaluation already handled these correctly via resolveAttachedPolicy, so only the write and list paths needed the guard. * test(integration): port remaining 4 ECR authorization tests Ports TestECRIAMAuthorization_PullOnly, _ReadOnly, _DetachPolicyDeniesImmediately, and _DeactivatedKeyReturns401 from tests/e2e/ecr now that AttachUserPolicy accepts AWS-managed policy ARNs. Each was proven equivalent by mutation before its E2E original was removed: PullOnly/ReadOnly/DetachPolicyDeniesImmediately fail when evaluatePrincipalPolicy is forced to always allow, and DeactivatedKeyReturns401 fails when the access-key-active check in resolveECRLongLivedPrincipal is bypassed. All mutants were reverted and a clean run confirmed before committing. tests/e2e/ecr/iam_authorization_test.go is now fully removed — every test it held has an in-process equivalent, and its helpers had no other callers in the package. * ci: select e2e suites by allow-list instead of deny-list Both jobs filtered SUITES by exclusion, so any suite named in neither deny-list ran in both jobs. That is how lb came to run twice per e2e run, and it makes adding a suite silently fan it out everywhere. Each job now names the suites it is eligible to run. Selection is still driven by SUITES, so this is behaviour-preserving for the current default: single/iam/cert on the single-node job, multinode/lb/cert on the multi-node job, exactly as before. eks, ecs and storagegrowth are listed as eligible on the single-node job but are not yet in the default SUITES. * test(e2e): delete seven tests with no coverage TestFinalClusterStats and TestInstanceLaunch assert nothing beyond what other callers of their shared helpers already imply. The multinode baseline, volume, guest-SSH, and preflight tests duplicate single-node coverage or exercise per-guest behaviour that isn't cluster-specific. TestMultinodeInstanceDistribution can't fail: three independent RunInstances calls with no placement group make a strict spread assertion flaky and a loose one meaningless. Preflight checks (/dev/kvm writability, peer SSH reachability) move into requireMultiNodeFixture so the package fixture singleton still fails fast with a clear message instead of disappearing along with the dedicated test. The remaining callers of needInstanceTrio (CrossNodeOps, CrossNodeGateway, NodeFailure, VolumeDurability) still trigger the shared trio launch on their own, so removing the distribution and volume-lifecycle tests doesn't leave it unbuilt. Also slims the manifest-lint baseline entry for the deleted multinode volume test. * fix(e2e): require full 3-node spread in TestMultinodeSpread SpreadPlacement passed at unique>=3, warned via t.Logf at unique==2, and only failed below that. A 33% spread failure (two instances colocated) passed with a log line. The 243.6s NAT-egress scenario that follows only proves what it claims to if the private trio actually landed on three distinct chassis, so that premise needs to be a hard assertion, not a warning. * refactor(e2e): drop the unused spread-assertion harness helpers AssertSpreadAcrossNodes and CountInstancesPerNode lost their only caller when the instance-distribution test was removed. Beyond being dead, the assertion helper documented minNodes=1 as an acceptable logging-only mode inherited from the bash driver, which is the pattern that let a placement test pass while every instance landed on one node. InstanceHostingNode and its ps helpers stay; five multinode tests use them. * fix(ec2): stop leaked goroutines on prepareEFIVolume error paths newViperblock starts the chunk uploader and WAL syncer unconditionally, but prepareEFIVolume only released them on the happy path via Close(). Six error returns between acquisition and Close left both goroutines running. Add a defer immediately after acquisition, matching the sibling prepareRootVolume; Close() remains the durability boundary. * test(predastore): add real predastore fixture for integration tests Extract predastore_integration_test.go's inline daemon startup into a reusable testutil/predastore package so tests/integration can point DaemonLite-style wiring at a real S3-compatible backend instead of an unreachable host. Kept as its own leaf package rather than folding into the shared testutil package: it transitively imports predastore's quicclient, whose package-level connection pool starts a permanent goroutine at init that would trip goleak checks in any of the ~80 other packages that import testutil for unrelated NATS helpers. * test(integration): migrate CreateVolume to assert real predastore persistence Add StartVolumeDaemonLite, an opt-in DaemonLite variant wired to the real predastore fixture, and migrate TestCreateVolume_PersistsToRealPredastore off memory-backed DaemonLite so CreateVolume/DescribeVolumes exercise viperblock's real S3 backend end to end instead of stopping at input validation. * test(integration): add real storage-state oracle for CreateVolume DescribeVolumes only proves the handler's own read path agrees with its own write path, both via the same deserialization; it cannot catch a bug in a field it never re-derives (e.g. a unit-conversion error in the bytes handed to viperblock, since VolumeMetadata.SizeGiB is copied from the request directly). vbscan reads a volume's persisted config.json and chunk objects straight over S3, independent of any spinifex handler code, so TestCreateVolume_PersistsToRealPredastore can assert on what viperblock actually wrote rather than only on what the handler claims. vbscan is a local reimplementation of scripts/vbrefscan's read-only scan rather than an import: spinifex cannot depend on mulga, the module that contains it. It only reimplements the config/chunk-listing half of vbrefscan — the checkpoint-based reference-integrity half needs viperblock.ParseBlockCheckpointBytes, which does not exist in the viperblock version spinifex currently depends on (v1.12.1) and is also moot for this volume: CreateVolume never calls SaveLiveCheckpoint, so a freshly created volume has no checkpoint to scan regardless of version. * refactor(test): move the predastore daemon fixture out of the coverage scope test-cover runs go test ./spinifex/..., so anything under spinifex/ is compiled and measured. The daemon fixture cannot be exercised by unit tests, so it reported a coverage gap no unit test could close. The e2e harness already avoids this by living under tests/; the fixture now does the same. vbscan stays put, being ordinary library code with its own unit tests. * test(integration): migrate RunInstances multi-count expansion off live guests RunInstances with MinCount/MaxCount=2 expands into a single per-node targeted launch (distributeInstances in gateway/ec2/instance/placement.go) before the daemon-facing NATS hop, so the real gateway router plus a stubbed daemon response is sufficient to prove it — no throwaway VM is needed to exercise the expansion logic itself. The live single-node E2E test's WaitForInstanceState/TerminateInstances sequence was teardown hygiene for the real guests it booted, not an assertion of the expansion behavior, so it is dropped rather than emulated. The migrated test additionally asserts the per-node launch request itself carries MinCount=MaxCount=2, proving the count actually propagates rather than the launch succeeding by coincidence or via the zero-daemon degenerate path in placement.go. * docs: drop a doc reference to the deleted live test * test(integration): migrate launch-template RunInstances expansion off live guests RunInstances --launch-template resolves ImageId/InstanceType from the stored template entirely gateway-side (expandLaunchTemplate, before validation/placement/the daemon-facing NATS hop), so proving it forwards the resolved fields never needed a real guest boot. Ports the assertion to tests/integration via a new opt-in StartLaunchTemplateDaemonLite (real LaunchTemplateServiceImpl over embedded JetStream) plus a direct subscription on the per-node launch subject to capture what the gateway actually dispatched, rather than only the client-visible response. The live test keeps its CRUD/versioning lifecycle (create/modify/delete version, tag-filtered describe) — none of it touches a guest — and drops only the RunInstances-from-template + wait-for-running + terminate tail, whose only job was waiting out a real boot to read back fields already proven resolved. * test(integration): migrate KV-only account-scoping checks off live tenants Ports the KeyPair/VPC-Subnet/IGW-EIGW/AccountSettings cross-account isolation assertions from tests/e2e/single/account_test.go to the in-process integration tier. Each of these resources resolves ownership from a plain account-scoped KV key (utils.AccountKey(accountID, id), or an account-keyed settings record) with no vm.Manager or real guest involved, so DaemonLite now also wires the real EgressOnlyIGWServiceImpl and AccountSettingsServiceImpl alongside the existing VPC/IGW/Key services. Two independently minted tenant accounts (via IAMService.CreateAccount/CreateUser/CreateAccessKey/CreatePolicy/ AttachUserPolicy, in-process — the same calls runAccountCreate makes without the CLI/stdout round-trip) replace the live tier's carousel so isolation is checked between real, separately owned accounts. Left live: instance scoping (Step2) and volume scoping (Step3) — checkInstanceOwnership, the actual cross-account gate for stop/start/ reboot/terminate/attach-volume, runs daemon-side only after a real vm.Manager lookup, so it cannot be exercised without a live guest. Snapshot scoping (Step5) needs a real backing volume for CreateSnapshot (viperblock, out of DaemonLite's scope), so it stays live entirely rather than split off only its two KV-only assertions. Global-resources (Step9) carries no account branching in the handlers at all, so asserting its equality across two accounts wouldn't exercise anything a fake node responder didn't already trivially satisfy — not migrated. * test(integration): migrate launch-template CRUD lifecycle off live guests TestLaunchTemplates booted zero guests: its RunInstances coverage had already moved to TestRunInstances_LaunchTemplateExpansion, leaving only template CRUD/versioning assertions occupying a full live-cluster slot. Port the remaining create/describe/version/modify/delete lifecycle, including the SourceVersion merge and a nested timestamp round-trip, into TestLaunchTemplates_CRUDLifecycle against the in-process integration gateway. * refactor(tests): move spot instance lifecycle coverage to integration tier RequestSpotInstances is a mock over on-demand RunInstances (no bidding, interruption, or reclamation), so booting real VMs in the e2e tier proved nothing spot-specific. Migrate the SIR CRUD lifecycle (request, describe, cancel, close-on-terminate, insufficient-capacity error) to a NATS-level integration test that exercises the real gateway orchestration and daemon service impl without a live guest. Real-VM-boot assertions are already covered by TestRunInstances_MultiCountExpansion and the e2e baseline tests. * fix(tests): share one embedded NATS server across the integration package ClientTokenStore in spinifex/gateway/ec2/instance is a process-wide sync.Once singleton bound to whichever *nats.Conn first initialises it. Every integration test booted its own embedded NATS server, so the first RunInstances-calling test captured the singleton, and any later test inherited a KV handle bound to that server's now-closed connection. TestMain now boots one embedded NATS+JetStream server for the whole package; every StartGateway call connects into its own dynamically provisioned NATS account (accountAuthenticator in shared_nats.go), giving each test a private JetStream namespace despite the shared server, and none of the per-test connections are closed until every test has finished - whichever one the singleton latches onto stays live for the run. Also wire the suite into the required PR gate: it was previously only invoked from e2e.yml's push-triggered (main/dev), self-hosted "Integration (front gate)" job, so a regression here never blocked a PR. The whole package runs in single-digit seconds, so it now runs inline in `make preflight` and as its own step in spinifex.yml's build_and_test job. * test(integration): migrate serial-console access checks off live guests Enable/Disable/GetSerialConsoleAccessEnabled is pure gateway+account-settings logic with no guest involved, so it moves to the in-process integration tier against the real AccountSettingsServiceImpl wired into DaemonLite. * test(integration): migrate DescribeImages round-trip off live guests runImage's AMI describe round-trip needs no real cluster: it registers an AMI against a hand-seeded snapshot via the real ImageServiceImpl, backed by a new StartImageDaemonLite helper wiring RegisterImage/DescribeImages onto an in-memory object store. * test(integration): migrate predastore object lifecycle off live guests runPredastoreObjectLifecycle only needs a real predastore daemon, not a tofu-provisioned cluster: it now runs against tests/fixtures/predastore's in-process fixture, covering the same create/put/get/delete/list contract. * test(integration): migrate ListFoundationModels catalog check off live cluster ListFoundationModels is pure gateway-side static catalog logic with no NATS/daemon hop, so it moves to the integration tier via a new BedrockClient harness accessor; this tier configures no credential resolver, so the Anthropic entry's absence is asserted explicitly rather than left optional. * test(integration): cover RunInstances client-token idempotency Proves replaying the same ClientToken through the real gateway returns the original reservation without a second daemon dispatch, and a different token launches independently. * test(integration): cover RunInstances launch-template expansion Proves version selection (omitted Version resolves to the template default, explicit Version selects that version) and field precedence (explicit RunInstances parameters override the template, unset fields inherit it) through the real gateway. * test(integration): cover RunInstances malformed-input rejection Proves a malformed ImageId and a malformed targeted-launch CapacityReservationId are rejected before any NATS dispatch, with no daemon stub registered so a regression that let either through would fail fast instead of hanging. * test(integration): cover RunInstances iam:PassRole enforcement Proves a caller granted ec2:RunInstances but not iam:PassRole on an instance profile's role is denied, and the identical session succeeds once PassRole is granted for that exact role ARN. * test(integration): cover RunInstances placement-group strategy routing Adds a placement-group daemon-lite wiring a real PlacementGroupServiceImpl over NATS, then proves spread strategy assigns exactly one instance per node while cluster strategy pins the whole launch to the single highest-capacity node. * test(integration): cover RunInstances capacity distribution across nodes Proves the default (non-placement-group) path packs a multi-instance launch across nodes proportionally to spare capacity rather than exhausting one node first. * test(e2e): merge SameSGComms and SGToSGDatapath around one client/target pair Both booted their own instance pair to check SG-to-SG datapath enforcement and same-default-SG east-west connectivity. Merging them around one shared client/target pair cuts four boots to two, and converting the inline harness.Step sequence into named t.Run stages (PortGroupMembership, AllowedTraffic, DeniedTraffic, RevokeAndVerifyBlocked, ReauthorizeAndVerifyRestored, SameSGComms) gives each its own pass/fail attribution instead of one opaque test result. SameSGComms now runs last and joins the default SG onto each ENI via ModifyNetworkInterfaceAttribute only after every SG-to-SG allow/deny assertion has already recorded its own result, since the default SG's self-reference rule admits all traffic between co-members and would otherwise defeat the denied-traffic check. * test(e2e): merge default-SG reachability, guest DNS, and egress enforcement DefaultSGReachabilityBaseline, GuestDNSResolution, and SecurityGroupEgress each booted their own instance to check one facet of "a guest is reachable only when policy says so": ingress deny/authorize, resolver + WAN egress, and egress ACL enforcement. Merging them around one shared guest cuts three boots to one, with named t.Run stages (Deny, Authorize, Resolver, WANEgress, RevokeEgress/RestoreEgress) giving each its own attribution. The egress revoke/restore round-trip now mutates this scenario's own dedicated SG instead of the shared default SG the rest of the suite depends on, isolating its egress-rule mutation from every other test. * fix(ci): single-quote e2e test_run regex across the ssh boundary * fix(e2e): anchor SG egress-enforcement probe to a WAN IP, not the gateway * test(e2e): merge single-node guest churn tests into one durability scenario Combines TestSSHProbe, TestENIHotplug, TestENIHotplugReconcile, TestVolumeDurability, TestModifyInstanceAttribute, and TestRebootInstance into TestGuestChurnDurability, a single shared-guest scenario that re-verifies a data-integrity sentinel sha256 after every perturbation stage (ENI hotplug, daemon restart, ENI detach, instance-type resize, stop/start, reboot). This proves the sentinel survives the full churn sequence rather than just one isolated stop/start. * fix(vpc): clear subnet drop gate when installing egress reroute A subnet with an attached IGW but no 0.0.0.0/0 route carries a priority-1100 drop gate on the VPC router. When a route later lands (NATGW reroute at 900, IGW reroute at 1000) the reroute is installed but the drop gate was never removed, so the higher-priority gate wins and black-holes the subnet's egress in lr_in_policy before it reaches routing or SNAT. This left NAT-gateway private-subnet egress with 100% packet loss: the SNAT stage saw zero packets and no conntrack was committed. Remove the subnet's drop gate for the same prefix once a reroute is installed. Idempotent and prefix-scoped, so a specific-prefix reroute is a no-op and leaves the default gate intact. * test(e2e): merge VPC subnet, NAT gateway, and EIP single-node tests Merges the former TestVPCSubnetE2E, TestNewVPCEgressBaseline, TestNATGateway, and TestInstanceEIP into one staged scenario, TestVPCEgressPaths, around one scenario-owned VPC and its two guests, cutting five guest boots to three. The public guest serves PublicSubnetEgress, then the NAT Gateway bastion hop, then (last, since association rewrites its public IP) the EIP flip. The private guest serves the NAT Gateway path, launched directly into the same VPC's already-existing private subnet instead of a dedicated one. RouteBeforeSubnet keeps its own second VPC and instance since it exercises the implicit main-route-table code path, a different one than the explicit route table this scenario's own VPC uses for its public subnet — collapsing the two would drop coverage of whichever path got merged away. The NAT Gateway up/down flip now repeats for an env-tunable round count (SPINIFEX_VPCEGRESS_NAT_ROUNDS, default 1), matching every other merged single-node scenario. * test: TEMPORARY mutation-check — revert |
||
|
Ben Duncan
|
28a876d17d | feat(dns): resolvable ALB/NLB + EKS DNS, forwarder hardening, cross-tenant reconcile (#532) | ||
|
Julian Sommer
|
8a94886b13 | feat: cloud-init EC2 datasource (#411) | ||
|
Julian Sommer
|
9288ecc7dc | chore: delete no op tests + speedup e2e | ||
|
Josh Siviour
|
9a474220ce |
fix: preserve terminated-instance describe-visibility window in GC reaper (#363)
* fix: preserve terminated-instance describe-visibility window in GC reaper The TerminatedTeardownReaper purged a terminated record the moment it drove the instance's pending NAT/OVN teardown to done. Normal terminations always arrive NAT/OVN-pending, so the first 2m GC sweep completed and purged them immediately, removing the instance from DescribeInstances before clients could observe state=terminated — an AWS-compat divergence that flaked e2e-single TestRunInstancesMultiCount. Stamp TerminatedAt on transition and purge a completed record only once it is older than a 15m visibility window; fresher records are left to the terminated bucket's 1h TTL, keeping the instance describable as terminated. The ADR-0003 §3 purge-on-completion invariants still hold (their seeded records predate the window). Closes mulga-siv-328. * test(e2e): poll intra-SG east-west ping until L2 converges TestSameSGComms fired a single un-retried 'ping -c 3 -W 2' immediately after the source instance's SSH became reachable, but the ICMP target is the destination's east-west data plane. Two instances boot in parallel and the destination guest NIC / OVN port flows come up several seconds after the EC2 running transition the launch waits on. When the probe wins that race the source kernel reports 'Destination Host Unreachable' (ARP unresolved), not an ACL drop, and the test flaked. Replace the one-shot ping with pingConverged, a bounded poll that retries until 0% packet loss or 45s. A genuine self-ingress/datapath break still stays at 100% loss for the full window and fails, so coverage is preserved. |
||
|
Josh Siviour
|
1873f45d26 |
Resilient resource lifecycle (ADR-0001..0006): EKS multi-node teardown, idempotent deletes, GC backstops (#357)
* fix: idempotent Delete* across VPC family + volume (RLC1) Delete handlers now return empty success when the resource is already absent (nats.ErrKeyNotFound), instead of *NotFound. Transient read errors still map to ServerInternal. Covers DeleteVpc, DeleteSubnet, DeleteSecurityGroup, DeleteNetworkInterface, DeleteInternetGateway, DeleteNatGateway, DeleteRouteTable and DeleteVolume. Makes destroy retries converge instead of failing mid-graph, removing a source of orphaned/zombie resources between deploys. Adds a TestRLC1_*DeleteIdempotentOnAbsent invariant per package and updates the daemon delegation table; supersedes the old absent->NotFound tests. * feat: prune orphan ENI LSPs in reconcile applyPorts (ADR-0003 §4) applyPorts was create-or-update only, so a guest LSP whose spinifex:eni_id no longer maps to an intent ENI survived instance terminate and VM-host reinstall — the orphan-LSP capacity-drift root cause. It now takes pruneOrphans (mirroring applySGs): on a full reconcile it lists LSPs and tears down each managed ENI port absent from intent via the composed topology.DeletePort cascade (clears port-group memberships, then deletes the LSP). The apply-only startup path does not prune, preserving in-flight ports until subscribers converge. Adds ListLogicalSwitchPorts to the OVN client (live + mock). Invariant TestRLC5_OrphanLSPPrunedAfterReconcile guards ADR-0003 §4; a startup-race test guards the apply-only path. * feat: idempotent + tracked instance terminate (ADR-0003 §1/§2) Make terminate idempotent across all three layers and break the un-terminable-ENI deadlock, and record per-dependency teardown so the GC backstop can later tell a fully-reaped instance from a dangling one. - vm.Manager.Terminate: absent / already-terminated / shutting-down now return nil instead of NotFound / invalid-transition (ADR-0003 §2). - VM.Teardown map + teardown.go marks: sync steps stamped Done/Failed via cleaner error returns; fire-and-forget OVN LSP and NAT rule stamped Pending for the reconciler / GC reaper (ADR-0003 §1). - InstanceCleaner: DeleteVolumes / ReleasePublicIP / DetachAndDeleteENI / RemoveFromPlacementGroup / ReleaseGPU now return error. - vpc ForceDeleteInstanceENI bypasses the in-use guard for the owning instance, breaking the failed-detach deadlock; plain delete still rejects an in-use ENI held by another instance. - gateway TerminateInstances: KV-health-gated idempotent absence (a failed terminated-bucket query does not fabricate success, ADR-0003 §3). - Invariants: TestRLC1_TerminateIdempotentOnAbsent, TestRLC4 teardown-map stamping + failed-dependency, TestRLC_ENINonDeadlock. * feat: reconciler GC backstop framework + terminated-teardown reaper (ADR-0003 §3) Add the shared reality->desired garbage-collection framework (inverse of Restore) and its first reaper, closing the gap 295.3 deferred: nothing retried pending/failed teardown or purged the terminated record when its dependents reached done. - vm/gc.go: GarbageCollector + Reaper interface (Class/Scope/Sweep). KV-health gated — holds and alarms when the desired-state cannot be trusted, never reaps against it. ClusterWide reapers run only on the elected leader (optional elector); NodeLocal run on every node. Start sweeps immediately then every GCInterval. - vm/teardown_reaper.go: TerminatedTeardownReaper (node-local). Scans this node's terminated records, re-drives each not-done dependent through the idempotent cleaner, and purges the record once all reach done. Records already complete on arrival are left to the bucket's 1h TTL. - StateStore gains DeleteTerminatedInstance (JetStream impl already existed). - daemon: wires the GC after the pending watchdog, gated on a cheap JetStream AccountInfo health probe. - Cluster OVN-LSP arm already lands via the reconciler drift prune (295.2). - Invariants: TestRLC5_GCRetriesPendingTeardownToDoneThenPurges, TestRLC5_GCNeverPurgesIncompleteRecord, TestRLC5_GCHoldsWhenKVDegraded, TestRLC5_GCSkipsOtherNodesRecords. * feat: VPC-family dependency guards (ADR-0004 rule #3) DeleteSubnet blocks while a live ENI attachment (a resident instance) remains; available/orphan ENIs do not pin it. DeleteNetworkInterface guards on the structured attachment (eniIsLiveAttachment), not a bare in-use string. DeleteNatGateway blocks while a live route table forwards to it and disassociates its EIP keeping the allocation (AWS parity); CreateNatGateway now marks the EIP associated so the disassociate is real and a second NAT GW cannot reuse the allocation. Binding invariants: TestRLC3 dependency-guard completeness across the subnet/ENI/NAT-gateway handlers. * feat: EBS volume data-safety GC override (ADR-0005) The shared GC backstop reaps actual-desired for every other class; for volumes that default is forbidden because they carry customer data. Add VolumeLeakReaper (node-local, rides the 295.5 GC): it marks a volume left attached to a definitively-gone instance as orphaned (a non-destructive tag) and alarms, but NEVER issues a delete. The daemon feeds it only this node's terminated instances whose volumes-teardown failed, so the shared predastore scan can never false-mark another node's live-instance volume. DeleteVolume idempotency + the detach/snapshot guards + the DeleteOnTermination teardown retry already landed (295.1/295.3/295.5). Binding invariants: TestRLC5 (GC marks+alarms, never deletes data), TestRLC3 (detach-before-delete). * test(elbv2): lock ADR-0002 §3 live-only TG in-use guard (mulga-siv-295.4) * feat(eks): idempotent DeleteCluster on absent + ADR-0006 lifecycle invariants (mulga-siv-295.8) DeleteCluster returns empty success when the cluster meta is already swept (Common Resource Lifecycle Contract #1), so tofu destroy retries converge. Adds TestRLC1 (idempotent delete), TestRLC2 (billable-before-KV-sweep ordering) and TestRLC3 (NLB target-group no-orphan, riding ADR-0002/172). * fix(eks): GC reaper terminates orphan control-plane VM after cluster delete (mulga-siv-294) A normal DeleteCluster can leave a running control-plane VM behind (terminate completes async; a daemon restart re-launches it from saved running-state) after the cluster meta is swept — a billable orphan with no owner. Add EKSBillableReaper on the 0003 GC backstop: a running managed-by=eks CP VM whose cluster meta is definitively gone is terminated. The CP ENI now carries the customer-account tag so a system-account VM can be mapped back to its cluster meta; the reaper never reaps on uncertainty (ENI gone/untagged or meta present). * feat: reclaim orphan EKS NLB + NAT-GW EIP in billable reaper The billable GC reaper terminated an orphan control-plane VM but left the cluster's NAT-gateway EIP and NLB front-end NAT behind when the meta was gone. After a successful terminate, run the name/tag-driven DeleteClusterNLB + DeleteClusterCPVPC teardown (idempotent, meta-independent) so the billable NAT-GW EIP and NLB NAT are reclaimed too. ADR-0006 §5. * fix: persist NAT-GW & route-table tags and reorder CP-VPC teardown to reclaim EKS NAT-GW EIP CreateNatGateway and CreateRouteTable both dropped input.TagSpecifications, so the tag-driven DeleteClusterCPVPC teardown could not discover the cluster NAT gateway or its private route table. The explicit DeleteNatGateway + ReleaseAddress were skipped and the billable NAT-GW EIP leaked under the system account on every clean DeleteCluster. Three fixes: - CreateNatGateway persists ExtractTags(TagSpecifications, natgateway). - CreateRouteTable persists ExtractTags(TagSpecifications, route-table). - DeleteClusterCPVPC tears down route tables before the NAT gateway, so the rule #3 route-ref guard (DependencyViolation / InvalidAddress.Locked) is not tripped and the EIP releases. Adds tag-persistence regression tests for both create paths and a teardown ordering invariant (TestRLC5). Validated live: NAT-GW EIP released cleanly, no DependencyViolation/Locked. * fix: route EKS control-plane VM terminate cross-node + DELETING teardown backstop Multi-node EKS cluster delete wedged in DELETING and stranded the billable NAT-GW EIP. Two root causes, both invisible on a single-node box: Cross-node terminate was never routed. TerminateSystemInstance was node-local, so terminating a CP VM owned by another node returned NotFound (treated as idempotent) and the teardown deleted a still-attached ENI (InvalidNetworkInterface.InUse). That error short-circuited purgeClusterInfra before the managed CP-VPC teardown that releases the NAT-GW EIP. Split TerminateSystemInstance into a local body plus a routing wrapper that issues system.TerminateInstance.<id> to the owning node. Bind that subscription for every system-managed VM (elbv2 + eks) via the OnInstanceUp funnel, so CP VMs placed on the coordinator's own node (and recovered after a daemon restart) are reachable too, not only remotely-launched ones. No DELETING backstop. A failed synchronous DeleteCluster left the cluster DELETING with nothing to re-drive it. Add EKSDeletingReaper (vm.Reaper, class eks-deleting) that re-runs purgeClusterInfra for clusters wedged past a min-age, gated by the per-cluster reconciler leader lease. Stamp ClusterMeta.DeletingSince on the transition into DELETING. TerminateK3sServerVM additionally waits out any residual ENI-InUse with a bounded budget as defense-in-depth. Validated live on env19 3-node: first delete-cluster converges in 29s, both remote CP VMs terminated cross-node, NAT-GW EIP released, backstop never fires. * fix(network): gate gateway reconcile on real datapath reachability Post-reboot the external OVN datapath can stay dead while every control-plane signal is green: the chassisredirect SB binding is claimed (up=true), NAT rows are present and the host neigh is primed, yet ovn-controller leaves stale gateway/localnet flows so EIP north-south and the VM->gateway heartbeat forward nothing. ensureGatewayClaimed only nudges recompute when the binding is unclaimed, so a claimed-but-dead datapath is never detected or repaired. Add ensureGatewayDatapath: after the SB claim converges, probe the gateway LRP IP (OVN answers ICMP natively, no guest or security-group dependency), and on a miss force one ovn-controller recompute and re-probe until a short deadline. Reuses the existing recompute nudge and no-ops when no verifier is wired or no gateway IP resolves. * fix: self-heal external uplink after reboot boot-race Post-reboot, ovs-vswitchd/ovn-controller order after the passive network.target rather than network-online.target, so they start concurrently with systemd-networkd. Under a heavy boot the WAN-glue veth (veth-wan-ovs) can come up admin-down before networkd brings it up, leaving br-wan with no carrier and the external datapath dead while every control-plane signal reconciles green. Broaden the datapath health-gate remediation from recompute-only to RepairDatapath: re-assert the veth admin state (covers the admin-down link, which a flow recompute cannot revive) then recompute (covers a stale gateway ofport). Idempotent; the veth ops are skipped in physical-uplink mode where the device is absent. Also capture ip -d link (veth admin/carrier) and ovs-ofctl show br-ext (ofport) in the reboot diag bundle to disambiguate the two failure shapes from CI artifacts. * fix: restore AWS-faithful Delete* NotFound; move idempotency to teardown callers (mulga-siv-306) RLC1 (f9218982) made EC2/VPC/EBS Delete* handlers return success on an absent resource, breaking AWS compatibility and the cross-account isolation signal (a cross-account delete reads as absent -> success instead of *.NotFound). Restore the AWS contract at the API while keeping lifecycle convergence: - Handlers return canonical *.NotFound on absent: DeleteVpc, DeleteSubnet, DeleteSecurityGroup, DeleteInternetGateway, DeleteNatGateway, DeleteRouteTable, DeleteVolume, and the public DeleteNetworkInterface (ForceDeleteInstanceENI keeps its idempotent force path, ADR-0003 section 2). - Add awserrors.IsNotFound; destroy orchestration (instance terminate, EKS CP-VPC teardown, EKS IGW/SG cleanup, k3s rollback, ELBv2 ENI/SG teardown) tolerates NotFound so re-issued/double-targeted deletes converge. - Flip the RLC1 unit invariants to assert *.NotFound on absent; revert the daemon round-trip cases; add IsNotFound + CP-VPC convergence tests. ELBv2 deletes and TerminateInstances stay idempotent (AWS itself is). * fix(network): gate ingress EIP datapath on guest SB Port_Binding convergence TestMultinodeGuestSSH intermittently times out (Connection timed out, not #355's reset-by-peer) connecting to a fresh trio instance's public IP after the suite churns instances across all three chassis. The instance is running and AddEIP has installed the DNAT row, flushed/primed the host neigh, and barriered the gateway-chassis flow — but none of that proves the gatewayLRP->guest geneve hop: until the guest ENI's southbound Port_Binding is up on its hosting chassis the cross-node flow is missing and the DNAT-translated packet blackholes. Add ensureGuestPortDatapath, the guest-side analog of ensureGatewayDatapath: after AddEIP, probe the guest LSP's SB Port_Binding (ovn-sbctl up column); on a miss force one ovn-controller recompute (binds the port / installs the remote flow) and re-probe until a short deadline. Recompute, not RepairDatapath: this is flow/binding convergence, not a WAN-uplink fault. No-op when no verifier is wired or the EIP carries no guest port. spec.PortName is populated for every associated EIP regardless of NAT mode. Extends GatewayClaimVerifier with GuestPortUp and wires the gate into applyEIPs so it runs in the continuous reconcile loop on every node. * Revert "fix(network): gate ingress EIP datapath on guest SB Port_Binding convergence" This reverts commit 728b1a9cec6eaf5c7ef5e9eda4469f239b4d8fc0. * fix(network): key EIP DNAT delete on external IP, not logical IP A dnat_and_snat row's identity on the VPC gateway router is its external IP. The logical (private) IP is not a stable key: the EIP pool round-robins and instance private IPs are reused as instances recycle, so the same logical IP is held by different EIPs over time. DeleteEIP deleted the row by logical IP, so a stale or retried delete for a freed EIP clobbered whichever live EIP now held that recycled private IP -- the next owner's row vanished with no re-add and its public IP went dark (SSH SYN never left br-wan: Connection timed out). Lifecycle-resilience teardown/GC retries amplified this by firing vpc.delete-nat repeatedly. Key the delete on the external IP via DeleteNATByExternalIP and skip when no external IP is supplied (no EIP means no dnat_and_snat row). A stale delete now targets the freed EIP's own external IP (ErrNATNotFound no-op) and can no longer touch the live row. * refactor: relax DeleteNatGateway to AWS-faithful Remove the route-reference dependency guard (checkNatGatewayRouteRefs) so a NAT gateway deletes even while route tables still forward to it, matching AWS (the route blackholes). The reconciler's loadNATGWs drops the SNAT once the record leaves the active bucket, so a forwarding route is not a delete-blocking dependent. EIP is still disassociated on delete (no leak). * test: invert NAT-GW route-ref invariant to AWS-faithful TestRLC3_NatGatewayBlocksWhileRouted asserted DependencyViolation while a route forwarded to the NAT gateway, which is AWS-infaithful. Replace with TestRLC3_NatGatewayDeletesWhileRouted: delete must succeed whether or not a live route still targets it (the route blackholes; the reconciler drops the SNAT). Per ADR-0004 §2. * fix: tear down NAT-GW SNAT on delete (restore delete-time fan-out) The guard commit (53ed55a9) dropped publishDeleteEventsForNatGateway on the assumption a routed NAT GW could never be deleted. With the route-ref guard removed, deleting a NAT GW while routes still target it left the SNAT in place (applyNATGWs is create-only, so the reconciler never prunes it) and egress kept working — TestNATGateway's post-teardown 'lost internet' check failed. Restore the fan-out: on delete, scan route tables and publish vpc.delete-nat-gateway per associated subnet so egress stops, while the route entry survives as a blackhole (AWS-faithful). * fix: probe EIP datapath in reboot self-heal, not the gateway LRP IP The post-reboot gateway datapath self-heal probed the gateway LRP IP for liveness, which OVN answers natively at the router — so it stayed green even when the EIP NAT pipeline was stranded (ofproto/trace dropped EIP traffic at table 0). RepairDatapath never fired and TestRebootResilience timed out, with the drift loop re-probing the same false-green every cycle. Prefer probing an associated EIP: force a fresh ARP of the dnat_and_snat external IP (flush neigh, ping to trigger, read the resolved MAC). OVN answers ARP for the external IP from the gateway chassis independent of the guest, so a resolved MAC proves the WAN uplink forwards and the NAT flows are installed — the signal the LRP IP cannot give. Fall back to the LRP IP for VPCs with no EIP. The existing drift-loop cadence now re-heals a later flow strand. * fix: lb-agent heartbeats over mgmt port, not the WAN/EIP datapath The lb-agent heartbeat to awsgw is control-plane and must survive a host reboot that strands the OVN/EIP north-south data plane. Customer (internet-facing) lb-agents heartbeated over the WAN GatewayURL, riding the EIP datapath, so after a reboot the ALB never reported healthy until the datapath self-healed. System LBs already heartbeated over the on-link br-mgmt NIC; this extends that path to every lb-agent. buildLBAgentEnv now always uses mgmtGatewayURL (on-link MgmtBridgeIP, the mgmt NIC the lb-agent VM already carries), falling back to the WAN URL only when no mgmt bridge exists. Drops the account-id branch. * fix: re-heal guest ENI SB Port_Binding post-reboot so EIP ingress converges Post-reboot every EIP in a VPC goes dark north-south: the DNAT row, host neigh and gateway-chassis flow are all in place (ARP to the EIP resolves) but the gatewayLRP->guest flow is missing until ovn-controller binds the guest ENI's SB Port_Binding, so the DNAT-translated packet blackholes and the public IP is unreachable on an otherwise-running instance. Restore the guest-port convergence gate in applyEIPs: probe the guest LSP's SB Port_Binding up column; on a miss force an ovn-controller recompute and re-probe until a deadline. Unlike the earlier reverted form, recompute on every miss, not once: after a host reboot the guest tap is replumbed several seconds after the reconcile runs, so a single early nudge fires before the port exists and never binds it. The window is widened to span guest boot. This is the southbound analog of the gateway datapath self-heal; the EIP ARP probe cannot detect it because the gateway chassis answers ARP for the external IP even while the guest delivery flow is absent. * fix(network): force post-reboot EIP recompute on guest-port bind, startup-only The reboot suite passed only by accident: the pre-#351 gateway-claim verifier polled the always-chassis-less gateway LRP, so it fired an ovn-controller inc-engine/recompute on every reconcile. That spurious recompute was load-bearing - it reinstalled the centralised gateway DNAT flows that ovn-controller's incremental engine misses on a cold (post-reboot) start. #351 correctly switched the check to the chassisredirect port, eliminating the every-cycle recompute, and the EIP datapath went dark post-reboot with every control-plane signal green. Restore the kick, scoped to the post-reboot startup path: applyEIPs now gates a single recompute on the guest ENI's SB Port_Binding chassis claim - the reliable signal the tap is back, unlike the up column which stays stale-true across a reboot. Drift never recomputes, preserving the #351 fix for freshly attached EIPs. Probe renamed GuestPortUp -> GuestPortBound (chassis, not up). * Revert "fix(network): force post-reboot EIP recompute on guest-port bind, startup-only" This reverts commit 792e318211ef03efbe24febeb470749d88c0b03c. * test(e2e/reboot): capture dataplane diag on ALB serve failure The post-reboot ALB-serve assertion (8.6) failed with no flow/ARP evidence: the OVN/OVS dataplane dumps only ran on the 8.5 targets-unhealthy path, but 8.5 passes on the daemon's own health view while the north-south EIP datapath is dark, so the serve failure (8.6) left nothing to localise the drop. On a serve failure, capture dumpALBProbe/dumpOVNDataplane/dumpOVNClaimState/ dumpHeartbeatDiagnostics/dumpAppDiagnostics before failing. Add to the OVN dataplane dump: dump-flows br-ext and fdb/show br-ext (north-south bridge, previously only br-int), and a real ARP probe that flushes the neigh entry first so primeReachability's pre-seeded reachable entry can no longer mask an unanswered ARP. Pure diagnostics; no production change. * test(e2e/reboot): decisive datapath diag — untruncate br-int, real-ingress ofproto/trace, conntrack + tap capture * test(e2e/reboot): add south->north return-path ofproto/trace to localize egress drop * test(e2e/reboot): derive LRP MAC from realized flow + remediation experiment (recompute vs ovn-controller restart) * test(e2e/reboot): dump persisted SB MAC_Binding + external MACs; flush experiments (conntrack vs MAC_Binding) to isolate recompute-resistant return-path drop * test(e2e/reboot): untruncate Port_Binding + enumerate DGP/chassisredirect claim state (does cr-gw row exist + resident chassis) * test(e2e/reboot): dump NB source of return-path drop (lflow stage, NAT external_mac/options, LR policy, LSP port_security) * fix: exempt public-IP instances from subnet egress drop gate (mulga-siv-311) Post-reboot reconcile installs the priority-1100 subnet egress drop gate for an IGW-attached subnet that has no 0.0.0.0/0 route, but nothing punched the subnet's public-IP instances through it. lr_in_policy runs before lr_out un-DNAT/SNAT, so an inbound connection's reply still carries its private source when it hits the gate and is dropped. The ALB/EIP datapath goes dark south->north post-reboot while NAT, ARP, chassis and guest-port binding all read green; the pre-reboot path survives only because the event-driven gate never evaluates a subnet created after IGW attach. Install the existing priority-1200 /32 reroute (above the gate) for every public-IP instance during reconcile. Reroute only: the instance's dnat_and_snat already supplies SNAT, unlike EnsureSystemInstanceEgress which also adds a plain snat. Public IPs come from two disjoint sources -- auto-assigned/ELB addresses on the ENI record and user EIPs in the EIP bucket -- so drive the exemption from both intent.Ports (new PortSpec.PublicIP) and intent.EIPs. * refactor: scope public-IP drop-gate exemption to gated subnets Only install the priority-1200 reroute when the instance's subnet actually carries a drop gate. Routed subnets egress via their priority-1000 reroute and need no exemption, so bounding on gate presence keeps the change inert for IGW-routed and NATGW-routed subnets (no redundant or overriding policy). Adds a negative test. * refactor: drop unused RemoveEIPInstanceEgress No production caller; the exemption is install-only in reconcile. Removed the interface method and impl rather than leave dead API surface. * test: dump guest console + OVN/datapath diag on SG-to-SG client SSH timeout TestSGToSGDatapath waited for client-vm SSH via EventuallyErr and reported only 'exit status 255' on timeout — no guest-side or datapath state. When the fresh public-IP VM is unreachable for the full window the failure was undiagnosable from CI artifacts. Swap the wait to the non-fatal trySSHReady probe and, on timeout, call DumpVPCFlowDiagnostics (guest console tail, ovn-nbctl/sbctl show, NAT rows, br-int flows / conntrack / host ARP keyed on the public + private IP) before Fatal, mirroring vpc_test/baseline_test. Captures persist to the artifact dir for the Stage 2 analyzer bundle. * fix: ownership-scope public-IP teardown to survive IP recycling A live instance's dnat_and_snat rule, host ARP, and IPAM lease were torn down by the delayed garbage-collection teardown of a different, long-dead instance that previously held the same recycled public IP. The 2-min vm/gc instance-teardown reaper re-runs ReleasePublicIP on a stale terminated record; because the external IPAM round-robins a small pool, the IP had already been reassigned to a live instance, and both teardown primitives acted on the IP by value with no ownership check: - policy.DeleteEIP deleted the dnat_and_snat row by external IP alone, discarding the logical IP it receives. - StaticPoolAllocator.Release freed the pool entry by IP value. Scope both to the current owner so a stale or duplicated teardown for a recycled IP is a no-op: - DeleteEIP looks up the row by external IP and skips the delete (and the ARP flush) when its logical IP no longer matches, mirroring AddEIP's idempotency lookup. Defeats both external- and logical-IP reuse races. - Allocator.Release / ExternalIPAM.ReleaseIP take an ownerENIID; the static allocator no-ops a release whose ENI differs from the live allocation, and treats an already-freed owner-scoped release as idempotent. Teardown and rollback callers pass their ENI; user-driven EIP release passes none. Proven via E2E single TestSGToSGDatapath (run 27515293563): client-vm SSH black-holed after its NAT was deleted by the dead instance's GC sweep. * fix(eks): single-flight DeleteCluster teardown per cluster SDK retries of one DeleteCluster get fanned across nodes by the worker queue group, so purgeClusterInfra raced the same cluster on multiple nodes: duplicate ENI/NLB teardown, duplicate TargetGroupNotFound deregisters, and a real double-release risk on the billable NAT-GW EIP. Idempotency kept it correct but noisy. Add a dedicated per-cluster teardown lease (teardownLeaderKey = reconcilerLeaderKey + "/teardown"), distinct from the reconciler lease so a delete never contends with the still-live reconciler that holds the reconciler lease until it observes DELETING and exits. DeleteCluster acquires it after the NotFound-idempotency check: the loser returns the cluster as DELETING (AWS-async semantics) without running the teardown; the winner drives purgeClusterInfra and releases. The DELETING backstop reaper shares the same lease, so at most one purgeClusterInfra runs per cluster cluster-wide. A nil leaderKV (single-node) skips gating, so local behavior is unchanged. * fix: env-driven SSH-ready budget for slow baremetal guests Add SPINIFEX_SSH_READY_TIMEOUT override for the single-node SSH-handshake probes (default unchanged at 3m). Baremetal boots q35 guests slower (OVMF + NBD-backed disks) than CI loop devices and intermittently misses the 3m window; the harness can now widen it to 5m without affecting shared-runner e2e. Applied at all single-node probe sites; failure messages now report the actual budget. * fix: shared external switch to stop per-VPC localnet ARP collision Per-VPC external switches each attached an untagged localnet port to the same physical network (br-ext). OVN cannot keep multiple untagged localnets on one physical network L2-separate, so with two or more concurrent public-IP VPCs ARP for a public IP landed on the wrong VPC's switch and the owning gateway router's ARP responder never fired, leaving the public IP unreachable. Move to a single shared external switch (provider-network pattern): one switch with one localnet on network_name=external. Each VPC's gateway LRP attaches via its own router-type switch port; per-VPC NAT isolation is unchanged. AttachIGW ensures the shared switch idempotently and is migration-safe; DetachIGW removes only the per-VPC port + LRP and preserves the shared switch. DeleteVPC removes the per-VPC gateway switch port from the shared switch so it does not dangle. Reconcile detects attachment via the gateway LRP since the shared switch carries no vpc_id. * fix: owner-scope DeleteEIP by ENI port to survive identical pair recycling A stale or GC-re-emitted vpc.delete-nat for a terminated instance tore down a live instance's dnat_and_snat rule (and flushed the host ARP) when the public IP and private IP both recycled, together, to that live instance in the same VPC. The (external_ip, logical_ip) pair-key matches under identical recycling, so the existing logical-IP guard fell through. Stamp the owning ENI port (spinifex:logical_port) on the dnat_and_snat row at AddEIP, thread the port through handleDeleteNAT -> DetachEIP -> DeleteEIP, and skip the delete when the row's stamped port differs from the incoming one. The IPAM release was already owner-scoped; this mirrors that guard at the NAT layer so any stale delete for a recycled pair is a no-op. Empty port preserves the prior pair-key behaviour. |
||
|
Julian Sommer
|
50b5495d26 | chore: more comment cleanup | ||
|
joshsiv-mulga
|
eef337fb80 |
test(e2e): make egress baseline reproduce the real IGW gap (fresh VPC)
The previous new-subnet egress baseline used the default VPC, whose main route table is prebaked with 0.0.0.0/0 -> IGW at bootstrap and reconciled by the vpcd drift loop, so a subnet created in it is reachable regardless of the create-time gap — the test passed and never flagged the bug. Replace it with runNewVPCEgressBaseline: a brand-new VPC where the IGW route is added to the MAIN route table BEFORE the public subnet is created. The subnet then implicitly joins a main RT that already carries the route, but CreateSubnet does not recompute per-subnet egress, so the subnet's OVN egress LRP is never installed and the instance has no datapath to the IGW. With an open SG, routing is the only barrier, so the instance is unreachable — red until CreateSubnet triggers a per-subnet egress recompute. PoolMode-gated like runVPCSubnetE2E (fresh VPC needs OVN IPAM for a public IP). runVPCSubnetE2E doesn't catch this: it associates a custom RT then adds the route, so the route mutation enumerates the subnet and installs the LRP. |
||
|
joshsiv-mulga
|
c9a4e904a8 |
test(e2e): fresh-install reachability baselines + drop qemu hostfwd fallback
Add baseline tests that confirm two fresh-install regressions the existing
suite masks (it authorizes tcp/22 on the default SG before every SSH probe):
- default SG default-deny gates external reach of instances in the
default subnet (SG, not routing)
- a newly-created public subnet never gets its OVN IGW egress LRP, so it
has no route to the VPC IGW despite MapPublicIpOnLaunch
Plus a positive test: instances sharing the default SG can communicate via
ICMP (same-host single-node, cross-host multinode).
Single-node baselines run after TestClusterStatsCLI (0-VM gate); multinode
after TestMultinodePreflight (fixture init). Instances are self-cleaning so
they don't perturb the singleton-VM ordering.
Remove the qemu hostfwd SSH fallback from the harness (InstancePublicSSHHost,
GuestSSHEndpoint) - it bypasses the OVN datapath and masks these regressions.
Endpoints now t.Fatal when no public IP is assigned.
|