Merge pull request 'mesh-build-image: a node that cannot build here now stays healthy and says not eligible, so it never holds the delivery (issue 1126)' (#546) from fix/1126-ineligible-healthy into main
This commit was merged in pull request #546.
This commit is contained in:
@@ -16,12 +16,14 @@ credential and an environment built from scratch.
|
|||||||
|
|
||||||
The account's subordinate user and group ranges are not declared anywhere: the node-engine makes every `user` account
|
The account's subordinate user and group ranges are not declared anywhere: the node-engine makes every `user` account
|
||||||
with `useradd`, which allocates them. The module's health reads `/etc/subuid` and `/etc/subgid` and is unhealthy
|
with `useradd`, which allocates them. The module's health reads `/etc/subuid` and `/etc/subgid` and is unhealthy
|
||||||
without a range for the account, without buildah, when a user namespace cannot be made, or when `/dev/net/tun`
|
when the module cannot run: without a range for the account, without buildah, when a user namespace cannot be made,
|
||||||
cannot be opened (buildah makes a RUN line's network with pasta, which needs the tun driver; on 2026-10-28 a build node had the
|
or when the disk is under its floor. A node where `/dev/net/tun` cannot be opened stays **healthy** and is **not
|
||||||
device node and no driver, so every image build there failed with "Failed to open() /dev/net/tun: No such device"). The
|
eligible** (buildah makes a RUN line's network with pasta, which needs the tun driver; on 2026-10-28 a build node had
|
||||||
kernel module loads `tun` at boot. Whatever the health refuses, the node is **not eligible**: the seat's puller judges
|
the device node and no driver, so every image build there failed with "Failed to open() /dev/net/tun: No such
|
||||||
by a capacity of 0 with that reason (`capacity.<node>`, which `build_usage` shows), so the asks go to a node that can
|
device"): the seat's puller judges by a capacity of 0 with that reason (`capacity.<node>`, which `build_usage` shows),
|
||||||
build them; a look that passes again wakes the puller.
|
so the asks go to a node that can build them, and its health says "not eligible" with the cause, so a node that
|
||||||
|
cannot build here now never holds the module's delivery for every node. The kernel module loads `tun` at boot; a look
|
||||||
|
that passes again wakes the puller.
|
||||||
|
|
||||||
The Dockerfile's steps may use the network (ADR 0487 §6); nothing a step can read holds a key.
|
The Dockerfile's steps may use the network (ADR 0487 §6); nothing a step can read holds a key.
|
||||||
|
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package main
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"net/http"
|
"net/http"
|
||||||
"sync"
|
"sync"
|
||||||
@@ -97,11 +98,25 @@ func (m *Module) look(ctx context.Context) (Look, error) {
|
|||||||
return l, &buildstep.Refusal{Text: err.Error(), Have: l.Buildah}
|
return l, &buildstep.Refusal{Text: err.Error(), Have: l.Buildah}
|
||||||
}
|
}
|
||||||
if err := m.tunDevice(); err != nil {
|
if err := m.tunDevice(); err != nil {
|
||||||
return l, &buildstep.Refusal{Text: err.Error(), Have: l.Buildah}
|
return l, ¬Eligible{&buildstep.Refusal{Text: err.Error(), Have: l.Buildah}}
|
||||||
}
|
}
|
||||||
return l, nil
|
return l, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// notEligible is a refusal of a machine that runs the module fine but cannot build here now (no tun device: its asks go
|
||||||
|
// to another node). It is a *buildstep.Refusal for a step, and the health stays healthy for it (issue 1126): the
|
||||||
|
// assigner reads unhealthy as a failed delivery and holds the build for every node. What cannot run (buildah missing,
|
||||||
|
// no ranges, no namespace, no runtime directory, no room) stays unhealthy.
|
||||||
|
type notEligible struct{ *buildstep.Refusal }
|
||||||
|
|
||||||
|
func (e *notEligible) Unwrap() error { return e.Refusal }
|
||||||
|
|
||||||
|
// isNotEligible says whether err is a refusal that leaves the module healthy.
|
||||||
|
func isNotEligible(err error) bool {
|
||||||
|
var n *notEligible
|
||||||
|
return errors.As(err, &n)
|
||||||
|
}
|
||||||
|
|
||||||
// Check is the guard: nil when the machine can build, else the refusal. Every look is kept for Eligible, and a change
|
// Check is the guard: nil when the machine can build, else the refusal. Every look is kept for Eligible, and a change
|
||||||
// wakes the puller, so a node that can build again takes asks at once.
|
// wakes the puller, so a node that can build again takes asks at once.
|
||||||
func (m *Module) Check(ctx context.Context) error {
|
func (m *Module) Check(ctx context.Context) error {
|
||||||
@@ -120,6 +135,13 @@ func (m *Module) keep(err error) {
|
|||||||
changed := m.last != why
|
changed := m.last != why
|
||||||
m.last = why
|
m.last = why
|
||||||
m.lastMu.Unlock()
|
m.lastMu.Unlock()
|
||||||
|
if changed && m.Say != nil {
|
||||||
|
if why == "" {
|
||||||
|
m.Say("this machine is eligible to build images again")
|
||||||
|
} else {
|
||||||
|
m.Say("this machine takes no image build now: %s", why)
|
||||||
|
}
|
||||||
|
}
|
||||||
if changed && m.Held != nil {
|
if changed && m.Held != nil {
|
||||||
m.Held.Wake()
|
m.Held.Wake()
|
||||||
}
|
}
|
||||||
@@ -155,12 +177,16 @@ func (m *Module) Start(ctx context.Context) {
|
|||||||
func (m *Module) Health(ctx context.Context) health.Answer {
|
func (m *Module) Health(ctx context.Context) health.Answer {
|
||||||
l, err := m.look(ctx)
|
l, err := m.look(ctx)
|
||||||
m.keep(err)
|
m.keep(err)
|
||||||
if err != nil {
|
if err != nil && !isNotEligible(err) {
|
||||||
return health.Unhealthy(err.Error())
|
return health.Unhealthy(err.Error())
|
||||||
}
|
}
|
||||||
if err := m.room(); err != nil {
|
if err := m.room(); err != nil {
|
||||||
return health.Unhealthy(err.Error())
|
return health.Unhealthy(err.Error())
|
||||||
}
|
}
|
||||||
|
if err != nil {
|
||||||
|
// Not eligible: the module runs, it takes no image ask here now, and says why; the capacity record says it too.
|
||||||
|
return health.Healthy("not eligible to build images, so it takes no image ask: " + err.Error())
|
||||||
|
}
|
||||||
return health.Healthy(fmt.Sprintf("buildah %s installed; %s holds %d subordinate uids and %d gids; a user namespace can be made; %s opens", l.Buildah, l.Account, l.UIDs, l.GIDs, m.tunPath()))
|
return health.Healthy(fmt.Sprintf("buildah %s installed; %s holds %d subordinate uids and %d gids; a user namespace can be made; %s opens", l.Buildah, l.Account, l.UIDs, l.GIDs, m.tunPath()))
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -168,7 +194,7 @@ func (m *Module) Health(ctx context.Context) health.Answer {
|
|||||||
func (m *Module) Status(ctx context.Context) map[string]any {
|
func (m *Module) Status(ctx context.Context) map[string]any {
|
||||||
l, err := m.look(ctx)
|
l, err := m.look(ctx)
|
||||||
out := map[string]any{"node": m.Node, "account": m.Account, "buildah": l.Buildah, "subuids": l.UIDs, "subgids": l.GIDs, "refusing": err != nil,
|
out := map[string]any{"node": m.Node, "account": m.Account, "buildah": l.Buildah, "subuids": l.UIDs, "subgids": l.GIDs, "refusing": err != nil,
|
||||||
"cpu": m.cpuStatus()}
|
"eligible": err == nil, "cpu": m.cpuStatus()}
|
||||||
if err != nil {
|
if err != nil {
|
||||||
out["why"] = err.Error()
|
out["why"] = err.Error()
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2,7 +2,9 @@ package main
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
@@ -54,7 +56,6 @@ func TestHealthIsUnhealthyWithoutWhatABuildNeeds(t *testing.T) {
|
|||||||
}{
|
}{
|
||||||
{"subuid", func(m *Module, p string) { m.SubUIDFile = p }, []string{"/subuid", "mesh-build-image"}},
|
{"subuid", func(m *Module, p string) { m.SubUIDFile = p }, []string{"/subuid", "mesh-build-image"}},
|
||||||
{"subgid", func(m *Module, p string) { m.SubGIDFile = p }, []string{"/subgid", "mesh-build-image"}},
|
{"subgid", func(m *Module, p string) { m.SubGIDFile = p }, []string{"/subgid", "mesh-build-image"}},
|
||||||
{"tun", func(m *Module, p string) { m.Tun = p + "/dev/net/tun" }, []string{"not eligible: ", "/dev/net/tun cannot be opened", "tun"}},
|
|
||||||
} {
|
} {
|
||||||
f := newFake(t)
|
f := newFake(t)
|
||||||
m := moduleWith(t, f, newStates())
|
m := moduleWith(t, f, newStates())
|
||||||
@@ -74,6 +75,61 @@ func TestHealthIsUnhealthyWithoutWhatABuildNeeds(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// 1126: a node that cannot build here now (no tun device) stays healthy, saying "not eligible" and why; the assigner reads
|
||||||
|
// unhealthy as a failed delivery and held the new build for every node. The eligibility, the status and the log say it too.
|
||||||
|
func TestANodeWithoutTunIsHealthyAndSaysItIsNotEligible(t *testing.T) {
|
||||||
|
f := newFake(t)
|
||||||
|
m := moduleWith(t, f, newStates())
|
||||||
|
var said []string
|
||||||
|
m.Say = func(format string, a ...any) { said = append(said, fmt.Sprintf(format, a...)) }
|
||||||
|
m.Tun = t.TempDir() + "/dev/net/tun"
|
||||||
|
a := m.Health(context.Background())
|
||||||
|
if a.State != health.StateHealthy || !a.Healthy {
|
||||||
|
t.Fatalf("health = %+v, want healthy", a)
|
||||||
|
}
|
||||||
|
for _, want := range []string{"not eligible", "/dev/net/tun cannot be opened", "tun"} {
|
||||||
|
if !strings.Contains(a.Why, want) {
|
||||||
|
t.Errorf("health says %q, lacks %q", a.Why, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if got := m.Eligible(); !strings.Contains(got, "not eligible: ") {
|
||||||
|
t.Errorf("eligible says %q", got)
|
||||||
|
}
|
||||||
|
if s := m.Status(context.Background()); s["eligible"] != false || s["refusing"] != true {
|
||||||
|
t.Errorf("status = %v, want not eligible", s)
|
||||||
|
}
|
||||||
|
// One log line for the change, none for the same look again; one when it can build again.
|
||||||
|
m.Health(context.Background())
|
||||||
|
if len(said) != 1 || !strings.Contains(said[0], "takes no image build now") {
|
||||||
|
t.Errorf("said = %q, want one line for the change", said)
|
||||||
|
}
|
||||||
|
m.Tun = filepath.Join(t.TempDir(), "tun")
|
||||||
|
os.WriteFile(m.Tun, nil, 0o644)
|
||||||
|
if a := m.Health(context.Background()); !a.Healthy || strings.Contains(a.Why, "not eligible") {
|
||||||
|
t.Errorf("health = %+v, want plain healthy", a)
|
||||||
|
}
|
||||||
|
if len(said) != 2 || !strings.Contains(said[1], "eligible to build images again") {
|
||||||
|
t.Errorf("said = %q, want the return said", said)
|
||||||
|
}
|
||||||
|
// A step still ends refused with the cause named.
|
||||||
|
m.Tun = t.TempDir() + "/none"
|
||||||
|
if err := m.Check(context.Background()); err == nil || !strings.Contains(err.Error(), "not eligible") {
|
||||||
|
t.Errorf("check = %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// 1126: what cannot run stays unhealthy even beside a missing tun: buildah missing is not "not eligible".
|
||||||
|
func TestWithoutBuildahTheNodeIsUnhealthyNotIneligible(t *testing.T) {
|
||||||
|
m := moduleWith(t, newFake(t), newStates())
|
||||||
|
m.Tun = t.TempDir() + "/none"
|
||||||
|
m.Run = func(ctx context.Context, env []string, name string, args ...string) forgekey.Ran {
|
||||||
|
return forgekey.Ran{Status: 127, Err: "exec: \"buildah\": executable file not found in $PATH"}
|
||||||
|
}
|
||||||
|
if a := m.Health(context.Background()); a.Healthy {
|
||||||
|
t.Errorf("health = %+v, want unhealthy", a)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestHealthIsUnhealthyWhenANamespaceCannotBeMade(t *testing.T) {
|
func TestHealthIsUnhealthyWhenANamespaceCannotBeMade(t *testing.T) {
|
||||||
f := newFake(t)
|
f := newFake(t)
|
||||||
m := moduleWith(t, f, newStates())
|
m := moduleWith(t, f, newStates())
|
||||||
|
|||||||
@@ -54,7 +54,7 @@ func (m *Module) Tools(steps []StepTool) []stdio.Tool {
|
|||||||
Run: func(map[string]any) (any, error) { return m.Status(m.Ctx), nil }},
|
Run: func(map[string]any) (any, error) { return m.Status(m.Ctx), nil }},
|
||||||
stdio.Tool{Name: HealthTool,
|
stdio.Tool{Name: HealthTool,
|
||||||
Description: "How the image build module is: unhealthy while buildah cannot be run, while its account holds no " +
|
Description: "How the image build module is: unhealthy while buildah cannot be run, while its account holds no " +
|
||||||
"subordinate user or group range, or while a user namespace cannot be made, saying which; healthy otherwise. Read-only.",
|
"subordinate user or group range, or while a user namespace cannot be made, saying which; healthy otherwise. A machine that cannot build here now (no tun device) stays healthy and says in its words \"not eligible\", with the cause; it takes no image ask meanwhile (issue 1126). Read-only.",
|
||||||
Run: func(map[string]any) (any, error) { return m.Health(m.Ctx), nil }},
|
Run: func(map[string]any) (any, error) { return m.Health(m.Ctx), nil }},
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user