A repeat assignment says nothing changed (ADR 0115)

One assignment of a module per node is now the rule, not a limitation —
the operator dropped the multi-assignment requirement, and the schema's
(node, module) key has been the decision since migration 0005. What
changed: Assign reports whether the assignment was new, and the command
says 'already runs — one node runs one of each (ADR 0115); nothing
changed' instead of printing 'is assigned' for a no-op, which read as
an action that happened. Idempotence stays: a repeat is exit 0, because
a script stating what is already true is not wrong.
This commit is contained in:
2026-09-26 19:02:35 +02:00
parent 95426e25cf
commit 50734095b8
9 changed files with 51 additions and 25 deletions
+8 -1
View File
@@ -46,9 +46,16 @@ func assign(ctx context.Context, open *stores, node, module string) (string, err
return "", err return "", err
} }
defer release() defer release()
if err := open.inventory.Assign(ctx, node, module); err != nil { fresh, err := open.inventory.Assign(ctx, node, module)
if err != nil {
return "", err return "", err
} }
if !fresh {
// Nothing changed, and saying "is assigned" would read as an action. One node runs one
// of each — the module's name is the assignment's identity (novox/hq ADR 0115).
return fmt.Sprintf("%s already runs %s — one node runs one of each (ADR 0115); nothing changed",
node, module), nil
}
said := fmt.Sprintf("%s is assigned %s", node, module) said := fmt.Sprintf("%s is assigned %s", node, module)
plan, _, err := planFor(ctx, open, node) plan, _, err := planFor(ctx, open, node)
if err != nil { if err != nil {
+1 -1
View File
@@ -355,7 +355,7 @@ func converge(ctx context.Context, open *stores, node string, yes bool, digest s
strings.Join(assigned, ", ")) strings.Join(assigned, ", "))
} }
if !slices.Contains(assigned, filter) { if !slices.Contains(assigned, filter) {
if err := inv.Assign(ctx, node, filter); err != nil { if _, err := inv.Assign(ctx, node, filter); err != nil {
return "", err return "", err
} }
if _, _, err := planFor(ctx, open, node); err != nil { if _, _, err := planFor(ctx, open, node); err != nil {
+1 -1
View File
@@ -73,7 +73,7 @@ func aMesh(t *testing.T) *stores {
if err := open.inventory.RecordOverlayKey(t.Context(), record.ID, aPublicKey(t)); err != nil { if err := open.inventory.RecordOverlayKey(t.Context(), record.ID, aPublicKey(t)); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if err := open.inventory.Assign(t.Context(), name, overlay.Name); err != nil { if _, err := open.inventory.Assign(t.Context(), name, overlay.Name); err != nil {
t.Fatal(err) t.Fatal(err)
} }
} }
+2 -2
View File
@@ -65,7 +65,7 @@ func TestTakingIsRefusedOnAConvergedNodeAndForAnUnassignedModule(t *testing.T) {
if _, err := inv.AddNode(t.Context(), "converged"); err != nil { if _, err := inv.AddNode(t.Context(), "converged"); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if err := inv.Assign(t.Context(), "converged", "hello-web"); err != nil { if _, err := inv.Assign(t.Context(), "converged", "hello-web"); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if err := inv.Take(t.Context(), "converged", "hello-web"); !errors.Is(err, ErrNotAdopted) { if err := inv.Take(t.Context(), "converged", "hello-web"); !errors.Is(err, ErrNotAdopted) {
@@ -91,7 +91,7 @@ func TestATakenModuleOutlivesItsAssignmentAndReturningToAdopted(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
for _, m := range []string{"hello-web", "postgres"} { for _, m := range []string{"hello-web", "postgres"} {
if err := inv.Assign(t.Context(), "anchor", m); err != nil { if _, err := inv.Assign(t.Context(), "anchor", m); err != nil {
t.Fatal(err) t.Fatal(err)
} }
} }
+16 -7
View File
@@ -430,27 +430,36 @@ func (i *Inventory) discard(ctx context.Context, name string) error {
return nil return nil
} }
// Assign puts a module on a node. // Assign puts a module on a node, and says whether that is new.
// //
// Records the intention and checks nothing. Whether the set of assignments can actually become a // Records the intention and checks nothing. Whether the set of assignments can actually become a
// declaration is resolution's question, asked over the whole set at once — and asking it here, // declaration is resolution's question, asked over the whole set at once — and asking it here,
// one module at a time, would let an assignment look accepted and then refuse when a second // one module at a time, would let an assignment look accepted and then refuse when a second
// arrives. // arrives.
func (i *Inventory) Assign(ctx context.Context, nodeName, module string) error { //
// **One assignment of a module per node is the rule, not a race lost** (novox/hq ADR 0115). The
// module's name is the assignment's identity — its database user, its broker account, its
// containers and its placed directory are all named by it — so the schema's (node, module) key
// is the decision, and a repeat is absorbed rather than refused. Absorbed audibly: the caller is
// told nothing changed, because "is assigned" printed for a no-op reads as an action.
func (i *Inventory) Assign(ctx context.Context, nodeName, module string) (bool, error) {
node, err := i.NodeByName(ctx, nodeName) node, err := i.NodeByName(ctx, nodeName)
if err != nil { if err != nil {
return err return false, err
} }
if err := i.runsSomewhere(ctx, module); err != nil { if err := i.runsSomewhere(ctx, module); err != nil {
return err return false, err
} }
_, err = i.store.Pool().Exec(ctx, tag, err := i.store.Pool().Exec(ctx,
`insert into assignment (node, module) values ($1, $2) on conflict do nothing`, `insert into assignment (node, module) values ($1, $2) on conflict do nothing`,
node.ID, module) node.ID, module)
if err != nil && strings.Contains(err.Error(), "assignment_module_fkey") { if err != nil && strings.Contains(err.Error(), "assignment_module_fkey") {
return fmt.Errorf("%w: %s", ErrNoSuchModule, module) return false, fmt.Errorf("%w: %s", ErrNoSuchModule, module)
} }
return err if err != nil {
return false, err
}
return tag.RowsAffected() > 0, nil
} }
// Unassign takes a module off a node. // Unassign takes a module off a node.
+19 -9
View File
@@ -77,7 +77,7 @@ func TestAModuleAMachineIsRunningCannotBeForgotten(t *testing.T) {
if err := inv.RegisterModule(t.Context(), manifest("thing", nil, nil), Source{}); err != nil { if err := inv.RegisterModule(t.Context(), manifest("thing", nil, nil), Source{}); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if err := inv.Assign(t.Context(), "laptop", "thing"); err != nil { if _, err := inv.Assign(t.Context(), "laptop", "thing"); err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -104,7 +104,7 @@ func TestRemovingANodeTakesItsAssignments(t *testing.T) {
if err := inv.RegisterModule(t.Context(), manifest("thing", nil, nil), Source{}); err != nil { if err := inv.RegisterModule(t.Context(), manifest("thing", nil, nil), Source{}); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if err := inv.Assign(t.Context(), "laptop", "thing"); err != nil { if _, err := inv.Assign(t.Context(), "laptop", "thing"); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if _, err := inv.store.Pool().Exec(t.Context(), `delete from node where id = $1`, node.ID); err != nil { if _, err := inv.store.Pool().Exec(t.Context(), `delete from node where id = $1`, node.ID); err != nil {
@@ -136,14 +136,16 @@ func TestAssigningAModuleTheMeshDoesNotKnowIsRefused(t *testing.T) {
if _, err := inv.AddNode(t.Context(), "laptop"); err != nil { if _, err := inv.AddNode(t.Context(), "laptop"); err != nil {
t.Fatal(err) t.Fatal(err)
} }
err := inv.Assign(t.Context(), "laptop", "not-a-module") _, err := inv.Assign(t.Context(), "laptop", "not-a-module")
if !errors.Is(err, ErrNoSuchModule) { if !errors.Is(err, ErrNoSuchModule) {
t.Fatalf("assigning an unknown module gave %v", err) t.Fatalf("assigning an unknown module gave %v", err)
} }
} }
func TestAssigningTwiceIsNotAnError(t *testing.T) { func TestAssigningTwiceIsNotAnErrorAndSaysSo(t *testing.T) {
// It is a statement of what should be true, and it already is. // It is a statement of what should be true, and it already is — one assignment of a module
// per node is the rule (novox/hq ADR 0115), so a repeat is absorbed, audibly: the caller is
// told nothing was new.
inv := fresh(t) inv := fresh(t)
if _, err := inv.AddNode(t.Context(), "laptop"); err != nil { if _, err := inv.AddNode(t.Context(), "laptop"); err != nil {
t.Fatal(err) t.Fatal(err)
@@ -151,10 +153,18 @@ func TestAssigningTwiceIsNotAnError(t *testing.T) {
if err := inv.RegisterModule(t.Context(), manifest("thing", nil, nil), Source{}); err != nil { if err := inv.RegisterModule(t.Context(), manifest("thing", nil, nil), Source{}); err != nil {
t.Fatal(err) t.Fatal(err)
} }
for i := 0; i < 3; i++ { first, err := inv.Assign(t.Context(), "laptop", "thing")
if err := inv.Assign(t.Context(), "laptop", "thing"); err != nil { if err != nil || !first {
t.Fatalf("the first assignment is the new one; got fresh=%v err=%v", first, err)
}
for i := 0; i < 2; i++ {
again, err := inv.Assign(t.Context(), "laptop", "thing")
if err != nil {
t.Fatalf("assigning again failed: %v", err) t.Fatalf("assigning again failed: %v", err)
} }
if again {
t.Fatal("a repeat must say nothing was new")
}
} }
assigned, err := inv.Assigned(t.Context(), "laptop") assigned, err := inv.Assigned(t.Context(), "laptop")
if err != nil { if err != nil {
@@ -277,7 +287,7 @@ func TestBeingBehindNamesTheMachinesRunningTheOldOne(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
for _, n := range []string{"laptop", "workstation"} { for _, n := range []string{"laptop", "workstation"} {
if err := inv.Assign(t.Context(), n, "thing"); err != nil { if _, err := inv.Assign(t.Context(), n, "thing"); err != nil {
t.Fatal(err) t.Fatal(err)
} }
} }
@@ -451,7 +461,7 @@ func TestTheCatalogueSaysWhereEachModuleCameFromAndWhoRunsIt(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
for _, n := range []string{"workstation", "laptop"} { for _, n := range []string{"workstation", "laptop"} {
if err := inv.Assign(ctx, n, "shell"); err != nil { if _, err := inv.Assign(ctx, n, "shell"); err != nil {
t.Fatal(err) t.Fatal(err)
} }
} }
+1 -1
View File
@@ -212,7 +212,7 @@ func TestAModuleStillAssignedRefusesBeforeAnythingAboutWhatItHolds(t *testing.T)
if err := inv.SetSettings(ctx, "anchor", "step-ca", map[string]any{"a": 1}); err != nil { if err := inv.SetSettings(ctx, "anchor", "step-ca", map[string]any{"a": 1}); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if err := inv.Assign(ctx, "anchor", "step-ca"); err != nil { if _, err := inv.Assign(ctx, "anchor", "step-ca"); err != nil {
t.Fatal(err) t.Fatal(err)
} }
for _, forget := range []func() error{ for _, forget := range []func() error{
+2 -2
View File
@@ -221,7 +221,7 @@ func TestWhatAMachineNoLongerHoldsIsAvailableAgain(t *testing.T) {
func TestUnassigningReleasesTheModulesPorts(t *testing.T) { func TestUnassigningReleasesTheModulesPorts(t *testing.T) {
inv, node := aNodeWithModules(t, "mailu", "other-mail") inv, node := aNodeWithModules(t, "mailu", "other-mail")
ctx := t.Context() ctx := t.Context()
if err := inv.Assign(ctx, node, "mailu"); err != nil { if _, err := inv.Assign(ctx, node, "mailu"); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if _, err := inv.PortFor(ctx, node, "mailu", 25, true); err != nil { if _, err := inv.PortFor(ctx, node, "mailu", 25, true); err != nil {
@@ -230,7 +230,7 @@ func TestUnassigningReleasesTheModulesPorts(t *testing.T) {
if err := inv.Unassign(ctx, node, "mailu"); err != nil { if err := inv.Unassign(ctx, node, "mailu"); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if err := inv.Assign(ctx, node, "other-mail"); err != nil { if _, err := inv.Assign(ctx, node, "other-mail"); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if _, err := inv.PortFor(ctx, node, "other-mail", 25, true); err != nil { if _, err := inv.PortFor(ctx, node, "other-mail", 25, true); err != nil {
+1 -1
View File
@@ -350,7 +350,7 @@ func TestACredentialGoesWhenTheConsumerStopsAskingForIt(t *testing.T) {
}, Source{}); err != nil { }, Source{}); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if err := inv.Assign(ctx, "consumer", "meshboard"); err != nil { if _, err := inv.Assign(ctx, "consumer", "meshboard"); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if _, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider", ""); err != nil { if _, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider", ""); err != nil {