Skip to content

Commit 1728baf

Browse files
kvapsclaude
andauthored
test(integration): make the suite honest under pipefail; pin post-#129/Bug-397 contracts (#137)
* test(integration): add SimulateNodeOffline knob to satellite mock The mock stamped every Node ONLINE unconditionally, making it impossible to exercise `linstor n lost` honestly: the Bug 111 gate correctly refuses node-lost against an ONLINE satellite, and the machine-readable CLI exits 0 on refusal envelopes, so the lost-cascade tests silently no-op'd and timed out. The knob models the documented n-lost precondition (a satellite that stopped heartbeating). Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com> * test(integration): drive evacuate-force and evict through the PUT routes The pinned linstor-client 1.27.1 (asserted by Group H) has no `node evacuate --force` flag and no `node evict` verb at all, so both tests crashed on CLI argument parsing before reaching the wire. Exercise the force override via PUT /v1/nodes/{node}/evacuate?force=true and the evict alias via PUT /v1/nodes/{node}/evict directly, asserting the []ApiCallRc envelope shape both routes must answer (Bug 78 wire). Node-lost cascade now takes the satellite offline first — the documented n-lost precondition — instead of silently hitting the Bug 111 online-refusal and timing out. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com> * test(integration): invert tiebreaker toggle pin to post-#129 reap contract TestGroupFRToggleDiskful2DisklessPreservesTieBreaker pinned the pre-#129 invariant (witness preserved at 1 diskful + 1 user-diskless). PR #129 collapsed the auto-witness contract to upstream parity — "witness lives iff exactly 2 diskful replicas" — so the toggle down to 1 diskful now reaps the witness. Rename to ...ReapsTieBreaker and assert the exact 2-replica composition (1 diskful + 1 user-diskless, 0 witnesses), mirroring the L1 spec inversion in internal/controller/ensure_tiebreaker_test.go. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com> * test(integration): pin Bug 397 restore contract; retry RunEvery stamp on conflict SnapShipCrossNode pinned the pre-Bug-397 behaviour where an explicit node_names restore onto a snapshot-less node succeeded and acted as a cross-node ship trigger. Bug 397 (P0 data integrity) now refuses that restore at the API edge — an empty diskful replica could latch UpToDate and be promoted on failover. Rename the subtest to SnapRestoreSnapshotHolderOnly and pin both halves of the new contract: 400 + no target RD for the snapshot-less node, 201 + the BlockstorRestoreFromSnapshot routing prop for a holder node. AutoSnapshotPeriodicTick stamped AutoSnapshot/RunEvery with a single-shot Update that races the live RD reconciler; wrap it in retry.RetryOnConflict. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com> * test(integration): assert NodeConnection property bag under upstream 'props' key The GET envelope assertion looked for a 'properties' map, but the upstream wire key is 'props' — golinstor's NodeConnection struct (client/connection.go) and blockstor's handler both spell it that way. The test (not the product) diverged from the wire contract. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com> * test(integration): align Group K workflows with CLI 1.27.1 and post-#129 witness - HappyPath: pin autoplace to the thin lvm-thin pool the way linstor-csi always does; unpinned placement could land on the thick 'file' pool where the snapshot step is correctly refused by the G5 thick-provider gate (the CLI exits 0 on refusal envelopes, so the failure surfaced as a misleading wait timeout). - NodeLostCascade: take the satellite offline before 'n lost' (Bug 111 gate refuses lost against an ONLINE satellite). - PoolDestroyedDropsFromPlacer: 'rd create' in linstor-client 1.27.1 has no --storage-pool flag; the pool constraint rides on autoplace. - LUKSStackEndToEnd: 1.27.1 spells the flag --passphrase, not --new-passphrase. - LateVD / ReplicasOnSame / ReplicasOnDifferent: exclude the auto-tiebreaker witness (expected on 2-diskful RDs post-#129) from diskful placement asserts it was never part of. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com> * test(integration): stamp blockstor.io/node-name label on fixture storage pools The k8s store's StoragePools().ListByNode runs a label selector on blockstor.io/node-name — the label pkg/store/k8s stamps on every store-created pool. Fixture pools seeded directly via the CRD client lacked it, so per-node store reads saw zero pools and the `n lost` storage-pool cascade silently deleted nothing, failing TestGroupKWFNodeLostCascade's SP assert. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com> * test(integration): fix r-create arg order for 1.27.1; witness-aware Bug 83 assert linstor-client 1.27.1's argparse rejects the trailing positional after two valued options ('too few arguments'); use the positional-first form Group F already uses. The Bug 83 dead-pool assert now excludes the auto-tiebreaker witness — a network-only DISKLESS attachment that consumes no pool and legitimately lands on the dead-pool node for a 2-diskful RD post-#129. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com> * test(integration): appease bodyclose/wrapcheck/godox on new test code Fold the PUT + envelope assertion into one helper so bodyclose sees the close in the same scope, wrap client errors inside the RetryOnConflict closure, and reword two comments that tripped godox. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com> * test(integration): seed Group G resources before the RD to dodge witness race seedRDWithVolume created the RD first, so the RD reconciler's auto-tiebreaker pass (witness iff exactly 2 diskful, post-#129) raced the fixture loop to worker-3 and the seeded third Resource flaked with AlreadyExists. Seeding all three diskful rows before the RD lands means the first reconcile already sees 3 diskful and wants no witness. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com> --------- Signed-off-by: Andrei Kvapil <kvapss@gmail.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 960f414 commit 1728baf

7 files changed

Lines changed: 356 additions & 108 deletions

File tree

tests/integration/group_a_test.go

Lines changed: 86 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ package integration
2727

2828
import (
2929
"context"
30+
"encoding/json"
31+
"io"
32+
"net/http"
3033
"os/exec"
3134
"sort"
3235
"testing"
@@ -272,10 +275,16 @@ func TestGroupANodeEvacuatePUT(t *testing.T) {
272275
t.Fatalf("EVICTED stamped despite in-use refusal; flags=%v", flagsMid)
273276
}
274277

275-
// Second call: --force MUST stamp EVICTED. This is the PUT
276-
// route smoke — CLI wrapper fatals on traceback so reaching
277-
// the next assert means the wire shape is intact.
278-
cli.JSON(t, "node", "evacuate", "--force", harness.NodeWorker2)
278+
// Second call: ?force=true MUST stamp EVICTED. The pinned CLI
279+
// (linstor-client 1.27.1, asserted by Group H) has no `--force`
280+
// flag on `node evacuate`, so the force override is exercised
281+
// through the PUT route directly — the same
282+
// `PUT /v1/nodes/{node}/evacuate?force=true` wire shape
283+
// handleRGDelete's force precedent documents. The envelope must
284+
// decode as the LINSTOR `[]ApiCallRc` array (Bug 78 wire).
285+
putRequireAPICallRcEnvelope(t,
286+
stack.RestURL+"/v1/nodes/"+harness.NodeWorker2+"/evacuate?force=true",
287+
http.StatusOK)
279288

280289
flagsAfter := nodeFlags(t, stack, harness.NodeWorker2)
281290
if !containsString(flagsAfter, "EVICTED") {
@@ -288,13 +297,19 @@ func TestGroupANodeEvacuatePUT(t *testing.T) {
288297
// evacuate (blockstor's REST shim wires both to handleNodeEvacuate);
289298
// without the PUT route, golinstor's `NodeService.Evict` (which
290299
// `doPUT`s) crashes the python decoder with an empty 405 body.
300+
//
301+
// The pinned CLI (linstor-client 1.27.1, asserted by Group H) does
302+
// not ship the `node evict` verb yet, so the PUT route is driven
303+
// directly over HTTP — the exact wire shape golinstor's
304+
// NodeService.Evict emits (`PUT /v1/nodes/{node}/evict`, empty JSON
305+
// body, `[]ApiCallRc` response).
291306
func TestGroupANodeEvictPUT(t *testing.T) {
292307
stack := harness.StartStack(t)
293308
harness.SeedThreeNodeCluster(t, stack)
294309

295-
cli := &harness.CLI{URL: stack.RestURL}
296-
// `linstor node evict <node>` — same wire as evacuate.
297-
cli.JSON(t, "node", "evict", harness.NodeWorker3)
310+
putRequireAPICallRcEnvelope(t,
311+
stack.RestURL+"/v1/nodes/"+harness.NodeWorker3+"/evict",
312+
http.StatusOK)
298313

299314
flags := nodeFlags(t, stack, harness.NodeWorker3)
300315
if !containsString(flags, "EVICTED") {
@@ -310,13 +325,25 @@ func TestGroupANodeEvictPUT(t *testing.T) {
310325
// DeletionTimestamp stamp would hang every orphan forever and brick
311326
// the next RD-create that recycles the name/port allocation.
312327
//
313-
// We seed a Resource on worker-1 + a peer replica on worker-2, then
314-
// call `n lost worker-1` and assert the worker-1 replica is gone
315-
// while worker-2 survives.
328+
// We seed a Resource on worker-1 + a peer replica on worker-2, take
329+
// worker-1's satellite offline (the documented `n lost` precondition
330+
// — the Bug 111 gate refuses `n lost` against an ONLINE satellite),
331+
// then call `n lost worker-1` and assert the worker-1 replica is
332+
// gone while worker-2 survives.
316333
func TestGroupANodeLostCascadesOrphans(t *testing.T) {
317334
stack := harness.StartStack(t)
318335
harness.SeedThreeNodeCluster(t, stack)
319336

337+
// Kill worker-1's satellite heartbeat and wait for the OFFLINE
338+
// status to land — `n lost` against an ONLINE satellite is
339+
// correctly refused with 409 (Bug 111), and the machine-readable
340+
// CLI exits 0 even on refusal envelopes, so calling too early
341+
// would silently no-op and the cascade assert below would time
342+
// out with a misleading message.
343+
stack.Satellite.SimulateNodeOffline(harness.NodeWorker1)
344+
waitForNodeConnectionStatus(t, stack, harness.NodeWorker1,
345+
blockstoriov1alpha1.NodeConnectionStatusOffline)
346+
320347
ctx := context.Background()
321348

322349
rd := &blockstoriov1alpha1.ResourceDefinition{
@@ -616,6 +643,55 @@ func retryStatusPatch(ctx context.Context, stack *harness.Stack, name string,
616643
return lastErr
617644
}
618645

646+
// putRequireAPICallRcEnvelope issues a JSON PUT with an empty-object
647+
// body and asserts the response status matches and the body decodes
648+
// as the LINSTOR `[]ApiCallRc` array shape — the envelope
649+
// python-linstor/golinstor expect from the node-lifecycle PUT routes
650+
// (Bug 78: an empty or non-JSON body crashes the python decoder).
651+
func putRequireAPICallRcEnvelope(t *testing.T, url string, wantStatus int) {
652+
t.Helper()
653+
654+
resp := httpPutGroupH(t, url, []byte("{}"))
655+
656+
defer func() { _ = resp.Body.Close() }()
657+
658+
body, err := io.ReadAll(resp.Body)
659+
if err != nil {
660+
t.Fatalf("read response body: %v", err)
661+
}
662+
663+
if resp.StatusCode != wantStatus {
664+
t.Fatalf("status: got %d, want %d; body: %s", resp.StatusCode, wantStatus, body)
665+
}
666+
667+
var envelope []map[string]any
668+
669+
err = json.Unmarshal(body, &envelope)
670+
if err != nil {
671+
t.Fatalf("body is not a []ApiCallRc envelope: %v; body: %s", err, body)
672+
}
673+
674+
if len(envelope) == 0 {
675+
t.Fatalf("empty []ApiCallRc envelope; body: %s", body)
676+
}
677+
}
678+
679+
// waitForNodeConnectionStatus blocks until the Node CRD's
680+
// Status.ConnectionStatus reaches `want` — used to sequence the
681+
// satellite-offline simulation before `n lost` (Bug 111 gate).
682+
func waitForNodeConnectionStatus(t *testing.T, stack *harness.Stack, node, want string) {
683+
t.Helper()
684+
685+
harness.Eventually(t, 10*time.Second, func() bool {
686+
var got blockstoriov1alpha1.Node
687+
688+
err := stack.Env.Client.Get(context.Background(),
689+
types.NamespacedName{Name: node}, &got)
690+
691+
return err == nil && got.Status.ConnectionStatus == want
692+
}, "node "+node+" never reached ConnectionStatus="+want)
693+
}
694+
619695
// runCLINoFatal runs the CLI and returns stdout + a best-effort
620696
// success flag. Unlike CLI.Run/JSON, this does NOT t.Fatal on
621697
// non-zero exit — used by tests that expect the call to fail

tests/integration/group_f_test.go

Lines changed: 33 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -260,19 +260,22 @@ func TestGroupFRToggleCancel(t *testing.T) {
260260
}
261261

262262
// ---------------------------------------------------------------------------
263-
// TestGroupFRToggleDiskful2DisklessPreservesTieBreaker — Bug 104
264-
// (P1, QUORUM HAZARD). Starting from the steady auto-place(2) state
265-
// (2 diskful + 1 TIE_BREAKER on the 3rd node), `linstor r td
266-
// --diskless <one-of-diskful>` MUST leave the TIE_BREAKER Resource
267-
// intact. Pre-fix, the RD reconciler recomputed wantWitness from
268-
// scratch after the toggle, saw "1 diskful + 1 non-witness diskless"
269-
// (the freshly-flipped replica), flipped its decision to "no witness
270-
// needed", and DELETED the TIE_BREAKER Resource — collapsing the
271-
// quorum surface to 1 diskful + 1 diskless with no third voter. The
272-
// next network partition would freeze the volume read-only.
263+
// TestGroupFRToggleDiskful2DisklessReapsTieBreaker — upstream-parity
264+
// contract for the toggle-to-1-diskful path (supersedes the Bug 104
265+
// "preserve" pin, inverted alongside the L1 specs in
266+
// internal/controller/ensure_tiebreaker_test.go). Starting from the
267+
// steady auto-place(2) state (2 diskful + 1 TIE_BREAKER on the 3rd
268+
// node), `linstor r td --diskless <one-of-diskful>` leaves 1 diskful
269+
// + 1 user-diskless — at which point quorumPolicy returns quorum=off:
270+
// there is no diskful tie to break and no majority to freeze.
271+
// Upstream LINSTOR's shouldTieBreakerExist never manages a witness
272+
// below 2 diskful, so blockstor REAPS the now-redundant TIE_BREAKER,
273+
// leaving exactly 2 replicas. (The former Bug 104/108 keep/create
274+
// branches rested on the false premise that 1 diskful + 1 diskless
275+
// freezes quorum:majority.)
273276
// ---------------------------------------------------------------------------
274277

275-
func TestGroupFRToggleDiskful2DisklessPreservesTieBreaker(t *testing.T) {
278+
func TestGroupFRToggleDiskful2DisklessReapsTieBreaker(t *testing.T) {
276279
stack, cli, rd := setupGroupFRD(t, "td-tb")
277280

278281
// Steady state: auto-place 2 lands diskful on worker-1+worker-2,
@@ -299,22 +302,22 @@ func TestGroupFRToggleDiskful2DisklessPreservesTieBreaker(t *testing.T) {
299302
return r != nil && groupFContains(r.Spec.Flags, "DISKLESS")
300303
}, "Resource "+rd+"."+target+" never gained DISKLESS flag")
301304

302-
// Bug 104 invariant: the TIE_BREAKER Resource on `witnessNode`
303-
// MUST still exist after the toggle settles. We give the RD
304-
// reconciler a generous beat (rdReconcileRequeue + apply) and
305-
// poll for a STABLE 3-replica composition: 1 diskful + 1
306-
// non-witness diskless + 1 TIE_BREAKER.
305+
// Upstream-parity invariant: the TIE_BREAKER on `witnessNode` is
306+
// REAPED once the toggle settles. We give the RD reconciler a
307+
// generous beat (rdReconcileRequeue + apply) and poll for a
308+
// STABLE 2-replica composition: 1 diskful + 1 user-diskless,
309+
// 0 witnesses.
307310
deadline := time.Now().Add(groupFAssertTimeout)
308311

309312
for time.Now().Before(deadline) {
310313
all := listResourcesByRD(t, stack, rd)
311314

312-
if assertBug104Composition(all, witnessNode) {
315+
if assertWitnessReapedComposition(all) {
313316
time.Sleep(2 * time.Second) // settle window
314317

315318
all = listResourcesByRD(t, stack, rd)
316319

317-
if assertBug104Composition(all, witnessNode) {
320+
if assertWitnessReapedComposition(all) {
318321
return // success
319322
}
320323
}
@@ -323,41 +326,40 @@ func TestGroupFRToggleDiskful2DisklessPreservesTieBreaker(t *testing.T) {
323326
}
324327

325328
all := listResourcesByRD(t, stack, rd)
326-
t.Fatalf("Bug 104: post-toggle replica set drifted from "+
327-
"{1 diskful + 1 user-diskless + 1 TIE_BREAKER on %s}; got %d entries: %v",
329+
t.Fatalf("post-toggle replica set drifted from "+
330+
"{1 diskful + 1 user-diskless, witness on %s reaped}; got %d entries: %v",
328331
witnessNode, len(all), all)
329332
}
330333

331-
// assertBug104Composition returns true iff the replica set looks
332-
// like {1 diskful + 1 user-diskless + 1 TIE_BREAKER on witnessNode}.
333-
// Bug 104's failure mode is the TIE_BREAKER on witnessNode getting
334-
// reaped, so we pin both the count (3) and the node identity.
335-
func assertBug104Composition(all []blockstoriov1alpha1.Resource, witnessNode string) bool {
336-
if len(all) != 3 {
334+
// assertWitnessReapedComposition returns true iff the replica set
335+
// looks like {1 diskful + 1 user-diskless} with NO TIE_BREAKER left.
336+
// The upstream-parity failure mode is the auto-witness surviving (or
337+
// being re-created) below 2 diskful, so we pin the exact count (2)
338+
// and a zero witness count.
339+
func assertWitnessReapedComposition(all []blockstoriov1alpha1.Resource) bool {
340+
if len(all) != 2 {
337341
return false
338342
}
339343

340-
witnessFound := false
341344
diskfulCount := 0
342345
userDisklessCount := 0
346+
witnessCount := 0
343347

344348
for i := range all {
345349
isDiskless := groupFContains(all[i].Spec.Flags, "DISKLESS")
346350
isTB := groupFContains(all[i].Spec.Flags, "TIE_BREAKER")
347351

348352
switch {
349353
case isTB:
350-
if all[i].Spec.NodeName == witnessNode {
351-
witnessFound = true
352-
}
354+
witnessCount++
353355
case isDiskless:
354356
userDisklessCount++
355357
default:
356358
diskfulCount++
357359
}
358360
}
359361

360-
return witnessFound && diskfulCount == 1 && userDisklessCount == 1
362+
return witnessCount == 0 && diskfulCount == 1 && userDisklessCount == 1
361363
}
362364

363365
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)