mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 09:01:31 +00:00
test: narrow the instance-state ratchet to instance-state access
Stop freezing the JetStreamManager type and constructor, which also serve cluster state; flag only instance-state methods, bucket and key identifiers, record codecs and local-state access. The cluster-state-only consumers in daemon_mgmt_ip.go and operator/cli/cluster.go leave the allowlist (73 to 67). The self-test now proves a cluster-state use is ignored and an instance-state method call is caught. Record the owner's disposition of the KV-unavailable restore result in the migration record.
This commit is contained in:
joshsiv-mulga
@@ -14,8 +14,6 @@ var allowlist = []allowance{
|
||||
{File: "spinifex/daemon/daemon.go", Symbol: "daemon.ReadLocalState", Inv: "INV-01"},
|
||||
{File: "spinifex/daemon/daemon.go", Symbol: "daemon.MarshalLocalState", Inv: "INV-01"},
|
||||
{File: "spinifex/daemon/daemon.go", Symbol: "daemon.WriteLocalStateBytes", Inv: "INV-01"},
|
||||
{File: "spinifex/daemon/daemon.go", Symbol: "daemon.JetStreamManager", Inv: "INV-02"},
|
||||
{File: "spinifex/daemon/daemon.go", Symbol: "daemon.NewJetStreamManager", Inv: "INV-02"},
|
||||
{File: "spinifex/daemon/daemon.go", Symbol: "JetStreamManager.InitKVBucket", Inv: "INV-02"},
|
||||
{File: "spinifex/daemon/daemon.go", Symbol: "JetStreamManager.InitTerminatedInstanceBucket", Inv: "INV-02"},
|
||||
{File: "spinifex/daemon/daemon.go", Symbol: "JetStreamManager.WriteNodeMarkerBestEffort", Inv: "INV-03"},
|
||||
@@ -23,7 +21,6 @@ var allowlist = []allowance{
|
||||
{File: "spinifex/daemon/instance_record_repair.go", Symbol: "JetStreamManager.WriteRunningSet", Inv: "INV-03"},
|
||||
{File: "spinifex/daemon/daemon.go", Symbol: "JetStreamManager.ListTerminatedInstances", Inv: "INV-04"},
|
||||
|
||||
{File: "spinifex/daemon/vm_adapters.go", Symbol: "daemon.JetStreamManager", Inv: "INV-05"},
|
||||
{File: "spinifex/daemon/vm_adapters.go", Symbol: "JetStreamManager.LoadState", Inv: "INV-05"},
|
||||
{File: "spinifex/daemon/vm_adapters.go", Symbol: "JetStreamManager.WriteStoppedInstance", Inv: "INV-05"},
|
||||
{File: "spinifex/daemon/vm_adapters.go", Symbol: "JetStreamManager.LoadStoppedInstance", Inv: "INV-05"},
|
||||
@@ -56,8 +53,6 @@ var allowlist = []allowance{
|
||||
{File: "spinifex/daemon/daemon_handlers_image.go", Symbol: "JetStreamManager.LoadStoppedInstance", Inv: "INV-08"},
|
||||
{File: "spinifex/daemon/daemon_handlers_image.go", Symbol: "JetStreamManager.UpdateStoppedInstance", Inv: "INV-08"},
|
||||
|
||||
{File: "spinifex/daemon/daemon_mgmt_ip.go", Symbol: "daemon.JetStreamManager", Inv: "INV-09"},
|
||||
|
||||
{File: "spinifex/daemon/instance_recovery.go", Symbol: "vm.InstanceRecord", Inv: "INV-10"},
|
||||
{File: "spinifex/daemon/instance_recovery.go", Symbol: "JetStreamManager.ListInstanceRecords", Inv: "INV-10"},
|
||||
{File: "spinifex/daemon/instance_recovery.go", Symbol: "JetStreamManager.ClaimRecoverableInstance", Inv: "INV-10"},
|
||||
@@ -76,7 +71,6 @@ var allowlist = []allowance{
|
||||
{File: "spinifex/vpcd/imds_instance_state.go", Symbol: "daemon.LocalState", Inv: "INV-13"},
|
||||
{File: "spinifex/vpcd/imds_instance_state.go", Symbol: "daemon.LocalStatePath", Inv: "INV-13"},
|
||||
{File: "spinifex/vpcd/imds_instance_state.go", Symbol: "daemon.ReadLocalState", Inv: "INV-13"},
|
||||
{File: "spinifex/vpcd/imds_instance_state.go", Symbol: "daemon.NewJetStreamManager", Inv: "INV-13"},
|
||||
{File: "spinifex/vpcd/imds_instance_state.go", Symbol: "daemon.InstanceStateBucket", Inv: "INV-13"},
|
||||
{File: "spinifex/vpcd/imds_instance_state.go", Symbol: "JetStreamManager.InitKVBucket", Inv: "INV-13"},
|
||||
{File: "spinifex/vpcd/imds_instance_state.go", Symbol: "vm.InstanceRecord", Inv: "INV-13"},
|
||||
@@ -96,6 +90,4 @@ var allowlist = []allowance{
|
||||
{File: "spinifex/runtime/compute/cache/cache.go", Symbol: "vm.VMFromRecord", Inv: "INV-17"},
|
||||
|
||||
{File: "spinifex/domains/admission/quota/records.go", Symbol: "vm.InstanceRecord", Inv: "INV-18"},
|
||||
|
||||
{File: "spinifex/operator/cli/cluster.go", Symbol: "daemon.NewJetStreamManager", Inv: "INV-19"},
|
||||
}
|
||||
|
||||
@@ -117,6 +117,9 @@ func use(h holder, raw []byte, jsManager *spxd.JetStreamManager, other interface
|
||||
_, _, _ = jsManager.LoadState("node")
|
||||
other.LoadState()
|
||||
_ = "spinifex-instance-state"
|
||||
cluster, _ := spxd.NewJetStreamManager(nil)
|
||||
_ = cluster.InitClusterStateBucket()
|
||||
_ = cluster.WriteRunningSet("node", nil)
|
||||
}
|
||||
`
|
||||
|
||||
@@ -139,9 +142,10 @@ func TestADR0007_S6_RatchetDetectsFaults(t *testing.T) {
|
||||
s := parseFault(t, rel, "widget")
|
||||
|
||||
want := []string{
|
||||
"daemon.InstanceStateBucket", "daemon.LocalStatePath", "daemon.JetStreamManager",
|
||||
"daemon.InstanceStateBucket", "daemon.LocalStatePath",
|
||||
"vm.InstanceRecord", "vm.VMFromRecord", "VM.Record()",
|
||||
"JetStreamManager.LoadInstanceRecord", "JetStreamManager.LoadState",
|
||||
"JetStreamManager.WriteRunningSet",
|
||||
`literal "spinifex-instance-state"`,
|
||||
}
|
||||
for _, sym := range want {
|
||||
@@ -150,7 +154,7 @@ func TestADR0007_S6_RatchetDetectsFaults(t *testing.T) {
|
||||
}
|
||||
}
|
||||
if len(s) != len(want) {
|
||||
t.Errorf("want exactly %d uses (field names, method declarations and an unrelated LoadState excluded), got %v",
|
||||
t.Errorf("want exactly %d uses (the manager type, its constructor, cluster-state methods, field names, method declarations and an unrelated LoadState excluded), got %v",
|
||||
len(want), s.sorted())
|
||||
}
|
||||
|
||||
|
||||
@@ -27,7 +27,8 @@ const (
|
||||
)
|
||||
|
||||
// daemonSymbols are the daemon's instance-state bucket, key and local-file
|
||||
// identifiers. A consumer naming one knows where instance state lives.
|
||||
// identifiers. The JetStreamManager type and constructor are not listed: they
|
||||
// also serve cluster state, so only the instance-state methods below count.
|
||||
var daemonSymbols = set(
|
||||
"InstanceStateBucket", "InstanceStateBucketVersion",
|
||||
"TerminatedInstanceBucket", "TerminatedInstanceBucketVersion",
|
||||
@@ -36,7 +37,6 @@ var daemonSymbols = set(
|
||||
"LocalState", "LocalStateSchemaVersion", "LocalStatePath", "ReadLocalState",
|
||||
"MarshalLocalState", "WriteLocalStateBytes", "LocalStateFileName",
|
||||
"LocalStateFileMode", "DefaultLocalStateDir",
|
||||
"JetStreamManager", "NewJetStreamManager",
|
||||
)
|
||||
|
||||
// vmSymbols are the raw persisted record and its conversion.
|
||||
|
||||
@@ -535,7 +535,7 @@ Line numbers are the first use in each file.
|
||||
| INV-06 | `runtime/compute/vm/{migrate,shutdown,orphan_qemu_reaper,teardown_reaper}.go` | `vm` | daemon | Uses the `StateStore` port for stopped and terminated writes, terminated listing, update and delete. |
|
||||
| INV-07 | `domains/ec2/instance/service_impl.go:511`–`:2847` | `ec2/instance` | daemon | Stopped and terminated read, update, claim, write and delete through the `StateStore`-shaped port. |
|
||||
| INV-08 | `daemon/daemon_handlers_image.go:35`, `:89` | `daemon` | daemon | Loads and updates a stopped instance for image creation. |
|
||||
| INV-09 | `daemon/daemon_mgmt_ip.go:18` | `daemon` | daemon | Names `JetStreamManager` for cluster-state management-IP allocation; no instance key, recorded because the type is frozen. |
|
||||
| INV-09 | `daemon/daemon_mgmt_ip.go:18` | `daemon` | daemon | Cluster-state consumer outside the ratchet: names `JetStreamManager` only for management-IP allocation in the cluster-state bucket and touches no instance key. |
|
||||
| INV-10 | `daemon/instance_recovery.go:85`–`:546` | `daemon` | daemon | Lists and decodes records, claims, releases and abandons recovery, reloads a record. Typed CAS writes. |
|
||||
| INV-11 | `daemon/dns_reconcile.go:38`, `:48`, `:179` | `daemon` | daemon | Constructs keys from the prefix, names the bucket, lists records for DNS. |
|
||||
| INV-12 | `daemon/eni_orphan_reaper.go:26`–`:27` | `daemon` | daemon | Lists live and terminated records to find orphaned ENIs. |
|
||||
@@ -545,7 +545,7 @@ Line numbers are the first use in each file.
|
||||
| INV-16 | `runtime/roles/awsgw/awsgw.go:573`–`:633` | `awsgw` | gateway | Opens the bucket by name with no migration hook for the instance cache and the quota reconcile loop. |
|
||||
| INV-17 | `runtime/compute/cache/cache.go:97`, `:248` | `instancecache` | gateway | Watches `i.` records into a `vm.VM` map through `kvstore.Store[vm.InstanceRecord]`. |
|
||||
| INV-18 | `domains/admission/quota/records.go:33` | `quota` | gateway | Snapshots records to count vCPUs. |
|
||||
| INV-19 | `operator/cli/cluster.go:170` | `operator/cli` | operator CLI | Constructs a `JetStreamManager` for the cluster-state shutdown marker; touches no instance key. |
|
||||
| INV-19 | `operator/cli/cluster.go:170` | `operator/cli` | operator CLI | Cluster-state consumer outside the ratchet: constructs a `JetStreamManager` only for the cluster-state shutdown marker and touches no instance key. |
|
||||
|
||||
Test packages are excluded; `daemon/instance_records_vpcd_test.go` pins INV-15's duplicated literals to the daemon constants.
|
||||
|
||||
@@ -578,6 +578,15 @@ Recovery on return (`daemon/instance_state_recovery_baseline_internal_test.go`),
|
||||
|
||||
The first two cases are the ones ADR-0007 S5 rules out: a node republishes local state as authority without the canonical record.
|
||||
|
||||
Owner disposition of the KV-unavailable result:
|
||||
|
||||
- Correct: while canonical KV is unavailable, the daemon does not launch, recreate, migrate or report a healthy new realization.
|
||||
- Permitted later: reattaching only to a demonstrably already-running local guest covered by a valid local journal assignment and fence.
|
||||
- Not acceptable as the final model: keeping an in-memory record as `running` when no live process has been confirmed; the later recovery slice must make that an explicit unconfirmed or degraded state.
|
||||
|
||||
The safety goal is that a partition does not kill an existing workload, while a daemon restart plus a stale file cannot resurrect one.
|
||||
`TestCurrentBehaviour_RestoreWithKVUnavailableLeavesLocalUnlaunched` stays as the current-behaviour pin until that slice changes it.
|
||||
|
||||
### Generation preservation
|
||||
|
||||
`Generation` and `ObservedGeneration` are the only generation fields; KV entry revisions are used for CAS and never persisted in the record.
|
||||
@@ -601,10 +610,12 @@ Unknown fields are a field a newer binary adds.
|
||||
|
||||
### Regression ratchet
|
||||
|
||||
`architecture/instancestate` parses every non-test production file with `go/parser` and fails on any new use of the daemon's instance-state bucket, key and local-file identifiers, `JetStreamManager` and its constructor, `vm.InstanceRecord`, `InstanceSpec`, `InstanceStatus`, `VMFromRecord` and `VM.Record()`, the `JetStreamManager` instance-state methods, or the bucket and file names as string literals.
|
||||
`TestADR0007_S6_NoNewRawInstanceStateConsumers` holds 73 allow-listed file and symbol pairs, each citing an inventory item above; the list only shrinks, a stale entry fails, and a walk that finds too little fails rather than passing vacuously.
|
||||
`TestADR0007_S6_RatchetDetectsFaults` and `TestADR0007_S6_RatchetSeesBareIdentifiersInOwnerPackages` inject synthetic sources to prove the matcher detects each kind of use and ignores declarations.
|
||||
`architecture/instancestate` parses every non-test production file with `go/parser` and fails on any new use of the daemon's instance-state bucket, key and local-file identifiers, `vm.InstanceRecord`, `InstanceSpec`, `InstanceStatus`, `VMFromRecord` and `VM.Record()`, the `JetStreamManager` instance-state methods (instance and terminated record access, the running-set projection, `LoadState`, and `InitKVBucket` and `InitTerminatedInstanceBucket`, which open the instance buckets), or the bucket and file names as string literals.
|
||||
The `JetStreamManager` type and `NewJetStreamManager` are not matched, because the manager also serves cluster state; a cluster-state-only consumer such as INV-09 or INV-19 is not instance-state debt and is not flagged.
|
||||
`TestADR0007_S6_NoNewRawInstanceStateConsumers` holds 67 allow-listed file and symbol pairs, each citing an inventory item above; the list only shrinks, a stale entry fails, and a walk that finds too little fails rather than passing vacuously.
|
||||
`TestADR0007_S6_RatchetDetectsFaults` and `TestADR0007_S6_RatchetSeesBareIdentifiersInOwnerPackages` inject synthetic sources to prove the matcher detects each kind of use, ignores declarations, and does not flag the manager type, its constructor or a cluster-state method.
|
||||
Adding a `daemon.LocalStatePath` use to `runtime/roles/awsgw` and renaming one allowlist file were each confirmed to fail the gate.
|
||||
After narrowing, a `runtime/roles/awsgw` file constructing a manager and calling `InitClusterStateBucket` passed, the same file calling `WriteRunningSet` failed `NoUnrecordedConsumer`, and restoring the removed INV-19 entry failed `Allowlist_NoStaleEntries`.
|
||||
|
||||
## Close-out status
|
||||
|
||||
|
||||
@@ -117,9 +117,9 @@ func TestCurrentBehaviour_RestoreTerminalCanonicalRecordRetiresLocal(t *testing.
|
||||
assert.NotNil(t, terminated, "CURRENT: migrated to the terminated bucket")
|
||||
}
|
||||
|
||||
// Current behaviour: with KV unreachable, restore stops after the failed
|
||||
// cluster read. Local instances stay in memory as the file left them, are
|
||||
// neither reset nor relaunched, and the local file is rewritten unchanged.
|
||||
// Current behaviour: with KV unreachable, restore stops after the failed read
|
||||
// and launches nothing, but a local instance stays in memory as running with no
|
||||
// live process confirmed; the target makes that an explicit unconfirmed state.
|
||||
func TestCurrentBehaviour_RestoreWithKVUnavailableLeavesLocalUnlaunched(t *testing.T) {
|
||||
d := createDaemonWithJetStream(t)
|
||||
d.vmMgr.Insert(&vm.VM{ID: "i-kvdown", Status: vm.StateRunning, InstanceType: "t3.micro"})
|
||||
|
||||
Reference in New Issue
Block a user