From f728405148f8a79babd8faac86435a67a6648679 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20ROBERT?= Date: Fri, 28 Aug 2026 00:41:00 +0200 Subject: [PATCH 1/3] feat(machine): a pack cannot name the runtime, and the compiler is what says so (#514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit machine.Binding's went unexported, and `p.binding().Driver.EnsureNetwork(…)` stopped compiling. It left the way to *name* one open. Measured on 154c204, in an isolated worktree: this file, dropped into internal/providers/scaleway/, compiled and `go build ./internal/providers/scaleway/` exited 0. package scaleway import "github.com/stephrobert/feint/internal/core/machine" var _ machine.Driver So the surface was held by TestNoPackReachesPastTheDeclaredDriverSurface alone — a convention plus an AST scan, which is what #514 §2.1 says is not enough, because a scan is a list somebody can widen and a build error is not. Driver, Router, Firewaller, Peerer, Isolator and Balancer are unexported now. That is stronger than the internal/ package rule the issue suggests and costs no new package: internal/ would still admit any future internal/core/machine/* sibling, and it would have dragged Spec, NetworkSpec, Attachment and the firewall and balancer vocabulary along with the interfaces or left a screen of aliases behind. What leaves the package instead is machine.Runtime, and it is a struct rather than a narrowed interface on purpose: an assertion needs no name, so rt.(interface{ Remove(context.Context, string) error }).Remove(ctx, victim) would have undone the whole thing in one line — and that shape is already in the repository twice, in machineDriver's Verify half and in `feint images remove`. A struct with an unexported field cannot be asserted on, so its method set is the whole of what a holder can do. It carries the operator's half — identity, capabilities, survey, prune, repair, watch, images — and no verb that moves a machine, a network, an address or a rule set. Those stay with Binding, Reconciler and GroupSync, where the ownership checks and the one order are. internal/cli is the caller that legitimately needed a name, and it needed thirteen in production plus some forty in tests. It holds machine.Runtime everywhere now, and five copies of `driver.(machine.Noop)` became one Runtime.Runs(): a question written five times is a question one caller answers differently. Every optional-half assertion it made — Surveyor, Pruner, Repairer, Watcher, UplinkReleaser, ImageBuilder, ImageLister and one anonymous RemoveImage — is a Runtime method keeping the three outcomes, so "nobody could be asked" never reads as "there is nothing". The proof is a test that compiles and requires the failure, not a comment claiming it. internal/cli/testdata/bypass holds nine packages: eight that must not build, one that must. TestThePacksCannotNameTheDriver builds each and checks the message names the right symbol; the ninth, admitted/, runs first and is fatal, because a probe failing for a wrong import or a module boundary reads exactly like the door being shut. tools/falsify/specs/driver-unnameable.json plants six mutations — the driver re-exported, the balancing half re-exported, the routing half re-exported past the ratchet, Env republishing its runtime, Binding republishing its driver, and the probe harness pointed at a directory that is not there. All six compile and all six bite. TestNoPackReachesPastTheDeclaredDriverSurface stays, and mustStayOutside gains a sibling that inverts: mustNotBeNameable asserts the six are *not* exported and still exist as unexported interfaces, so re-exporting one fails a named test instead of quietly putting the boundary back behind a scan. What this does not close, written down rather than implied: machine.Noop, machine.Incus and machine.Recorder stay exported — the metadata-only default, the only runtime, and #515's shared recorder are all needed by name outside the package — so `machine.Noop{}.Remove(ctx, n)` in a pack still compiles and is caught by the scan. The two controls are not alternatives. PackSurface and its exemption ledger are untouched, the ledger is still empty, machine.NewRecorder and its twenty-one gestures are untouched, and testdata/provider-four compiles and replays green through the contract alone. Four falsification specs named fragments this rename moved and were retargeted at the code that carries the same guard today; falsify:lint finds all 865 mutations again. Co-authored-by: Claude Opus 5 (1M context) --- internal/cli/clean.go | 27 +- internal/cli/clean_ledger.go | 20 +- internal/cli/clean_ledger_test.go | 30 +- internal/cli/clean_traps.go | 36 ++- internal/cli/clean_traps_test.go | 22 +- internal/cli/cli.go | 64 ++-- internal/cli/dhcp_leftover_test.go | 2 +- internal/cli/doctor.go | 14 +- internal/cli/doctor_images_test.go | 6 +- internal/cli/driver_surface_test.go | 109 ++++++- internal/cli/driver_unnameable_test.go | 184 ++++++++++++ internal/cli/images.go | 29 +- internal/cli/leftovers.go | 11 +- internal/cli/leftovers_test.go | 16 +- internal/cli/provider_four_test.go | 12 +- internal/cli/provider_four_wiring_test.go | 18 +- internal/cli/runtime_blindness_test.go | 30 +- internal/cli/shutdown_sweep_test.go | 6 +- .../cli/testdata/bypass/admitted/probe.go | 34 +++ .../cli/testdata/bypass/balancer/probe.go | 21 ++ .../testdata/bypass/bindingdriver/probe.go | 28 ++ internal/cli/testdata/bypass/driver/probe.go | 22 ++ .../cli/testdata/bypass/envdriver/probe.go | 22 ++ .../cli/testdata/bypass/firewaller/probe.go | 21 ++ .../cli/testdata/bypass/isolator/probe.go | 20 ++ internal/cli/testdata/bypass/peerer/probe.go | 21 ++ internal/cli/testdata/bypass/router/probe.go | 21 ++ internal/cli/timeout.go | 4 +- internal/cli/timeout_test.go | 4 +- internal/cli/up.go | 12 +- internal/cli/up_wait_test.go | 12 +- internal/core/emulator/conformance.go | 5 +- internal/core/emulator/emulator.go | 59 ++-- internal/core/emulator/enforcement.go | 3 +- internal/core/emulator/evidence_test.go | 2 +- internal/core/emulator/ui_test.go | 42 ++- internal/core/machine/address.go | 6 +- internal/core/machine/balancer.go | 32 +- internal/core/machine/binding.go | 18 +- internal/core/machine/binding_boot_test.go | 6 +- internal/core/machine/capabilities.go | 9 +- internal/core/machine/firewall.go | 14 +- internal/core/machine/firewall_binding.go | 6 +- internal/core/machine/groupsync.go | 10 +- internal/core/machine/images.go | 28 +- internal/core/machine/incus.go | 22 +- internal/core/machine/isolate.go | 28 +- internal/core/machine/machine.go | 31 +- internal/core/machine/ownership_test.go | 2 +- internal/core/machine/placement.go | 14 +- internal/core/machine/plan.go | 6 +- internal/core/machine/prune.go | 8 +- internal/core/machine/recorder.go | 26 +- internal/core/machine/recorder_test.go | 12 +- internal/core/machine/runtime.go | 275 ++++++++++++++++++ internal/core/machine/surface.go | 48 ++- internal/core/machine/watch.go | 2 +- .../exoscale/detach_internal_test.go | 3 +- .../exoscale/elasticip_routing_test.go | 2 +- internal/providers/exoscale/entry_test.go | 6 +- .../exoscale/firewall_internal_test.go | 10 +- .../exoscale/firewall_scope_internal_test.go | 3 +- internal/providers/exoscale/loadbalancers.go | 4 +- .../exoscale/machines_internal_test.go | 8 +- .../exoscale/pools_machines_internal_test.go | 12 +- internal/providers/outscale/audit_test.go | 18 +- .../outscale/firewall_internal_test.go | 8 +- .../outscale/isolate_pass_internal_test.go | 2 +- .../outscale/loadbalancer_dataplane_test.go | 16 +- .../outscale/machines_internal_test.go | 10 +- .../providers/outscale/netpeerings_test.go | 4 +- .../outscale/nets_teardown_internal_test.go | 2 +- .../outscale/privateips_lock_test.go | 4 +- .../outscale/publicip_routing_test.go | 4 +- internal/providers/replay_test.go | 2 +- .../scaleway/address_routing_test.go | 8 +- internal/providers/scaleway/barrage_test.go | 10 +- internal/providers/scaleway/boot_test.go | 8 +- .../providers/scaleway/concurrency_test.go | 9 +- internal/providers/scaleway/detach_test.go | 4 +- .../scaleway/firewall_internal_test.go | 2 +- .../providers/scaleway/lostupdate_test.go | 3 +- .../scaleway/ownership_audit_test.go | 8 +- internal/providers/scaleway/routes_test.go | 4 +- .../a-run-ends-where-it-could-start.json | 4 +- tools/falsify/specs/driver-unnameable.json | 47 +++ .../specs/one-machine-per-address.json | 4 +- tools/falsify/specs/run-leaves-nothing.json | 12 +- tools/falsify/specs/trapped-station.json | 4 +- 89 files changed, 1335 insertions(+), 502 deletions(-) create mode 100644 internal/cli/driver_unnameable_test.go create mode 100644 internal/cli/testdata/bypass/admitted/probe.go create mode 100644 internal/cli/testdata/bypass/balancer/probe.go create mode 100644 internal/cli/testdata/bypass/bindingdriver/probe.go create mode 100644 internal/cli/testdata/bypass/driver/probe.go create mode 100644 internal/cli/testdata/bypass/envdriver/probe.go create mode 100644 internal/cli/testdata/bypass/firewaller/probe.go create mode 100644 internal/cli/testdata/bypass/isolator/probe.go create mode 100644 internal/cli/testdata/bypass/peerer/probe.go create mode 100644 internal/cli/testdata/bypass/router/probe.go create mode 100644 internal/core/machine/runtime.go create mode 100644 tools/falsify/specs/driver-unnameable.json diff --git a/internal/cli/clean.go b/internal/cli/clean.go index bceb8d81..5c8af5c6 100644 --- a/internal/cli/clean.go +++ b/internal/cli/clean.go @@ -59,7 +59,7 @@ func clean(args []string, stdout io.Writer) error { return err } - driver, err := resolveDriver(*vm, stdout) + rt, err := resolveDriver(*vm, stdout) if err != nil { return err } @@ -70,13 +70,12 @@ func clean(args []string, stdout io.Writer) error { // and the delete itself — so sweeping first would report the same five // objects the issue's reproduction reports and change nothing (#455). if *force { - if err := clearRuntimeTraps(stdout, led, driver); err != nil { + if err := clearRuntimeTraps(stdout, led, rt); err != nil { return err } } - pruner, ok := driver.(machine.Pruner) - if !ok { + if !rt.Sweeps() { // --vm off has no runtime, so there is nothing to sweep and that is not a // failure. It became worth distinguishing when this command started // collecting instance records as well: an operator with no machine @@ -85,7 +84,7 @@ func clean(args []string, stdout io.Writer) error { // the ambiguity this project refuses everywhere else. // // TestCleanSucceedsWithNoRuntimeToSweep fails without this. - if _, noRuntime := driver.(machine.Noop); noRuntime { + if !rt.Runs() { // The same sentence as the swept case, because it is just as true // with no runtime and network.sh asserts on it. An early return that // stayed silent would make the line depend on the mode, and a caller @@ -95,7 +94,7 @@ func clean(args []string, stdout io.Writer) error { // it is findable and endable with no runtime answering at all. return sweepLeftoverDHCP(stdout, led, *vm) } - return fmt.Errorf("the %s runtime cannot be swept", driver.Name()) + return fmt.Errorf("the %s runtime cannot be swept", rt.Name()) } // Read before, so the sweep can be judged on the host rather than on its own @@ -103,16 +102,16 @@ func clean(args []string, stdout io.Writer) error { // the object standing is invisible to every count the remover produces, and // that is the shape this repository has now met twice. ctx := context.Background() - before, surveyable, surveyErr := surveyRuntime(ctx, driver) + before, surveyable, surveyErr := surveyRuntime(ctx, rt) if surveyErr != nil { // Never an empty list on a failed read. "I could not look" and "there is // nothing" are different facts, and reporting the first as the second is // how an inventory once called a live account empty. - led.record(leftoverRecord{Kind: "survey", Name: driver.Name(), Attribution: "none", + led.record(leftoverRecord{Kind: "survey", Name: rt.Name(), Attribution: "none", Stage: stageSweep, Why: whyUnreadable, Action: actionNone}) } - pruned, err := pruner.Prune(ctx) + pruned, _, err := rt.Prune(ctx) // Reported either way: a partial sweep still removed something, and saying // what went is what tells the operator whether to look further. led.prose("removed %d machine(s), %d network(s), %d rule set(s)\n", @@ -125,10 +124,10 @@ func clean(args []string, stdout io.Writer) error { // is still there: the case no return code reveals. // TestTheSweepNamesWhatSurvivedItsOwnSuccessfulDelete fails without this. if surveyable && surveyErr == nil { - after, _, afterErr := surveyRuntime(ctx, driver) + after, _, afterErr := surveyRuntime(ctx, rt) switch { case afterErr != nil: - led.record(leftoverRecord{Kind: "survey", Name: driver.Name(), Attribution: "none", + led.record(leftoverRecord{Kind: "survey", Name: rt.Name(), Attribution: "none", Stage: stageSweep, Why: whyUnreadable, Action: actionNone}) default: led.recordAll(survivors(before, after), stageSweep, whySurvived, actionNone) @@ -247,7 +246,7 @@ func reportStuckLeftovers(stdout io.Writer, led *ledger, vm string, doorstep boo // A runtime that will not resolve is not a clean host and the caller must // not read it as one, so this fails rather than skipping: the same position // refuseRuntimeLeftovers already took for the doorstep. - driver, err := resolveDriver(vm, stdout) + rt, err := resolveDriver(vm, stdout) if err != nil { led.record(leftoverRecord{Kind: "survey", Name: vm, Attribution: "none", Stage: stageDoorstep, Why: whyUnreadable, Action: actionNone}) @@ -255,7 +254,7 @@ func reportStuckLeftovers(stdout io.Writer, led *ledger, vm string, doorstep boo } if doorstep { - if err := refuseRuntimeLeftovers(stdout, led, vm, driver); err != nil { + if err := refuseRuntimeLeftovers(stdout, led, vm, rt); err != nil { return err } } @@ -275,7 +274,7 @@ func reportStuckLeftovers(stdout io.Writer, led *ledger, vm string, doorstep boo // and TestCleanCheckReportsARuleSetHeldByATrappedNetwork fail without it; // TestCleanCheckStaysQuietOnARuntimeNothingHoldsBeyondItsSweep is the // accepting half, and it is the one that keeps this usable mid-run. - trapped, err := reportRuntimeTraps(stdout, led, driver) + trapped, err := reportRuntimeTraps(stdout, led, rt) if err != nil { return err } diff --git a/internal/cli/clean_ledger.go b/internal/cli/clean_ledger.go index 8a6c5487..6c869f53 100644 --- a/internal/cli/clean_ledger.go +++ b/internal/cli/clean_ledger.go @@ -168,16 +168,12 @@ var surveyRuntime = surveyLeftovers // surveyLeftovers reads what the runtime holds, and distinguishes the three // outcomes rather than two. A driver that cannot survey is not an empty host: // it is a host nobody looked at, and it says so. -func surveyLeftovers(ctx context.Context, driver machine.Driver) (machine.Leftovers, bool, error) { - surveyor, ok := driver.(machine.Surveyor) - if !ok { - return machine.Leftovers{}, false, nil - } - left, err := surveyor.Survey(ctx) +func surveyLeftovers(ctx context.Context, rt machine.Runtime) (machine.Leftovers, bool, error) { + left, asked, err := rt.Survey(ctx) if err != nil { - return machine.Leftovers{}, true, err + return machine.Leftovers{}, asked, err } - return left, true, nil + return left, asked, nil } // recordAll writes one line per object of a survey, sorted so two runs of the @@ -228,11 +224,11 @@ func (l *ledger) recordAll(left machine.Leftovers, stage, why, action string) { // The driver is resolved by the caller and passed in, since the check now asks // this runtime two questions rather than one and resolving it twice would let // them disagree about which host they are talking about. -func refuseRuntimeLeftovers(out io.Writer, led *ledger, vm string, driver machine.Driver) error { +func refuseRuntimeLeftovers(out io.Writer, led *ledger, vm string, rt machine.Runtime) error { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - left, surveyable, err := surveyRuntime(ctx, driver) + left, surveyable, err := surveyRuntime(ctx, rt) if !surveyable { // --vm off, and every driver that cannot be asked. Said out loud rather // than returned in silence: a precondition that passes quietly on the @@ -241,10 +237,10 @@ func refuseRuntimeLeftovers(out io.Writer, led *ledger, vm string, driver machin return nil } if err != nil { - led.record(leftoverRecord{Kind: "survey", Name: driver.Name(), Attribution: "none", + led.record(leftoverRecord{Kind: "survey", Name: rt.Name(), Attribution: "none", Stage: stageDoorstep, Why: whyUnreadable, Action: actionNone}) return fmt.Errorf("could not look at what the %s runtime holds, so this host cannot be called clean: %w", - driver.Name(), err) + rt.Name(), err) } if len(left.Machines) == 0 && len(left.Networks) == 0 { led.prose("no machine or network of an earlier run is left on this runtime\n") diff --git a/internal/cli/clean_ledger_test.go b/internal/cli/clean_ledger_test.go index 54e12c8c..ac1aed50 100644 --- a/internal/cli/clean_ledger_test.go +++ b/internal/cli/clean_ledger_test.go @@ -57,10 +57,10 @@ func (d *sweptDriver) Prune(context.Context) (machine.Pruned, error) { } // withDriver points the doorstep and the sweep at a runtime a test controls. -func withDriver(t *testing.T, d machine.Driver) { +func withDriver(t *testing.T, d machine.Runtime) { t.Helper() previous := resolveDriver - resolveDriver = func(string, io.Writer) (machine.Driver, error) { return d, nil } + resolveDriver = func(string, io.Writer) (machine.Runtime, error) { return d, nil } t.Cleanup(func() { resolveDriver = previous }) // And the real host read back on top of whatever quietDHCP silenced: this // runtime is a fake holding known objects, so reading it is the point. @@ -79,7 +79,7 @@ func withDriver(t *testing.T, d machine.Driver) { // DHCP services says it is not about runtime objects. func noRuntime(t *testing.T) { t.Helper() - withDriver(t, machine.Noop{}) + withDriver(t, machine.Use(machine.Noop{})) } // quietDHCP silences the real /proc scan: these tests are about runtime @@ -105,7 +105,7 @@ func TestTheDoorstepRefusesAHostHoldingAPreviousRunsNetwork(t *testing.T) { Networks: []string{"fnt-5df8d7080c7"}, Firewalls: []string{"iso-fnt-5df8d7080c7"}, }} - withDriver(t, held) + withDriver(t, machine.Use(held)) var out bytes.Buffer err := reportStuckLeftovers(&out, newLedger(&out, false, time.Now()), "incus", true) @@ -124,7 +124,7 @@ func TestTheDoorstepRefusesAHostHoldingAPreviousRunsNetwork(t *testing.T) { } // The accepting half, on the same path. - withDriver(t, &sweptDriver{}) + withDriver(t, machine.Use(&sweptDriver{})) var clean bytes.Buffer if err := reportStuckLeftovers(&clean, newLedger(&clean, false, time.Now()), "incus", true); err != nil { t.Fatalf("the doorstep refused a runtime holding nothing: %v (%q)", err, clean.String()) @@ -138,7 +138,7 @@ func TestTheDoorstepRefusesAHostHoldingAPreviousRunsNetwork(t *testing.T) { // forty minutes. func TestTheDoorstepSaysItCouldNotLookRatherThanCallingTheHostClean(t *testing.T) { quietDHCP(t) - withDriver(t, &sweptDriver{blind: true}) + withDriver(t, machine.Use(&sweptDriver{blind: true})) var out bytes.Buffer led := newLedger(&out, true, time.Now()) @@ -163,10 +163,10 @@ func TestTheSweepNamesWhatSurvivedItsOwnSuccessfulDelete(t *testing.T) { t.Setenv("XDG_RUNTIME_DIR", t.TempDir()) t.Setenv("XDG_STATE_HOME", "") - withDriver(t, &sweptDriver{ + withDriver(t, machine.Use(&sweptDriver{ keeps: true, left: machine.Leftovers{Networks: []string{"fnt-5df8d7080c7"}}, - }) + })) var out bytes.Buffer if err := clean([]string{"--vm", "incus", "--format", "json"}, &out); err != nil { @@ -201,7 +201,7 @@ func TestTheSweepNamesWhatSurvivedItsOwnSuccessfulDelete(t *testing.T) { // The witness: the same sweep against a runtime that really removes must // record no survivor. Without it this test would pass on code that labels // every object a survivor. - withDriver(t, &sweptDriver{left: machine.Leftovers{Networks: []string{"fnt-5df8d7080c7"}}}) + withDriver(t, machine.Use(&sweptDriver{left: machine.Leftovers{Networks: []string{"fnt-5df8d7080c7"}}})) var honest bytes.Buffer if err := clean([]string{"--vm", "incus", "--format", "json"}, &honest); err != nil { t.Fatalf("clean on a runtime that removes: %v", err) @@ -221,14 +221,14 @@ func TestTheLedgerAnswersWhichMechanismProducesTheWaste(t *testing.T) { t.Setenv("XDG_RUNTIME_DIR", t.TempDir()) t.Setenv("XDG_STATE_HOME", "") - withDriver(t, &sweptDriver{ + withDriver(t, machine.Use(&sweptDriver{ keeps: true, left: machine.Leftovers{ Machines: []string{"feint-scw-a"}, Networks: []string{"fnt-a", "fnt-b"}, Firewalls: []string{"iso-fnt-a"}, }, - }) + })) var out bytes.Buffer if err := clean([]string{"--vm", "incus", "--format", "json"}, &out); err != nil { @@ -293,10 +293,10 @@ func TestTheLedgerIsParseableEndToEnd(t *testing.T) { t.Setenv("XDG_RUNTIME_DIR", t.TempDir()) t.Setenv("XDG_STATE_HOME", "") - withDriver(t, &sweptDriver{keeps: true, left: machine.Leftovers{ + withDriver(t, machine.Use(&sweptDriver{keeps: true, left: machine.Leftovers{ Machines: []string{"feint-scw-a"}, Networks: []string{"fnt-a"}, - }}) + }})) var out bytes.Buffer if err := clean([]string{"--vm", "incus", "--format", "json"}, &out); err != nil { @@ -319,7 +319,7 @@ func TestTheLedgerIsParseableEndToEnd(t *testing.T) { // The text half, unchanged: network.sh decides the runtime is clean on this // exact sentence. - withDriver(t, &sweptDriver{}) + withDriver(t, machine.Use(&sweptDriver{})) var text bytes.Buffer if err := clean([]string{"--vm", "incus"}, &text); err != nil { t.Fatalf("clean in text mode: %v", err) @@ -355,7 +355,7 @@ func TestTheLeftoverCheckMidRunIgnoresTheRunsOwnObjects(t *testing.T) { Networks: []string{"fnt-default", "fnt-feba907ed4e"}, Firewalls: []string{"scw-31a308684ad"}, }} - withDriver(t, live) + withDriver(t, machine.Use(live)) var out bytes.Buffer if err := reportStuckLeftovers(&out, newLedger(&out, false, time.Now()), "incus", false); err != nil { diff --git a/internal/cli/clean_traps.go b/internal/cli/clean_traps.go index 3a8399cf..2a4e4d6a 100644 --- a/internal/cli/clean_traps.go +++ b/internal/cli/clean_traps.go @@ -50,21 +50,20 @@ const whyTrapped = "beyond-an-ordinary-command" // Distinguishes three outcomes rather than two, like every other reader here: a // runtime that cannot be asked is not a clean one, and it says so instead of // answering zero. -func reportRuntimeTraps(out io.Writer, led *ledger, driver machine.Driver) (int, error) { - repairer, ok := driver.(machine.Repairer) - if !ok { - led.prose("the %s runtime cannot be asked what holds it, so nothing was asked\n", driver.Name()) - return 0, nil - } +func reportRuntimeTraps(out io.Writer, led *ledger, rt machine.Runtime) (int, error) { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - traps, err := repairer.Traps(ctx) + traps, asked, err := rt.Traps(ctx) + if !asked { + led.prose("the %s runtime cannot be asked what holds it, so nothing was asked\n", rt.Name()) + return 0, nil + } if err != nil { - led.record(leftoverRecord{Kind: "survey", Name: driver.Name(), Attribution: "none", + led.record(leftoverRecord{Kind: "survey", Name: rt.Name(), Attribution: "none", Stage: stageDoorstep, Why: whyUnreadable, Action: actionNone}) return 0, fmt.Errorf("could not look at what holds the %s runtime, so this host cannot be called clean: %w", - driver.Name(), err) + rt.Name(), err) } if len(traps) == 0 { led.prose("nothing on this runtime is beyond an ordinary sweep\n") @@ -88,21 +87,20 @@ func reportRuntimeTraps(out io.Writer, led *ledger, driver machine.Driver) (int, // The announcement is not politeness. What this touches is the runtime's own // database, so the row is printed whole before it goes and again as it goes, // and an operator who disagrees has everything needed to put it back. -func clearRuntimeTraps(out io.Writer, led *ledger, driver machine.Driver) error { - repairer, ok := driver.(machine.Repairer) - if !ok { - return fmt.Errorf("--force has nothing to reach on the %s runtime: it cannot be asked what holds it", - driver.Name()) - } +func clearRuntimeTraps(out io.Writer, led *ledger, rt machine.Runtime) error { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - traps, err := repairer.Traps(ctx) + traps, asked, err := rt.Traps(ctx) + if !asked { + return fmt.Errorf("--force has nothing to reach on the %s runtime: it cannot be asked what holds it", + rt.Name()) + } if err != nil { - led.record(leftoverRecord{Kind: "survey", Name: driver.Name(), Attribution: "none", + led.record(leftoverRecord{Kind: "survey", Name: rt.Name(), Attribution: "none", Stage: stageSweep, Why: whyUnreadable, Action: actionNone}) return fmt.Errorf("could not look at what holds the %s runtime, so nothing was forced: %w", - driver.Name(), err) + rt.Name(), err) } var repairable []machine.Trap for _, trap := range traps { @@ -125,7 +123,7 @@ func clearRuntimeTraps(out io.Writer, led *ledger, driver machine.Driver) error led.prose(" %s %s: %s\n %s\n", trap.Kind, trap.Name, trap.Why, trap.Row) } - cleared, repairErr := repairer.Repair(ctx) + cleared, _, repairErr := rt.Repair(ctx) for _, trap := range cleared { led.record(trapRecord(trap, stageSweep, actionRemoved)) led.prose("removed %s %s\n", trap.Kind, trap.Name) diff --git a/internal/cli/clean_traps_test.go b/internal/cli/clean_traps_test.go index 27a3d122..04543001 100644 --- a/internal/cli/clean_traps_test.go +++ b/internal/cli/clean_traps_test.go @@ -53,7 +53,7 @@ func (d *trappedDriver) Repair(context.Context) ([]machine.Trap, error) { } // checkAgainst runs `feint clean --check` against a runtime a test controls. -func checkAgainst(t *testing.T, driver machine.Driver) (string, error) { +func checkAgainst(t *testing.T, driver machine.Runtime) (string, error) { t.Helper() quietDHCP(t) withDriver(t, driver) @@ -67,13 +67,13 @@ func checkAgainst(t *testing.T, driver machine.Driver) (string, error) { // because this reports them: before the fix all three answered 0. func TestCleanCheckReportsADanglingPeerRow(t *testing.T) { - report, err := checkAgainst(t, &trappedDriver{traps: []machine.Trap{{ + report, err := checkAgainst(t, machine.Use(&trappedDriver{traps: []machine.Trap{{ Kind: machine.TrapDanglingPeer, Name: "fnt-c10fedc7f6c/fnt-e41278b8c3a", Why: "its peering names network 2617, which no longer exists", Repairable: true, Row: `{"id":401}`, - }}}) + }}})) if err == nil { t.Fatal("a host holding a peering row no command can remove was reported as ready") } @@ -88,11 +88,11 @@ func TestCleanCheckReportsADanglingPeerRow(t *testing.T) { } func TestCleanCheckReportsAStrippedUplink(t *testing.T) { - report, err := checkAgainst(t, &trappedDriver{traps: []machine.Trap{{ + report, err := checkAgainst(t, machine.Use(&trappedDriver{traps: []machine.Trap{{ Kind: machine.TrapStrippedUplink, Name: "fnt-ad48c26e025", Why: "its block 10.2.2.0/24 is no longer delegated to the uplink feint-uplink", - }}}) + }}})) if err == nil { t.Fatal("a host whose uplink lost the block of a network still standing was reported as ready") } @@ -102,11 +102,11 @@ func TestCleanCheckReportsAStrippedUplink(t *testing.T) { } func TestCleanCheckReportsARuleSetHeldByATrappedNetwork(t *testing.T) { - report, err := checkAgainst(t, &trappedDriver{traps: []machine.Trap{{ + report, err := checkAgainst(t, machine.Use(&trappedDriver{traps: []machine.Trap{{ Kind: machine.TrapHeldFirewall, Name: "iso-fnt-c10fedc7f6c", Why: "it is attached to fnt-c10fedc7f6c, which is trapped", - }}}) + }}})) if err == nil { t.Fatal("a rule set neither the network nor the sweep can release was reported as ready") } @@ -126,7 +126,7 @@ func TestCleanCheckReportsARuleSetHeldByATrappedNetwork(t *testing.T) { // say so out loud, because "checked and fine" and "never looked" must not read // the same. func TestCleanCheckStaysQuietOnARuntimeNothingHoldsBeyondItsSweep(t *testing.T) { - report, err := checkAgainst(t, &trappedDriver{}) + report, err := checkAgainst(t, machine.Use(&trappedDriver{})) if err != nil { t.Fatalf("a runtime holding nothing beyond its sweep was refused: %v\n%s", err, report) } @@ -137,7 +137,7 @@ func TestCleanCheckStaysQuietOnARuntimeNothingHoldsBeyondItsSweep(t *testing.T) // A runtime that cannot be asked is not an empty one. Three outcomes, never two. func TestCleanCheckRefusesARuntimeItCannotAsk(t *testing.T) { - report, err := checkAgainst(t, &trappedDriver{blind: true}) + report, err := checkAgainst(t, machine.Use(&trappedDriver{blind: true})) if err == nil { t.Fatalf("a runtime that answered nothing was reported as a clean host:\n%s", report) } @@ -159,10 +159,10 @@ func TestForceNamesEveryRowBeforeItRemovesIt(t *testing.T) { Why: "its block is no longer delegated", }, }} - withDriver(t, driver) + withDriver(t, machine.Use(driver)) var out bytes.Buffer - if err := clearRuntimeTraps(&out, newLedger(&out, false, time.Now()), driver); err != nil { + if err := clearRuntimeTraps(&out, newLedger(&out, false, time.Now()), machine.Use(driver)); err != nil { t.Fatalf("--force: %v\n%s", err, out.String()) } report := out.String() diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 3a08d411..d1142b0e 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -621,7 +621,7 @@ Every one of these is also a mise task: run "mise tasks" to list them. // creates nothing, so a probe that fails costs one call and falls through to the // bridge. What is chosen is printed, because a runtime selected in silence is a // runtime nobody can reason about. -func machineDriver(mode string, stdout io.Writer) (machine.Driver, error) { +func machineDriver(mode string, stdout io.Writer) (machine.Runtime, error) { ctx := context.Background() // verify asks the host what it delivers, once, before anything is published. @@ -634,46 +634,40 @@ func machineDriver(mode string, stdout io.Writer) (machine.Driver, error) { // // The narrowing is announced rather than silent, because a capability that // quietly drops is how a suite starts skipping what it used to assert. - verify := func(d machine.Driver) []string { - v, ok := d.(interface { - Verify(context.Context) (machine.Capabilities, []string) - }) - if !ok { - return nil - } - _, unmet := v.Verify(ctx) + verify := func(rt machine.Runtime) []string { + _, unmet := rt.Verify(ctx) return unmet } - requested := func(d machine.Driver) (machine.Driver, error) { - if !d.Available(ctx) { - return nil, fmt.Errorf("--vm %s requested but the Incus daemon does not answer", mode) + requested := func(rt machine.Runtime) (machine.Runtime, error) { + if !rt.Available(ctx) { + return machine.Runtime{}, fmt.Errorf("--vm %s requested but the Incus daemon does not answer", mode) } // Asked for by name, and the host cannot serve it: refuse at startup // naming the missing half, the same shape as the line above. Accepting // it would publish a capability the first create disproves, and blame // the client for it. - if unmet := verify(d); len(unmet) > 0 { - return nil, fmt.Errorf("--vm %s requested but this host cannot deliver it:\n %s", + if unmet := verify(rt); len(unmet) > 0 { + return machine.Runtime{}, fmt.Errorf("--vm %s requested but this host cannot deliver it:\n %s", mode, strings.Join(unmet, "\n ")) } - return d, nil + return rt, nil } switch mode { case "off", "none", "": - return machine.Noop{}, nil + return machine.Use(machine.Noop{}), nil case "incus": - return requested(machine.NewIncus()) + return requested(machine.Use(machine.NewIncus())) case "incus-vm", "kvm": - return requested(machine.NewIncusVM()) + return requested(machine.Use(machine.NewIncusVM())) case "incus-ovn", "ovn": - return requested(machine.NewIncusOVN()) + return requested(machine.Use(machine.NewIncusOVN())) case "auto": // Most capable first. Never incus-vm: a virtual machine costs tens of // seconds to boot where a container costs seconds, and that is a trade // an operator makes on purpose rather than one auto makes for them. - for _, d := range []machine.Driver{machine.NewIncusOVN(), machine.NewIncus()} { + for _, d := range []machine.Runtime{machine.Use(machine.NewIncusOVN()), machine.Use(machine.NewIncus())} { if !d.Available(ctx) { continue } @@ -682,9 +676,9 @@ func machineDriver(mode string, stdout io.Writer) (machine.Driver, error) { // a mode whose defining capability the host cannot deliver is passed // over, so the ordinary host that never installed OVN lands on the // bridge that works instead of on a promise that does not. - declared := machine.CapabilitiesOf(d) + declared := d.Capabilities() unmet := verify(d) - caps := machine.CapabilitiesOf(d) + caps := d.Capabilities() if declared.Isolation && !caps.Isolation { // Isolation is the only reason this mode is tried first, so a // host that cannot deliver it gets the next mode rather than @@ -705,9 +699,9 @@ func machineDriver(mode string, stdout io.Writer) (machine.Driver, error) { return d, nil } fmt.Fprintln(stdout, "no machine runtime available, falling back to metadata-only machines") - return machine.Noop{}, nil + return machine.Use(machine.Noop{}), nil default: - return nil, fmt.Errorf("unknown --vm mode %q (off, incus, incus-vm, incus-ovn, auto)", mode) + return machine.Runtime{}, fmt.Errorf("unknown --vm mode %q (off, incus, incus-vm, incus-ovn, auto)", mode) } } @@ -932,11 +926,11 @@ func serve(args []string, stdout io.Writer) error { } } - driver, err := machineDriver(*vm, stdout) + rt, err := machineDriver(*vm, stdout) if err != nil { return err } - env.UseMachines(driver) + env.UseMachines(rt) // The operator's own identifier declarations, the door through the boot // refusal (#465). Read here in the composition root like the other // deployment choices (FEINT_OUTSCALE_REGION), and only on the serve path: @@ -959,12 +953,12 @@ func serve(args []string, stdout io.Writer) error { // exactly the step nobody thinks of taking. // What a previous life left on the runtime, said before this one serves // beside it. The policy and its boundaries live in leftovers.go. - reportLeftovers(driver, env.Log) + reportLeftovers(rt, env.Log) watchCtx, stopWatching := context.WithCancel(context.Background()) defer stopWatching() - if watcher, ok := driver.(machine.Watcher); ok { - if events, err := watcher.Watch(watchCtx); err == nil { + if events, asked, err := rt.Watch(watchCtx); asked { + if err == nil { go reportRuntimeEvents(events, env.Log, srv) } else { env.Log.Warn("could not watch the machine runtime", "error", err) @@ -993,7 +987,7 @@ func serve(args []string, stdout io.Writer) error { // caller asked for against what this process is bound to. Handler: emulator.GuardRebinding(srv.Handler(), *addr), ReadHeaderTimeout: 10 * time.Second, - WriteTimeout: writeTimeoutFor(driver), + WriteTimeout: writeTimeoutFor(rt), } ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) @@ -1036,7 +1030,7 @@ func serve(args []string, stdout io.Writer) error { // Swept before the state is written: what the runtime no longer holds must // not be described as running in a snapshot the next run restores. - shutdownSweep(driver, *cleanup, stdout) + shutdownSweep(rt, *cleanup, stdout) if *state != "" { f, err := os.OpenFile(*state, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600) //nolint:gosec // operator-supplied path, by design @@ -1069,10 +1063,9 @@ func serve(args []string, stdout io.Writer) error { // not a measurement of anything, it is a leftover by construction — the one // that made two green conformance runs fail their successor's doorstep. // TestAGracefulExitReleasesTheUplink fails without the call. -func shutdownSweep(driver machine.Driver, cleanup bool, stdout io.Writer) { +func shutdownSweep(rt machine.Runtime, cleanup bool, stdout io.Writer) { if cleanup { - if pruner, ok := driver.(machine.Pruner); ok { - pruned, err := pruner.Prune(context.Background()) + if pruned, asked, err := rt.Prune(context.Background()); asked { fmt.Fprintf(stdout, "cleanup: removed %d machine(s), %d network(s), %d rule set(s)\n", pruned.Machines, pruned.Networks, pruned.Firewalls) if err != nil { @@ -1080,8 +1073,7 @@ func shutdownSweep(driver machine.Driver, cleanup bool, stdout io.Writer) { } } } - if releaser, ok := driver.(machine.UplinkReleaser); ok { - released, err := releaser.ReleaseUplink(context.Background()) + if released, asked, err := rt.ReleaseUplink(context.Background()); asked { switch { case err != nil: // Said rather than swallowed: an uplink this exit could not judge diff --git a/internal/cli/dhcp_leftover_test.go b/internal/cli/dhcp_leftover_test.go index 0a178027..40665864 100644 --- a/internal/cli/dhcp_leftover_test.go +++ b/internal/cli/dhcp_leftover_test.go @@ -45,7 +45,7 @@ func swapLeftoverSeams(t *testing.T, find func() ([]machine.DHCPLeftover, error) // afterwards; withDriver in clean_ledger_test.go is that override, and it // works because a later swap wins. savedSurvey := surveyRuntime - surveyRuntime = func(context.Context, machine.Driver) (machine.Leftovers, bool, error) { + surveyRuntime = func(context.Context, machine.Runtime) (machine.Leftovers, bool, error) { return machine.Leftovers{}, false, nil } t.Cleanup(func() { surveyRuntime = savedSurvey }) diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index 2c8e1a18..bcfa72d2 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -155,11 +155,11 @@ func checkRuntime(ctx context.Context, mode string) []check { }} } - driver, err := machineDriver(mode, io.Discard) + rt, err := machineDriver(mode, io.Discard) if err != nil { return []check{{title: "the --vm mode is not one this binary knows", state: verdictFail, detail: err.Error()}} } - if _, isNoop := driver.(machine.Noop); isNoop { + if !rt.Runs() { return []check{{ title: "no machine runtime is available", state: verdictWarn, @@ -168,9 +168,9 @@ func checkRuntime(ctx context.Context, mode string) []check { }} } - caps := machine.CapabilitiesOf(driver) + caps := rt.Capabilities() out := []check{{ - title: "machine runtime: " + driver.Name(), + title: "machine runtime: " + rt.Name(), state: verdictOK, detail: fmt.Sprintf("addresses %v, firewall %v, isolation %v, own kernel %v", caps.Addresses, caps.Firewall, caps.Isolation, caps.OwnKernel), }} @@ -194,7 +194,7 @@ func checkRuntime(ctx context.Context, mode string) []check { }) } out = append(out, checkIncusVersion(ctx)) - out = append(out, checkImages(ctx, driver)) + out = append(out, checkImages(ctx, rt)) return out } @@ -212,8 +212,8 @@ func checkRuntime(ctx context.Context, mode string) []check { // to tell "checked and fine" from "never looked". // // TestDoctorNamesTheMissingImagesAndHowToBuildThem fails without this. -func checkImages(ctx context.Context, driver machine.Driver) check { - inventory, err := machine.ImageInventory(ctx, driver) +func checkImages(ctx context.Context, rt machine.Runtime) check { + inventory, err := rt.Inventory(ctx) if err != nil { return check{ title: "could not read the machine images", diff --git a/internal/cli/doctor_images_test.go b/internal/cli/doctor_images_test.go index d6f3dcd4..c5357cf5 100644 --- a/internal/cli/doctor_images_test.go +++ b/internal/cli/doctor_images_test.go @@ -48,7 +48,7 @@ func TestDoctorNamesTheMissingImagesAndHowToBuildThem(t *testing.T) { // One built, the rest missing: the ordinary state of a station where // somebody ran `feint images` before the table grew. - got := checkImages(context.Background(), hostWith(all[0].Alias())) + got := checkImages(context.Background(), machine.Use(hostWith(all[0].Alias()))) if got.state != verdictWarn { t.Errorf("missing images reported as %v, want a warning", got.state) } @@ -71,7 +71,7 @@ func TestDoctorNamesTheMissingImagesAndHowToBuildThem(t *testing.T) { for _, spec := range all { every = append(every, spec.Alias()) } - got = checkImages(context.Background(), hostWith(every...)) + got = checkImages(context.Background(), machine.Use(hostWith(every...))) if got.state != verdictOK { t.Errorf("a complete image set reported as %v, want ok (detail %q)", got.state, got.detail) } @@ -83,7 +83,7 @@ func TestDoctorNamesTheMissingImagesAndHowToBuildThem(t *testing.T) { // "every image is present" because nobody could look is the shape of claim this // project exists to remove. func TestDoctorDoesNotReadSilenceAsACompleteImageSet(t *testing.T) { - got := checkImages(context.Background(), machine.Noop{}) + got := checkImages(context.Background(), machine.Use(machine.Noop{})) if got.state == verdictOK { t.Errorf("a driver that lists nothing reported an ok: %q / %q", got.title, got.detail) } diff --git a/internal/cli/driver_surface_test.go b/internal/cli/driver_surface_test.go index d6b893b9..9e6c4970 100644 --- a/internal/cli/driver_surface_test.go +++ b/internal/cli/driver_surface_test.go @@ -39,6 +39,14 @@ type machinePackage struct { // results maps a member to every type it returns, positionally, for the // `a, b := x.M()` form. results map[string][]string + // hidden maps an *unexported* interface of the package to its method + // names. Since #514 that is where the driver and its five pack-facing + // halves live, so two things need reading out of them: the method set the + // runtime-blindness derivation compares implementations against, and the + // fact that they are unexported at all — which is what + // TestThePacksCannotNameTheDriver's companion asserts here rather than + // leaving to a build that would simply stop failing. + hidden map[string][]string } // readMachinePackage parses internal/core/machine and reports what it exports. @@ -64,6 +72,7 @@ func readMachinePackage(t *testing.T) machinePackage { members: map[string]bool{}, yields: map[string]string{}, results: map[string][]string{}, + hidden: map[string][]string{}, } parsed := 0 for _, file := range files { @@ -120,6 +129,13 @@ func readMachineGenDecl(pkg machinePackage, d *ast.GenDecl) { switch s := spec.(type) { case *ast.TypeSpec: if !s.Name.IsExported() { + if t, ok := s.Type.(*ast.InterfaceType); ok { + for _, m := range t.Methods.List { + for _, name := range m.Names { + pkg.hidden[s.Name.Name] = append(pkg.hidden[s.Name.Name], name.Name) + } + } + } continue } pkg.names[s.Name.Name] = "type" @@ -625,26 +641,69 @@ func (s *surfaceScanner) add(key string, node ast.Node, how string) { // differently here; there is neither today. var notInTheDriverSurface = map[string]string{} +// mustNotBeNameable names what internal/core/machine must not export at all. +// +// This list is the inversion of the one below, and the inversion is the point +// of #514. Everything here used to be exported and excluded from PackSurface, +// which held it by an AST scan over the packs' sources — a convention. On +// 154c204 a pack could still write `var _ machine.Driver` and +// `go build ./internal/providers/scaleway/` exited 0, measured. Since #514 the +// six are unexported, so the sentence fails the build instead, and what this +// list holds is the *return*: re-exporting any of them, under any spelling, +// reopens the door in one edit and nothing else in the repository would say +// so. internal/core/machine's own package documentation (runtime.go) carries +// the reasoning; TestThePacksCannotNameTheDriver compiles the sentence and +// requires the failure. +// +// Each entry must still exist as an unexported interface of the package, so a +// deletion or a rename is a failure too: an exclusion naming nothing +// constrains nothing, which is exactly what the sibling list below asserts the +// other way round. +var mustNotBeNameable = []string{ + // The runtime itself. A pack holding one calls Start, Remove or + // RemoveNetwork past Binding.ours and past the driver's mustOwn — the hole + // a crafted snapshot walked through. + "driver", + // Its five pack-facing halves. Reaching one by assertion bypasses the + // shared layer exactly as surely as calling a method does, which is the + // correction that took #511's count from eleven sites to twenty-nine. + "router", "firewaller", "peerer", "isolator", "balancer", +} + // mustStayOutside names what the declared surface may never admit. // // It is the answer to the trap #511 names first: a surface that authorises // everything a pack reaches today documents the state of affairs instead of // constraining it. So what is excluded is asserted, not merely absent — the -// raw driver and its optional halves, the driver's own argument vocabulary, -// and the low-level Binding verbs the two orchestrators exist to sequence. -// Every entry below is exported by internal/core/machine and therefore -// nameable; none may appear in PackSurface. +// driver's implementations, its operator-facing halves, its argument +// vocabulary, and the low-level Binding verbs the two orchestrators exist to +// sequence. Every entry below is exported by internal/core/machine and +// therefore nameable; none may appear in PackSurface. +// +// The driver interface and its five pack-facing halves left this list for +// mustNotBeNameable above, and that is a promotion rather than a removal: a +// name the compiler refuses needs no scan behind it, and a name the scan still +// has to catch is a name somebody can still write. var mustStayOutside = []string{ - // The runtime itself, and every implementation of it. A pack holding one - // calls Start, Remove or RemoveNetwork past Binding.ours and past the - // driver's mustOwn — the hole a crafted snapshot walked through. - "Driver", "Noop", "Incus", "Recorder", - // Its optional halves. Reaching one by assertion bypasses the shared layer - // exactly as surely as calling a method does, which is the correction that - // took #511's count from eleven sites to twenty-nine. - "Router", "Firewaller", "Peerer", "Isolator", "Balancer", "Capable", "Waiter", + // Every implementation of the runtime. These stay exported — Noop is the + // metadata-only default the emulator and forty tests build on, Recorder is + // the shared contract recorder of #515, Incus is the runtime itself — so + // the scan is what keeps them out of a pack, and the residue is written + // down rather than implied: `machine.Noop{}.Remove(ctx, name)` still + // compiles in a pack, and is caught here rather than by the build. + "Noop", "Incus", "Recorder", + // The operator-facing halves. internal/cli reaches these through + // machine.Runtime's own methods, so no pack needs them; they are excluded + // by the scan rather than by the compiler because `feint clean`, `feint + // doctor` and `feint images` are not packs and the handle answers for + // them. + "Capable", "Waiter", "ImageBuilder", "ImageLister", "Pruner", "Repairer", "Surveyor", "Watcher", "UplinkReleaser", + // The handle itself, and its door. It is the emulator's and the CLI's + // spelling of a runtime; a pack that names it has gone looking for the + // value #511 took out of its reach. + "Runtime", "Use", // The driver's own argument vocabulary: what the shared layer builds from // what a pack declares. A pack assembling one of these is a pack writing // the call the layer exists to write. @@ -673,7 +732,7 @@ var mustStayOutside = []string{ "Binding.AddressOf", "Binding.RouteAddress", "Binding.UnrouteAddress", "Binding.SyncRuleSet", "Binding.ApplyRuleSets", "Binding.DropRuleSet", - "Binding.WithDriver", + "Binding.WithRuntime", // The firewall step of the boot replay. The Reconciler runs it, last, and // a pack running it itself puts the expansion before the interfaces it is // supposed to see. @@ -749,16 +808,34 @@ func TestNoPackReachesPastTheDeclaredDriverSurface(t *testing.T) { // The declared surface says something, and still means what it says. // -// Two halves, and each answers one way the list could become decoration. It +// Three halves, and each answers one way the list could become decoration. It // must resolve: every entry names something internal/core/machine really // exports, so a rename makes this fail instead of silently emptying the -// contract. And it must exclude: mustStayOutside is asserted absent, because a +// contract. It must exclude: mustStayOutside is asserted absent, because a // list that admits everything a pack reaches today is an inventory, not a -// boundary — #511 names that trap first and this is what answers it. +// boundary — #511 names that trap first and this is what answers it. And the +// six names of mustNotBeNameable must not come back as exported names at all, +// which is #514's half: the compiler holds them today, and a single edit +// re-exporting one would put them back behind a scan without anything saying +// so. func TestTheDeclaredDriverSurfaceIsSmallerThanThePackage(t *testing.T) { pkg := readMachinePackage(t) surface := machine.PackSurface() + for _, key := range mustNotBeNameable { + exported := strings.ToUpper(key[:1]) + key[1:] + if kind, back := pkg.names[exported]; back { + t.Errorf("internal/core/machine exports %s again (as a %s): #514 unexported it so that "+ + "`var _ machine.%s` in a pack fails the build, and an exported spelling puts the "+ + "boundary back behind a scan somebody can widen", exported, kind, exported) + } + if len(pkg.hidden[key]) == 0 { + t.Errorf("internal/core/machine has no unexported interface %q any more: an exclusion "+ + "naming nothing constrains nothing, and the driver contract cannot have lost its "+ + "verbs", key) + } + } + for key, why := range surface { if len(strings.Fields(why)) < 4 { t.Errorf("the surface entry for %s says %q, which does not say what a pack asks it for", key, why) diff --git a/internal/cli/driver_unnameable_test.go b/internal/cli/driver_unnameable_test.go new file mode 100644 index 00000000..5b804707 --- /dev/null +++ b/internal/cli/driver_unnameable_test.go @@ -0,0 +1,184 @@ +package cli + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "sort" + "strings" + "testing" + "time" +) + +// bypassProbe is one package under testdata/bypass and the verdict the +// compiler owes it. +type bypassProbe struct { + // dir is the directory name under testdata/bypass. + dir string + // wants is the fragment the build error must carry. Empty means the probe + // must build: that is the positive control. + wants string + // why says which sentence this probe is, so a failure names the gesture + // somebody just made compilable again rather than a directory. + why string +} + +// bypassProbes is the closed list, and it is asserted against the directory +// listing in both directions below. +var bypassProbes = []bypassProbe{ + {"admitted", "", "the positive control: what a pack may legitimately name"}, + {"driver", "undefined: machine.Driver", + "the runtime itself — #514's acceptance criterion (a), verbatim"}, + {"router", "undefined: machine.Router", + "the address half, reached past Reconciler.Route and the emulated-block guard"}, + {"firewaller", "undefined: machine.Firewaller", + "the rule-set half, reached past GroupSync — the layer #475 was born in"}, + {"peerer", "undefined: machine.Peerer", + "the peering half, the second writer the audit measured severing a live peering"}, + {"isolator", "undefined: machine.Isolator", + "the isolation half, the other side of the same fork"}, + {"balancer", "undefined: machine.Balancer", + "the balancing half, reached by a bare assertion until Binding gained the verbs"}, + {"envdriver", "emulator.Env{}.Machines undefined", + "the field that put a driver in every pack's hand before #511"}, + {"bindingdriver", "b.Driver undefined", + "`p.binding().Driver.EnsureNetwork(…)`, the sentence surface.go cites by name"}, +} + +// A provider pack cannot name the runtime, and the compiler is what says so +// (#514). +// +// # What this measures that no other test can +// +// #511 unexported emulator.Env's driver field and machine.Binding's, so a pack +// could no longer *obtain* a driver. It could still *name* the type. Measured +// on 154c204, in this repository, before the change this test arrived with: +// +// package scaleway +// import "github.com/stephrobert/feint/internal/core/machine" +// var _ machine.Driver +// +// dropped into internal/providers/scaleway/ compiled, and +// `go build ./internal/providers/scaleway/` exited 0. So the boundary was held +// by TestNoPackReachesPastTheDeclaredDriverSurface alone — a convention plus an +// AST scan, which is what #514 §2.1 says is not enough, because a scan is a +// list somebody can widen and a build error is not. +// +// # Why it is a subprocess and not an assertion +// +// A test cannot assert that an expression does not compile from inside a +// package that compiles. The sentence has to live in a package of its own, +// outside every ./... pattern, and be handed to the toolchain. That is what +// testdata/bypass is: nine packages, eight of which must fail to build, one of +// which must succeed. +// +// # Why the positive control is not optional +// +// A probe that fails to build for the wrong reason — a mistyped import, a +// module boundary the internal rule refuses, no toolchain on PATH — reads +// exactly like the door being shut, and this repository has paid seven times +// in one day for instruments that reported success because they looked +// nowhere. So testdata/bypass/admitted names only what machine.PackSurface +// admits, imports the same two packages, and must build. It runs first, and +// its failure is fatal: nothing below is worth reading after it. +// +// # What this does NOT hold, written down rather than implied +// +// machine.Noop, machine.Incus and machine.Recorder stay exported, because the +// emulator's default, the only runtime and the shared contract recorder are +// all needed by name outside this package. So `machine.Noop{}.Remove(ctx, n)` +// in a pack still compiles, and is caught by +// TestNoPackReachesPastTheDeclaredDriverSurface instead — mustStayOutside +// names all three. The two tests are not alternatives: the compiler holds the +// vocabulary, the scan holds what is left. +func TestThePacksCannotNameTheDriver(t *testing.T) { + root := repoRoot(t) + base := filepath.Join(root, "internal", "cli", "testdata", "bypass") + + // The population, both ways. A probe directory nobody registered would be + // built by nothing; a registered probe whose directory is gone would be a + // verdict about an empty package, which every compiler grants. + entries, err := os.ReadDir(base) + if err != nil { + t.Fatalf("read %s: the probes are the whole measurement, and a listing that fails "+ + "leaves this test asserting nothing: %v", base, err) + } + onDisk := map[string]bool{} + for _, e := range entries { + if e.IsDir() { + onDisk[e.Name()] = true + } + } + registered := map[string]bool{} + for _, p := range bypassProbes { + registered[p.dir] = true + if !onDisk[p.dir] { + t.Fatalf("testdata/bypass/%s is registered and does not exist: a probe with no source "+ + "is a verdict about an empty package, which compiles", p.dir) + } + } + var unregistered []string + for dir := range onDisk { + if !registered[dir] { + unregistered = append(unregistered, dir) + } + } + sort.Strings(unregistered) + if len(unregistered) > 0 { + t.Fatalf("testdata/bypass holds %v, which this test builds nothing for: a probe nobody "+ + "runs is a sentence nobody checks", unregistered) + } + + if _, err := exec.LookPath("go"); err != nil { + // Never a skip. A skip here reports "the door is shut" for a run that + // asked nothing, which is the failure mode this whole file exists to + // refuse. + t.Fatalf("no go toolchain on PATH, so nothing was compiled and nothing was proved: %v", err) + } + + // The control first, and fatally: every verdict below is about the + // toolchain answering the question that was asked. + if out, err := buildProbe(t, root, "admitted"); err != nil { + t.Fatalf("testdata/bypass/admitted must compile and did not, so every refusal below is "+ + "unreadable — a probe can fail for a reason that has nothing to do with the driver:\n%s", + out) + } + + for _, probe := range bypassProbes { + if probe.wants == "" { + continue + } + out, err := buildProbe(t, root, probe.dir) + if err == nil { + t.Errorf("testdata/bypass/%s compiles: %s. A pack can name the runtime again, and the "+ + "boundary is back to a convention an AST scan enforces — which is the state "+ + "#514 §2.1 measured and refused", probe.dir, probe.why) + continue + } + if !strings.Contains(out, probe.wants) { + t.Errorf("testdata/bypass/%s failed to build, but not on %q — so this probe is "+ + "measuring something else and its refusal proves nothing about %s:\n%s", + probe.dir, probe.wants, probe.why, out) + } + } +} + +// buildProbe compiles one probe package and returns everything the toolchain +// said. A non-nil error means the build failed, which is what most of these +// probes are for. +func buildProbe(t *testing.T, root, dir string) (string, error) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + cmd := exec.CommandContext(ctx, "go", "build", + "./internal/cli/testdata/bypass/"+dir+"/") + cmd.Dir = root + out, err := cmd.CombinedOutput() + if ctx.Err() != nil { + // A timeout is not a refusal. Reported as its own failure so a slow or + // wedged toolchain never reads as the compiler rejecting the sentence. + t.Fatalf("building testdata/bypass/%s did not finish: %v\n%s", dir, ctx.Err(), out) + } + return string(out), err +} diff --git a/internal/cli/images.go b/internal/cli/images.go index 049906a3..b877c154 100644 --- a/internal/cli/images.go +++ b/internal/cli/images.go @@ -53,19 +53,19 @@ func images(args []string, stdout, stderr io.Writer) int { return 1 } - driver, err := machineDriver(*vm, stderr) + rt, err := machineDriver(*vm, stderr) if err != nil { fmt.Fprintf(stderr, "feint: %v\n", err) return 1 } - if _, isNoop := driver.(machine.Noop); isNoop { + if !rt.Runs() { fmt.Fprintln(stderr, "feint: no machine runtime answers, so there is nowhere to build an image") fmt.Fprintln(stderr, " `feint doctor --vm incus` says what is missing") return 1 } ctx := context.Background() - inventory, err := machine.ImageInventory(ctx, driver) + inventory, err := rt.Inventory(ctx) if err != nil { fmt.Fprintf(stderr, "feint: %v\n", err) return 1 @@ -74,7 +74,7 @@ func images(args []string, stdout, stderr io.Writer) int { // derived an image put it under the prefix without any warm-up row naming // it, and an inventory that only answered the list would leave it // invisible on the operator's own machine. - derived, err := machine.DerivedImages(ctx, driver) + derived, err := rt.DerivedInventory(ctx) if err != nil { fmt.Fprintf(stderr, "feint: %v\n", err) return 1 @@ -88,7 +88,7 @@ func images(args []string, stdout, stderr io.Writer) int { // first image rather than in the middle of the set. The build itself goes // through machine.BuildIfMissing below, which is the seam the boot // path (machine.EnsureImage) drives too. - if _, ok := driver.(machine.ImageBuilder); !ok { + if !rt.BuildsImages() { fmt.Fprintln(stderr, "feint: this runtime cannot build images") return 1 } @@ -110,7 +110,7 @@ func images(args []string, stdout, stderr io.Writer) int { // callers, one lock, written once. Measured on 2026-08-25: two builds // that met on the old fixed-name builder killed each other, one on // `apt-get update … 137`, the other on a publish whose rootfs moved. - made, err := machine.BuildIfMissing(ctx, driver, status.Spec, stdout) + made, err := rt.BuildImage(ctx, status.Spec, stdout) if err != nil { fmt.Fprintf(stderr, "feint: %s: %v\n", status.Spec.Name, err) return 1 @@ -138,7 +138,7 @@ func images(args []string, stdout, stderr io.Writer) int { return 1 } fmt.Fprintf(stdout, "== %s (outside the warm-up set, derived from the family table)\n", spec.Alias()) - made, err := machine.BuildIfMissing(ctx, driver, spec, stdout) + made, err := rt.BuildImage(ctx, spec, stdout) if err != nil { fmt.Fprintf(stderr, "feint: %s: %v\n", spec.Name, err) return 1 @@ -219,24 +219,19 @@ func imagesRemove(args []string, stdout, stderr io.Writer) int { return 1 } - driver, err := machineDriver(*vm, stderr) + rt, err := machineDriver(*vm, stderr) if err != nil { fmt.Fprintf(stderr, "feint: %v\n", err) return 1 } - remover, ok := driver.(interface { - RemoveImage(context.Context, string) error - }) - if !ok { + if !rt.RemovesImages() { fmt.Fprintln(stderr, "feint: this runtime holds no images to remove") return 1 } ctx := context.Background() held := map[string]string{} - if lister, canAsk := driver.(machine.ImageLister); canAsk { - if listed, err := lister.LocalImages(ctx); err == nil { - held = listed - } + if listed, _, err := rt.LocalImages(ctx); err == nil { + held = listed } for _, name := range names { @@ -249,7 +244,7 @@ func imagesRemove(args []string, stdout, stderr io.Writer) int { return 1 } fmt.Fprintf(stdout, "removing %s (%s)\n", alias, short(fingerprint)) - if err := remover.RemoveImage(ctx, name); err != nil { + if _, err := rt.RemoveImage(ctx, name); err != nil { fmt.Fprintf(stderr, "feint: %v\n", err) return 1 } diff --git a/internal/cli/leftovers.go b/internal/cli/leftovers.go index ed4d490d..aeb75913 100644 --- a/internal/cli/leftovers.go +++ b/internal/cli/leftovers.go @@ -44,16 +44,15 @@ import ( // reportLeftovers names the labelled machines a previous run left on the // runtime. TestStartupNamesTheLeftoversItDidNotAdopt fails without it. -func reportLeftovers(driver machine.Driver, log *slog.Logger) { - surveyor, ok := driver.(machine.Surveyor) - if !ok { - return - } +func reportLeftovers(rt machine.Runtime, log *slog.Logger) { // Bounded: a hung runtime must delay the listener by seconds, not hold it. ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - left, err := surveyor.Survey(ctx) + left, asked, err := rt.Survey(ctx) + if !asked { + return + } if err != nil { // Silence on error would be the defect this file exists to remove: an // operator who cannot be told "there are leftovers" must at least be diff --git a/internal/cli/leftovers_test.go b/internal/cli/leftovers_test.go index 912b67b4..dc5bc24b 100644 --- a/internal/cli/leftovers_test.go +++ b/internal/cli/leftovers_test.go @@ -30,7 +30,7 @@ func (d surveyingDriver) Survey(context.Context) (machine.Leftovers, error) { return d.left, d.err } -func noticed(t *testing.T, driver machine.Driver) string { +func noticed(t *testing.T, driver machine.Runtime) string { t.Helper() var buf bytes.Buffer reportLeftovers(driver, slog.New(slog.NewTextHandler(&buf, nil))) @@ -38,10 +38,10 @@ func noticed(t *testing.T, driver machine.Driver) string { } func TestStartupNamesTheLeftoversItDidNotAdopt(t *testing.T) { - out := noticed(t, surveyingDriver{left: machine.Leftovers{ + out := noticed(t, machine.Use(surveyingDriver{left: machine.Leftovers{ Machines: []string{"feint-scw-79e3ef40", "feint-osc-i-21519d57"}, Networks: []string{"fnt-default"}, - }}) + }})) // The line must name the machines: "2 machines exist" sends the operator // to the runtime to find out which, and the whole point is that the @@ -65,7 +65,7 @@ func TestStartupNamesTheLeftoversItDidNotAdopt(t *testing.T) { } func TestStartupStaysSilentWhenTheRuntimeIsClean(t *testing.T) { - if out := noticed(t, surveyingDriver{}); out != "" { + if out := noticed(t, machine.Use(surveyingDriver{})); out != "" { t.Errorf("a clean runtime produced a notice:\n%s", out) } } @@ -75,10 +75,10 @@ func TestStartupStaysSilentOverPlumbingAlone(t *testing.T) { // bridge is reused under its name on the next boot, and the OVN uplink is // kept across runs by design. A notice that fired on every restart would // be read by nobody, which is worse than none. - out := noticed(t, surveyingDriver{left: machine.Leftovers{ + out := noticed(t, machine.Use(surveyingDriver{left: machine.Leftovers{ Networks: []string{"feint-uplink", "fnt-default"}, Firewalls: []string{"scw-bbbb"}, - }}) + }})) if out != "" { t.Errorf("plumbing without machines produced a notice:\n%s", out) } @@ -87,7 +87,7 @@ func TestStartupStaysSilentOverPlumbingAlone(t *testing.T) { func TestStartupSaysWhenItCouldNotLook(t *testing.T) { // Silence on error would be the exact defect the notice removes: the // operator must at least learn that nobody looked. - out := noticed(t, surveyingDriver{err: errors.New("incus: connection refused")}) + out := noticed(t, machine.Use(surveyingDriver{err: errors.New("incus: connection refused")})) if !strings.Contains(out, "could not look") || !strings.Contains(out, "connection refused") { t.Errorf("a failed survey must be said, got:\n%s", out) } @@ -96,7 +96,7 @@ func TestStartupSaysWhenItCouldNotLook(t *testing.T) { func TestStartupAsksNothingOfADriverThatCannotAnswer(t *testing.T) { // Noop backs --vm off: no runtime, nothing to survey, and the notice must // not manufacture one. - if out := noticed(t, machine.Noop{}); out != "" { + if out := noticed(t, machine.Use(machine.Noop{})); out != "" { t.Errorf("a driver without Survey produced a notice:\n%s", out) } } diff --git a/internal/cli/provider_four_test.go b/internal/cli/provider_four_test.go index a37b346a..24339175 100644 --- a/internal/cli/provider_four_test.go +++ b/internal/cli/provider_four_test.go @@ -160,13 +160,13 @@ func TestTheDisciplineDetectorsReadTheFourthPack(t *testing.T) { func fourthPack(t *testing.T) (*providerfour.Pack, *machine.Recorder, *emulator.Env) { t.Helper() rec := machine.NewRecorder() - env := fourthEnv(t, rec) + env := fourthEnv(t, machine.Use(rec)) return providerfour.New(env), rec, env } // fourthEnv is fourthPack's environment half, for the tests that need a // runtime other than a plain recorder. -func fourthEnv(t *testing.T, runtime machine.Driver) *emulator.Env { +func fourthEnv(t *testing.T, runtime machine.Runtime) *emulator.Env { t.Helper() machine.Binding{Provider: providerfour.Name}.ForgetPlacements() n := 0 @@ -593,7 +593,7 @@ func (refusingRuntime) Start(context.Context, machine.Spec) (machine.Machine, er // for an address that never comes, on a machine that does not exist. func TestTheFourthPackPublishesTheStateTheEffectProduced(t *testing.T) { ctx := context.Background() - env := fourthEnv(t, refusingRuntime{machine.NewRecorder()}) + env := fourthEnv(t, machine.Use(refusingRuntime{machine.NewRecorder()})) pack := providerfour.New(env) node, err := pack.CreateNode(ctx, providerfour.NodeRequest{Name: "web-1", Image: "four-linux"}) @@ -756,7 +756,7 @@ func TestTheFourthPacksSegmentsReachEachOtherOnlyInTheSameRealm(t *testing.T) { // the pack says nothing about which. joined := machine.NewRecorder() joined.Joined = true - env := fourthEnv(t, joined) + env := fourthEnv(t, machine.Use(joined)) other := providerfour.New(env) first, err := other.CreateSegment(ctx, "green-1", "10.40.0.0/24", "green") must(t, err) @@ -814,7 +814,7 @@ func TestTheFourthPacksSpreaderRecordsTheDeliveryAndNotTheIntent(t *testing.T) { // A runtime that cannot balance: the pack leaves its family a record and // asks the host for nothing, rather than reporting nothing distributed // with no reason beside it. - silent := fourthEnv(t, machine.Noop{}) + silent := fourthEnv(t, machine.Use(machine.Noop{})) quiet := providerfour.New(silent) other, err := quiet.CreateSegment(ctx, "front", "10.40.0.0/24", "green") must(t, err) @@ -874,7 +874,7 @@ func TestTheFourthPacksSpreaderKeepsItsPortAcrossASnapshot(t *testing.T) { } rec := machine.NewRecorder() - next := fourthEnv(t, rec) + next := fourthEnv(t, machine.Use(rec)) next.Store = restored revived := providerfour.New(next) must(t, revived.RegisterBackend(ctx, spreader.ID, node.ID)) diff --git a/internal/cli/provider_four_wiring_test.go b/internal/cli/provider_four_wiring_test.go index 38830532..6d5fa2c8 100644 --- a/internal/cli/provider_four_wiring_test.go +++ b/internal/cli/provider_four_wiring_test.go @@ -51,7 +51,7 @@ import ( // reportingEnv is fourthEnv with a logger a test can read, because "reports // rather than panics" is an assertion about what was said, and a discarded log // makes it an assertion that nothing crashed. -func reportingEnv(t *testing.T, runtime machine.Driver) (*emulator.Env, *bytes.Buffer) { +func reportingEnv(t *testing.T, runtime machine.Runtime) (*emulator.Env, *bytes.Buffer) { t.Helper() var log bytes.Buffer n := 0 @@ -103,7 +103,7 @@ func aNode(env *emulator.Env) *resource.Resource { // — the one only `--vm incus` and `--vm incus-ovn` used to reach — without // needing a host. func TestABootUnderAnEnforcingRuntimeReportsAnUnwiredGroupSync(t *testing.T) { - env, log := reportingEnv(t, machine.NewRecorder()) + env, log := reportingEnv(t, machine.Use(machine.NewRecorder())) res := aNode(env) // A panic here fails the test by crashing it, which is the honest form: a @@ -142,10 +142,10 @@ func TestABootUnderAnEnforcingRuntimeReportsAnUnwiredGroupSync(t *testing.T) { func TestAnUnwiredGroupSyncIsReportedUnderEveryRuntime(t *testing.T) { for _, tc := range []struct { mode string - runtime machine.Driver + runtime machine.Runtime }{ - {"an enforcing runtime, what --vm incus-ovn gives", machine.NewRecorder()}, - {"machine.Noop, the --vm off default and what CI runs", machine.Noop{}}, + {"an enforcing runtime, what --vm incus-ovn gives", machine.Use(machine.NewRecorder())}, + {"machine.Noop, the --vm off default and what CI runs", machine.Use(machine.Noop{})}, } { t.Run(tc.mode, func(t *testing.T) { env, log := reportingEnv(t, tc.runtime) @@ -217,7 +217,7 @@ func TestAPackThatWiredNoGroupSyncIsToldWhichFieldIsMissing(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - env, log := reportingEnv(t, machine.NewRecorder()) + env, log := reportingEnv(t, machine.Use(machine.NewRecorder())) res := aNode(env) rec := machine.Reconciler{ Groups: tc.build(env), @@ -270,10 +270,10 @@ func TestAPackThatWiredNoGroupSyncIsToldWhichFieldIsMissing(t *testing.T) { func TestABootWithNoDeclaredPlanIsRefusedRatherThanPanicking(t *testing.T) { for _, tc := range []struct { mode string - runtime machine.Driver + runtime machine.Runtime }{ - {"an enforcing runtime", machine.NewRecorder()}, - {"machine.Noop, the --vm off default", machine.Noop{}}, + {"an enforcing runtime", machine.Use(machine.NewRecorder())}, + {"machine.Noop, the --vm off default", machine.Use(machine.Noop{})}, } { t.Run(tc.mode, func(t *testing.T) { env, log := reportingEnv(t, tc.runtime) diff --git a/internal/cli/runtime_blindness_test.go b/internal/cli/runtime_blindness_test.go index 2274103a..40615e14 100644 --- a/internal/cli/runtime_blindness_test.go +++ b/internal/cli/runtime_blindness_test.go @@ -200,28 +200,29 @@ type runtimeScan struct { } // driverImplementations names the exported types of internal/core/machine that -// implement machine.Driver, derived from the package's own declarations. +// implement its driver interface, derived from the package's own declarations. // // Derived rather than listed, and for the reason #511 wrote down after three // wrong counts of the same surface: a list somebody remembered is a list that // stops being true. A fourth runtime — the remote or libvirt driver #514 leaves // the door open for — becomes forbidden vocabulary the day it compiles, with // nobody having to remember this file exists. +// +// The method set is read off the *unexported* interface since #514, which is +// where it lives now. That is not a detail of this reader: a derivation still +// looking for an exported "Driver" would find nothing, call every type an +// implementation or none, and report a disciplined repository — which is what +// it did for one commit while this change was being made. func driverImplementations(t *testing.T, pkg machinePackage) []string { t.Helper() - var methods []string - for key := range pkg.members { - if typ, method, ok := strings.Cut(key, "."); ok && typ == "Driver" { - methods = append(methods, method) - } - } + methods := pkg.hidden["driver"] if len(methods) < 5 { - t.Fatalf("machine.Driver reads as %d method(s): the derivation is broken, and a broken "+ - "derivation names every type an implementation or none", len(methods)) + t.Fatalf("the machine package's driver interface reads as %d method(s): the derivation is "+ + "broken, and a broken derivation names every type an implementation or none", len(methods)) } var impls []string for name, kind := range pkg.names { - if kind != "type" || name == "Driver" { + if kind != "type" { continue } complete := true @@ -240,8 +241,8 @@ func driverImplementations(t *testing.T, pkg machinePackage) []string { // is the only runtime. A derivation that found one found the interface and // missed the types. if len(impls) < 2 { - t.Fatalf("only %v implement machine.Driver: the derivation is broken, and its silence "+ - "about the rest would read as a pack naming no runtime", impls) + t.Fatalf("only %v implement the machine package's driver: the derivation is broken, and "+ + "its silence about the rest would read as a pack naming no runtime", impls) } return impls } @@ -257,8 +258,9 @@ func runtimeTells(impls []string) []runtimeTell { // are all the same knowledge, and only the first would survive a // word boundary. match: regexp.MustCompile(`(?i)` + regexp.QuoteMeta(impl)), - why: "machine." + impl + " implements machine.Driver: it is one of the runtimes behind " + - "--vm, and which one is running is the operator's business and never the pack's", + why: "machine." + impl + " implements the machine package's driver: it is one of the " + + "runtimes behind --vm, and which one is running is the operator's business and " + + "never the pack's", }) } return append(tells, declaredRuntimeTells...) diff --git a/internal/cli/shutdown_sweep_test.go b/internal/cli/shutdown_sweep_test.go index 23c9fe96..59389c4f 100644 --- a/internal/cli/shutdown_sweep_test.go +++ b/internal/cli/shutdown_sweep_test.go @@ -36,7 +36,7 @@ func TestAGracefulExitReleasesTheUplink(t *testing.T) { var buf bytes.Buffer driver := &releasingDriver{released: true} - shutdownSweep(driver, false, &buf) + shutdownSweep(machine.Use(driver), false, &buf) if !driver.asked { t.Fatal("the exit never asked the driver to release the uplink; the next run's doorstep refuses what stays (#521)") @@ -53,7 +53,7 @@ func TestAnExitSaysNothingWhenTheUplinkIsNotItsToRelease(t *testing.T) { var buf bytes.Buffer driver := &releasingDriver{released: false} - shutdownSweep(driver, false, &buf) + shutdownSweep(machine.Use(driver), false, &buf) if !driver.asked { t.Fatal("the exit never asked the driver") @@ -68,7 +68,7 @@ func TestAnExitSaysNothingWhenTheUplinkIsNotItsToRelease(t *testing.T) { func TestAnExitWithoutAReleaserStaysAnExit(t *testing.T) { var buf bytes.Buffer - shutdownSweep(machine.Noop{}, false, &buf) + shutdownSweep(machine.Use(machine.Noop{}), false, &buf) if buf.Len() != 0 { t.Errorf("a driver with no uplink produced output:\n%s", buf.String()) diff --git a/internal/cli/testdata/bypass/admitted/probe.go b/internal/cli/testdata/bypass/admitted/probe.go new file mode 100644 index 00000000..c459a8f1 --- /dev/null +++ b/internal/cli/testdata/bypass/admitted/probe.go @@ -0,0 +1,34 @@ +// Package bypass is one sentence a provider pack must not be able to write. +// +// Each directory here is a package of its own, holding a single expression, +// and every one of them but admitted/ must FAIL to compile. +// internal/cli's TestThePacksCannotNameTheDriver builds each in turn and +// requires the failure, naming the symbol; admitted/ is its positive control +// and must build, so a probe broken for any other reason — a wrong import +// path, a module the internal rule refuses, no toolchain — cannot read as the +// door being shut. +// +// They live under testdata/ so `go build ./...`, `go vet ./...` and +// golangci-lint never see them: a package that must not compile would +// otherwise break every build in the repository. `go list ./...` never names +// them either, so no coverage, evidence or drift artefact can count them. +package admittedprobe + +import ( + "github.com/stephrobert/feint/internal/core/emulator" + "github.com/stephrobert/feint/internal/core/machine" +) + +// The positive control. Everything here is something a pack may legitimately +// name — machine.PackSurface admits the three types, and emulator.Env.Store is +// the field every pack reads — so this package must build. If it stops +// building, the probes beside it stop measuring the door and start measuring +// whatever broke here, which is the exact shape of an instrument that reports +// success because it looked nowhere. +var ( + _ machine.Binding + _ machine.Reconciler + _ machine.GroupSync + _ = emulator.Env{}.Store + _ = machine.Binding{}.Prefix +) diff --git a/internal/cli/testdata/bypass/balancer/probe.go b/internal/cli/testdata/bypass/balancer/probe.go new file mode 100644 index 00000000..726e93b9 --- /dev/null +++ b/internal/cli/testdata/bypass/balancer/probe.go @@ -0,0 +1,21 @@ +// Package bypass is one sentence a provider pack must not be able to write. +// +// Each directory here is a package of its own, holding a single expression, +// and every one of them but admitted/ must FAIL to compile. +// internal/cli's TestThePacksCannotNameTheDriver builds each in turn and +// requires the failure, naming the symbol; admitted/ is its positive control +// and must build, so a probe broken for any other reason — a wrong import +// path, a module the internal rule refuses, no toolchain — cannot read as the +// door being shut. +// +// They live under testdata/ so `go build ./...`, `go vet ./...` and +// golangci-lint never see them: a package that must not compile would +// otherwise break every build in the repository. `go list ./...` never names +// them either, so no coverage, evidence or drift artefact can count them. +package balancerprobe + +import "github.com/stephrobert/feint/internal/core/machine" + +// The balancing half, reached by a bare interface assertion in one pack until +// Binding gained the three verbs. +var _ machine.Balancer diff --git a/internal/cli/testdata/bypass/bindingdriver/probe.go b/internal/cli/testdata/bypass/bindingdriver/probe.go new file mode 100644 index 00000000..5c5d6182 --- /dev/null +++ b/internal/cli/testdata/bypass/bindingdriver/probe.go @@ -0,0 +1,28 @@ +// Package bypass is one sentence a provider pack must not be able to write. +// +// Each directory here is a package of its own, holding a single expression, +// and every one of them but admitted/ must FAIL to compile. +// internal/cli's TestThePacksCannotNameTheDriver builds each in turn and +// requires the failure, naming the symbol; admitted/ is its positive control +// and must build, so a probe broken for any other reason — a wrong import +// path, a module the internal rule refuses, no toolchain — cannot read as the +// door being shut. +// +// They live under testdata/ so `go build ./...`, `go vet ./...` and +// golangci-lint never see them: a package that must not compile would +// otherwise break every build in the repository. `go list ./...` never names +// them either, so no coverage, evidence or drift artefact can count them. +package bindingdriverprobe + +import ( + "context" + + "github.com/stephrobert/feint/internal/core/machine" +) + +// The sentence internal/core/machine/surface.go cites by name: it compiled +// while Binding's driver field was exported, and a discipline test could only +// report it. +func bypass(ctx context.Context, b machine.Binding) { + _ = b.Driver.EnsureNetwork(ctx, machine.NetworkSpec{}) +} diff --git a/internal/cli/testdata/bypass/driver/probe.go b/internal/cli/testdata/bypass/driver/probe.go new file mode 100644 index 00000000..2bb1fab6 --- /dev/null +++ b/internal/cli/testdata/bypass/driver/probe.go @@ -0,0 +1,22 @@ +// Package bypass is one sentence a provider pack must not be able to write. +// +// Each directory here is a package of its own, holding a single expression, +// and every one of them but admitted/ must FAIL to compile. +// internal/cli's TestThePacksCannotNameTheDriver builds each in turn and +// requires the failure, naming the symbol; admitted/ is its positive control +// and must build, so a probe broken for any other reason — a wrong import +// path, a module the internal rule refuses, no toolchain — cannot read as the +// door being shut. +// +// They live under testdata/ so `go build ./...`, `go vet ./...` and +// golangci-lint never see them: a package that must not compile would +// otherwise break every build in the repository. `go list ./...` never names +// them either, so no coverage, evidence or drift artefact can count them. +package driverprobe + +import "github.com/stephrobert/feint/internal/core/machine" + +// The sentence of #514's acceptance criterion (a), verbatim. On 154c204 this +// file compiled and `go build ./internal/providers/scaleway/` exited 0, which +// is what left the boundary held by a convention plus an AST scan. +var _ machine.Driver diff --git a/internal/cli/testdata/bypass/envdriver/probe.go b/internal/cli/testdata/bypass/envdriver/probe.go new file mode 100644 index 00000000..999e3792 --- /dev/null +++ b/internal/cli/testdata/bypass/envdriver/probe.go @@ -0,0 +1,22 @@ +// Package bypass is one sentence a provider pack must not be able to write. +// +// Each directory here is a package of its own, holding a single expression, +// and every one of them but admitted/ must FAIL to compile. +// internal/cli's TestThePacksCannotNameTheDriver builds each in turn and +// requires the failure, naming the symbol; admitted/ is its positive control +// and must build, so a probe broken for any other reason — a wrong import +// path, a module the internal rule refuses, no toolchain — cannot read as the +// door being shut. +// +// They live under testdata/ so `go build ./...`, `go vet ./...` and +// golangci-lint never see them: a package that must not compile would +// otherwise break every build in the repository. `go list ./...` never names +// them either, so no coverage, evidence or drift artefact can count them. +package envdriverprobe + +import "github.com/stephrobert/feint/internal/core/emulator" + +// The value every pack held before #511: `Machines machine.Driver` was a +// public field of the environment, and `p.env.Machines` was how three packs +// built their bindings. +var _ = emulator.Env{}.Machines diff --git a/internal/cli/testdata/bypass/firewaller/probe.go b/internal/cli/testdata/bypass/firewaller/probe.go new file mode 100644 index 00000000..d0d3b07d --- /dev/null +++ b/internal/cli/testdata/bypass/firewaller/probe.go @@ -0,0 +1,21 @@ +// Package bypass is one sentence a provider pack must not be able to write. +// +// Each directory here is a package of its own, holding a single expression, +// and every one of them but admitted/ must FAIL to compile. +// internal/cli's TestThePacksCannotNameTheDriver builds each in turn and +// requires the failure, naming the symbol; admitted/ is its positive control +// and must build, so a probe broken for any other reason — a wrong import +// path, a module the internal rule refuses, no toolchain — cannot read as the +// door being shut. +// +// They live under testdata/ so `go build ./...`, `go vet ./...` and +// golangci-lint never see them: a package that must not compile would +// otherwise break every build in the repository. `go list ./...` never names +// them either, so no coverage, evidence or drift artefact can count them. +package firewallerprobe + +import "github.com/stephrobert/feint/internal/core/machine" + +// The rule-set half. Reaching it is how a pack writes EnsureFirewall itself +// instead of going through GroupSync, which is the layer #475 was born in. +var _ machine.Firewaller diff --git a/internal/cli/testdata/bypass/isolator/probe.go b/internal/cli/testdata/bypass/isolator/probe.go new file mode 100644 index 00000000..58421f5d --- /dev/null +++ b/internal/cli/testdata/bypass/isolator/probe.go @@ -0,0 +1,20 @@ +// Package bypass is one sentence a provider pack must not be able to write. +// +// Each directory here is a package of its own, holding a single expression, +// and every one of them but admitted/ must FAIL to compile. +// internal/cli's TestThePacksCannotNameTheDriver builds each in turn and +// requires the failure, naming the symbol; admitted/ is its positive control +// and must build, so a probe broken for any other reason — a wrong import +// path, a module the internal rule refuses, no toolchain — cannot read as the +// door being shut. +// +// They live under testdata/ so `go build ./...`, `go vet ./...` and +// golangci-lint never see them: a package that must not compile would +// otherwise break every build in the repository. `go list ./...` never names +// them either, so no coverage, evidence or drift artefact can count them. +package isolatorprobe + +import "github.com/stephrobert/feint/internal/core/machine" + +// The isolation half, the other side of the same fork. +var _ machine.Isolator diff --git a/internal/cli/testdata/bypass/peerer/probe.go b/internal/cli/testdata/bypass/peerer/probe.go new file mode 100644 index 00000000..70f5560e --- /dev/null +++ b/internal/cli/testdata/bypass/peerer/probe.go @@ -0,0 +1,21 @@ +// Package bypass is one sentence a provider pack must not be able to write. +// +// Each directory here is a package of its own, holding a single expression, +// and every one of them but admitted/ must FAIL to compile. +// internal/cli's TestThePacksCannotNameTheDriver builds each in turn and +// requires the failure, naming the symbol; admitted/ is its positive control +// and must build, so a probe broken for any other reason — a wrong import +// path, a module the internal rule refuses, no toolchain — cannot read as the +// door being shut. +// +// They live under testdata/ so `go build ./...`, `go vet ./...` and +// golangci-lint never see them: a package that must not compile would +// otherwise break every build in the repository. `go list ./...` never names +// them either, so no coverage, evidence or drift artefact can count them. +package peererprobe + +import "github.com/stephrobert/feint/internal/core/machine" + +// The peering half. Outscale reached it directly as a second writer, and the +// audit measured a peering severed on CreateSubnet because of it. +var _ machine.Peerer diff --git a/internal/cli/testdata/bypass/router/probe.go b/internal/cli/testdata/bypass/router/probe.go new file mode 100644 index 00000000..ef1ee7ff --- /dev/null +++ b/internal/cli/testdata/bypass/router/probe.go @@ -0,0 +1,21 @@ +// Package bypass is one sentence a provider pack must not be able to write. +// +// Each directory here is a package of its own, holding a single expression, +// and every one of them but admitted/ must FAIL to compile. +// internal/cli's TestThePacksCannotNameTheDriver builds each in turn and +// requires the failure, naming the symbol; admitted/ is its positive control +// and must build, so a probe broken for any other reason — a wrong import +// path, a module the internal rule refuses, no toolchain — cannot read as the +// door being shut. +// +// They live under testdata/ so `go build ./...`, `go vet ./...` and +// golangci-lint never see them: a package that must not compile would +// otherwise break every build in the repository. `go list ./...` never names +// them either, so no coverage, evidence or drift artefact can count them. +package routerprobe + +import "github.com/stephrobert/feint/internal/core/machine" + +// The address half. A pack asserting its way to this one reaches +// RouteAddress past Reconciler.Route and past the emulated-block guard. +var _ machine.Router diff --git a/internal/cli/timeout.go b/internal/cli/timeout.go index c649fe83..41971ff4 100644 --- a/internal/cli/timeout.go +++ b/internal/cli/timeout.go @@ -26,8 +26,8 @@ import ( // stays; without one, the original figure keeps its measured justification. // // TestTheWriteDeadlineFollowsTheRuntime fails if either mode loses its value. -func writeTimeoutFor(driver machine.Driver) time.Duration { - if _, none := driver.(machine.Noop); none { +func writeTimeoutFor(rt machine.Runtime) time.Duration { + if !rt.Runs() { return 60 * time.Second } return 10 * time.Minute diff --git a/internal/cli/timeout_test.go b/internal/cli/timeout_test.go index 18e1a4fc..d6ce9f06 100644 --- a/internal/cli/timeout_test.go +++ b/internal/cli/timeout_test.go @@ -14,10 +14,10 @@ import ( // connection on work that then succeeds: the client's retry meets its own // subnet as a conflict (#473, links 2 and 3). func TestTheWriteDeadlineFollowsTheRuntime(t *testing.T) { - if got := writeTimeoutFor(machine.Noop{}); got != 60*time.Second { + if got := writeTimeoutFor(machine.Use(machine.Noop{})); got != 60*time.Second { t.Errorf("with no runtime the deadline is %v; sixty seconds is the measured figure for small local responses", got) } - withRuntime := writeTimeoutFor(machine.NewIncusOVN()) + withRuntime := writeTimeoutFor(machine.Use(machine.NewIncusOVN())) if withRuntime <= 60*time.Second { t.Errorf("under a machine runtime the deadline is %v; anything at or under a minute cuts a fifteen-subnet apply (#473)", withRuntime) } diff --git a/internal/cli/up.go b/internal/cli/up.go index 6f65b8d5..df8270cb 100644 --- a/internal/cli/up.go +++ b/internal/cli/up.go @@ -316,7 +316,7 @@ func preflight(decl *environment.File, skipIaC bool, stdout io.Writer) error { // This is the same check `serve` makes at startup (#181) — a mode the host // cannot deliver is refused naming the missing half — moved ahead of // everything so that nothing has started when it fires. - driver, err := resolveRuntime(decl.Runtime.Mode, stdout) + rt, err := resolveRuntime(decl.Runtime.Mode, stdout) if err != nil { // A guard with no way past it gets worked around by copying the // emulator, which teaches nobody anything — the reasoning that named @@ -328,11 +328,11 @@ func preflight(decl *environment.File, skipIaC bool, stdout io.Writer) error { if len(decl.Runtime.Images) == 0 { return nil } - if _, isNoop := driver.(machine.Noop); isNoop { + if !rt.Runs() { fmt.Fprintf(stdout, " runtime.images: not checked, nothing boots under `%s`\n", decl.Runtime.Mode) return nil } - return checkDeclaredImages(decl, driver, stdout) + return checkDeclaredImages(decl, rt, stdout) } // resolveRuntime is machineDriver, behind a name a test can replace. @@ -436,14 +436,14 @@ func waysPastTheRuntimeRefusal(decl *environment.File) string { // // TestADeclaredImageTheStationLacksIsRefusedWithTheCommandThatBuildsIt fails // without the refusal, and its sibling holds the accepting half. -func checkDeclaredImages(decl *environment.File, driver machine.Driver, stdout io.Writer) error { +func checkDeclaredImages(decl *environment.File, rt machine.Runtime, stdout io.Writer) error { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - inventory, err := machine.ImageInventory(ctx, driver) + inventory, err := rt.Inventory(ctx) if err != nil { return fmt.Errorf("reading the machine images: %w", err) } - derived, err := machine.DerivedImages(ctx, driver) + derived, err := rt.DerivedInventory(ctx) if err != nil { return fmt.Errorf("reading the images this station derived: %w", err) } diff --git a/internal/cli/up_wait_test.go b/internal/cli/up_wait_test.go index 827a53c1..a06580b2 100644 --- a/internal/cli/up_wait_test.go +++ b/internal/cli/up_wait_test.go @@ -307,12 +307,12 @@ func TestUpRefusesADeclaredRuntimeTheHostCannotDeliver(t *testing.T) { asked := "" restore := resolveRuntime - resolveRuntime = func(mode string, _ io.Writer) (machine.Driver, error) { + resolveRuntime = func(mode string, _ io.Writer) (machine.Runtime, error) { asked = mode if mode == "off" { - return machine.Noop{}, nil + return machine.Use(machine.Noop{}), nil } - return nil, fmt.Errorf("--vm %s requested but this host cannot deliver it:\n"+ + return machine.Runtime{}, fmt.Errorf("--vm %s requested but this host cannot deliver it:\n"+ " isolation: the daemon did not answer for network.ovn.northbound", mode) } t.Cleanup(func() { resolveRuntime = restore }) @@ -355,7 +355,7 @@ func TestUpRefusesADeclaredRuntimeTheHostCannotDeliver(t *testing.T) { // every mode would pass the test above and serve nobody. func TestUpAcceptsARuntimeTheHostDoesDeliver(t *testing.T) { restore := resolveRuntime - resolveRuntime = func(_ string, _ io.Writer) (machine.Driver, error) { return machine.Noop{}, nil } + resolveRuntime = func(_ string, _ io.Writer) (machine.Runtime, error) { return machine.Use(machine.Noop{}), nil } t.Cleanup(func() { resolveRuntime = restore }) decl, err := environment.Parse("version: 1\nruntime:\n mode: incus-ovn\n") @@ -387,7 +387,7 @@ func TestADeclaredImageTheStationLacksIsRefusedWithTheCommandThatBuildsIt(t *tes t.Fatalf("parse: %v", err) } var buf bytes.Buffer - err = checkDeclaredImages(decl, emptyStation{}, &buf) + err = checkDeclaredImages(decl, machine.Use(emptyStation{}), &buf) if err == nil { t.Fatalf("a station holding no image accepted a declaration that needs %s", want) } @@ -408,7 +408,7 @@ func TestAnImageOutsideTheWarmUpSetIsAnnouncedAndNeverRefused(t *testing.T) { t.Fatalf("parse: %v", err) } var out bytes.Buffer - if err := checkDeclaredImages(decl, emptyStation{}, &out); err != nil { + if err := checkDeclaredImages(decl, machine.Use(emptyStation{}), &out); err != nil { t.Fatalf("an image the boot path can derive was refused: %v", err) } if !strings.Contains(out.String(), "nixos/25.05") || !strings.Contains(out.String(), "derives one") { diff --git a/internal/core/emulator/conformance.go b/internal/core/emulator/conformance.go index aa1ba0e0..8a1c8e25 100644 --- a/internal/core/emulator/conformance.go +++ b/internal/core/emulator/conformance.go @@ -871,10 +871,7 @@ func (s *Server) handleConformance(w http.ResponseWriter, _ *http.Request) { // The dataplane axis reads the driver's own declaration, never a mode // name: Noop names itself "none", and that is the whole comparison. - machines := "none" - if s.env.machines != nil { - machines = s.env.machines.Name() - } + machines := s.env.machines.Name() runtimeOn := machines != "none" evidence := make(map[string]Evidence, len(routes)) diff --git a/internal/core/emulator/emulator.go b/internal/core/emulator/emulator.go index 0ce8ef04..5eff44dd 100644 --- a/internal/core/emulator/emulator.go +++ b/internal/core/emulator/emulator.go @@ -35,13 +35,18 @@ type Env struct { // operator asked for it. // // Unexported since #511, and that is the point: it was the one value that - // put a machine.Driver in every pack's hand, and a pack holding one can - // call any driver verb — Start, Remove, RemoveNetwork — past the ownership - // checks and past the shared order. A pack now declares its binding and - // receives it back through Bind, and no expression in internal/providers - // can name a driver at all. Wiring and tests use UseMachines; readers of - // the runtime's identity use RuntimeName. - machines machine.Driver + // put a driver in every pack's hand, and a pack holding one can call any + // driver verb — Start, Remove, RemoveNetwork — past the ownership checks + // and past the shared order. A pack now declares its binding and receives + // it back through Bind. Wiring and tests use UseMachines; readers of the + // runtime's identity use RuntimeName. + // + // A machine.Runtime since #514, not a driver: the field being private + // closed the way to obtain one and left the way to name one open, so + // `var _ machine.Driver` in a pack still compiled. The handle is now the + // only spelling of a runtime outside internal/core/machine, and it offers + // no verb that moves a machine, a network or a rule set. + machines machine.Runtime // BootImages maps an opaque image identifier onto the operating system the // operator declared it to be (FEINT_BOOT_IMAGES, parsed by // machine.ParseDeclaredImages). Each pack hands it to its Binding, which @@ -70,31 +75,29 @@ type Env struct { // The pack declares what only it knows — its prefix, its login, the key its // address is published under, its image table — and the environment supplies // the driver. It used to write `Driver: p.env.Machines` itself, which meant -// every pack held a machine.Driver value and could call any of its verbs, -// ownership checks and shared order included; machine.Binding's driver field -// is unexported since, so this is the only way in and there is no way back -// out. A pack naming this field again does not fail a test — it fails the -// build, which is the strongest of the three ranks #514 lists; internal/cli's +// every pack held a driver value and could call any of its verbs, ownership +// checks and shared order included; machine.Binding's driver field is +// unexported since, so this is the only way in and there is no way back out. A +// pack naming this field again does not fail a test — it fails the build, +// which is the strongest of the three ranks #514 lists; internal/cli's // TestNoPackReachesPastTheDeclaredDriverSurface holds what typing cannot, // starting with a pack that builds a driver of its own. func (e *Env) Bind(b machine.Binding) machine.Binding { - return b.WithDriver(e.machines) + return b.WithRuntime(e.machines) } // UseMachines points this environment at a machine runtime. It is how the CLI // wires the driver the operator asked for, and how a test injects a fake one; // nothing hands the value back out. -func (e *Env) UseMachines(d machine.Driver) { e.machines = d } +// +// It takes the handle rather than the driver, so a caller wraps its own value +// once — machine.Use(fake) — and never names a driver type to do it. +func (e *Env) UseMachines(r machine.Runtime) { e.machines = r } // RuntimeName identifies the runtime in logs, in `feint status` and on // /_feint/health — the one thing about the driver a caller outside this // package may read, because a name is not a handle. -func (e *Env) RuntimeName() string { - if e.machines == nil { - return machine.Noop{}.Name() - } - return e.machines.Name() -} +func (e *Env) RuntimeName() string { return e.machines.Name() } // DefaultEnv returns an environment backed by a fresh store, the wall clock, // random UUIDs and no machine runtime. @@ -103,7 +106,7 @@ func DefaultEnv() *Env { Store: store.New(), Now: func() time.Time { return time.Now().UTC() }, NewID: NewUUID, - machines: machine.Noop{}, + machines: machine.Use(machine.Noop{}), Log: slog.Default(), } } @@ -617,15 +620,11 @@ func (s *Server) handleHealth(w http.ResponseWriter, _ *http.Request) { for _, p := range s.packs { providers = append(providers, p.Name()) } - driver := "none" - var capabilities *machine.Capabilities - if s.env.machines != nil { - driver = s.env.machines.Name() - // Declared rather than CapabilitiesOf: null on the wire when the driver - // said nothing, so a reader can tell "no" from "nobody promised". Five - // falses cannot. - capabilities = machine.Declared(s.env.machines) - } + driver := s.env.machines.Name() + // Declared rather than Capabilities: null on the wire when the driver said + // nothing, so a reader can tell "no" from "nobody promised". Five falses + // cannot. + capabilities := s.env.machines.DeclaredCapabilities() // The capabilities are published because the difference between the runtime // modes was recorded only in documentation — that is, nowhere at the moment // it matters. A conformance suite gating on `capabilities.isolation` asserts diff --git a/internal/core/emulator/enforcement.go b/internal/core/emulator/enforcement.go index 72e7d282..e7ac23d1 100644 --- a/internal/core/emulator/enforcement.go +++ b/internal/core/emulator/enforcement.go @@ -46,7 +46,8 @@ type FirewallEnforcer interface { // It exists because the firewall's history repeated itself on the balancer, // measured on 2026-08-25: `capabilities.balancing` was true under OVN — the // runtime really can distribute — while a Scaleway stack's load balancer left -// no trace on the host, because that pack hands nothing to `machine.Balancer`. +// no trace on the host, because that pack hands nothing to the runtime's +// balancing half. // Both statements were true, and a suite following this repository's own advice // ("key on the declared capability, never on a mode name") would have asserted // distribution on a cloud that never promised it. The vocabulary was one word diff --git a/internal/core/emulator/evidence_test.go b/internal/core/emulator/evidence_test.go index e3773c87..0094076c 100644 --- a/internal/core/emulator/evidence_test.go +++ b/internal/core/emulator/evidence_test.go @@ -187,7 +187,7 @@ func (namedDriver) Available(context.Context) bool { return true } func TestEvidenceDataplaneFollowsTheDriversOwnDeclaration(t *testing.T) { env := contractEnv(t) - env.UseMachines(namedDriver{}) + env.UseMachines(machine.Use(namedDriver{})) srv := evidenceServer(t, env, `{"ok": true}`) ts := httptest.NewServer(srv.Handler()) defer ts.Close() diff --git a/internal/core/emulator/ui_test.go b/internal/core/emulator/ui_test.go index a254fb99..a5108511 100644 --- a/internal/core/emulator/ui_test.go +++ b/internal/core/emulator/ui_test.go @@ -1,6 +1,7 @@ package emulator_test import ( + "context" "encoding/json" "net/http" "net/http/httptest" @@ -396,15 +397,32 @@ func TestThePageCarriesEveryOperationBehindTheCounts(t *testing.T) { // the fourth one will, and it must not be read as refusing five capabilities it // was never asked about. // -// It embeds the Driver interface rather than the no-op driver, and that is the -// whole trick: embedding Noop would inherit Noop's own Capabilities method and -// make this driver a declaring one, which is the opposite of what it is for. A -// nil embedded interface satisfies Driver at compile time and panics if anything -// calls through it — nothing here does, because the health endpoint asks for the -// name and for a type assertion, and both are answered above. -type mute struct{ machine.Driver } +// It spells out the ten verbs rather than embedding machine.Noop, and that is +// the whole trick: Noop carries its own Capabilities method, so embedding it +// would make this driver a declaring one, which is the opposite of what it is +// for. It used to embed the driver interface as a nil field, which satisfied +// the interface and panicked on any call; since #514 that type is unnameable +// outside internal/core/machine, and writing the methods out is the better +// answer anyway — a runtime nothing can call is a runtime this test cannot +// grow into. +type mute struct{} -func (mute) Name() string { return "mute" } +func (mute) Name() string { return "mute" } +func (mute) Available(context.Context) bool { return true } +func (mute) Stop(context.Context, string) error { return nil } +func (mute) Remove(context.Context, string) error { return nil } +func (mute) EnsureNetwork(context.Context, machine.NetworkSpec) error { return nil } +func (mute) Attach(context.Context, string, machine.Attachment) error { return nil } +func (mute) Detach(context.Context, string, string) error { return nil } +func (mute) RemoveNetwork(context.Context, string) error { return nil } + +func (mute) Start(_ context.Context, spec machine.Spec) (machine.Machine, error) { + return machine.Machine{Name: spec.Name, Running: true}, nil +} + +func (mute) Inspect(_ context.Context, name string) (machine.Machine, bool, error) { + return machine.Machine{Name: name}, false, nil +} // A driver that declared nothing is not a driver that refused everything. // @@ -417,9 +435,9 @@ func (mute) Name() string { return "mute" } // made on the wire rather than in the page: a second driver, a second reader, or // a script with jq would each have to reinvent it otherwise. func TestAnUndeclaredDriverIsNotTheSameAsOneThatDeclaresNothing(t *testing.T) { - read := func(driver machine.Driver) map[string]any { + read := func(rt machine.Runtime) map[string]any { env := emulator.DefaultEnv() - env.UseMachines(driver) + env.UseMachines(rt) srv, err := emulator.NewServer(env) if err != nil { t.Fatalf("build emulator: %v", err) @@ -432,7 +450,7 @@ func TestAnUndeclaredDriverIsNotTheSameAsOneThatDeclaresNothing(t *testing.T) { } // The no-op driver declares: it runs nothing and says so, which is a claim. - silentButDeclaring := read(machine.Noop{}) + silentButDeclaring := read(machine.Use(machine.Noop{})) caps, ok := silentButDeclaring["capabilities"].(map[string]any) if !ok { t.Fatalf("a driver that declares got no capability object: %v", silentButDeclaring["capabilities"]) @@ -443,7 +461,7 @@ func TestAnUndeclaredDriverIsNotTheSameAsOneThatDeclaresNothing(t *testing.T) { // A driver that never implemented Capable declares nothing, and the payload // has to be able to say so. - undeclared := read(mute{}) + undeclared := read(machine.Use(mute{})) if undeclared["capabilities"] != nil { t.Errorf("a driver that declares nothing published %v; "+ "absent and refused would then read the same", undeclared["capabilities"]) diff --git a/internal/core/machine/address.go b/internal/core/machine/address.go index 896a6346..c9dde839 100644 --- a/internal/core/machine/address.go +++ b/internal/core/machine/address.go @@ -27,10 +27,10 @@ type AddressSpec struct { Network string } -// Router is the optional half of a Driver that can give a machine a public -// address. Separate from Driver so a runtime without the capability is a +// router is the optional half of a driver that can give a machine a public +// address. Separate from the driver so a runtime without the capability is a // compile-time fact rather than a silent no-op. -type Router interface { +type router interface { // RouteAddress makes the address reach the machine, and the machine carry // it. Calling it twice with the same pair is harmless. RouteAddress(ctx context.Context, spec AddressSpec) error diff --git a/internal/core/machine/balancer.go b/internal/core/machine/balancer.go index fd61602e..6a08f16e 100644 --- a/internal/core/machine/balancer.go +++ b/internal/core/machine/balancer.go @@ -204,13 +204,13 @@ func RecordBalancerDelivery(st *store.Store, res *resource.Resource, now time.Ti st.Commit(base, res, now) } -// Balancer is the optional half of a Driver that can distribute packets. +// balancer is the optional half of a driver that can distribute packets. // -// Optional for the same reason Firewaller is: absence has to be a compile-time -// fact and a declared capability, never a silent no-op. A pack whose driver is -// not a Balancer serves the load-balancer family exactly as before — the -// configuration round-trips, nothing forwards — and says so. -type Balancer interface { +// Optional for the same reason the firewalling half is: absence has to be a +// compile-time fact and a declared capability, never a silent no-op. A pack +// whose driver does not balance serves the load-balancer family exactly as +// before — the configuration round-trips, nothing forwards — and says so. +type balancer interface { // EnsureBalancer creates or replaces the balancer as a whole. It must // succeed when called again with the same specification, and its delivery // reports what the host now holds: the targets distributed, and the ones @@ -231,12 +231,12 @@ type Balancer interface { // Verify clears the claim on a host whose daemon has no northbound connection. // Asking only whether the interface is implemented would drive a // bridge-backed run into a refusal on every register. -func (b Binding) balancer() Balancer { +func (b Binding) balancer() balancer { if b.driver == nil || !CapabilitiesOf(b.driver).Balancing { return nil } - balancer, _ := b.driver.(Balancer) - return balancer + bal, _ := b.driver.(balancer) + return bal } // Balances reports whether this runtime both implements and declares @@ -255,22 +255,22 @@ func (b Binding) Balances() bool { return b.balancer() != nil } // through, so the caller would record "nothing distributed" with no reason // beside it. Three outcomes, never two. func (b Binding) EnsureBalancer(ctx context.Context, spec BalancerSpec) (BalancerDelivery, error) { - balancer := b.balancer() - if balancer == nil { + bal := b.balancer() + if bal == nil { return BalancerDelivery{}, ErrBalancerNotDistributed } - return balancer.EnsureBalancer(ctx, spec) + return bal.EnsureBalancer(ctx, spec) } // RemoveBalancer withdraws it. It succeeds when nothing is there, which is the // normal path: a delete runs after an emptied listener set that may never have // reached the host. A runtime that does not balance holds nothing to withdraw, // so this is a no-op rather than an error — the asymmetry with EnsureBalancer -// is deliberate, and it is the same one Driver.Remove documents. +// is deliberate, and it is the same one the driver's Remove documents. func (b Binding) RemoveBalancer(ctx context.Context, network, listen string) error { - balancer := b.balancer() - if balancer == nil { + bal := b.balancer() + if bal == nil { return nil } - return balancer.RemoveBalancer(ctx, network, listen) + return bal.RemoveBalancer(ctx, network, listen) } diff --git a/internal/core/machine/binding.go b/internal/core/machine/binding.go index 4b0e4e4b..8a48d02e 100644 --- a/internal/core/machine/binding.go +++ b/internal/core/machine/binding.go @@ -32,14 +32,18 @@ type Binding struct { // the default: starting machines is a side effect on the operator's host. // // Unexported, and that is the point of #511: a pack declares the fields - // below and receives the driver from the emulator through WithDriver, so + // below and receives the runtime from the emulator through WithRuntime, so // no expression in a provider pack can name a driver method through the // binding. The field was exported until then, and `p.binding().Driver. // EnsureNetwork(…)` was a working sentence — a discipline test can only // report such a line, while an unexported field means it does not compile. + // + // Its *type* is unexported too since #514: the field being private closed + // the way to obtain a driver, and left the way to name one open, so + // `var _ machine.Driver` in a pack still compiled. See Runtime. // internal/cli's TestNoPackReachesPastTheDeclaredDriverSurface holds what // typing cannot. - driver Driver + driver driver // Provider labels everything this binding creates, so a sweep can find its // own work and an operator's machines are never touched. Provider string @@ -105,15 +109,19 @@ type Binding struct { Log *slog.Logger } -// WithDriver returns the binding bound to a runtime. +// WithRuntime returns the binding bound to a runtime. // // This is the one door a driver goes through on its way into a binding, and it // is deliberately one-way: nothing hands it back out. The emulator calls it // (Env.Bind) while mounting a pack, which is why a pack declares the fields // above and never the driver — see the driver field for the sentence that used // to compile and no longer does. -func (b Binding) WithDriver(d Driver) Binding { - b.driver = d +// +// It takes a Runtime rather than a driver since #514: the handle is the only +// spelling of a runtime that exists outside this package, so emulator.Env can +// carry one and hand it over without ever naming what is inside it. +func (b Binding) WithRuntime(r Runtime) Binding { + b.driver = r.d return b } diff --git a/internal/core/machine/binding_boot_test.go b/internal/core/machine/binding_boot_test.go index b8ecfef9..285fa9b2 100644 --- a/internal/core/machine/binding_boot_test.go +++ b/internal/core/machine/binding_boot_test.go @@ -44,9 +44,9 @@ func (d *recordingDriver) Attach(context.Context, string, Attachment) error { re func (d *recordingDriver) Detach(context.Context, string, string) error { return nil } func (d *recordingDriver) RemoveNetwork(context.Context, string) error { return nil } -func bootBinding(driver Driver) Binding { +func bootBinding(d driver) Binding { return Binding{ - driver: driver, + driver: d, Provider: "acme", Prefix: "feint-acme-", User: "root", @@ -78,7 +78,7 @@ func TestAnUnknownImageStaysMetadataOnlyWithoutARuntime(t *testing.T) { // Noop boots nothing, so there is nothing to substitute: the control plane // must keep accepting — docs/limits.md promises hardcoded production // identifiers keep working, and CI runs the conformance suites this way. - for name, driver := range map[string]Driver{"noop": Noop{}, "nil": nil} { + for name, driver := range map[string]driver{"noop": Noop{}, "nil": nil} { b := bootBinding(driver) res := &resource.Resource{ID: "srv-1", State: "stopped"} if !b.PowerOn(context.Background(), res, Boot{Requested: "totalement-inconnue"}) { diff --git a/internal/core/machine/capabilities.go b/internal/core/machine/capabilities.go index 7adf96c6..a0426527 100644 --- a/internal/core/machine/capabilities.go +++ b/internal/core/machine/capabilities.go @@ -110,8 +110,9 @@ type Capabilities struct { } // Capable is implemented by a driver that declares what it delivers. It is an -// optional interface, like Firewaller and Pruner: a driver that does not -// implement it is read through CapabilitiesOf below, which assumes nothing. +// optional interface, like the firewalling half and Pruner: a driver that does +// not implement it is read through CapabilitiesOf below, which assumes +// nothing. type Capable interface { Capabilities() Capabilities } @@ -123,7 +124,7 @@ type Capable interface { // so a suite gating on one skips rather than asserting something nobody // promised. A driver that gains a capability declares it; nothing here guesses // from a type name. -func CapabilitiesOf(d Driver) Capabilities { +func CapabilitiesOf(d driver) Capabilities { if c := Declared(d); c != nil { return *c } @@ -144,7 +145,7 @@ func CapabilitiesOf(d Driver) Capabilities { // declared, and the page prints "not declared". // TestAnUndeclaredDriverIsNotTheSameAsOneThatDeclaresNothing in // internal/core/emulator fails without this. -func Declared(d Driver) *Capabilities { +func Declared(d driver) *Capabilities { c, ok := d.(Capable) if !ok { return nil diff --git a/internal/core/machine/firewall.go b/internal/core/machine/firewall.go index d91acea9..9159ff32 100644 --- a/internal/core/machine/firewall.go +++ b/internal/core/machine/firewall.go @@ -108,14 +108,14 @@ func (b FirewallBinding) covers(network string) bool { return network == "" || !slices.Contains(b.Unfiltered, network) } -// Firewaller is the optional half of a Driver: a runtime that can enforce rules -// implements it, one that cannot does not, and the pack degrades to serving the -// rules as metadata. +// firewaller is the optional half of a driver: a runtime that can enforce +// rules implements it, one that cannot does not, and the pack degrades to +// serving the rules as metadata. // -// Kept separate from Driver on purpose. Enforcement is the one capability whose -// absence a user has to be told about, and a separate interface makes that -// absence a compile-time fact rather than a silent no-op. -type Firewaller interface { +// Kept separate from the driver on purpose. Enforcement is the one capability +// whose absence a user has to be told about, and a separate interface makes +// that absence a compile-time fact rather than a silent no-op. +type firewaller interface { // EnsureFirewall creates or replaces the rule set as a whole. Replacing // rather than patching is what makes a rule removed upstream disappear here // instead of lingering. diff --git a/internal/core/machine/firewall_binding.go b/internal/core/machine/firewall_binding.go index c101e782..5045ad25 100644 --- a/internal/core/machine/firewall_binding.go +++ b/internal/core/machine/firewall_binding.go @@ -73,7 +73,7 @@ func (s FirewallSpec) WithPermissiveCatchAll(allow string) FirewallSpec { // now be applied. A failure is logged and never fails the control plane: the // API still serves the group, which is the honest degraded state, and this log // is the only place an operator learns the rules exist nowhere. -func (b Binding) SyncRuleSet(ctx context.Context, fw Firewaller, spec FirewallSpec) bool { +func (b Binding) SyncRuleSet(ctx context.Context, fw firewaller, spec FirewallSpec) bool { if fw == nil { return false } @@ -100,7 +100,7 @@ func (b Binding) SyncRuleSet(ctx context.Context, fw Firewaller, spec FirewallSp // (#574). It travels beside the sets rather than being applied here, because // the decision is per interface and only the driver knows which interface sits // on which network. -func (b Binding) ApplyRuleSets(ctx context.Context, fw Firewaller, machine string, unfiltered []string, specs ...FirewallSpec) { +func (b Binding) ApplyRuleSets(ctx context.Context, fw firewaller, machine string, unfiltered []string, specs ...FirewallSpec) { if fw == nil || machine == "" { return } @@ -166,7 +166,7 @@ func (b Binding) reportFirewall(err error, keyvals ...any) { // rather than fatal: the group is already gone from the control plane, and // refusing the delete afterwards would leave the client with a resource it // cannot remove. -func (b Binding) DropRuleSet(ctx context.Context, fw Firewaller, name string) { +func (b Binding) DropRuleSet(ctx context.Context, fw firewaller, name string) { if fw == nil { return } diff --git a/internal/core/machine/groupsync.go b/internal/core/machine/groupsync.go index 90a79610..902cab96 100644 --- a/internal/core/machine/groupsync.go +++ b/internal/core/machine/groupsync.go @@ -220,9 +220,9 @@ func (s GroupSync) wired() bool { // enforcer is the runtime's firewall half, nil when it has none — the // assertion every pack wrote for itself, now out of their vocabulary entirely, // which is what lets the enforcement test mark a wired pack by this type -// instead of by machine.Firewaller. -func (s GroupSync) enforcer() Firewaller { - fw, _ := s.Binding.driver.(Firewaller) +// instead of by the driver's firewalling half. +func (s GroupSync) enforcer() firewaller { + fw, _ := s.Binding.driver.(firewaller) return fw } @@ -230,8 +230,8 @@ func (s GroupSync) enforcer() Firewaller { // construction, in which case reject rules against foreign subnets are dead // weight: the blocks would name subnets the machine cannot reach anyway. func (s GroupSync) nativeIsolation() bool { - peerer, ok := s.Binding.driver.(Peerer) - return ok && peerer.NativeIsolation() + peer, ok := s.Binding.driver.(peerer) + return ok && peer.NativeIsolation() } // spec assembles the rule set of one group: the pack's translation, with the diff --git a/internal/core/machine/images.go b/internal/core/machine/images.go index c89dc22f..49d4a97c 100644 --- a/internal/core/machine/images.go +++ b/internal/core/machine/images.go @@ -283,7 +283,7 @@ func ParseDeclaredImages(s string) (map[string]Image, error) { return out, nil } -// ImageBuilder is the optional half of a Driver that can build the images +// ImageBuilder is the optional half of a driver that can build the images // RequiredImages names. // // It is the seam `feint images` drives, named here so EnsureImage can drive the @@ -331,16 +331,16 @@ func imageBuildLock(alias string) *sync.Mutex { // alias somebody may be booting from. // // TestOneBuilderPerImageAndPerProcess fails without this. -func BuildIfMissing(ctx context.Context, driver Driver, spec ImageSpec, progress io.Writer) (bool, error) { - builder, canBuild := driver.(ImageBuilder) +func BuildIfMissing(ctx context.Context, d driver, spec ImageSpec, progress io.Writer) (bool, error) { + builder, canBuild := d.(ImageBuilder) if !canBuild { - return false, fmt.Errorf("the %s runtime cannot build images", driver.Name()) + return false, fmt.Errorf("the %s runtime cannot build images", d.Name()) } mu := imageBuildLock(spec.Alias()) mu.Lock() defer mu.Unlock() - if lister, canAsk := driver.(ImageLister); canAsk { + if lister, canAsk := d.(ImageLister); canAsk { held, err := lister.LocalImages(ctx) // A station that cannot be asked is not a station that holds nothing: // the build goes ahead, because the caller asked for this image and @@ -378,7 +378,7 @@ func BuildIfMissing(ctx context.Context, driver Driver, spec ImageSpec, progress // // TestAFailedImageBuildRefusesTheBootAndNamesTheSource and // TestABootDerivesAndBuildsTheImageItNames fail without this. -func EnsureImage(ctx context.Context, driver Driver, ref string, log *slog.Logger) (bool, error) { +func EnsureImage(ctx context.Context, d driver, ref string, log *slog.Logger) (bool, error) { if log == nil { log = slog.Default() } @@ -386,14 +386,14 @@ func EnsureImage(ctx context.Context, driver Driver, ref string, log *slog.Logge if !ours { return false, nil } - lister, canAsk := driver.(ImageLister) - _, canBuild := driver.(ImageBuilder) + lister, canAsk := d.(ImageLister) + _, canBuild := d.(ImageBuilder) if !canAsk || !canBuild { // An undeclared capability counts as absent, so this says "nobody could // look" rather than "the image is there": the boot goes on, and the // driver's own fallback warning is what the operator reads next. log.Debug("this runtime cannot be asked about base images, so none is built", - "image", ref, "runtime", driver.Name()) + "image", ref, "runtime", d.Name()) return false, nil } held, err := lister.LocalImages(ctx) @@ -419,7 +419,7 @@ func EnsureImage(ctx context.Context, driver Driver, ref string, log *slog.Logge buildCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), imageBuildTimeout) defer cancel() started := time.Now() - built, err := BuildIfMissing(buildCtx, driver, spec, nil) + built, err := BuildIfMissing(buildCtx, d, spec, nil) if err != nil { // Said here with the duration; the caller owns the refusal and its // message, because only it knows the resource and the provider. @@ -475,12 +475,12 @@ type ImageLister interface { // never as present: an undeclared property counts as absent, which is the same // rule CapabilitiesOf applies. Reporting "nothing is missing" because nobody // could look is the failure this project exists to remove. -func ImageInventory(ctx context.Context, driver Driver) ([]ImageStatus, error) { +func ImageInventory(ctx context.Context, d driver) ([]ImageStatus, error) { specs := RequiredImages() sort.Slice(specs, func(i, j int) bool { return specs[i].Name < specs[j].Name }) held := map[string]string{} - if lister, ok := driver.(ImageLister); ok { + if lister, ok := d.(ImageLister); ok { var err error held, err = lister.LocalImages(ctx) if err != nil { @@ -514,8 +514,8 @@ func ImageInventory(ctx context.Context, driver Driver) ([]ImageStatus, error) { // control, and the report names the removal gesture beside each derived row. // // TestDerivedImagesAreNamedBesideTheWarmupSet fails without this. -func DerivedImages(ctx context.Context, driver Driver) ([]ImageStatus, error) { - lister, ok := driver.(ImageLister) +func DerivedImages(ctx context.Context, d driver) ([]ImageStatus, error) { + lister, ok := d.(ImageLister) if !ok { return nil, nil } diff --git a/internal/core/machine/incus.go b/internal/core/machine/incus.go index 830e9e1c..dc3913c6 100644 --- a/internal/core/machine/incus.go +++ b/internal/core/machine/incus.go @@ -222,7 +222,7 @@ func (d *Incus) logger() *slog.Logger { return slog.Default() } -// Name implements Driver. +// Name implements driver. func (d *Incus) Name() string { switch { case d.OVN && d.VM: @@ -236,7 +236,7 @@ func (d *Incus) Name() string { } } -// Available implements Driver: it queries the daemon, since an installed client +// Available implements driver: it queries the daemon, since an installed client // with no reachable server is the common broken case. func (d *Incus) Available(ctx context.Context) bool { _, err := d.run(ctx, "list", "--format", "json") @@ -403,7 +403,7 @@ func (d *Incus) publicRouteKey() string { return "ipv4.routes" } -// Start implements Driver. +// Start implements driver. // // The instance is initialised cold, its devices configured, then started — // rather than launched in one step — because every device key must be in place @@ -681,7 +681,7 @@ func (d *Incus) attachExtra(ctx context.Context, spec Spec) error { return nil } -// Attach implements Driver. +// Attach implements driver. // // Two steps, and the second is the one that is easy to miss. Adding the device // reserves the address on the managed bridge, which is all `ipv4.address` does: @@ -806,9 +806,9 @@ func (d *Incus) Attach(ctx context.Context, name string, att Attachment) error { return d.reconcileSecondary(ctx, name, device, att) } -// Detach implements Driver. +// Detach implements driver. // -// The measurement it exists for (#426). Before it, the Driver had Attach and no +// The measurement it exists for (#426). Before it, the driver had Attach and no // counterpart, so a pack deleting a private NIC could only forget it in the // store: the `eth1` stayed on the container, DeletePrivateNetwork then ran // RemoveNetwork, Incus answered "The network is currently in use", the pack @@ -1244,7 +1244,7 @@ func (d *Incus) lockNetworks(names []string) func() { } } -// EnsureNetwork implements Driver. It creates a managed bridge carrying the +// EnsureNetwork implements driver. It creates a managed bridge carrying the // block the pack computed — or an OVN network in OVN mode — and succeeds when // the network is already there. // @@ -1421,7 +1421,7 @@ func leftoverHolds(leftover DHCPLeftover, block netip.Prefix) bool { return false } -// RemoveNetwork implements Driver. Incus refuses to delete a network still in +// RemoveNetwork implements driver. Incus refuses to delete a network still in // use, and that refusal is propagated rather than forced: a subnet whose // machines are still running must not be deletable, which is what the client // expects and what a DependencyViolation is for. @@ -1597,7 +1597,7 @@ func gatewayAddress(cidr, gateway string) (string, error) { return fmt.Sprintf("%s/%d", addr, prefix.Bits()), nil } -// Stop implements Driver. +// Stop implements driver. func (d *Incus) Stop(ctx context.Context, name string) error { if !safeName.MatchString(name) { return fmt.Errorf("refusing to stop %q: not a name this emulator creates", name) @@ -1684,7 +1684,7 @@ func (d *Incus) runUntilFree(ctx context.Context, args ...string) ([]byte, error } } -// Remove implements Driver. +// Remove implements driver. // // Stopped first, then deleted, so a machine still initialising is torn down in // order rather than deleted under its own init. This does not prevent the @@ -1759,7 +1759,7 @@ func (d *Incus) WaitRunning(ctx context.Context, name string) (Machine, error) { } } -// Inspect implements Driver. An instance that is running but has not obtained an +// Inspect implements driver. An instance that is running but has not obtained an // address yet reports an empty IP rather than an error: a booting VM is a normal // state, not a failure. func (d *Incus) Inspect(ctx context.Context, name string) (Machine, bool, error) { diff --git a/internal/core/machine/isolate.go b/internal/core/machine/isolate.go index 410cbfd4..cab94ff7 100644 --- a/internal/core/machine/isolate.go +++ b/internal/core/machine/isolate.go @@ -31,9 +31,9 @@ var ErrNetworkGone = errors.New("the network was removed while its isolation was // The Incus halves of both are in incus_isolate.go, with the measurements that // made the OVN mode necessary. -// Isolator is the optional half of a Driver whose networks are born joined, and -// that can keep them apart with rules. -type Isolator interface { +// isolator is the optional half of a driver whose networks are born joined, +// and that can keep them apart with rules. +type isolator interface { // IsolateNetwork rejects traffic from the network towards each foreign // block, and leaves everything else alone. Called again with a different // list, it replaces the previous one: blocks appear and disappear as @@ -41,12 +41,12 @@ type Isolator interface { IsolateNetwork(ctx context.Context, network string, foreign []string) error } -// Peerer is the optional half of a Driver whose networks are born separate -// and joined on request — the exact inverse of Isolator. A pack asks -// NativeIsolation to know which of the two it is talking to: with a Peerer that -// answers true, reject rules against foreign blocks are dead weight, and +// peerer is the optional half of a driver whose networks are born separate +// and joined on request — the exact inverse of the isolator. The layer asks +// NativeIsolation to know which of the two it is talking to: with a peerer +// that answers true, reject rules against foreign blocks are dead weight, and // reachability is granted by peering instead. -type Peerer interface { +type peerer interface { // NativeIsolation reports whether two networks of this driver are // unreachable from each other unless peered. NativeIsolation() bool @@ -89,13 +89,13 @@ type IsolationMember struct { // says what happened — native peering, rule-set isolation, or nothing, when // the driver has neither capability — because at least one pack does extra // work (a security-group resync) only in the rule-set case. -func ReconcileIsolation(ctx context.Context, driver Driver, log *slog.Logger, noun string, +func ReconcileIsolation(ctx context.Context, d driver, log *slog.Logger, noun string, members []IsolationMember, reachable func(from, to int) bool) (native, applied bool) { if log == nil { log = slog.Default() } - if peerer, ok := driver.(Peerer); ok && peerer.NativeIsolation() { + if peer, ok := d.(peerer); ok && peer.NativeIsolation() { for i, m := range members { if m.Network == "" { continue @@ -109,14 +109,14 @@ func ReconcileIsolation(ctx context.Context, driver Driver, log *slog.Logger, no peers = append(peers, other.Network) } } - if err := peerer.PeerNetworks(ctx, m.Network, peers); err != nil { + if err := peer.PeerNetworks(ctx, m.Network, peers); err != nil { report(log, noun, m, err, "peer") } } return true, true } - isolator, ok := driver.(Isolator) + iso, ok := d.(isolator) if !ok { return false, false } @@ -133,7 +133,7 @@ func ReconcileIsolation(ctx context.Context, driver Driver, log *slog.Logger, no foreign = append(foreign, other.Block) } } - if err := isolator.IsolateNetwork(ctx, m.Network, foreign); err != nil { + if err := iso.IsolateNetwork(ctx, m.Network, foreign); err != nil { report(log, noun, m, err, "isolate") } } @@ -141,7 +141,7 @@ func ReconcileIsolation(ctx context.Context, driver Driver, log *slog.Logger, no } // ReconcileIsolation is the binding's door onto the pass above, and the only -// one a provider pack has: the package-level form takes a Driver, which is +// one a provider pack has: the package-level form takes a driver, which is // precisely the value #511 took out of every pack's reach, and it stays // exported for the core's own tests and for nothing else. // diff --git a/internal/core/machine/machine.go b/internal/core/machine/machine.go index ce27696f..16374131 100644 --- a/internal/core/machine/machine.go +++ b/internal/core/machine/machine.go @@ -134,9 +134,14 @@ type Machine struct { Running bool } -// Driver runs machines and the networks they sit on. Implementations must be +// driver runs machines and the networks they sit on. Implementations must be // safe for concurrent use. -type Driver interface { +// +// Unexported since #514, and that is the point: a pack could not obtain one +// after #511 and could still name one, so `var _ machine.Driver` compiled in +// internal/providers/scaleway on 154c204. What leaves this package is Runtime +// (runtime.go), which carries a driver and offers none of its verbs. +type driver interface { // Name identifies the driver in logs and in the health endpoint. Name() string // Available reports whether the driver can actually run anything. The @@ -171,7 +176,7 @@ type Driver interface { RemoveNetwork(ctx context.Context, name string) error } -// Waiter is the optional half of a Driver that can tell when a machine is +// Waiter is the optional half of a driver that can tell when a machine is // ready. Start never waits, because an API call must not block for the tens of // seconds a virtual machine takes to boot; a caller that needs to reach the // machine asks here instead. @@ -185,36 +190,36 @@ type Waiter interface { // on a machine runtime being present. type Noop struct{} -// Name implements Driver. +// Name implements driver. func (Noop) Name() string { return "none" } -// Available implements Driver. +// Available implements driver. func (Noop) Available(context.Context) bool { return true } -// Start implements Driver. +// Start implements driver. func (Noop) Start(_ context.Context, spec Spec) (Machine, error) { return Machine{ID: "", Name: spec.Name, Running: true}, nil } -// Stop implements Driver. +// Stop implements driver. func (Noop) Stop(context.Context, string) error { return nil } -// Remove implements Driver. +// Remove implements driver. func (Noop) Remove(context.Context, string) error { return nil } -// Inspect implements Driver. +// Inspect implements driver. func (Noop) Inspect(_ context.Context, name string) (Machine, bool, error) { return Machine{Name: name}, false, nil } -// EnsureNetwork implements Driver. +// EnsureNetwork implements driver. func (Noop) EnsureNetwork(context.Context, NetworkSpec) error { return nil } -// Attach implements Driver. +// Attach implements driver. func (Noop) Attach(context.Context, string, Attachment) error { return nil } -// Detach implements Driver. +// Detach implements driver. func (Noop) Detach(context.Context, string, string) error { return nil } -// RemoveNetwork implements Driver. +// RemoveNetwork implements driver. func (Noop) RemoveNetwork(context.Context, string) error { return nil } diff --git a/internal/core/machine/ownership_test.go b/internal/core/machine/ownership_test.go index 4c4ba726..fbc2f289 100644 --- a/internal/core/machine/ownership_test.go +++ b/internal/core/machine/ownership_test.go @@ -21,7 +21,7 @@ import ( // // Every case here fails if its guard is removed. That is the point of the file. -func quietBinding(driver Driver) Binding { +func quietBinding(driver driver) Binding { return Binding{ driver: driver, Provider: "scaleway", diff --git a/internal/core/machine/placement.go b/internal/core/machine/placement.go index e7ef1755..1bedd5fd 100644 --- a/internal/core/machine/placement.go +++ b/internal/core/machine/placement.go @@ -49,20 +49,20 @@ func placementKey(provider, address string) string { return provider + "|" + add // A failure to unroute the previous holder is returned rather than swallowed. The // caller wanted the address moved, and a move whose first half failed has left // the address where it was. -func (b Binding) RouteAddress(ctx context.Context, router Router, spec AddressSpec) error { - if router == nil || spec.Address == "" || spec.Machine == "" { +func (b Binding) RouteAddress(ctx context.Context, rt router, spec AddressSpec) error { + if rt == nil || spec.Address == "" || spec.Machine == "" { return nil } key := placementKey(b.Provider, spec.Address) if held, ok := placements.Load(key); ok { if previous, _ := held.(string); previous != "" && previous != spec.Machine { - if err := router.UnrouteAddress(ctx, previous, spec.Address); err != nil { + if err := rt.UnrouteAddress(ctx, previous, spec.Address); err != nil { return err } } } - if err := router.RouteAddress(ctx, spec); err != nil { + if err := rt.RouteAddress(ctx, spec); err != nil { return err } placements.Store(key, spec.Machine) @@ -76,11 +76,11 @@ func (b Binding) RouteAddress(ctx context.Context, router Router, spec AddressSp // — and forgetting the placement then would leave the current holder unknown, so // the next move would not take it back from anybody. Only a call naming the // recorded holder clears the record. -func (b Binding) UnrouteAddress(ctx context.Context, router Router, machine, address string) error { - if router == nil || address == "" { +func (b Binding) UnrouteAddress(ctx context.Context, rt router, machine, address string) error { + if rt == nil || address == "" { return nil } - err := router.UnrouteAddress(ctx, machine, address) + err := rt.UnrouteAddress(ctx, machine, address) key := placementKey(b.Provider, address) if held, ok := placements.Load(key); ok { diff --git a/internal/core/machine/plan.go b/internal/core/machine/plan.go index 8dcca40b..3c161bc7 100644 --- a/internal/core/machine/plan.go +++ b/internal/core/machine/plan.go @@ -110,9 +110,9 @@ func (r Reconciler) plan(res *resource.Resource) (Plan, bool) { // router is the runtime's routing half, nil when it has none — the assertion // every pack wrote for itself, now inside the layer. -func (r Reconciler) router() Router { - router, _ := r.binding().driver.(Router) - return router +func (r Reconciler) router() router { + rt, _ := r.binding().driver.(router) + return rt } // PowerOn starts the machine on its declared plan and replays the post-boot diff --git a/internal/core/machine/prune.go b/internal/core/machine/prune.go index 90c325ac..a81e1e4e 100644 --- a/internal/core/machine/prune.go +++ b/internal/core/machine/prune.go @@ -27,7 +27,7 @@ type Pruned struct { // Total reports whether anything was found at all. func (p Pruned) Total() int { return p.Machines + p.Networks + p.Firewalls } -// Pruner is the optional half of a Driver that can find and remove everything +// Pruner is the optional half of a driver that can find and remove everything // the emulator created through it. type Pruner interface { // Prune removes the machines, networks and rule sets carrying the label, @@ -47,7 +47,7 @@ type Leftovers struct { // Total reports whether anything was found at all. func (l Leftovers) Total() int { return len(l.Machines) + len(l.Networks) + len(l.Firewalls) } -// Surveyor is the optional half of a Driver that can name the emulator's +// Surveyor is the optional half of a driver that can name the emulator's // labelled work without acting on it. A restart uses it to notice what a // previous life left behind; adoption is deliberately not on offer, because // the store that gave those objects meaning died with the process that @@ -59,7 +59,7 @@ type Surveyor interface { Survey(ctx context.Context) (Leftovers, error) } -// UplinkReleaser is the optional half of a Driver whose networks share one +// UplinkReleaser is the optional half of a driver whose networks share one // piece of host plumbing no resource delete will ever remove: the uplink. // Every emulated resource goes when a client deletes it, so a run whose // clients cleaned up after themselves still leaves exactly one labelled @@ -124,7 +124,7 @@ type Trap struct { Row string } -// Repairer is the optional half of a Driver that can name the states its own +// Repairer is the optional half of a driver that can name the states its own // sweep cannot get out of, and clear the ones no ordinary command reaches. // // The split matters. Traps is a read and is always safe to run; Repair reaches diff --git a/internal/core/machine/recorder.go b/internal/core/machine/recorder.go index 3be87194..1a79a6b1 100644 --- a/internal/core/machine/recorder.go +++ b/internal/core/machine/recorder.go @@ -31,7 +31,7 @@ import ( // the natural name and is already taken by the watcher's type in watch.go; // gesture is the word #514 uses for exactly this.) type Gesture struct { - // Kind names the gesture: the Driver (or optional-half) method that was + // Kind names the gesture: the driver (or optional-half) method that was // called. It must be a key of the contract vocabulary. Kind string // Resource is what the gesture acted on — the machine, network, rule set @@ -43,7 +43,7 @@ type Gesture struct { } // contractGestures is the closed vocabulary of the driver contract: every -// method of Driver and its optional halves (Router, Firewaller, Peerer, +// method of the driver and its optional halves (routing, rule sets, peering, // Isolator, Balancer, Capable), and nothing else. The value says whether the // gesture changes the host: true is a gesture the Recorder records, false is a // read, deliberately left out of the recording — a read changes nothing on the @@ -90,7 +90,7 @@ func KnownGesture(kind string) bool { return known } -// Recorder is the shared fake runtime: it implements Driver and every optional +// Recorder is the shared fake runtime: it implements the driver and every optional // half, runs machines instantly, and records each host-changing gesture as a // typed Gesture, in call order. // @@ -165,13 +165,13 @@ func (r *Recorder) OutsideContract() []Gesture { return out } -// Name implements Driver. +// Name implements driver. func (r *Recorder) Name() string { return "recorder" } -// Available implements Driver. +// Available implements driver. func (r *Recorder) Available(context.Context) bool { return true } -// Start implements Driver: the machine runs at once, on the address its first +// Start implements driver: the machine runs at once, on the address its first // attachment fixes, or on a stable placeholder when the pack fixed none. func (r *Recorder) Start(_ context.Context, spec Spec) (Machine, error) { ip := "10.230.0.10" @@ -185,7 +185,7 @@ func (r *Recorder) Start(_ context.Context, spec Spec) (Machine, error) { return Machine{Name: spec.Name, IP: ip, Running: true}, nil } -// Stop implements Driver. +// Stop implements driver. func (r *Recorder) Stop(_ context.Context, name string) error { r.mu.Lock() if m, found := r.machines[name]; found { @@ -197,7 +197,7 @@ func (r *Recorder) Stop(_ context.Context, name string) error { return nil } -// Remove implements Driver. It succeeds when nothing is there, as the contract +// Remove implements driver. It succeeds when nothing is there, as the contract // requires. func (r *Recorder) Remove(_ context.Context, name string) error { r.mu.Lock() @@ -207,7 +207,7 @@ func (r *Recorder) Remove(_ context.Context, name string) error { return nil } -// Inspect implements Driver. A read, so it is not recorded. +// Inspect implements driver. A read, so it is not recorded. func (r *Recorder) Inspect(_ context.Context, name string) (Machine, bool, error) { r.mu.Lock() defer r.mu.Unlock() @@ -218,25 +218,25 @@ func (r *Recorder) Inspect(_ context.Context, name string) (Machine, bool, error return Machine{Name: name, IP: m.ip, Running: m.running}, true, nil } -// EnsureNetwork implements Driver. +// EnsureNetwork implements driver. func (r *Recorder) EnsureNetwork(_ context.Context, spec NetworkSpec) error { r.Record(Gesture{Kind: "EnsureNetwork", Resource: spec.Name, Args: spec}) return nil } -// Attach implements Driver. +// Attach implements driver. func (r *Recorder) Attach(_ context.Context, name string, att Attachment) error { r.Record(Gesture{Kind: "Attach", Resource: name, Args: att}) return nil } -// Detach implements Driver. +// Detach implements driver. func (r *Recorder) Detach(_ context.Context, name, network string) error { r.Record(Gesture{Kind: "Detach", Resource: name, Args: network}) return nil } -// RemoveNetwork implements Driver. +// RemoveNetwork implements driver. func (r *Recorder) RemoveNetwork(_ context.Context, name string) error { r.Record(Gesture{Kind: "RemoveNetwork", Resource: name}) return nil diff --git a/internal/core/machine/recorder_test.go b/internal/core/machine/recorder_test.go index de189762..4ce85e4a 100644 --- a/internal/core/machine/recorder_test.go +++ b/internal/core/machine/recorder_test.go @@ -16,12 +16,12 @@ import ( // contractInterfaces is the surface the vocabulary must cover: Driver and its // optional halves, exactly the set NewRecorder implements. var contractInterfaces = map[string]reflect.Type{ - "Driver": reflect.TypeOf((*Driver)(nil)).Elem(), - "Router": reflect.TypeOf((*Router)(nil)).Elem(), - "Firewaller": reflect.TypeOf((*Firewaller)(nil)).Elem(), - "Peerer": reflect.TypeOf((*Peerer)(nil)).Elem(), - "Isolator": reflect.TypeOf((*Isolator)(nil)).Elem(), - "Balancer": reflect.TypeOf((*Balancer)(nil)).Elem(), + "Driver": reflect.TypeOf((*driver)(nil)).Elem(), + "Router": reflect.TypeOf((*router)(nil)).Elem(), + "Firewaller": reflect.TypeOf((*firewaller)(nil)).Elem(), + "Peerer": reflect.TypeOf((*peerer)(nil)).Elem(), + "Isolator": reflect.TypeOf((*isolator)(nil)).Elem(), + "Balancer": reflect.TypeOf((*balancer)(nil)).Elem(), "Capable": reflect.TypeOf((*Capable)(nil)).Elem(), } diff --git a/internal/core/machine/runtime.go b/internal/core/machine/runtime.go new file mode 100644 index 00000000..ae15f8ff --- /dev/null +++ b/internal/core/machine/runtime.go @@ -0,0 +1,275 @@ +package machine + +import ( + "context" + "io" +) + +// Runtime is the only handle on a machine runtime that leaves this package. +// +// # Why it exists +// +// #511 unexported emulator.Env's driver field and machine.Binding's, so a pack +// could no longer *obtain* a driver value. It could still *name* the type: on +// 154c204 this file, dropped into internal/providers/scaleway, compiled and +// `go build ./internal/providers/scaleway/` exited 0 — +// +// package scaleway +// import "github.com/stephrobert/feint/internal/core/machine" +// var _ machine.Driver +// +// — which left the boundary held by a convention plus an AST scan, exactly +// what #514 §2.1 says is not enough. So Driver and the five halves a pack +// would reach for (Router, Firewaller, Peerer, Isolator, Balancer) are +// unexported now, and a pack that writes any of those names fails the build. +// internal/cli's TestThePacksCannotNameTheDriver compiles that very file and +// requires the failure, with a companion that must still compile so a probe +// broken for some other reason cannot read as the door being shut. +// +// # Why the operator side needs a handle at all +// +// internal/cli builds the runtime the operator asked for (`--vm`), asks it +// what the host delivers, sweeps it on the way out, and substitutes doubles +// for it in some forty tests. None of that is a pack reaching past the shared +// layer, and none of it can name a driver type any more. This is what it names +// instead. +// +// # Why a struct and not an interface +// +// An interface narrowed to Name and Available would be defeated in one line, +// because a type assertion needs no name: +// +// rt.(interface{ Remove(context.Context, string) error }).Remove(ctx, victim) +// +// That is not hypothetical — internal/cli already reaches two capabilities +// that way (a Verify half in machineDriver, a RemoveImage half in `feint +// images remove`), so the shape is in the repository and would be copied. A +// struct with an unexported field cannot be asserted on at all, and its method +// set is therefore the whole of what a holder can do. +// +// # What it deliberately does not offer +// +// Nothing that moves a machine, a network, an address or a rule set: no Start, +// Stop, Remove, Inspect, Attach, Detach, EnsureNetwork, RemoveNetwork, +// RouteAddress, EnsureFirewall, PeerNetworks, EnsureBalancer. Those belong to +// Binding, Reconciler and GroupSync, which apply the ownership checks and the +// one order, and machine.PackSurface is the list of what a pack may ask them +// for. What is here is the operator's half — identity, capability, sweep, +// repair, images — the half `feint doctor`, `feint clean` and `feint images` +// exist to serve. +// +// The zero value is the metadata-only runtime: no machine ever starts, which +// is what `--vm off` and CI get. +type Runtime struct { + d driver +} + +// Use binds a driver into the handle. It is the one door in, and deliberately +// one-way: nothing hands the driver back out. +// +// The parameter type is unexported on purpose. A caller outside this package +// can still pass any value whose method set satisfies it — which is how +// internal/cli passes the Incus driver, and how every fake runtime in the +// packs' tests is still injected — but it cannot declare a variable of that +// type, name it in a signature, or assert its way back to one. +func Use(d driver) Runtime { return Runtime{d: d} } + +// backing returns the driver, or the metadata-only one for a zero Runtime, so +// every method below can be written without a nil branch. +func (r Runtime) backing() driver { + if r.d == nil { + return Noop{} + } + return r.d +} + +// Name identifies the runtime in logs, in `feint status` and on +// /_feint/health. +func (r Runtime) Name() string { return r.backing().Name() } + +// Available reports whether the runtime can actually run anything. +func (r Runtime) Available(ctx context.Context) bool { return r.backing().Available(ctx) } + +// Runs reports whether anything is actually backed by a host. False is the +// metadata-only runtime: servers change state and nothing starts. +// +// It is one question asked in five places — the serve write deadline, `feint +// doctor`, `feint up`, `feint clean` and `feint images` each used to write +// `driver.(machine.Noop)` for themselves — and a question written five times +// is a question one caller answers differently. +func (r Runtime) Runs() bool { + if r.d == nil { + return false + } + _, none := r.d.(Noop) + return !none +} + +// Capabilities reports what this runtime delivers. See CapabilitiesOf: an +// undeclared capability counts as absent, so a check skips rather than +// asserting what nobody promised. +func (r Runtime) Capabilities() Capabilities { return CapabilitiesOf(r.backing()) } + +// DeclaredCapabilities is the nullable half CapabilitiesOf cannot express: +// nil when the driver declares nothing, so /_feint/health can tell "this +// runtime says it cannot" from "this runtime was never asked". See Declared. +func (r Runtime) DeclaredCapabilities() *Capabilities { return Declared(r.backing()) } + +// Verify asks the host what it actually delivers and names what it narrowed. +// +// A driver with no verification half answers its declared capabilities and an +// empty list, which is the honest reading of "nobody could check": it claims +// no narrowing it did not measure. +func (r Runtime) Verify(ctx context.Context) (Capabilities, []string) { + v, ok := r.backing().(interface { + Verify(context.Context) (Capabilities, []string) + }) + if !ok { + return r.Capabilities(), nil + } + return v.Verify(ctx) +} + +// Survey names the labelled machines, networks and rule sets this runtime +// still holds, touching none of them. +// +// Three outcomes, never two: asked reports whether the runtime could be asked +// at all, because a runtime nobody looked at is not an empty host. That +// distinction is measurement-integrity's second rule and it is why this +// returns a bool the callers must read. +func (r Runtime) Survey(ctx context.Context) (left Leftovers, asked bool, err error) { + s, ok := r.backing().(Surveyor) + if !ok { + return Leftovers{}, false, nil + } + left, err = s.Survey(ctx) + return left, true, err +} + +// Sweeps reports whether this runtime can be swept at all, which `feint clean` +// asks before it decides what an empty result means: a runtime nobody can +// sweep is not a clean one. +func (r Runtime) Sweeps() bool { + _, ok := r.backing().(Pruner) + return ok +} + +// Prune removes everything this runtime carries the emulator's label on. +// asked is false for a runtime that cannot sweep. +func (r Runtime) Prune(ctx context.Context) (pruned Pruned, asked bool, err error) { + p, ok := r.backing().(Pruner) + if !ok { + return Pruned{}, false, nil + } + pruned, err = p.Prune(ctx) + return pruned, true, err +} + +// ReleaseUplink gives back the host plumbing no resource delete removes, when +// and only when this process is the one holding it. asked is false for a +// runtime with no uplink to give back. +func (r Runtime) ReleaseUplink(ctx context.Context) (released, asked bool, err error) { + u, ok := r.backing().(UplinkReleaser) + if !ok { + return false, false, nil + } + released, err = u.ReleaseUplink(ctx) + return released, true, err +} + +// Traps names the states this runtime's own sweep cannot get out of. It issues +// no mutating command. asked is false for a runtime that cannot be asked. +func (r Runtime) Traps(ctx context.Context) (traps []Trap, asked bool, err error) { + rep, ok := r.backing().(Repairer) + if !ok { + return nil, false, nil + } + traps, err = rep.Traps(ctx) + return traps, true, err +} + +// Repair clears the traps that can be cleared and returns those it cleared. It +// reaches past the runtime's own commands, so it runs only when an operator +// asks for it by name. +func (r Runtime) Repair(ctx context.Context) (cleared []Trap, asked bool, err error) { + rep, ok := r.backing().(Repairer) + if !ok { + return nil, false, nil + } + cleared, err = rep.Repair(ctx) + return cleared, true, err +} + +// Watch streams what the runtime is doing until the context is cancelled. +// asked is false for a runtime that cannot report. +func (r Runtime) Watch(ctx context.Context) (events <-chan Event, asked bool, err error) { + w, ok := r.backing().(Watcher) + if !ok { + return nil, false, nil + } + events, err = w.Watch(ctx) + return events, true, err +} + +// BuildsImages reports whether this runtime can build the images +// RequiredImages names. +func (r Runtime) BuildsImages() bool { + _, ok := r.backing().(ImageBuilder) + return ok +} + +// LocalImages answers which of the emulator's images the station holds, keyed +// by alias. asked is false for a runtime that cannot be asked, which is not +// the same as a station holding none. +func (r Runtime) LocalImages(ctx context.Context) (held map[string]string, asked bool, err error) { + l, ok := r.backing().(ImageLister) + if !ok { + return nil, false, nil + } + held, err = l.LocalImages(ctx) + return held, true, err +} + +// RemovesImages reports whether this runtime holds images it can be asked to +// remove, so `feint images remove` refuses before it names anything. +func (r Runtime) RemovesImages() bool { + _, ok := r.backing().(imageRemover) + return ok +} + +// RemoveImage deletes one image this emulator published. asked is false for a +// runtime that holds no images to remove. +func (r Runtime) RemoveImage(ctx context.Context, alias string) (asked bool, err error) { + rm, ok := r.backing().(imageRemover) + if !ok { + return false, nil + } + return true, rm.RemoveImage(ctx, alias) +} + +// imageRemover is the optional half `feint images remove` drives. It was an +// anonymous interface at the call site until #514, which is the shape a +// Runtime holder can no longer write: an assertion needs no name, so a handle +// that returned a driver-shaped value would hand back everything this one +// exists to withhold. +type imageRemover interface { + RemoveImage(ctx context.Context, alias string) error +} + +// BuildImage builds one image unless the station already holds it, and reports +// whether it built anything. It is BuildIfMissing's door for a Runtime holder; +// the exclusion and the second look live there. +func (r Runtime) BuildImage(ctx context.Context, spec ImageSpec, progress io.Writer) (bool, error) { + return BuildIfMissing(ctx, r.backing(), spec, progress) +} + +// Inventory is what RequiredImages asks of this station: one status per image +// the emulator needs, present or not. +func (r Runtime) Inventory(ctx context.Context) ([]ImageStatus, error) { + return ImageInventory(ctx, r.backing()) +} + +// DerivedInventory is the same reading for the images a boot derives. +func (r Runtime) DerivedInventory(ctx context.Context) ([]ImageStatus, error) { + return DerivedImages(ctx, r.backing()) +} diff --git a/internal/core/machine/surface.go b/internal/core/machine/surface.go index 19e0a0e8..843b3946 100644 --- a/internal/core/machine/surface.go +++ b/internal/core/machine/surface.go @@ -28,14 +28,14 @@ package machine // TestTheDeclaredDriverSurfaceIsSmallerThanThePackage asserts the exclusions // by name rather than leaving them to absence: // -// - Driver itself and every implementation of it. A pack holding one calls +// - The driver and every implementation of it. A pack holding one calls // Start, Remove or RemoveNetwork past Binding.ours and past the driver's // mustOwn — the path a crafted snapshot walked through, and the reason // Resource.Runtime is untrusted input. -// - Its optional halves — Router, Firewaller, Peerer, Isolator, Balancer. -// Reaching one by type assertion bypasses the shared layer as surely as -// calling a method does; that correction is what took the measurement of -// this defect from eleven sites to twenty-nine. +// - Its five pack-facing halves — the routing, rule-set, peering, isolation +// and balancing ones. Reaching one by type assertion bypasses the shared +// layer as surely as calling a method does; that correction is what took +// the measurement of this defect from eleven sites to twenty-nine. // - The driver's own argument vocabulary — Spec, NetworkSpec, AddressSpec, // FirewallBinding — which the layer builds from what a pack declares. // - Thirteen of Binding's own verbs, including Start, Stop, RouteAddress and @@ -47,17 +47,27 @@ package machine // - GroupSync.AfterBoot, for the same reason one level up: the Reconciler // runs it, last. // +// The first two bullets are not on this list at all any more, and that is +// #514: the driver and its five pack-facing halves are unexported since, so a +// pack cannot write their names and no list has to keep them out. What +// internal/cli's mustStayOutside still asserts is what a pack could otherwise +// write — Noop, Incus, Recorder, the operator-side halves, the Runtime handle +// — and its sibling mustNotBeNameable asserts that the six have not come back. +// // Measured on 2026-08-26, the day the door closed: this package exports 97 // package-level names and 297 members of them; the list below admitted 17 and // 41 that day. Re-counted on 2026-08-27 while #541 moved the address reader // into the layer: 17 and 43, Binding down to 13 of its 36 exported members. // Re-counted again on 2026-08-27 when #574 gave a pack a way to say where its -// security groups apply: 17 and 45. +// security groups apply: 17 and 45. Re-counted on 2026-08-28, when #514 took +// the driver and its five pack-facing halves out of the exported set and put +// the Runtime handle in: 93 package-level names and 304 members, the list +// below still 17 and 45, Binding still 13 of its 36. // The middle figure had already drifted by one before that change, which is // what a number written in prose does — it is re-counted here rather than // left standing, and the count is a fact about the list, never a gate. What -// the three packs actually reach is 216 sites, every one of them named here -// and none of them a driver. +// the three packs actually reach is 221 sites, 293 with the fourth, every one +// of them named here and none of them a driver. // // # The rule when something is missing // @@ -76,10 +86,24 @@ package machine // caught too. Its exemption ledger is empty today, and an exemption without a // written reason is refused by TestEveryBarrageExemptionSaysWhy. // -// The compiler holds the rest and holds it harder: emulator.Env carries no -// Driver a pack can read, and Binding's driver field is unexported behind -// WithDriver, so `p.binding().Driver.EnsureNetwork(…)` — a sentence that -// compiled before this change — no longer does. +// The compiler holds the rest and holds it harder, in two steps that are +// worth telling apart because the first was taken for the second for a while. +// +// #511 closed the way to *obtain* a driver: emulator.Env carries no field a +// pack can read, and Binding's driver field is unexported behind WithRuntime, +// so `p.binding().Driver.EnsureNetwork(…)` — a sentence that compiled before +// that change — no longer does. +// +// #514 closed the way to *name* one. Until it, `var _ machine.Driver` in a +// pack compiled and `go build ./internal/providers/scaleway/` exited 0, +// measured on 154c204, so the surface was still held by a convention plus an +// AST scan. The driver interface and the five halves above are unexported +// since; what leaves this package is machine.Runtime, a struct whose driver +// field is private — a struct rather than a narrowed interface, because an +// assertion needs no name and +// `rt.(interface{ Remove(context.Context, string) error })` would have undone +// the whole thing in one line. internal/cli's TestThePacksCannotNameTheDriver +// compiles the forbidden sentence and requires the failure. // PackSurface is the closed list of what a provider pack may name in this // package: package-level names as they are written (machine.Attachment), and diff --git a/internal/core/machine/watch.go b/internal/core/machine/watch.go index 74927cf5..43f24177 100644 --- a/internal/core/machine/watch.go +++ b/internal/core/machine/watch.go @@ -29,7 +29,7 @@ type Event struct { Message string } -// Watcher is the optional half of a Driver that can report what its runtime is +// Watcher is the optional half of a driver that can report what its runtime is // doing. A driver without one leaves the operator reading the runtime's log by // hand, which is what this exists to avoid. type Watcher interface { diff --git a/internal/providers/exoscale/detach_internal_test.go b/internal/providers/exoscale/detach_internal_test.go index 857b2a42..4848fa4c 100644 --- a/internal/providers/exoscale/detach_internal_test.go +++ b/internal/providers/exoscale/detach_internal_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/stephrobert/feint/internal/core/machine" "github.com/stephrobert/feint/internal/core/resource" ) @@ -26,7 +27,7 @@ import ( // operation was already correct while nothing at all was sent. func TestDetachingAnInstanceTakesTheDeviceOffTheRuntime(t *testing.T) { driver := &recordingDriver{} - p := runtimePack(driver) + p := runtimePack(machine.Use(driver)) const instanceID = "11111111-1111-4111-8111-111111111111" const networkID = "22222222-2222-4222-8222-222222222222" diff --git a/internal/providers/exoscale/elasticip_routing_test.go b/internal/providers/exoscale/elasticip_routing_test.go index ce93154d..96c686af 100644 --- a/internal/providers/exoscale/elasticip_routing_test.go +++ b/internal/providers/exoscale/elasticip_routing_test.go @@ -68,7 +68,7 @@ func routedServe(t *testing.T) (http.Handler, *routedRuntime, *emulator.Env) { t.Helper() env := emulator.DefaultEnv() rt := &routedRuntime{} - env.UseMachines(rt) + env.UseMachines(machine.Use(rt)) srv, err := emulator.NewServer(env, exoscale.New(env)) if err != nil { t.Fatalf("build the server: %v", err) diff --git a/internal/providers/exoscale/entry_test.go b/internal/providers/exoscale/entry_test.go index 599edfd6..5190a132 100644 --- a/internal/providers/exoscale/entry_test.go +++ b/internal/providers/exoscale/entry_test.go @@ -152,7 +152,7 @@ func TestExoscaleNeverPublishesKeyMaterial(t *testing.T) { // dead code on the only pack that motivated it. func TestAnExoscaleKeyReachesTheMachine(t *testing.T) { driver := &recordingRuntime{} - h := serveWith(t, driver) + h := serveWith(t, machine.Use(driver)) call(t, h, "POST", "/v2/ssh-key", `{"name":"mine","public-key":"`+realKey+`"}`) call(t, h, "POST", "/v2/instance", `{ @@ -224,7 +224,7 @@ func (r *recordingRuntime) user() string { return r.specs[0].User } -func serveWith(t *testing.T, drv machine.Driver) http.Handler { +func serveWith(t *testing.T, drv machine.Runtime) http.Handler { t.Helper() env := emulator.DefaultEnv() env.UseMachines(drv) @@ -371,5 +371,5 @@ func TestExoscaleAnswersTheCloudsStatusForAKeyItCannotRead(t *testing.T) { } } -// Detach implements machine.Driver; *recordingRuntime needs no behaviour here. +// Detach completes the machine package's driver contract; *recordingRuntime needs no behaviour here. func (r *recordingRuntime) Detach(context.Context, string, string) error { return nil } diff --git a/internal/providers/exoscale/firewall_internal_test.go b/internal/providers/exoscale/firewall_internal_test.go index d08f593f..99ff17e2 100644 --- a/internal/providers/exoscale/firewall_internal_test.go +++ b/internal/providers/exoscale/firewall_internal_test.go @@ -88,7 +88,7 @@ func storedInstance(p *Pack, publicIP string, groupIDs ...any) *resource.Resourc // forbidden until a rule allows it. func TestAnExoscaleGroupReachesTheHostWhenAnInstanceBoots(t *testing.T) { driver := newFirewallDriver() - p := sequencedPack(driver) + p := sequencedPack(machine.Use(driver)) group := storedSecurityGroup(p, "platform-web", []any{map[string]any{ "id": "r1", "flow-direction": "ingress", "protocol": "tcp", "network": "0.0.0.0/0", "start-port": 443, "end-port": 443, @@ -116,7 +116,7 @@ func TestAnExoscaleGroupReachesTheHostWhenAnInstanceBoots(t *testing.T) { // OVN NIC's own default is deny — until the group defines one outbound rule, // after which only the defined outbound rules pass. func TestAnEgressRuleFlipsTheEgressDefault(t *testing.T) { - p := sequencedPack(&recordingDriver{}) + p := sequencedPack(machine.Use(&recordingDriver{})) open := storedSecurityGroup(p, "no-egress", []any{map[string]any{ "id": "r1", "flow-direction": "ingress", "protocol": "tcp", "network": "0.0.0.0/0", "start-port": 22, "end-port": 22, @@ -157,7 +157,7 @@ func TestAnEgressRuleFlipsTheEgressDefault(t *testing.T) { // arrive expanded into the member machines' addresses — and the group's // external sources, which upstream defines as extending that membership. func TestAGroupSourcedRuleExpandsToTheMembersAddresses(t *testing.T) { - p := sequencedPack(&recordingDriver{}) + p := sequencedPack(machine.Use(&recordingDriver{})) web := storedSecurityGroup(p, "web", nil) _ = p.env.Store.Update(Name, kindSecurityGroup, web.ID, func(stored *resource.Resource) error { stored.Attrs["external-sources"] = []any{"203.0.113.0/24"} @@ -190,7 +190,7 @@ func TestAGroupSourcedRuleExpandsToTheMembersAddresses(t *testing.T) { // member that does not inherit them is a machine the group never reaches. func TestAPoolMemberWearsItsPoolsGroups(t *testing.T) { driver := newFirewallDriver() - p := sequencedPack(driver) + p := sequencedPack(machine.Use(driver)) group := storedSecurityGroup(p, "platform-app", []any{map[string]any{ "id": "r1", "flow-direction": "ingress", "protocol": "tcp", "network": "0.0.0.0/0", "start-port": 8080, "end-port": 8080, @@ -252,7 +252,7 @@ func (p *Pack) poolMembersOfOnlyPool(t *testing.T) []*resource.Resource { // and this one keeps the resync itself from being dropped along with it. func TestALateNetworkAttachCarriesTheRuleSets(t *testing.T) { driver := newFirewallDriver() - p := sequencedPack(driver) + p := sequencedPack(machine.Use(driver)) group := storedSecurityGroup(p, "web", []any{map[string]any{ "id": "r1", "flow-direction": "ingress", "protocol": "tcp", "network": "0.0.0.0/0", "start-port": 443, "end-port": 443, diff --git a/internal/providers/exoscale/firewall_scope_internal_test.go b/internal/providers/exoscale/firewall_scope_internal_test.go index e423485a..7447cf8b 100644 --- a/internal/providers/exoscale/firewall_scope_internal_test.go +++ b/internal/providers/exoscale/firewall_scope_internal_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/stephrobert/feint/internal/core/machine" "github.com/stephrobert/feint/internal/core/resource" ) @@ -26,7 +27,7 @@ import ( // NICs, so this is a field on the attachment, not a rule of the layer. func TestALateNetworkAttachLeavesThePrivateInterfaceUnfiltered(t *testing.T) { driver := newFirewallDriver() - p := sequencedPack(driver) + p := sequencedPack(machine.Use(driver)) group := storedSecurityGroup(p, "web", []any{map[string]any{ "id": "r1", "flow-direction": "ingress", "protocol": "tcp", "network": "0.0.0.0/0", "start-port": 443, "end-port": 443, diff --git a/internal/providers/exoscale/loadbalancers.go b/internal/providers/exoscale/loadbalancers.go index dbcb8cd5..2d10262e 100644 --- a/internal/providers/exoscale/loadbalancers.go +++ b/internal/providers/exoscale/loadbalancers.go @@ -82,8 +82,8 @@ import ( // So the interface is not the obstacle — an in-block address is accepted and // created — and neither is a provider-shaped gap in it: the runtime refuses // this pack's address on its own, before any guard of ours is consulted. What -// is missing is an address, and no field of `machine.Balancer` could supply -// one that upstream does not publish. +// is missing is an address, and no field of the runtime's balancing half +// could supply one that upstream does not publish. // // Hence: this pack never calls the runtime, and it does not call it and swallow // the error either. A call whose refusal is guaranteed is noise, and the honest diff --git a/internal/providers/exoscale/machines_internal_test.go b/internal/providers/exoscale/machines_internal_test.go index a6a804dd..620ff6ce 100644 --- a/internal/providers/exoscale/machines_internal_test.go +++ b/internal/providers/exoscale/machines_internal_test.go @@ -41,7 +41,7 @@ func (d *recordingDriver) Detach(_ context.Context, name, network string) error } func (d *recordingDriver) RemoveNetwork(context.Context, string) error { return nil } -func runtimePack(driver machine.Driver) *Pack { +func runtimePack(driver machine.Runtime) *Pack { env := &emulator.Env{ Store: store.New(), Now: func() time.Time { return time.Unix(1700000000, 0).UTC() }, @@ -86,7 +86,7 @@ func TestExoscaleTemplateResolutionIsExact(t *testing.T) { // and the runtime is never asked for anything. func TestAnUnknownTemplateDoesNotBootASubstitute(t *testing.T) { driver := &recordingDriver{} - p := runtimePack(driver) + p := runtimePack(machine.Use(driver)) res := &resource.Resource{ ID: "00000000-0000-4000-8000-0000000000aa", State: "stopped", @@ -109,7 +109,7 @@ func TestAnUnknownTemplateDoesNotBootASubstitute(t *testing.T) { // The accepting half: a served template still boots, as its own default-user. func TestAServedTemplateBootsAsItsDefaultUser(t *testing.T) { driver := &recordingDriver{} - p := runtimePack(driver) + p := runtimePack(machine.Use(driver)) res := &resource.Resource{ ID: "00000000-0000-4000-8000-0000000000ab", State: "stopped", @@ -155,7 +155,7 @@ func TestAnInstanceWithNoPublicIPPublishesNone(t *testing.T) { {"an address of the emulated elastic block is", "192.0.2.7", "192.0.2.7"}, } { t.Run(tc.name, func(t *testing.T) { - p := runtimePack(&recordingDriver{}) + p := runtimePack(machine.Use(&recordingDriver{})) res := &resource.Resource{ ID: "00000000-0000-4000-8000-0000000000c1", State: "running", diff --git a/internal/providers/exoscale/pools_machines_internal_test.go b/internal/providers/exoscale/pools_machines_internal_test.go index f82b9aa2..89d4bd4c 100644 --- a/internal/providers/exoscale/pools_machines_internal_test.go +++ b/internal/providers/exoscale/pools_machines_internal_test.go @@ -65,7 +65,7 @@ func (d *failingDriver) Start(context.Context, machine.Spec) (machine.Machine, e // sequencedPack is runtimePack with unique identifiers, which address // allocation needs: two resources under one constant id are one resource. -func sequencedPack(driver machine.Driver) *Pack { +func sequencedPack(driver machine.Runtime) *Pack { n := 0 var mu sync.Mutex env := &emulator.Env{ @@ -128,7 +128,7 @@ func publicIPs(p *Pack) (byAddress map[string][]string, total int) { // machines carry one /32. func TestAPoolMemberAndAStandaloneInstanceNeverShareAnAddress(t *testing.T) { driver := &gatedDriver{gate: make(chan struct{}), started: make(chan string, 8)} - p := sequencedPack(driver) + p := sequencedPack(machine.Use(driver)) done := make(chan struct{}) go func() { @@ -185,7 +185,7 @@ func TestAPoolMemberAndAStandaloneInstanceNeverShareAnAddress(t *testing.T) { // the runtime refused must answer "error" — the state Exoscale's own // instance-state enum declares — and record no machine, because none exists. func TestAFailedPoolMemberStartIsPublishedAsError(t *testing.T) { - p := sequencedPack(&failingDriver{}) + p := sequencedPack(machine.Use(&failingDriver{})) createOnePool(p, 1) members := p.env.Store.List(kindInstance, resource.Tenant{Provider: Name}) @@ -209,7 +209,7 @@ func TestAFailedPoolMemberStartIsPublishedAsError(t *testing.T) { // kept "no machine" for the member's whole life. func TestAPoolMemberStartIsRecordedInTheStore(t *testing.T) { driver := &recordingDriver{} - p := sequencedPack(driver) + p := sequencedPack(machine.Use(driver)) createOnePool(p, 1) members := p.env.Store.List(kindInstance, resource.Tenant{Provider: Name}) @@ -277,7 +277,7 @@ func createPoolOnNetwork(p *Pack, size int, networkID string) { // `private-networks: [{id, mac-address}]` on get-instance. func TestAPoolMemberJoinsThePoolsPrivateNetworks(t *testing.T) { driver := &attachRecordingDriver{} - p := sequencedPack(driver) + p := sequencedPack(machine.Use(driver)) pn := createManagedNetwork(t, p, "back") createPoolOnNetwork(p, 2, pn.ID) @@ -323,7 +323,7 @@ func TestAPoolMemberJoinsThePoolsPrivateNetworks(t *testing.T) { // is free for whoever comes next. func TestAScaleDownReleasesTheMembersLeases(t *testing.T) { driver := &attachRecordingDriver{} - p := sequencedPack(driver) + p := sequencedPack(machine.Use(driver)) pn := createManagedNetwork(t, p, "back") createPoolOnNetwork(p, 2, pn.ID) diff --git a/internal/providers/outscale/audit_test.go b/internal/providers/outscale/audit_test.go index 38d4ab6e..1f09805c 100644 --- a/internal/providers/outscale/audit_test.go +++ b/internal/providers/outscale/audit_test.go @@ -362,7 +362,7 @@ func TestASubnetDoesNotDeleteUnderARace(t *testing.T) { // that boots. func TestACreateWhoseResourceVanishesLeavesNoMachineBehind(t *testing.T) { runtime := newBlockingRuntime() - ts := newRuntimeServer(t, runtime) + ts := newRuntimeServer(t, machine.Use(runtime)) _, subnetID := netAndSubnet(t, ts, "10.51.0.0/16", "10.51.1.0/24") done := make(chan struct{}) @@ -479,7 +479,7 @@ func (f *blockingRuntime) running() []string { return out } -func newRuntimeServer(t *testing.T, drv machine.Driver) *httptest.Server { +func newRuntimeServer(t *testing.T, drv machine.Runtime) *httptest.Server { t.Helper() env := emulator.DefaultEnv() env.UseMachines(drv) @@ -502,7 +502,7 @@ func newRuntimeServer(t *testing.T, drv machine.Driver) *httptest.Server { func TestTwoConcurrentStartsReachTheRuntimeOnce(t *testing.T) { runtime := newCountingRuntime() runtime.blockStarts = make(chan struct{}) - ts := newRuntimeServer(t, runtime) + ts := newRuntimeServer(t, machine.Use(runtime)) _, subnetID := netAndSubnet(t, ts, "10.52.0.0/16", "10.52.1.0/24") _, out := post(t, ts, "CreateVms", @@ -590,7 +590,7 @@ func TestUpdateVmValidatesWhatCreateValidates(t *testing.T) { func TestANetDoesNotDeleteUnderASubnetBeingCreated(t *testing.T) { runtime := newCountingRuntime() runtime.blockNetworks = make(chan struct{}) - ts := newRuntimeServer(t, runtime) + ts := newRuntimeServer(t, machine.Use(runtime)) _, out := post(t, ts, "CreateNet", `{"IpRange":"10.54.0.0/16"}`) n, _ := out["Net"].(map[string]any) @@ -635,7 +635,7 @@ func TestANetDoesNotDeleteUnderASubnetBeingCreated(t *testing.T) { func TestSubnetCreateDoesNotHoldTheAddressingLockAcrossTheRuntime(t *testing.T) { runtime := newCountingRuntime() runtime.blockNetworks = make(chan struct{}) - ts := newRuntimeServer(t, runtime) + ts := newRuntimeServer(t, machine.Use(runtime)) _, out := post(t, ts, "CreateNet", `{"IpRange":"10.55.0.0/16"}`) n, _ := out["Net"].(map[string]any) @@ -760,7 +760,7 @@ func (f *countingRuntime) starts(id string) int { func TestUpdateVmAndStartVmsDoNotOverwriteEachOther(t *testing.T) { runtime := newCountingRuntime() runtime.blockStarts = make(chan struct{}) - ts := newRuntimeServer(t, runtime) + ts := newRuntimeServer(t, machine.Use(runtime)) _, subnetID := netAndSubnet(t, ts, "10.57.0.0/16", "10.57.1.0/24") _, out := post(t, ts, "CreateVms", @@ -1045,7 +1045,7 @@ func outscaleContract(t *testing.T) *contract.Doc { // keeps the address until the machine is terminated. func TestAStoppedVmKeepsItsPrivateAddress(t *testing.T) { runtime := newCountingRuntime() - ts := newRuntimeServer(t, runtime) + ts := newRuntimeServer(t, machine.Use(runtime)) // No Subnet: this is the case that had the address only in the binding. _, out := post(t, ts, "CreateVms", `{"ImageId":"ami-00000001"}`) @@ -1441,8 +1441,8 @@ func TestReadVmsStateAnswersRunningByDefault(t *testing.T) { } } -// Detach implements machine.Driver; *blockingRuntime needs no behaviour here. +// Detach completes the machine package's driver contract; *blockingRuntime needs no behaviour here. func (f *blockingRuntime) Detach(context.Context, string, string) error { return nil } -// Detach implements machine.Driver; *countingRuntime needs no behaviour here. +// Detach completes the machine package's driver contract; *countingRuntime needs no behaviour here. func (f *countingRuntime) Detach(context.Context, string, string) error { return nil } diff --git a/internal/providers/outscale/firewall_internal_test.go b/internal/providers/outscale/firewall_internal_test.go index 6016ac44..fafa9b38 100644 --- a/internal/providers/outscale/firewall_internal_test.go +++ b/internal/providers/outscale/firewall_internal_test.go @@ -61,7 +61,7 @@ func (d *firewallDriver) RemoveFirewall(_ context.Context, name string) error { // firewallPack is runtimePack with unique identifiers, which several stored // groups need. -func firewallPack(driver machine.Driver) *Pack { +func firewallPack(driver machine.Runtime) *Pack { n := 0 env := &emulator.Env{ Store: store.New(), @@ -111,7 +111,7 @@ func storedVM(p *Pack, groupIDs ...string) *resource.Resource { // directions, which is what the API describes and the host never received. func TestAnOutscaleGroupReachesTheHostWhenItsVmBoots(t *testing.T) { driver := newFirewallDriver() - p := firewallPack(driver) + p := firewallPack(machine.Use(driver)) group := storedGroup(p, "witness-osc", []any{map[string]any{ "FromPortRange": 22, "ToPortRange": 22, "IpProtocol": "tcp", "IpRanges": []any{"0.0.0.0/0"}, @@ -142,7 +142,7 @@ func TestAnOutscaleGroupReachesTheHostWhenItsVmBoots(t *testing.T) { // member boots. func TestAMemberSourcedRuleExpandsToTheMembersAddresses(t *testing.T) { driver := newFirewallDriver() - p := firewallPack(driver) + p := firewallPack(machine.Use(driver)) web := storedGroup(p, "web", nil) data := storedGroup(p, "data", []any{map[string]any{ "FromPortRange": 5432, "ToPortRange": 5432, "IpProtocol": "tcp", @@ -181,7 +181,7 @@ func TestAMemberSourcedRuleExpandsToTheMembersAddresses(t *testing.T) { // holds. func TestARevokedRuleLeavesTheRuleSet(t *testing.T) { driver := newFirewallDriver() - p := firewallPack(driver) + p := firewallPack(machine.Use(driver)) group := storedGroup(p, "witness-osc", []any{map[string]any{ "FromPortRange": 22, "ToPortRange": 22, "IpProtocol": "tcp", "IpRanges": []any{"0.0.0.0/0"}, "SecurityGroupRuleId": "sgr-1", diff --git a/internal/providers/outscale/isolate_pass_internal_test.go b/internal/providers/outscale/isolate_pass_internal_test.go index f0955f63..3305bb44 100644 --- a/internal/providers/outscale/isolate_pass_internal_test.go +++ b/internal/providers/outscale/isolate_pass_internal_test.go @@ -64,7 +64,7 @@ func (f *passIsolator) networksSeen() []string { func TestConcurrentSubnetCreatesShareTheirIsolationPasses(t *testing.T) { env := emulator.DefaultEnv() driver := newPassIsolator() - env.UseMachines(driver) + env.UseMachines(machine.Use(driver)) p := New(env) subnet := func(id, network, block string) *resource.Resource { diff --git a/internal/providers/outscale/loadbalancer_dataplane_test.go b/internal/providers/outscale/loadbalancer_dataplane_test.go index d6c10b46..ec7442be 100644 --- a/internal/providers/outscale/loadbalancer_dataplane_test.go +++ b/internal/providers/outscale/loadbalancer_dataplane_test.go @@ -124,7 +124,7 @@ func aBalancedStack(t *testing.T, ts *httptest.Server) (vmA, vmB, vip string) { func TestAnUndeclaredBalancingCapabilityIsNeverUsed(t *testing.T) { runtime := newRecordingBalancer(false) close(runtime.release) - ts := newRuntimeServer(t, runtime) + ts := newRuntimeServer(t, machine.Use(runtime)) aBalancedStack(t, ts) @@ -144,7 +144,7 @@ func TestAnUndeclaredBalancingCapabilityIsNeverUsed(t *testing.T) { func TestTheBalancerSpecIsWhatTheApiDescribes(t *testing.T) { runtime := newRecordingBalancer(true) close(runtime.release) - ts := newRuntimeServer(t, runtime) + ts := newRuntimeServer(t, machine.Use(runtime)) _, _, vip := aBalancedStack(t, ts) @@ -182,7 +182,7 @@ func TestTheBalancerSpecIsWhatTheApiDescribes(t *testing.T) { func TestUnlinkingAndDeletingReachTheRuntime(t *testing.T) { runtime := newRecordingBalancer(true) close(runtime.release) - ts := newRuntimeServer(t, runtime) + ts := newRuntimeServer(t, machine.Use(runtime)) vmA, _, vip := aBalancedStack(t, ts) @@ -221,7 +221,7 @@ func TestUnlinkingAndDeletingReachTheRuntime(t *testing.T) { func TestEmptyingTheListenersRemovesTheBalancerFromTheRuntime(t *testing.T) { runtime := newRecordingBalancer(true) close(runtime.release) - ts := newRuntimeServer(t, runtime) + ts := newRuntimeServer(t, machine.Use(runtime)) _, _, vip := aBalancedStack(t, ts) if len(runtime.specs()) == 0 { @@ -296,7 +296,7 @@ func (r *withholdingBalancer) EnsureBalancer(ctx context.Context, spec machine.B // newLoggedRuntimeServer is newRuntimeServer with somewhere to read the log, // because the level a line carries is the subject here. -func newLoggedRuntimeServer(t *testing.T, drv machine.Driver, log *bytes.Buffer) *httptest.Server { +func newLoggedRuntimeServer(t *testing.T, drv machine.Runtime, log *bytes.Buffer) *httptest.Server { t.Helper() env := emulator.DefaultEnv() env.UseMachines(drv) @@ -328,7 +328,7 @@ func TestAnUndistributableShapeIsNotLoggedAsAnError(t *testing.T) { "outside that network's own block 10.188.3.0/24", machine.ErrBalancerNotDistributed), } close(runtime.release) - ts := newLoggedRuntimeServer(t, runtime, &log) + ts := newLoggedRuntimeServer(t, machine.Use(runtime), &log) aBalancedStack(t, ts) if len(runtime.specs()) == 0 { @@ -352,7 +352,7 @@ func TestAnUndistributableShapeIsNotLoggedAsAnError(t *testing.T) { err: errors.New("incus query: Error: Failed creating load balancer: something new"), } close(failing.release) - aBalancedStack(t, newLoggedRuntimeServer(t, failing, &broken)) + aBalancedStack(t, newLoggedRuntimeServer(t, machine.Use(failing), &broken)) if !strings.Contains(broken.String(), "level=ERROR") { t.Errorf("a runtime failure must stay an error: %q", broken.String()) } @@ -406,7 +406,7 @@ func TestAPartialDeliveryIsRecordedAndSaidAtWarn(t *testing.T) { var log bytes.Buffer runtime := &withholdingBalancer{recordingBalancer: newRecordingBalancer(true), withheld: map[string]string{}} close(runtime.release) - ts := newLoggedRuntimeServer(t, runtime, &log) + ts := newLoggedRuntimeServer(t, machine.Use(runtime), &log) doc := contractDoc(t) out := call(t, ts, doc, "CreateNet", `{"IpRange":"10.188.0.0/16"}`) diff --git a/internal/providers/outscale/machines_internal_test.go b/internal/providers/outscale/machines_internal_test.go index 3959937a..dd01349f 100644 --- a/internal/providers/outscale/machines_internal_test.go +++ b/internal/providers/outscale/machines_internal_test.go @@ -37,7 +37,7 @@ func (d *recordingDriver) Attach(context.Context, string, machine.Attachment) er func (d *recordingDriver) Detach(context.Context, string, string) error { return nil } func (d *recordingDriver) RemoveNetwork(context.Context, string) error { return nil } -func runtimePack(driver machine.Driver) *Pack { +func runtimePack(driver machine.Runtime) *Pack { env := &emulator.Env{ Store: store.New(), Now: func() time.Time { return time.Unix(1700000000, 0).UTC() }, @@ -81,7 +81,7 @@ func TestOutscaleImageResolutionIsExact(t *testing.T) { // and the runtime is never asked for anything. func TestAnUnknownOmiDoesNotBootASubstitute(t *testing.T) { driver := &recordingDriver{} - p := runtimePack(driver) + p := runtimePack(machine.Use(driver)) res := &resource.Resource{ ID: "i-00000001", State: stateStopped, @@ -107,7 +107,7 @@ func TestAnUnknownOmiDoesNotBootASubstitute(t *testing.T) { func TestARegisteredImageRefusesToBootAndSaysWhy(t *testing.T) { driver := &recordingDriver{} var log bytes.Buffer - p := runtimePack(driver) + p := runtimePack(machine.Use(driver)) p.env.Log = slog.New(slog.NewTextHandler(&log, nil)) p.env.Store.Put(&resource.Resource{ ID: "ami-0000cafe", @@ -139,7 +139,7 @@ func TestARegisteredImageRefusesToBootAndSaysWhy(t *testing.T) { // provisions on its images. func TestAServedOmiBootsWithItsLogin(t *testing.T) { driver := &recordingDriver{} - p := runtimePack(driver) + p := runtimePack(machine.Use(driver)) res := &resource.Resource{ ID: "i-00000001", State: stateStopped, @@ -182,7 +182,7 @@ func TestAVmPublishesNoPublicAddressAsItsPrivateOne(t *testing.T) { {"an address of the emulated public block is not", "198.51.100.7", ""}, } { t.Run(tc.name, func(t *testing.T) { - p := runtimePack(&recordingDriver{}) + p := runtimePack(machine.Use(&recordingDriver{})) res := &resource.Resource{ ID: "i-00000541", State: stateRunning, diff --git a/internal/providers/outscale/netpeerings_test.go b/internal/providers/outscale/netpeerings_test.go index 7ae2a937..572c144f 100644 --- a/internal/providers/outscale/netpeerings_test.go +++ b/internal/providers/outscale/netpeerings_test.go @@ -392,7 +392,7 @@ func (r *peererRuntime) peersOf(network string) []string { func TestAnAcceptedPeeringPeersTheBackingNetworks(t *testing.T) { env := emulator.DefaultEnv() rt := newPeererRuntime() - env.UseMachines(rt) + env.UseMachines(machine.Use(rt)) srv, err := emulator.NewServer(env, outscale.New(env)) if err != nil { t.Fatalf("build emulator: %v", err) @@ -445,7 +445,7 @@ func TestAnAcceptedPeeringPeersTheBackingNetworks(t *testing.T) { func TestACreateSubnetDoesNotSeverAnActivePeering(t *testing.T) { env := emulator.DefaultEnv() rt := newPeererRuntime() - env.UseMachines(rt) + env.UseMachines(machine.Use(rt)) srv, err := emulator.NewServer(env, outscale.New(env)) if err != nil { t.Fatalf("build emulator: %v", err) diff --git a/internal/providers/outscale/nets_teardown_internal_test.go b/internal/providers/outscale/nets_teardown_internal_test.go index d8ff9be3..59319db3 100644 --- a/internal/providers/outscale/nets_teardown_internal_test.go +++ b/internal/providers/outscale/nets_teardown_internal_test.go @@ -24,7 +24,7 @@ import ( // default group from the store without removing its rule set from the host. func TestDeleteNetDropsTheDefaultGroupsRuleSet(t *testing.T) { driver := newFirewallDriver() - p := firewallPack(driver) + p := firewallPack(machine.Use(driver)) w := httptest.NewRecorder() p.createNet(w, httptest.NewRequest(http.MethodPost, "/api/v1/CreateNet", diff --git a/internal/providers/outscale/privateips_lock_test.go b/internal/providers/outscale/privateips_lock_test.go index f50defa1..412d05a8 100644 --- a/internal/providers/outscale/privateips_lock_test.go +++ b/internal/providers/outscale/privateips_lock_test.go @@ -88,7 +88,7 @@ func newSlowAttachServer(t *testing.T) (*httptest.Server, *slowAttach) { release: make(chan struct{}), } env := emulator.DefaultEnv() - env.UseMachines(rt) + env.UseMachines(machine.Use(rt)) srv, err := emulator.NewServer(env, outscale.New(env)) if err != nil { t.Fatalf("build emulator: %v", err) @@ -189,5 +189,5 @@ func netOf(t *testing.T, ts *httptest.Server, subnetID string) string { return netID } -// Detach implements machine.Driver; *slowAttach needs no behaviour here. +// Detach completes the machine package's driver contract; *slowAttach needs no behaviour here. func (s *slowAttach) Detach(context.Context, string, string) error { return nil } diff --git a/internal/providers/outscale/publicip_routing_test.go b/internal/providers/outscale/publicip_routing_test.go index 63f5bc9c..e6cf83fb 100644 --- a/internal/providers/outscale/publicip_routing_test.go +++ b/internal/providers/outscale/publicip_routing_test.go @@ -113,7 +113,7 @@ func newRoutedServer(t *testing.T) (*httptest.Server, *routedRuntime, *emulator. t.Helper() env := emulator.DefaultEnv() rt := newRoutedRuntime() - env.UseMachines(rt) + env.UseMachines(machine.Use(rt)) srv, err := emulator.NewServer(env, outscale.New(env)) if err != nil { t.Fatalf("build emulator: %v", err) @@ -229,5 +229,5 @@ func TestAPoisonedPublicIpIsNeverRouted(t *testing.T) { } } -// Detach implements machine.Driver; *routedRuntime needs no behaviour here. +// Detach completes the machine package's driver contract; *routedRuntime needs no behaviour here. func (r *routedRuntime) Detach(context.Context, string, string) error { return nil } diff --git a/internal/providers/replay_test.go b/internal/providers/replay_test.go index 280f6e08..cb83c876 100644 --- a/internal/providers/replay_test.go +++ b/internal/providers/replay_test.go @@ -150,7 +150,7 @@ func recorderEnv() (*emulator.Env, *machine.Recorder) { }, Log: slog.New(slog.NewTextHandler(io.Discard, nil)), } - env.UseMachines(rec) + env.UseMachines(machine.Use(rec)) return env, rec } diff --git a/internal/providers/scaleway/address_routing_test.go b/internal/providers/scaleway/address_routing_test.go index b8507586..65e1c59b 100644 --- a/internal/providers/scaleway/address_routing_test.go +++ b/internal/providers/scaleway/address_routing_test.go @@ -88,7 +88,7 @@ func (r *addressRuntime) withdrawn() []string { // newAddressTestServer is newRuntimeTestServer with the env kept in reach, so a // test can poison the store the way a restored snapshot would. -func newAddressTestServer(t testing.TB, drv machine.Driver) (*httptest.Server, *emulator.Env) { +func newAddressTestServer(t testing.TB, drv machine.Runtime) (*httptest.Server, *emulator.Env) { t.Helper() var seq atomic.Int64 @@ -127,7 +127,7 @@ func contains(list []string, want string) bool { // the replay hands the guest its address. func TestPowerOnRoutesAnAddressAttachedBeforeBoot(t *testing.T) { rt := newAddressRuntime() - ts, _ := newAddressTestServer(t, rt) + ts, _ := newAddressTestServer(t, machine.Use(rt)) _, out := do(t, ts, "POST", zone+"/ips", `{}`) ip, _ := out["ip"].(map[string]any) @@ -160,7 +160,7 @@ func TestPowerOnRoutesAnAddressAttachedBeforeBoot(t *testing.T) { // decoded, echoed back, and read by nobody. func TestADynamicAddressFollowsThePowerCycle(t *testing.T) { rt := newAddressRuntime() - ts, _ := newAddressTestServer(t, rt) + ts, _ := newAddressTestServer(t, machine.Use(rt)) _, out := do(t, ts, "POST", zone+"/servers", `{"name":"ephemeral","commercial_type":"DEV1-S","image":"ubuntu_jammy","dynamic_ip_required":true}`) @@ -229,7 +229,7 @@ func TestADynamicAddressFollowsThePowerCycle(t *testing.T) { // authorisation half, and this holds it on both stored paths. func TestAPoisonedStoredAddressIsNeverRouted(t *testing.T) { rt := newAddressRuntime() - ts, env := newAddressTestServer(t, rt) + ts, env := newAddressTestServer(t, machine.Use(rt)) const poison = "10.76.154.1" // a host bridge gateway, well-formed and not ours diff --git a/internal/providers/scaleway/barrage_test.go b/internal/providers/scaleway/barrage_test.go index f25092a1..b866271e 100644 --- a/internal/providers/scaleway/barrage_test.go +++ b/internal/providers/scaleway/barrage_test.go @@ -30,7 +30,7 @@ import ( // the sweep report the harness rather than the pack. Measured, not assumed — // this repository's most expensive recurring defect is the instrument, not the // subject. -func newBarrageServer(t *testing.T, drv machine.Driver) (*httptest.Server, *store.Store) { +func newBarrageServer(t *testing.T, drv machine.Runtime) (*httptest.Server, *store.Store) { t.Helper() var seq atomic.Int64 @@ -184,7 +184,7 @@ const ( // allocator's own lock. func TestABarrageLeavesTheStoreCoherent(t *testing.T) { runtime := newBarrageRuntime() - ts, st := newBarrageServer(t, runtime) + ts, st := newBarrageServer(t, machine.Use(runtime)) var wg sync.WaitGroup // Errors are collected rather than reported from the goroutines: t.Fatalf @@ -363,7 +363,7 @@ func firstFew(all []string, n int) []string { // reference another that the other world deleted. func TestARestoreDuringTrafficLandsInOneWorld(t *testing.T) { runtime := newBarrageRuntime() - ts, st := newBarrageServer(t, runtime) + ts, st := newBarrageServer(t, machine.Use(runtime)) // A world worth restoring, captured before the traffic starts. _, before := doRaw(ts, "POST", zoneURL+"/ips", `{}`) @@ -444,7 +444,7 @@ func TestARestoreDuringTrafficLandsInOneWorld(t *testing.T) { // of ipam/v1 that nothing exercised: book, attach through a NIC, release. func TestASharedNetworkUnderBarrageNeverHandsOutOneAddressTwice(t *testing.T) { runtime := newBarrageRuntime() - ts, st := newBarrageServer(t, runtime) + ts, st := newBarrageServer(t, machine.Use(runtime)) // One network for everybody. A /24 leaves 250-odd addresses, so exhaustion // is not what this measures. @@ -531,5 +531,5 @@ func TestASharedNetworkUnderBarrageNeverHandsOutOneAddressTwice(t *testing.T) { } } -// Detach implements machine.Driver; *barrageRuntime needs no behaviour here. +// Detach completes the machine package's driver contract; *barrageRuntime needs no behaviour here. func (b *barrageRuntime) Detach(context.Context, string, string) error { return nil } diff --git a/internal/providers/scaleway/boot_test.go b/internal/providers/scaleway/boot_test.go index 9b2882bb..480091aa 100644 --- a/internal/providers/scaleway/boot_test.go +++ b/internal/providers/scaleway/boot_test.go @@ -3,6 +3,8 @@ package scaleway_test import ( "net/http" "testing" + + "github.com/stephrobert/feint/internal/core/machine" ) // The measurement of #83, replayed against the pack the way the issue took it: @@ -13,7 +15,7 @@ import ( func TestAnUnknownImageDoesNotBootASubstitute(t *testing.T) { rt := newFakeRuntime() close(rt.release) // nothing here needs to hold a start open - ts := newRuntimeTestServer(t, rt) + ts := newRuntimeTestServer(t, machine.Use(rt)) const zone = "/instance/v1/zones/fr-par-1" status, out := do(t, ts, "POST", zone+"/servers", @@ -43,7 +45,7 @@ func TestAnUnknownImageDoesNotBootASubstitute(t *testing.T) { func TestTheMarketplaceAnswersOneImagePerLabel(t *testing.T) { rt := newFakeRuntime() close(rt.release) - ts := newRuntimeTestServer(t, rt) + ts := newRuntimeTestServer(t, machine.Use(rt)) idOf := func(label string) string { _, out := do(t, ts, "GET", "/marketplace/v2/local-images?image_label="+label, "") @@ -71,7 +73,7 @@ func TestTheMarketplaceAnswersOneImagePerLabel(t *testing.T) { func TestAKnownImageBootsWhatItNamesWithItsLogin(t *testing.T) { rt := newFakeRuntime() close(rt.release) - ts := newRuntimeTestServer(t, rt) + ts := newRuntimeTestServer(t, machine.Use(rt)) const zone = "/instance/v1/zones/fr-par-1" status, out := do(t, ts, "POST", zone+"/servers", diff --git a/internal/providers/scaleway/concurrency_test.go b/internal/providers/scaleway/concurrency_test.go index eda262f7..1151fabc 100644 --- a/internal/providers/scaleway/concurrency_test.go +++ b/internal/providers/scaleway/concurrency_test.go @@ -18,7 +18,8 @@ import ( "github.com/stephrobert/feint/internal/providers/scaleway" ) -// fakeRuntime is a machine.Driver that records what it was asked to do and +// fakeRuntime satisfies the machine package's driver contract, records what +// it was asked to do and // refuses a name it has already given out, the way Incus refuses to launch an // instance whose name exists. That refusal is the whole point: it is what turned // a second poweron into a "stopped" server with a container still running. @@ -132,7 +133,7 @@ func (f *fakeRuntime) running() []string { // newRuntimeTestServer is newTestServer with a machine runtime behind it, and an // id generator that is safe to call from two requests at once — the sequential // one races under -race the moment a test issues concurrent calls. -func newRuntimeTestServer(t testing.TB, drv machine.Driver) *httptest.Server { +func newRuntimeTestServer(t testing.TB, drv machine.Runtime) *httptest.Server { t.Helper() var seq atomic.Int64 @@ -183,7 +184,7 @@ func call(ts *httptest.Server, method, path, body string) (int, error) { // this project exists not to give. func TestConcurrentPowerOnStartsTheMachineOnce(t *testing.T) { rt := newFakeRuntime() - ts := newRuntimeTestServer(t, rt) + ts := newRuntimeTestServer(t, machine.Use(rt)) const zone = "/instance/v1/zones/fr-par-1" status, out := do(t, ts, "POST", zone+"/servers", `{"name":"demo","commercial_type":"DEV1-S"}`) @@ -263,7 +264,7 @@ func TestConcurrentPowerOnStartsTheMachineOnce(t *testing.T) { func TestPowerOnIsIdempotentOnARunningServer(t *testing.T) { rt := newFakeRuntime() close(rt.release) // nothing to hold: this test is sequential - ts := newRuntimeTestServer(t, rt) + ts := newRuntimeTestServer(t, machine.Use(rt)) const zone = "/instance/v1/zones/fr-par-1" status, out := do(t, ts, "POST", zone+"/servers", `{"name":"demo","commercial_type":"DEV1-S"}`) diff --git a/internal/providers/scaleway/detach_test.go b/internal/providers/scaleway/detach_test.go index 86d7a179..35d88604 100644 --- a/internal/providers/scaleway/detach_test.go +++ b/internal/providers/scaleway/detach_test.go @@ -67,7 +67,7 @@ func nicOnRunningServer(t *testing.T, ts *httptest.Server, subnet string) (pnID, func TestDeletingAPrivateNICDetachesItFromTheRuntime(t *testing.T) { rt := newFakeRuntime() close(rt.release) - ts := newRuntimeTestServer(t, rt) + ts := newRuntimeTestServer(t, machine.Use(rt)) _, serverID, nicID := nicOnRunningServer(t, ts, "10.71.0.0/24") @@ -110,7 +110,7 @@ func TestDeletingAPrivateNICDetachesItFromTheRuntime(t *testing.T) { func TestAPrivateNetworkTheRuntimeKeptIsNotReportedDeleted(t *testing.T) { rt := &keepingRuntime{fakeRuntime: newFakeRuntime()} close(rt.release) - ts := newRuntimeTestServer(t, rt) + ts := newRuntimeTestServer(t, machine.Use(rt)) status, pn := do(t, ts, "POST", "/vpc/v2/regions/fr-par/private-networks", `{"name":"kept","subnets":["10.72.0.0/24"]}`) diff --git a/internal/providers/scaleway/firewall_internal_test.go b/internal/providers/scaleway/firewall_internal_test.go index 888bf191..739ca6ea 100644 --- a/internal/providers/scaleway/firewall_internal_test.go +++ b/internal/providers/scaleway/firewall_internal_test.go @@ -72,7 +72,7 @@ func TestBridgeModeRejectsForeignSubnetsThroughTheGroup(t *testing.T) { rec := machine.NewRecorder() rec.Joined = true // the bridge shape: networks reach each other unless rejected env := emulator.DefaultEnv() - env.UseMachines(rec) + env.UseMachines(machine.Use(rec)) p := New(env) tenant := resource.Tenant{Provider: Name, Project: defaultProject, Zone: "fr-par-1"} diff --git a/internal/providers/scaleway/lostupdate_test.go b/internal/providers/scaleway/lostupdate_test.go index 1510ebaa..95a1e9aa 100644 --- a/internal/providers/scaleway/lostupdate_test.go +++ b/internal/providers/scaleway/lostupdate_test.go @@ -8,6 +8,7 @@ import ( "sync" "testing" + "github.com/stephrobert/feint/internal/core/machine" "github.com/stephrobert/feint/internal/core/store/storetest" ) @@ -127,7 +128,7 @@ const nicBarrageTrials = 12 func TestAttachingANICDoesNotResurrectADeletedServer(t *testing.T) { runtime := newBarrageRuntime() - ts, st := newBarrageServer(t, runtime) + ts, st := newBarrageServer(t, machine.Use(runtime)) const zone = "/instance/v1/zones/fr-par-1" const region = "/vpc/v2/regions/fr-par" diff --git a/internal/providers/scaleway/ownership_audit_test.go b/internal/providers/scaleway/ownership_audit_test.go index 8606211b..0510412a 100644 --- a/internal/providers/scaleway/ownership_audit_test.go +++ b/internal/providers/scaleway/ownership_audit_test.go @@ -219,7 +219,7 @@ func TestTerminateReleasesWhatDeleteReleases(t *testing.T) { func TestCreatingAServerDoesNotStealALiveAddress(t *testing.T) { runtime := &routingRuntime{fakeRuntime: newFakeRuntime()} close(runtime.release) // nothing here needs to block - ts := newRuntimeTestServer(t, runtime) + ts := newRuntimeTestServer(t, machine.Use(runtime)) _, out := do(t, ts, "POST", zone+"/ips", `{}`) ip, _ := out["ip"].(map[string]any) @@ -425,7 +425,7 @@ func TestAnAddressIsAValidIPReference(t *testing.T) { func TestARefusedAttachmentIsVisibleOnTheNIC(t *testing.T) { refusing := &refusingRuntime{fakeRuntime: newFakeRuntime()} close(refusing.release) - ts := newRuntimeTestServer(t, refusing) + ts := newRuntimeTestServer(t, machine.Use(refusing)) srvID := aServer(t, ts, "vm-host") if status, _ := do(t, ts, "POST", zone+"/servers/"+srvID+"/action", @@ -468,7 +468,7 @@ func TestARefusedAttachmentIsVisibleOnTheNIC(t *testing.T) { func TestARefusedAttachmentIsVisibleOnTheV2alpha1View(t *testing.T) { refusing := &refusingRuntime{fakeRuntime: newFakeRuntime()} close(refusing.release) - ts := newRuntimeTestServer(t, refusing) + ts := newRuntimeTestServer(t, machine.Use(refusing)) srvID := aServer(t, ts, "vm-host") if status, _ := do(t, ts, "POST", zone+"/servers/"+srvID+"/action", @@ -510,5 +510,5 @@ func (r *refusingRuntime) Attach(context.Context, string, machine.Attachment) er return errors.New(`Failed to start device "eth1": PCI: slot 0 function 0 not available`) } -// Detach implements machine.Driver; *refusingRuntime needs no behaviour here. +// Detach completes the machine package's driver contract; *refusingRuntime needs no behaviour here. func (r *refusingRuntime) Detach(context.Context, string, string) error { return nil } diff --git a/internal/providers/scaleway/routes_test.go b/internal/providers/scaleway/routes_test.go index 48799287..b5a0e4c9 100644 --- a/internal/providers/scaleway/routes_test.go +++ b/internal/providers/scaleway/routes_test.go @@ -99,7 +99,7 @@ func (r *peeringRuntime) peersOf(network string) []string { // VPC's networks, which the machine driver enforces, reconciles when it flips. func TestEnableRoutingReconcilesThePeering(t *testing.T) { rt := newPeeringRuntime() - ts, _ := newAddressTestServer(t, rt) + ts, _ := newAddressTestServer(t, machine.Use(rt)) vpc := createVPC(t, ts, `{"name":"routed","enable_routing":false}`) vpcID, _ := vpc["id"].(string) @@ -289,7 +289,7 @@ func TestAVPCCreatedWithoutEnableRoutingRoutes(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { rt := newPeeringRuntime() - ts, _ := newAddressTestServer(t, rt) + ts, _ := newAddressTestServer(t, machine.Use(rt)) vpc := createVPC(t, ts, tc.body) vpcID, _ := vpc["id"].(string) diff --git a/tools/falsify/specs/a-run-ends-where-it-could-start.json b/tools/falsify/specs/a-run-ends-where-it-could-start.json index baf38d5d..3dee983a 100644 --- a/tools/falsify/specs/a-run-ends-where-it-could-start.json +++ b/tools/falsify/specs/a-run-ends-where-it-could-start.json @@ -33,8 +33,8 @@ { "label": "serve stops asking the driver on its way out, so the uplink outlives every graceful stop again and the next run's doorstep refuses the host", "file": "internal/cli/cli.go", - "find": "\tif releaser, ok := driver.(machine.UplinkReleaser); ok {", - "replace": "\tif releaser, ok := driver.(machine.UplinkReleaser); ok && false {", + "find": "\tif released, asked, err := rt.ReleaseUplink(context.Background()); asked {", + "replace": "\tif released, asked, err := rt.ReleaseUplink(context.Background()); asked && false {", "test": "TestAGracefulExitReleasesTheUplink", "package": "./internal/cli/" }, diff --git a/tools/falsify/specs/driver-unnameable.json b/tools/falsify/specs/driver-unnameable.json new file mode 100644 index 00000000..650f5b88 --- /dev/null +++ b/tools/falsify/specs/driver-unnameable.json @@ -0,0 +1,47 @@ +{ + "package": "./internal/cli/", + "mutations": [ + { + "label": "the driver interface is exported again, so `var _ machine.Driver` in a pack compiles and the boundary is back to a convention an AST scan enforces (#514)", + "file": "internal/core/machine/machine.go", + "find": "type driver interface {", + "replace": "type Driver = driver\n\ntype driver interface {", + "test": "TestThePacksCannotNameTheDriver" + }, + { + "label": "the balancing half is exported again, and one of the five type assertions #511 counted becomes writable in a pack once more (#514)", + "file": "internal/core/machine/balancer.go", + "find": "type balancer interface {", + "replace": "type Balancer = balancer\n\ntype balancer interface {", + "test": "TestThePacksCannotNameTheDriver" + }, + { + "label": "the routing half comes back as an exported name and only the ratchet says so: the surface list still excludes it, and excluding a nameable type is the state #514 replaced (#514)", + "file": "internal/core/machine/address.go", + "find": "type router interface {", + "replace": "type Router = router\n\ntype router interface {", + "test": "TestTheDeclaredDriverSurfaceIsSmallerThanThePackage" + }, + { + "label": "the environment publishes its runtime again, which is the field that put a driver in every pack's hand before #511 (#514)", + "file": "internal/core/emulator/emulator.go", + "find": "\tmachines machine.Runtime\n", + "replace": "\tMachines machine.Runtime\n\tmachines machine.Runtime\n", + "test": "TestThePacksCannotNameTheDriver" + }, + { + "label": "the binding publishes its driver again, and `p.binding().Driver.EnsureNetwork(…)` — the sentence surface.go cites by name — compiles once more (#514)", + "file": "internal/core/machine/binding.go", + "find": "\tdriver driver\n", + "replace": "\tDriver driver\n\tdriver driver\n", + "test": "TestThePacksCannotNameTheDriver" + }, + { + "label": "the probe harness compiles a package that is not there, so every refusal is a refusal about nothing — and the positive control is what has to notice (#514)", + "file": "internal/cli/driver_unnameable_test.go", + "find": "\t\t\"./internal/cli/testdata/bypass/\"+dir+\"/\")", + "replace": "\t\t\"./internal/cli/testdata/bypass/\"+dir+\"/nowhere/\")", + "test": "TestThePacksCannotNameTheDriver" + } + ] +} diff --git a/tools/falsify/specs/one-machine-per-address.json b/tools/falsify/specs/one-machine-per-address.json index 9f194cf6..76f7a044 100644 --- a/tools/falsify/specs/one-machine-per-address.json +++ b/tools/falsify/specs/one-machine-per-address.json @@ -11,8 +11,8 @@ { "label": "a move whose withdrawal failed carries on and routes the address anyway", "file": "internal/core/machine/placement.go", - "find": "\t\t\tif err := router.UnrouteAddress(ctx, previous, spec.Address); err != nil {\n\t\t\t\treturn err\n\t\t\t}", - "replace": "\t\t\tif err := router.UnrouteAddress(ctx, previous, spec.Address); err != nil && false {\n\t\t\t\treturn err\n\t\t\t}", + "find": "\t\t\tif err := rt.UnrouteAddress(ctx, previous, spec.Address); err != nil {\n\t\t\t\treturn err\n\t\t\t}", + "replace": "\t\t\tif err := rt.UnrouteAddress(ctx, previous, spec.Address); err != nil && false {\n\t\t\t\treturn err\n\t\t\t}", "test": "TestRouteAddressReportsAFailedWithdrawalInsteadOfMovingAnyway" }, { diff --git a/tools/falsify/specs/run-leaves-nothing.json b/tools/falsify/specs/run-leaves-nothing.json index e893b251..c639bbda 100644 --- a/tools/falsify/specs/run-leaves-nothing.json +++ b/tools/falsify/specs/run-leaves-nothing.json @@ -44,8 +44,8 @@ { "label": "the doorstep finds the previous run's networks and lets the run start anyway, which is the state that fails thirty steps later on a message naming only the block", "file": "internal/cli/clean.go", - "find": "\tif err := refuseRuntimeLeftovers(stdout, led, vm, driver); err != nil {", - "replace": "\tif err := refuseRuntimeLeftovers(stdout, led, vm, driver); err != nil && false {", + "find": "\t\tif err := refuseRuntimeLeftovers(stdout, led, vm, rt); err != nil {", + "replace": "\t\tif err := refuseRuntimeLeftovers(stdout, led, vm, rt); err != nil && false {", "test": "TestTheDoorstepRefusesAHostHoldingAPreviousRunsNetwork", "package": "./internal/cli/" }, @@ -60,8 +60,8 @@ { "label": "a survey that could not be taken is reported as a clean host, which is two outcomes where three are owed", "file": "internal/cli/clean_ledger.go", - "find": "\t\treturn fmt.Errorf(\"could not look at what the %s runtime holds, so this host cannot be called clean: %w\",\n\t\t\tdriver.Name(), err)", - "replace": "\t\t_ = fmt.Errorf(\"could not look at what the %s runtime holds, so this host cannot be called clean: %w\",\n\t\t\tdriver.Name(), err)\n\t\treturn nil", + "find": "\t\treturn fmt.Errorf(\"could not look at what the %s runtime holds, so this host cannot be called clean: %w\",\n\t\t\trt.Name(), err)", + "replace": "\t\t_ = fmt.Errorf(\"could not look at what the %s runtime holds, so this host cannot be called clean: %w\",\n\t\t\trt.Name(), err)\n\t\treturn nil", "test": "TestTheDoorstepSaysItCouldNotLookRatherThanCallingTheHostClean", "package": "./internal/cli/" }, @@ -76,8 +76,8 @@ { "label": "the doorstep question is asked at every moment, so a run is refused for owning the machines and networks it just created", "file": "internal/cli/clean.go", - "find": "\tif doorstep {\n\t\tif err := refuseRuntimeLeftovers(stdout, led, vm, driver); err != nil {", - "replace": "\tif doorstep || true {\n\t\tif err := refuseRuntimeLeftovers(stdout, led, vm, driver); err != nil {", + "find": "\tif doorstep {\n\t\tif err := refuseRuntimeLeftovers(stdout, led, vm, rt); err != nil {", + "replace": "\tif doorstep || true {\n\t\tif err := refuseRuntimeLeftovers(stdout, led, vm, rt); err != nil {", "test": "TestTheLeftoverCheckMidRunIgnoresTheRunsOwnObjects", "package": "./internal/cli/" }, diff --git a/tools/falsify/specs/trapped-station.json b/tools/falsify/specs/trapped-station.json index 2726122c..802bbc30 100644 --- a/tools/falsify/specs/trapped-station.json +++ b/tools/falsify/specs/trapped-station.json @@ -103,8 +103,8 @@ { "label": "the check stops asking the runtime what holds it, the exact shape it was in when it answered 0 on a station nothing could clean", "file": "internal/cli/clean.go", - "find": "\ttrapped, err := reportRuntimeTraps(stdout, led, driver)\n\tif err != nil {\n\t\treturn err\n\t}", - "replace": "\ttrapped, err := reportRuntimeTraps(stdout, led, driver)\n\ttrapped = 0\n\tif err != nil {\n\t\treturn err\n\t}", + "find": "\ttrapped, err := reportRuntimeTraps(stdout, led, rt)\n\tif err != nil {\n\t\treturn err\n\t}", + "replace": "\ttrapped, err := reportRuntimeTraps(stdout, led, rt)\n\ttrapped = 0\n\tif err != nil {\n\t\treturn err\n\t}", "test": "TestCleanCheckReportsAStrippedUplink", "package": "./internal/cli/" } From 0697c21a32cd29420adb446afeef8be30b8ed8e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20ROBERT?= Date: Fri, 28 Aug 2026 01:00:37 +0200 Subject: [PATCH 2/3] docs: the architecture said the door was shut when only half of it was (#514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two passages claimed what #514 measured to be false. docs/architecture.md: "a pack receives no machine.Driver value at all … so the call it would have written does not compile". docs/fourth-pack.md: "it could not name machine.Driver if it tried: since #511 emulator.Env hands out no driver value". Both were true of *obtaining* a driver and neither was true of *naming* one. On 154c204, `var _ machine.Driver` in internal/providers/scaleway compiled and `go build ./internal/providers/scaleway/` exited 0. That is the shape this repository calls a comment standing in for a control, and documentation is where it survives longest, because nothing runs it. They now say which step closed which half, and name the test that compiles the forbidden sentence. docs/limits.md loses two references to types that are unexported since: the driver's Detach and the runtime's balancing half. The passage on #475 keeps `machine.Firewaller` — it is explicitly "until #475 was fixed", and rewriting a dated record to match today's spelling would make it say something that was not what was measured. Co-authored-by: Claude Opus 5 (1M context) --- docs/architecture.md | 16 ++++++++++++---- docs/fourth-pack.md | 6 ++++-- docs/limits.md | 4 ++-- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 168bf552..e7d8268a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -99,10 +99,18 @@ And what a pack may ask of the runtime is a closed list rather than whatever it can reach: `machine.PackSurface()` names eight service families, and `internal/cli`'s `TestNoPackReachesPastTheDeclaredDriverSurface` holds the packs' own sources against it, naming the pack, the gesture and the line. The strongest -half is not that test: a pack receives no `machine.Driver` value at all — -`emulator.Env` keeps it unexported and hands back a finished `Binding` — so the -call it would have written does not compile. A gesture the list lacks is added -to it; the pack never works around it. +half is not that test, and it took two steps. #511 closed the way to *obtain* +a driver: `emulator.Env` keeps it unexported and hands back a finished +`Binding`, so `p.binding().Driver.EnsureNetwork(…)` stopped compiling. #514 +closed the way to *name* one, because until it `var _ machine.Driver` in a pack +still compiled — measured on `154c204`, `go build ./internal/providers/scaleway/` +exited 0 — which left the surface held by a convention plus a scan. The driver +interface and its five pack-facing halves are unexported now, and what leaves +the package is `machine.Runtime`, a struct rather than a narrowed interface +because a type assertion needs no name. `internal/cli`'s +`TestThePacksCannotNameTheDriver` compiles the forbidden sentence and requires +the failure. A gesture the list lacks is added to it; the pack never works +around it. ## A request, end to end diff --git a/docs/fourth-pack.md b/docs/fourth-pack.md index 5bd28dd2..a11d4c5e 100644 --- a/docs/fourth-pack.md +++ b/docs/fourth-pack.md @@ -281,8 +281,10 @@ spreaders — which drives the whole runtime dataplane through the shared contract alone: it boots a machine, declares a network and joins it at boot and after boot, publishes and withdraws a public address, hands a rule set over and re-expands it, asks for a balancer, and keeps two subnets apart. It names no -runtime, and it could not name `machine.Driver` if it tried: since #511 -`emulator.Env` hands out no driver value. +runtime, and it could not name the driver if it tried: since #511 +`emulator.Env` hands out no driver value, and since #514 there is no exported +name for one — `var _ machine.Driver` in a pack fails the build, which it did +not until then. Three things it is deliberately not: diff --git a/docs/limits.md b/docs/limits.md index 81b669ee..8f6a51f8 100644 --- a/docs/limits.md +++ b/docs/limits.md @@ -1110,7 +1110,7 @@ are never chosen by the emulator. Three things changed, and the third is the one that generalises: -- `machine.Driver.Detach`, required rather than optional, asking both ownership +- the driver's `Detach`, required rather than optional, asking both ownership questions and removing only a device the instance itself carries. Both packs that attach now detach; the Exoscale handler had documented the gap as unclosable ("the driver deliberately has no hot-unplug"), which is how one @@ -3037,7 +3037,7 @@ its wording differs; the verdict does not. So `capabilities.balancing` is irrelevant to this family: the pack never asks the runtime at all, because the only call it could make is one whose refusal is -guaranteed. `machine.Balancer` needed no provider-shaped concession to reach +guaranteed. The runtime's balancing half needed no provider-shaped concession to reach that answer, and `internal/core` gained no Exoscale knowledge — what is missing is an address upstream does not publish, and no field of an interface can supply one. From cde80224e4cb7792804ee04a9d77ac0148a53bf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20ROBERT?= Date: Fri, 28 Aug 2026 01:29:04 +0200 Subject: [PATCH 3/3] docs(machine): Runtime.RemoveImage names its argument the way the operator types it The parameter was called alias while every caller passes the bare / the CLI reads, and the driver is what prepends the emulator's own prefix. A name that asserts a shape the value does not have is the smallest version of the defect this lot is about. Co-authored-by: Claude Opus 5 (1M context) --- internal/core/machine/runtime.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/core/machine/runtime.go b/internal/core/machine/runtime.go index ae15f8ff..a67c9b56 100644 --- a/internal/core/machine/runtime.go +++ b/internal/core/machine/runtime.go @@ -237,14 +237,16 @@ func (r Runtime) RemovesImages() bool { return ok } -// RemoveImage deletes one image this emulator published. asked is false for a -// runtime that holds no images to remove. -func (r Runtime) RemoveImage(ctx context.Context, alias string) (asked bool, err error) { +// RemoveImage deletes one image this emulator published, named the way the +// operator types it — "/", without the emulator's own prefix, +// which the driver adds. asked is false for a runtime that holds no images to +// remove. +func (r Runtime) RemoveImage(ctx context.Context, name string) (asked bool, err error) { rm, ok := r.backing().(imageRemover) if !ok { return false, nil } - return true, rm.RemoveImage(ctx, alias) + return true, rm.RemoveImage(ctx, name) } // imageRemover is the optional half `feint images remove` drives. It was an