diff --git a/tests/integration/group_a_test.go b/tests/integration/group_a_test.go index 73fea709..f5965d37 100644 --- a/tests/integration/group_a_test.go +++ b/tests/integration/group_a_test.go @@ -27,6 +27,9 @@ package integration import ( "context" + "encoding/json" + "io" + "net/http" "os/exec" "sort" "testing" @@ -272,10 +275,16 @@ func TestGroupANodeEvacuatePUT(t *testing.T) { t.Fatalf("EVICTED stamped despite in-use refusal; flags=%v", flagsMid) } - // Second call: --force MUST stamp EVICTED. This is the PUT - // route smoke — CLI wrapper fatals on traceback so reaching - // the next assert means the wire shape is intact. - cli.JSON(t, "node", "evacuate", "--force", harness.NodeWorker2) + // Second call: ?force=true MUST stamp EVICTED. The pinned CLI + // (linstor-client 1.27.1, asserted by Group H) has no `--force` + // flag on `node evacuate`, so the force override is exercised + // through the PUT route directly — the same + // `PUT /v1/nodes/{node}/evacuate?force=true` wire shape + // handleRGDelete's force precedent documents. The envelope must + // decode as the LINSTOR `[]ApiCallRc` array (Bug 78 wire). + putRequireAPICallRcEnvelope(t, + stack.RestURL+"/v1/nodes/"+harness.NodeWorker2+"/evacuate?force=true", + http.StatusOK) flagsAfter := nodeFlags(t, stack, harness.NodeWorker2) if !containsString(flagsAfter, "EVICTED") { @@ -288,13 +297,19 @@ func TestGroupANodeEvacuatePUT(t *testing.T) { // evacuate (blockstor's REST shim wires both to handleNodeEvacuate); // without the PUT route, golinstor's `NodeService.Evict` (which // `doPUT`s) crashes the python decoder with an empty 405 body. +// +// The pinned CLI (linstor-client 1.27.1, asserted by Group H) does +// not ship the `node evict` verb yet, so the PUT route is driven +// directly over HTTP — the exact wire shape golinstor's +// NodeService.Evict emits (`PUT /v1/nodes/{node}/evict`, empty JSON +// body, `[]ApiCallRc` response). func TestGroupANodeEvictPUT(t *testing.T) { stack := harness.StartStack(t) harness.SeedThreeNodeCluster(t, stack) - cli := &harness.CLI{URL: stack.RestURL} - // `linstor node evict ` — same wire as evacuate. - cli.JSON(t, "node", "evict", harness.NodeWorker3) + putRequireAPICallRcEnvelope(t, + stack.RestURL+"/v1/nodes/"+harness.NodeWorker3+"/evict", + http.StatusOK) flags := nodeFlags(t, stack, harness.NodeWorker3) if !containsString(flags, "EVICTED") { @@ -310,13 +325,25 @@ func TestGroupANodeEvictPUT(t *testing.T) { // DeletionTimestamp stamp would hang every orphan forever and brick // the next RD-create that recycles the name/port allocation. // -// We seed a Resource on worker-1 + a peer replica on worker-2, then -// call `n lost worker-1` and assert the worker-1 replica is gone -// while worker-2 survives. +// We seed a Resource on worker-1 + a peer replica on worker-2, take +// worker-1's satellite offline (the documented `n lost` precondition +// — the Bug 111 gate refuses `n lost` against an ONLINE satellite), +// then call `n lost worker-1` and assert the worker-1 replica is +// gone while worker-2 survives. func TestGroupANodeLostCascadesOrphans(t *testing.T) { stack := harness.StartStack(t) harness.SeedThreeNodeCluster(t, stack) + // Kill worker-1's satellite heartbeat and wait for the OFFLINE + // status to land — `n lost` against an ONLINE satellite is + // correctly refused with 409 (Bug 111), and the machine-readable + // CLI exits 0 even on refusal envelopes, so calling too early + // would silently no-op and the cascade assert below would time + // out with a misleading message. + stack.Satellite.SimulateNodeOffline(harness.NodeWorker1) + waitForNodeConnectionStatus(t, stack, harness.NodeWorker1, + blockstoriov1alpha1.NodeConnectionStatusOffline) + ctx := context.Background() rd := &blockstoriov1alpha1.ResourceDefinition{ @@ -616,6 +643,55 @@ func retryStatusPatch(ctx context.Context, stack *harness.Stack, name string, return lastErr } +// putRequireAPICallRcEnvelope issues a JSON PUT with an empty-object +// body and asserts the response status matches and the body decodes +// as the LINSTOR `[]ApiCallRc` array shape — the envelope +// python-linstor/golinstor expect from the node-lifecycle PUT routes +// (Bug 78: an empty or non-JSON body crashes the python decoder). +func putRequireAPICallRcEnvelope(t *testing.T, url string, wantStatus int) { + t.Helper() + + resp := httpPutGroupH(t, url, []byte("{}")) + + defer func() { _ = resp.Body.Close() }() + + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("read response body: %v", err) + } + + if resp.StatusCode != wantStatus { + t.Fatalf("status: got %d, want %d; body: %s", resp.StatusCode, wantStatus, body) + } + + var envelope []map[string]any + + err = json.Unmarshal(body, &envelope) + if err != nil { + t.Fatalf("body is not a []ApiCallRc envelope: %v; body: %s", err, body) + } + + if len(envelope) == 0 { + t.Fatalf("empty []ApiCallRc envelope; body: %s", body) + } +} + +// waitForNodeConnectionStatus blocks until the Node CRD's +// Status.ConnectionStatus reaches `want` — used to sequence the +// satellite-offline simulation before `n lost` (Bug 111 gate). +func waitForNodeConnectionStatus(t *testing.T, stack *harness.Stack, node, want string) { + t.Helper() + + harness.Eventually(t, 10*time.Second, func() bool { + var got blockstoriov1alpha1.Node + + err := stack.Env.Client.Get(context.Background(), + types.NamespacedName{Name: node}, &got) + + return err == nil && got.Status.ConnectionStatus == want + }, "node "+node+" never reached ConnectionStatus="+want) +} + // runCLINoFatal runs the CLI and returns stdout + a best-effort // success flag. Unlike CLI.Run/JSON, this does NOT t.Fatal on // non-zero exit — used by tests that expect the call to fail diff --git a/tests/integration/group_f_test.go b/tests/integration/group_f_test.go index 3bb5754f..1edfcfeb 100644 --- a/tests/integration/group_f_test.go +++ b/tests/integration/group_f_test.go @@ -260,19 +260,22 @@ func TestGroupFRToggleCancel(t *testing.T) { } // --------------------------------------------------------------------------- -// TestGroupFRToggleDiskful2DisklessPreservesTieBreaker — Bug 104 -// (P1, QUORUM HAZARD). Starting from the steady auto-place(2) state -// (2 diskful + 1 TIE_BREAKER on the 3rd node), `linstor r td -// --diskless ` MUST leave the TIE_BREAKER Resource -// intact. Pre-fix, the RD reconciler recomputed wantWitness from -// scratch after the toggle, saw "1 diskful + 1 non-witness diskless" -// (the freshly-flipped replica), flipped its decision to "no witness -// needed", and DELETED the TIE_BREAKER Resource — collapsing the -// quorum surface to 1 diskful + 1 diskless with no third voter. The -// next network partition would freeze the volume read-only. +// TestGroupFRToggleDiskful2DisklessReapsTieBreaker — upstream-parity +// contract for the toggle-to-1-diskful path (supersedes the Bug 104 +// "preserve" pin, inverted alongside the L1 specs in +// internal/controller/ensure_tiebreaker_test.go). Starting from the +// steady auto-place(2) state (2 diskful + 1 TIE_BREAKER on the 3rd +// node), `linstor r td --diskless ` leaves 1 diskful +// + 1 user-diskless — at which point quorumPolicy returns quorum=off: +// there is no diskful tie to break and no majority to freeze. +// Upstream LINSTOR's shouldTieBreakerExist never manages a witness +// below 2 diskful, so blockstor REAPS the now-redundant TIE_BREAKER, +// leaving exactly 2 replicas. (The former Bug 104/108 keep/create +// branches rested on the false premise that 1 diskful + 1 diskless +// freezes quorum:majority.) // --------------------------------------------------------------------------- -func TestGroupFRToggleDiskful2DisklessPreservesTieBreaker(t *testing.T) { +func TestGroupFRToggleDiskful2DisklessReapsTieBreaker(t *testing.T) { stack, cli, rd := setupGroupFRD(t, "td-tb") // Steady state: auto-place 2 lands diskful on worker-1+worker-2, @@ -299,22 +302,22 @@ func TestGroupFRToggleDiskful2DisklessPreservesTieBreaker(t *testing.T) { return r != nil && groupFContains(r.Spec.Flags, "DISKLESS") }, "Resource "+rd+"."+target+" never gained DISKLESS flag") - // Bug 104 invariant: the TIE_BREAKER Resource on `witnessNode` - // MUST still exist after the toggle settles. We give the RD - // reconciler a generous beat (rdReconcileRequeue + apply) and - // poll for a STABLE 3-replica composition: 1 diskful + 1 - // non-witness diskless + 1 TIE_BREAKER. + // Upstream-parity invariant: the TIE_BREAKER on `witnessNode` is + // REAPED once the toggle settles. We give the RD reconciler a + // generous beat (rdReconcileRequeue + apply) and poll for a + // STABLE 2-replica composition: 1 diskful + 1 user-diskless, + // 0 witnesses. deadline := time.Now().Add(groupFAssertTimeout) for time.Now().Before(deadline) { all := listResourcesByRD(t, stack, rd) - if assertBug104Composition(all, witnessNode) { + if assertWitnessReapedComposition(all) { time.Sleep(2 * time.Second) // settle window all = listResourcesByRD(t, stack, rd) - if assertBug104Composition(all, witnessNode) { + if assertWitnessReapedComposition(all) { return // success } } @@ -323,23 +326,24 @@ func TestGroupFRToggleDiskful2DisklessPreservesTieBreaker(t *testing.T) { } all := listResourcesByRD(t, stack, rd) - t.Fatalf("Bug 104: post-toggle replica set drifted from "+ - "{1 diskful + 1 user-diskless + 1 TIE_BREAKER on %s}; got %d entries: %v", + t.Fatalf("post-toggle replica set drifted from "+ + "{1 diskful + 1 user-diskless, witness on %s reaped}; got %d entries: %v", witnessNode, len(all), all) } -// assertBug104Composition returns true iff the replica set looks -// like {1 diskful + 1 user-diskless + 1 TIE_BREAKER on witnessNode}. -// Bug 104's failure mode is the TIE_BREAKER on witnessNode getting -// reaped, so we pin both the count (3) and the node identity. -func assertBug104Composition(all []blockstoriov1alpha1.Resource, witnessNode string) bool { - if len(all) != 3 { +// assertWitnessReapedComposition returns true iff the replica set +// looks like {1 diskful + 1 user-diskless} with NO TIE_BREAKER left. +// The upstream-parity failure mode is the auto-witness surviving (or +// being re-created) below 2 diskful, so we pin the exact count (2) +// and a zero witness count. +func assertWitnessReapedComposition(all []blockstoriov1alpha1.Resource) bool { + if len(all) != 2 { return false } - witnessFound := false diskfulCount := 0 userDisklessCount := 0 + witnessCount := 0 for i := range all { isDiskless := groupFContains(all[i].Spec.Flags, "DISKLESS") @@ -347,9 +351,7 @@ func assertBug104Composition(all []blockstoriov1alpha1.Resource, witnessNode str switch { case isTB: - if all[i].Spec.NodeName == witnessNode { - witnessFound = true - } + witnessCount++ case isDiskless: userDisklessCount++ default: @@ -357,7 +359,7 @@ func assertBug104Composition(all []blockstoriov1alpha1.Resource, witnessNode str } } - return witnessFound && diskfulCount == 1 && userDisklessCount == 1 + return witnessCount == 0 && diskfulCount == 1 && userDisklessCount == 1 } // --------------------------------------------------------------------------- diff --git a/tests/integration/group_g_test.go b/tests/integration/group_g_test.go index acf8b953..0760b407 100644 --- a/tests/integration/group_g_test.go +++ b/tests/integration/group_g_test.go @@ -55,6 +55,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/retry" blockstoriov1alpha1 "github.com/cozystack/blockstor/api/v1alpha1" "github.com/cozystack/blockstor/internal/controller" @@ -101,21 +102,16 @@ func seedRDWithVolume(t *testing.T, stack *harness.Stack, suffix string) string ctx := context.Background() rdName := groupGRDName(suffix) - rd := &blockstoriov1alpha1.ResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: rdName}, - Spec: blockstoriov1alpha1.ResourceDefinitionSpec{ - ResourceGroupName: harness.FixtureDefaultRG, - VolumeDefinitions: []blockstoriov1alpha1.ResourceDefinitionVolume{ - {VolumeNumber: 0, SizeKib: 1024 * 1024}, - }, - }, - } - - err := stack.Env.Client.Create(ctx, rd) - if err != nil { - t.Fatalf("seed RD %q: %v", rdName, err) - } - + // Seed the per-node Resources BEFORE the RD: the RD reconciler's + // auto-tiebreaker pass fires as soon as the RD lands, and with + // only worker-1/worker-2 visible (2 diskful) it races this loop + // to worker-3 with its own DISKLESS TIE_BREAKER row — the seeded + // worker-3 create then flakes with AlreadyExists. With all three + // diskful rows in place first, the reconciler sees 3 diskful and + // wants no witness (post-#129 invariant: witness iff exactly 2). + // Fresh Resource stubs without a parent RD are deliberately left + // alone by the orphan path (see resource_controller.go + // resourceWasApplied), so the reversed order is safe. for _, node := range harness.FixtureNodes() { r := &blockstoriov1alpha1.Resource{ ObjectMeta: metav1.ObjectMeta{ @@ -142,6 +138,21 @@ func seedRDWithVolume(t *testing.T, stack *harness.Stack, suffix string) string } } + rd := &blockstoriov1alpha1.ResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: rdName}, + Spec: blockstoriov1alpha1.ResourceDefinitionSpec{ + ResourceGroupName: harness.FixtureDefaultRG, + VolumeDefinitions: []blockstoriov1alpha1.ResourceDefinitionVolume{ + {VolumeNumber: 0, SizeKib: 1024 * 1024}, + }, + }, + } + + err := stack.Env.Client.Create(ctx, rd) + if err != nil { + t.Fatalf("seed RD %q: %v", rdName, err) + } + return rdName } @@ -282,7 +293,7 @@ func TestGroupG(t *testing.T) { t.Run("SnapDeleteIdempotent", func(t *testing.T) { testGroupGSnapDeleteIdempotent(t, stack, cli) }) t.Run("SnapRestoreCreatesNewRD", func(t *testing.T) { testGroupGSnapRestoreCreatesNewRD(t, stack, cli) }) t.Run("SnapRollbackOnExistingRD", func(t *testing.T) { testGroupGSnapRollbackOnExistingRD(t, stack, cli) }) - t.Run("SnapShipCrossNode", func(t *testing.T) { testGroupGSnapShipCrossNode(t, stack, cli) }) + t.Run("SnapRestoreSnapshotHolderOnly", func(t *testing.T) { testGroupGSnapRestoreSnapshotHolderOnly(t, stack, cli) }) t.Run("SnapOrphanCleanup", func(t *testing.T) { testGroupGSnapOrphanCleanup(t, stack, cli) }) t.Run("SnapDeleteBlockedByLater", func(t *testing.T) { testGroupGSnapDeleteBlockedByLater(t, stack, cli) }) t.Run("AutoSnapshotPeriodicTick", func(t *testing.T) { testGroupGAutoSnapshotPeriodicTick(t, stack) }) @@ -564,20 +575,24 @@ func testGroupGSnapRollbackOnExistingRD(t *testing.T, stack *harness.Stack, cli } } -// testGroupGSnapShipCrossNode pins F8: cross-node snapshot ship. +// testGroupGSnapRestoreSnapshotHolderOnly pins the Bug 397 (P0 DATA +// INTEGRITY) restore-target contract at the integration tier +// (supersedes the pre-397 "cross-node ship via restore" pin): +// +// 1. An explicit `node_names` restore onto a node that does NOT +// hold the snapshot is REFUSED with 400 before any Store +// mutation — a diskful replica on a snapshot-less node would be +// created empty and could silently latch UpToDate without the +// snapshot's data (promotable-on-failover empty copy). +// 2. A restore onto a snapshot-HOLDING node succeeds (201) and the +// restored RD carries the `BlockstorRestoreFromSnapshot= +// :` routing prop the satellite-side dispatcher +// reads to materialise the clone. // -// Phase 0 caveat (harness/satellite.go:152): the satellite mock's -// FakeExec is a slot-only stub — it does not actually capture -// shell-outs. So instead of asserting that `zfs send | zfs recv` -// was invoked, we assert the controller-side seed shape a real -// ship flow depends on: the cross-node snapshot-restore endpoint -// accepts a `node_names` body, the restored RD is created with the -// satellite-dispatcher routing prop set, and the Snapshot CRD's -// per-node materialisation includes only the source-side nodes (so -// populating the destination node would require an actual ship). -// The send-recv command capture lands in Group F or in the -// satellite unit tests once FakeExec graduates from stub. -func testGroupGSnapShipCrossNode(t *testing.T, stack *harness.Stack, cli *harness.CLI) { +// The per-node REST/store seed shape (snapshot present only on the +// source diskful set) is asserted en route, matching the unit pin in +// pkg/rest/bug_397_restore_snapshotless_node_test.go. +func testGroupGSnapRestoreSnapshotHolderOnly(t *testing.T, stack *harness.Stack, cli *harness.CLI) { ctx := context.Background() srcRD := groupGRDName("ship-src") @@ -631,8 +646,10 @@ func testGroupGSnapShipCrossNode(t *testing.T, stack *harness.Stack, cli *harnes t.Fatalf("snap.Spec.Nodes = %v, want %v (source diskful set)", snap.Spec.Nodes, want) } - // Restore into a fresh RD — production semantics would have - // the satellite-side reconciler then ship from N1/N2 to N3. + // The Bug-397 refusal: worker-3 does NOT hold the snapshot, so an + // explicit node_names restore onto it must be rejected with 400 + // (and no target RD created) — never silently stamp a diskful + // Resource that would be materialised empty. dstRD := groupGRDName("ship-dst") status, body := httpPostJSON(t, stack.RestURL+"/v1/resource-definitions/"+srcRD+"/snapshot-restore-resource", @@ -641,8 +658,30 @@ func testGroupGSnapShipCrossNode(t *testing.T, stack *harness.Stack, cli *harnes "snapshot_name": "ship-snap", "node_names": []string{harness.NodeWorker3}, }) + if status != http.StatusBadRequest { + t.Fatalf("snapshot-restore onto snapshot-less node: status %d, want 400; body %s", + status, string(body)) + } + + var refusedRD blockstoriov1alpha1.ResourceDefinition + + err := stack.Env.Client.Get(ctx, types.NamespacedName{Name: dstRD}, &refusedRD) + if err == nil { + t.Fatalf("refused restore still created target RD %q", dstRD) + } + + // Holder-node restore: worker-1 holds the snapshot, so the same + // request against it succeeds and stamps the dispatcher routing + // prop on the restored RD. + status, body = httpPostJSON(t, + stack.RestURL+"/v1/resource-definitions/"+srcRD+"/snapshot-restore-resource", + map[string]any{ + "to_resource": dstRD, + "snapshot_name": "ship-snap", + "node_names": []string{harness.NodeWorker1}, + }) if status != http.StatusCreated { - t.Fatalf("snapshot-restore: status %d, body %s", status, string(body)) + t.Fatalf("snapshot-restore onto holder node: status %d, body %s", status, string(body)) } // The restored RD carries the cross-node ship marker @@ -820,22 +859,34 @@ func testGroupGAutoSnapshotPeriodicTick(t *testing.T, stack *harness.Stack) { ctx := context.Background() rdName := seedRDWithVolume(t, stack, "auto-snap") - var rd blockstoriov1alpha1.ResourceDefinition - if err := stack.Env.Client.Get(ctx, - types.NamespacedName{Name: rdName}, &rd); err != nil { - t.Fatalf("Get RD: %v", err) - } - - if rd.Spec.Props == nil { - rd.Spec.Props = map[string]string{} - } - // Stamp `AutoSnapshot/RunEvery=1` (minutes) so the runnable // considers this RD due. Keep defaults to 10 — prune is a // no-op until we exceed that. - rd.Spec.Props[controller.PropAutoSnapshotRunEvery] = "1" + // + // Retry-on-conflict: the RD reconciler runs live on the manager + // and routinely bumps the RD's resourceVersion between our Get + // and Update (e.g. the auto-tiebreaker pass right after the + // fixture spawn) — a single-shot Update flakes with "the object + // has been modified". + if err := retry.RetryOnConflict(retry.DefaultBackoff, func() error { + var rd blockstoriov1alpha1.ResourceDefinition + if gErr := stack.Env.Client.Get(ctx, + types.NamespacedName{Name: rdName}, &rd); gErr != nil { + return fmt.Errorf("get RD %q: %w", rdName, gErr) + } + + if rd.Spec.Props == nil { + rd.Spec.Props = map[string]string{} + } + + rd.Spec.Props[controller.PropAutoSnapshotRunEvery] = "1" + + if uErr := stack.Env.Client.Update(ctx, &rd); uErr != nil { + return fmt.Errorf("update RD %q: %w", rdName, uErr) + } - if err := stack.Env.Client.Update(ctx, &rd); err != nil { + return nil + }); err != nil { t.Fatalf("set AutoSnapshot/RunEvery: %v", err) } diff --git a/tests/integration/group_i_test.go b/tests/integration/group_i_test.go index 8dfe0eaf..1cfcdbb4 100644 --- a/tests/integration/group_i_test.go +++ b/tests/integration/group_i_test.go @@ -184,9 +184,12 @@ func TestGroupI(t *testing.T) { // the contract that matters — accepting the modify body without // rejecting on shape. // -// Re-reading the connection via GET confirms the canonical empty-bag -// envelope still surfaces (golinstor decodes `{node_a, node_b, -// properties:{}}` into a zero-value NodeConnection without error). +// Re-reading the connection via GET confirms the canonical envelope +// still surfaces. The upstream wire key for the property bag is +// `props` — golinstor's NodeConnection struct is `{node_a, node_b, +// props, flags, port}` (client/connection.go) and blockstor's +// handler emits an empty `props` object (never `null`) when no +// properties are set. func groupINodeConnectionSetProperty(t *testing.T, stack *harness.Stack) { t.Helper() @@ -234,8 +237,8 @@ func groupINodeConnectionSetProperty(t *testing.T, stack *harness.Stack) { t.Errorf("envelope node fields: got %+v", got) } - if _, ok := got["properties"].(map[string]any); !ok { - t.Errorf("envelope missing properties map: %+v", got) + if _, ok := got["props"].(map[string]any); !ok { + t.Errorf("envelope missing props map (upstream NodeConnection wire key): %+v", got) } } diff --git a/tests/integration/group_k_test.go b/tests/integration/group_k_test.go index 6d9119b6..b2476c3d 100644 --- a/tests/integration/group_k_test.go +++ b/tests/integration/group_k_test.go @@ -97,7 +97,17 @@ func TestGroupKWFHappyPath(t *testing.T) { cli.Run(t, "resource-definition", "create", rdName, "--resource-group", wfRG) cli.Run(t, "volume-definition", "create", rdName, "4M") - cli.Run(t, "resource", "create", "--auto-place", "2", rdName) + // Pin the autoplace to a thin (snapshot-capable) pool the way + // linstor-csi always does via its StorageClass `storagePool` + // parameter. Unpinned, the placer is free to land on the + // fixture's thick `file` pool and the snapshot step below is + // then correctly refused by the G5 thick-provider gate (only + // LVM_THIN / ZFS_THIN / FILE_THIN can take COW snapshots). + // (Positional-first ordering: 1.27.1's argparse mis-counts the + // trailing positional after two valued options — same form + // Group F uses.) + cli.Run(t, "resource", "create", rdName, "--auto-place", "2", + "--storage-pool", "lvm-thin") waitForDiskfulReplicaCount(t, stack, rdName, 2) waitForDRBDUpToDate(t, stack, rdName, 2) @@ -154,13 +164,21 @@ func TestGroupKWFLateVD(t *testing.T) { waitForDRBDUpToDate(t, stack, rdName, 2) + // The Bug-79 failure mode is a DISKFUL replica left pinned to + // DISKLESS. The auto-tiebreaker WITNESS on the third node is + // expected here — post-#129 the invariant is "witness lives iff + // exactly 2 diskful replicas", and this RD has exactly 2 — so + // the TIE_BREAKER (which is DISKLESS by construction) is + // excluded from the regression check. resources := listResourcesOfRD(t, stack, rdName) for i := range resources { - for _, fl := range resources[i].Spec.Flags { - if fl == "DISKLESS" { - t.Fatalf("Bug 79 regression: replica %s pinned to DISKLESS after late VD add", - resources[i].Name) - } + if resourceHasFlag(&resources[i], "TIE_BREAKER") { + continue + } + + if resourceHasFlag(&resources[i], "DISKLESS") { + t.Fatalf("Bug 79 regression: replica %s pinned to DISKLESS after late VD add", + resources[i].Name) } } @@ -324,7 +342,18 @@ func TestGroupKWFNodeLostCascade(t *testing.T) { cli.Run(t, "resource", "create", "--auto-place", "3", rdName) waitForDiskfulReplicaCount(t, stack, rdName, 3) + // Take worker-2's satellite offline first — that is the + // documented `n lost` precondition. The Bug 111 gate refuses + // `n lost` against an ONLINE satellite with 409 (the safe + // alternatives are `n d` / `n evacuate`), and the + // machine-readable CLI exits 0 even on refusal envelopes, so + // losing an online node here would silently no-op and the + // cascade asserts below would time out. lost := harness.NodeWorker2 + stack.Satellite.SimulateNodeOffline(lost) + waitForNodeConnectionStatus(t, stack, lost, + blockstoriov1alpha1.NodeConnectionStatusOffline) + cli.Run(t, "node", "lost", lost) // Node row must vanish. @@ -418,17 +447,30 @@ func TestGroupKWFPoolDestroyedDropsFromPlacer(t *testing.T) { // Now spawn an RD pinned to the lvm-thin storage pool with // PlaceCount=2. The placer MUST skip worker-1's missing pool and // land both replicas on worker-2/worker-3. + // `rd create` in the pinned linstor-client 1.27.1 has no + // `--storage-pool` flag — the pool constraint rides on the + // autoplace call below, exactly like the operator flow. rdName := "wf-pool-gone" - cli.Run(t, "resource-definition", "create", rdName, "--resource-group", wfRG, "--storage-pool", deadPool) + cli.Run(t, "resource-definition", "create", rdName, "--resource-group", wfRG) cli.Run(t, "volume-definition", "create", rdName, "4M") - cli.Run(t, "resource", "create", "--auto-place", "2", - "--storage-pool", deadPool, rdName) + cli.Run(t, "resource", "create", rdName, "--auto-place", "2", + "--storage-pool", deadPool) waitForDiskfulReplicaCount(t, stack, rdName, 2) - for _, r := range listResourcesOfRD(t, stack, rdName) { - if r.Spec.NodeName == deadNode { - t.Errorf("placer landed replica on node %s with PoolMissing pool %s — Bug 83 regression", + // Diskful replicas only: the auto-tiebreaker witness (expected on + // the remaining node for a 2-diskful RD post-#129) is a + // network-only DISKLESS attachment that consumes no pool, so a + // witness on the dead-pool node is fine — Bug 83 is about the + // placer backing a DISKFUL replica with a missing pool. + resources := listResourcesOfRD(t, stack, rdName) + for i := range resources { + if resourceHasFlag(&resources[i], "DISKLESS") { + continue + } + + if resources[i].Spec.NodeName == deadNode { + t.Errorf("placer landed diskful replica on node %s with PoolMissing pool %s — Bug 83 regression", deadNode, deadPool) } } @@ -467,15 +509,25 @@ func TestGroupKWFReplicasOnSame(t *testing.T) { cli.Run(t, "resource", "create", "--auto-place", "2", rdName) waitForDiskfulReplicaCount(t, stack, rdName, 2) + // `replicas-on-same` constrains DISKFUL placement. The + // auto-tiebreaker witness (DISKLESS + TIE_BREAKER, expected on + // the third node post-#129 because the RD has exactly 2 diskful + // replicas) is a network-only attachment that deliberately lands + // OUTSIDE the diskful zone group, so it is excluded from the + // zone-spread assertion. resources := listResourcesOfRD(t, stack, rdName) zoneSeen := map[string]bool{} for i := range resources { + if resourceHasFlag(&resources[i], "DISKLESS") { + continue + } + zoneSeen[nodeZones[resources[i].Spec.NodeName]] = true } if len(zoneSeen) != 1 { - t.Errorf("replicas-on-same violated: replicas span %d zones %v; %+v", + t.Errorf("replicas-on-same violated: diskful replicas span %d zones %v; %+v", len(zoneSeen), zoneSeen, resources) } @@ -514,13 +566,22 @@ func TestGroupKWFReplicasOnDifferent(t *testing.T) { cli.Run(t, "resource", "create", "--auto-place", "2", rdName) waitForDiskfulReplicaCount(t, stack, rdName, 2) + // Diskful replicas only — the auto-tiebreaker witness is a + // network-only attachment outside the placement constraint and + // would otherwise mask a violation by adding its own zone. zones := map[string]bool{} - for _, r := range listResourcesOfRD(t, stack, rdName) { - zones[nodeZones[r.Spec.NodeName]] = true + + resources := listResourcesOfRD(t, stack, rdName) + for i := range resources { + if resourceHasFlag(&resources[i], "DISKLESS") { + continue + } + + zones[nodeZones[resources[i].Spec.NodeName]] = true } if len(zones) < 2 { - t.Errorf("replicas-on-different violated: replicas all in %v", zones) + t.Errorf("replicas-on-different violated: diskful replicas all in %v", zones) } } @@ -541,7 +602,9 @@ func TestGroupKWFLUKSStackEndToEnd(t *testing.T) { cli := &harness.CLI{URL: stack.RestURL} // Unlock the controller before any LUKS-stack provisioning. - cli.Run(t, "encryption", "create-passphrase", "--new-passphrase", "supersecret-passphrase-1") + // The pinned linstor-client 1.27.1 spells the flag + // `-p/--passphrase` (no `--new-passphrase`). + cli.Run(t, "encryption", "create-passphrase", "--passphrase", "supersecret-passphrase-1") rdName := "wf-luks" cli.Run(t, "resource-definition", "create", rdName, "--resource-group", wfRG, @@ -869,6 +932,19 @@ func waitForSnapshotAbsent(t *testing.T, stack *harness.Stack, rdName, snapName }, "Snapshot "+full+" not deleted") } +// resourceHasFlag reports whether the Resource's Spec.Flags carries +// the named flag. Used to tell auto-tiebreaker witnesses (DISKLESS + +// TIE_BREAKER) apart from diskful replicas in placement asserts. +func resourceHasFlag(r *blockstoriov1alpha1.Resource, flag string) bool { + for _, fl := range r.Spec.Flags { + if fl == flag { + return true + } + } + + return false +} + // getRDWithVDs fetches the RD CRD; tests use it to read // Spec.VolumeDefinitions and Spec.LayerStack. func getRDWithVDs(t *testing.T, stack *harness.Stack, rdName string) *blockstoriov1alpha1.ResourceDefinition { diff --git a/tests/integration/harness/fixtures.go b/tests/integration/harness/fixtures.go index fee2984b..9984ab10 100644 --- a/tests/integration/harness/fixtures.go +++ b/tests/integration/harness/fixtures.go @@ -134,7 +134,16 @@ func seedStoragePool(ctx context.Context, t *testing.T, cli client.Client, node, pool2 := &blockstoriov1alpha1.StoragePool{ // Composite name pinned by the CEL XValidation rule on // StoragePool — keep `.` exactly. - ObjectMeta: metav1.ObjectMeta{Name: pool + "." + node}, + // + // The `blockstor.io/node-name` label mirrors what + // pkg/store/k8s.(*storagePools).Create stamps on every + // store-created pool: the store's ListByNode runs a label + // selector, so a fixture pool seeded without it is invisible + // to per-node store reads (e.g. the `n lost` SP cascade). + ObjectMeta: metav1.ObjectMeta{ + Name: pool + "." + node, + Labels: map[string]string{"blockstor.io/node-name": node}, + }, Spec: blockstoriov1alpha1.StoragePoolSpec{ NodeName: node, PoolName: pool, diff --git a/tests/integration/harness/satellite.go b/tests/integration/harness/satellite.go index d0c4b549..3bde33f6 100644 --- a/tests/integration/harness/satellite.go +++ b/tests/integration/harness/satellite.go @@ -82,6 +82,13 @@ type Satellite struct { // drbdState[rdName][node] → forced DrbdState for that replica. drbdState map[string]map[string]string + // nodeOffline[node] true → satellite stamps ConnectionStatus + // OFFLINE instead of ONLINE — models a dead satellite so `linstor + // n lost` flows can pass the Bug 111 online-refusal gate the way + // they do in production (the documented `n lost` precondition is + // a satellite that no longer heartbeats). + nodeOffline map[string]bool + // failNext is the queue of pending FakeExec-style failures the // satellite mock is asked to inject the next time it sees a // matching command. Phase 0 stores them but does not consult @@ -99,6 +106,7 @@ func NewSatellite(cli client.Client) *Satellite { client: cli, poolMissing: map[string]map[string]bool{}, drbdState: map[string]map[string]string{}, + nodeOffline: map[string]bool{}, tickInterval: satelliteTickInterval, } } @@ -137,6 +145,18 @@ func (s *Satellite) SimulatePoolMissing(node, pool string) { s.poolMissing[node][pool] = true } +// SimulateNodeOffline makes the mock stamp ConnectionStatus OFFLINE +// for the named node from the next tick on (instead of the default +// ONLINE heartbeat). Models a dead/unreachable satellite — the +// documented precondition for `linstor n lost` (Bug 111 gate: the +// REST handler refuses `n lost` against an ONLINE satellite). +func (s *Satellite) SimulateNodeOffline(node string) { + s.mu.Lock() + defer s.mu.Unlock() + + s.nodeOffline[node] = true +} + // SimulateDRBDState pins the DrbdState the satellite will stamp on // (rdName, node). Use to force `SyncTarget`, `Outdated`, `Failed`, // etc. for tests that need a non-healthy resource. @@ -191,6 +211,10 @@ func (s *Satellite) reconcileNodes(ctx context.Context) { node := &nodes.Items[i] desiredStatus := blockstoriov1alpha1.NodeConnectionStatusOnline + if s.isNodeOffline(node.Name) { + desiredStatus = blockstoriov1alpha1.NodeConnectionStatusOffline + } + if node.Status.ConnectionStatus == desiredStatus && hasReadyTrue(node.Status.Conditions) { continue } @@ -320,6 +344,13 @@ func (s *Satellite) isPoolMissing(node, pool string) bool { return s.poolMissing[node][pool] } +func (s *Satellite) isNodeOffline(node string) bool { + s.mu.Lock() + defer s.mu.Unlock() + + return s.nodeOffline[node] +} + // hasReadyTrue is a tiny helper kept off the global namespace so // internal callers don't accidentally use it as a public assertion. func hasReadyTrue(conds []metav1.Condition) bool {