From c57daaf861d418b5fbb9be8629b4f3d9b744f372 Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Thu, 24 Sep 2026 10:35:14 +0800 Subject: [PATCH] feat(cli): add felis converge for fields a newer desired spec never delivered (tracker #1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Provisioning is create-if-absent, so a field the desired spec gained after an install (spec.rcon, spec.startup.healthHTTPPort, a derived env key) never reaches the existing login/lobby CR while every re-run of setup reports success — the reported 'configuration updates never reach an installed deployment' symptom. converge is the explicit pass: it fills exactly the zero-valued whitelist fields and the derived env (including a missing key, which refreshDerivedEnv deliberately never adds), and never overwrites a non-zero value. The timing stays with the operator because enabling RCON or the HTTP readiness gate on a pre-listener image would wedge that server in Starting until it was marked Failed. Tests: fills predated fields while operator edits survive / non-zero values left alone / absent + foreign + unset-image guards. usage table updated so the router-parity test passes; troubleshooting gains §12b. --- cmd/felis/converge.go | 73 +++++++++++++++ cmd/felis/converge_test.go | 185 +++++++++++++++++++++++++++++++++++++ cmd/felis/run.go | 2 + cmd/felis/systemservers.go | 159 ++++++++++++++++++++++++++----- docs/troubleshooting.md | 24 +++++ 5 files changed, 421 insertions(+), 22 deletions(-) create mode 100644 cmd/felis/converge.go create mode 100644 cmd/felis/converge_test.go diff --git a/cmd/felis/converge.go b/cmd/felis/converge.go new file mode 100644 index 0000000..0861fd5 --- /dev/null +++ b/cmd/felis/converge.go @@ -0,0 +1,73 @@ +package main + +import ( + "context" + "errors" + "flag" + "fmt" + "io" + "os" + "strings" + + "felis.lolicon.best/internal/config" + "felis.lolicon.best/internal/platform" +) + +// cmdConverge is the explicit convergence pass over already-installed system +// servers (#1). Provisioning is create-if-absent, so a field the desired spec +// gained after an install (spec.rcon, spec.startup.healthHTTPPort, a derived env +// key) never reaches the existing CR — and nothing says so. This command fills +// exactly those zero-value fields; see convergeSystemServers for the full contract +// and why it is a separate, operator-timed step rather than part of setup. +// +// It reads the same host config as setup (the control plane's felis.toml) and +// talks to the cluster with the local kubeconfig, so it must run as root on the +// control-plane host. +func cmdConverge(args []string, stdout, stderr io.Writer) int { + fs := flag.NewFlagSet("converge", flag.ContinueOnError) + fs.SetOutput(stderr) + cfgPath := fs.String("config", defaultSetupConfigPath, "path to felis.toml") + if err := fs.Parse(args); err != nil { + if errors.Is(err, flag.ErrHelp) { + return 0 + } + return 2 + } + if os.Geteuid() != 0 { + fmt.Fprintln(stderr, "felis converge: refused — converging needs the cluster credentials, so it must run as root (try: sudo felis converge)") + return 1 + } + + cfg, err := config.Load(*cfgPath) + if err != nil { + fmt.Fprintf(stderr, "felis converge: %v\n", err) + fmt.Fprintln(stderr, "If this host was never installed, run `sudo felis setup` first.") + return 1 + } + cl, err := buildSystemServerClient() + if err != nil { + fmt.Fprintf(stderr, "felis converge: %v\n", err) + return 1 + } + + controlNS := platform.DefaultControlNamespace + outcomes := convergeSystemServers(context.Background(), cl, cfg.K8s.Namespace, + cfg.Velocity.LoginImage, cfg.Velocity.LobbyImage, + platform.InternalAPIBaseURL(controlNS), cfg.Server.RootDomain, + defaultPanelHostname(cfg.Server.RootDomain, cfg.Auth.PanelHostname)) + + fmt.Fprintln(stdout, "felis converge: filling fields an installed system server predates (operator-set values are never overwritten):") + exit := 0 + for _, o := range outcomes { + switch { + case o.err != nil: + fmt.Fprintf(stdout, " - %s: ERROR %v\n", o.name, o.err) + exit = 1 + case len(o.changes) > 0: + fmt.Fprintf(stdout, " - %s: updated (%s)\n", o.name, strings.Join(o.changes, ", ")) + default: + fmt.Fprintf(stdout, " - %s: %s\n", o.name, o.skipped) + } + } + return exit +} diff --git a/cmd/felis/converge_test.go b/cmd/felis/converge_test.go new file mode 100644 index 0000000..b18ee0f --- /dev/null +++ b/cmd/felis/converge_test.go @@ -0,0 +1,185 @@ +package main + +import ( + "context" + "slices" + "strings" + "testing" + + "felis.lolicon.best/internal/apis/felis/v1alpha1" + "felis.lolicon.best/internal/naming" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +// converge is the explicit pass over an installed system server whose CR predates +// a field the desired spec has since gained (#1). It must fill exactly the +// zero-valued whitelist fields and the derived env, and must not touch anything a +// non-zero value already occupies — that is the operator's. +func TestConvergeSystemServersFillsPredatedFields(t *testing.T) { + scheme := newSystemServerScheme(t) + ctx := context.Background() + + // An old install: the lobby CR was created before the desired spec began + // rendering spec.rcon, and the login CR before the HTTP readiness gate existed. + // One derived env key is absent entirely (as if it were added later), and one + // hand-added env var plus a non-whitelisted spec field must survive. + lobby, err := lobbySystemServer("reg/lobby:1", "minecraft") + if err != nil { + t.Fatalf("build lobby: %v", err) + } + lobby.Spec.Rcon = v1alpha1.RconSpec{} + lobby.Spec.JavaMemory = "999Mi" + + login, err := loginSystemServer("reg/limbo:1", "minecraft", + "http://felis-api.felis.svc.cluster.local:8081", "mc.example.net", "console.mc.example.net") + if err != nil { + t.Fatalf("build login: %v", err) + } + login.Spec.Startup.HealthHTTPPort = 0 + kept := login.Spec.Env + login.Spec.Env = nil + for _, e := range kept { + if e.Name != envPanelHostname { + login.Spec.Env = append(login.Spec.Env, e) + } + } + login.Spec.Env = append(login.Spec.Env, v1alpha1.EnvVar{Name: "OPERATOR_TUNING", Value: "keep-me"}) + + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(lobby, login).Build() + outcomes := convergeSystemServers(ctx, cl, "minecraft", "reg/limbo:1", "reg/lobby:1", + "http://felis-api.felis.svc.cluster.local:8081", "mc.example.net", "console.mc.example.net") + + byName := map[string]systemServerOutcome{} + for _, o := range outcomes { + if o.err != nil { + t.Fatalf("%s: unexpected error: %v", o.name, o.err) + } + byName[o.name] = o + } + lobbyOut := byName[naming.SystemLobbyServer] + if len(lobbyOut.changes) != 1 || lobbyOut.changes[0] != "spec.rcon" { + t.Errorf("lobby changes = %v, want [spec.rcon] (only the zero-valued field)", lobbyOut.changes) + } + loginOut := byName[naming.SystemLoginServer] + if !slices.Contains(loginOut.changes, "spec.startup.healthHTTPPort") || !slices.Contains(loginOut.changes, "env "+envPanelHostname) { + t.Errorf("login changes = %v, want the health port plus the missing derived env key", loginOut.changes) + } + + var gotLobby v1alpha1.MinecraftServer + if err := cl.Get(ctx, client.ObjectKey{Namespace: "minecraft", Name: naming.SystemLobbyServer}, &gotLobby); err != nil { + t.Fatalf("get lobby: %v", err) + } + if !gotLobby.Spec.Rcon.Enabled || + gotLobby.Spec.Rcon.SecretRef.Name != naming.RconSecretName(naming.SystemLobbyServer) || + gotLobby.Spec.Rcon.SecretRef.Key != naming.RconSecretKey { + t.Errorf("lobby rcon = %+v, want the desired block with the %s secret", + gotLobby.Spec.Rcon, naming.RconSecretName(naming.SystemLobbyServer)) + } + if gotLobby.Spec.JavaMemory != "999Mi" { + t.Errorf("lobby javaMemory = %q, want 999Mi — converge fills new fields, it does not rewrite the spec", gotLobby.Spec.JavaMemory) + } + + var gotLogin v1alpha1.MinecraftServer + if err := cl.Get(ctx, client.ObjectKey{Namespace: "minecraft", Name: naming.SystemLoginServer}, &gotLogin); err != nil { + t.Fatalf("get login: %v", err) + } + if gotLogin.Spec.Startup.HealthHTTPPort != felisLimboHealthPort { + t.Errorf("login healthHTTPPort = %d, want %d", gotLogin.Spec.Startup.HealthHTTPPort, felisLimboHealthPort) + } + env := map[string]string{} + for _, e := range gotLogin.Spec.Env { + env[e.Name] = e.Value + } + if env[envPanelHostname] != "console.mc.example.net" { + t.Errorf("%s was not added back: %q", envPanelHostname, env[envPanelHostname]) + } + if env["OPERATOR_TUNING"] != "keep-me" { + t.Error("a hand-added env var was dropped; converge only touches config-derived names") + } +} + +// A field already holding a non-zero value belongs to the operator: converge must +// report "already converged" and write nothing. +func TestConvergeSystemServersLeavesNonZeroFieldsAlone(t *testing.T) { + scheme := newSystemServerScheme(t) + ctx := context.Background() + + lobby, err := lobbySystemServer("reg/lobby:1", "minecraft") + if err != nil { + t.Fatalf("build lobby: %v", err) + } + lobby.Spec.Rcon = v1alpha1.RconSpec{ + Enabled: true, + SecretRef: v1alpha1.SecretKeyRef{Name: "operator-rotated", Key: "password"}, + } + login, err := loginSystemServer("reg/limbo:1", "minecraft", + "http://felis-api.felis.svc.cluster.local:8081", "mc.example.net", "console.mc.example.net") + if err != nil { + t.Fatalf("build login: %v", err) + } + + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(lobby, login).Build() + for _, o := range convergeSystemServers(ctx, cl, "minecraft", "reg/limbo:1", "reg/lobby:1", + "http://felis-api.felis.svc.cluster.local:8081", "mc.example.net", "console.mc.example.net") { + if o.err != nil { + t.Fatalf("%s: unexpected error: %v", o.name, o.err) + } + if len(o.changes) != 0 || o.skipped != "already converged" { + t.Errorf("%s outcome = %+v, want already converged with no writes", o.name, o) + } + } + var got v1alpha1.MinecraftServer + if err := cl.Get(ctx, client.ObjectKey{Namespace: "minecraft", Name: naming.SystemLobbyServer}, &got); err != nil { + t.Fatalf("get lobby: %v", err) + } + if got.Spec.Rcon.SecretRef.Name != "operator-rotated" { + t.Errorf("lobby rcon secretRef = %q — converge overwrote a field the operator had already set", + got.Spec.Rcon.SecretRef.Name) + } +} + +// Guards: an absent CR is reported (creation is setup's job), a foreign CR is +// refused rather than adopted, and an unset image skips like the provisioner does. +func TestConvergeSystemServersGuards(t *testing.T) { + scheme := newSystemServerScheme(t) + ctx := context.Background() + run := func(cl client.Client, loginImage, lobbyImage string) []systemServerOutcome { + return convergeSystemServers(ctx, cl, "minecraft", loginImage, lobbyImage, + "http://felis-api.felis.svc.cluster.local:8081", "mc.example.net", "console.mc.example.net") + } + + t.Run("absent CRs are reported, not created", func(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(scheme).Build() + for _, o := range run(cl, "reg/limbo:1", "reg/lobby:1") { + if o.err != nil { + t.Fatalf("%s: %v", o.name, o.err) + } + if o.created || !strings.Contains(o.skipped, "not present") { + t.Errorf("%s outcome = %+v, want a not-present skip", o.name, o) + } + } + }) + + t.Run("foreign CR is refused", func(t *testing.T) { + foreign := &v1alpha1.MinecraftServer{} + foreign.Name = naming.SystemLoginServer + foreign.Namespace = "minecraft" + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(foreign).Build() + out := run(cl, "reg/limbo:1", "") + if len(out) != 2 { + t.Fatalf("outcomes = %d, want 2", len(out)) + } + if out[0].err == nil || !strings.Contains(out[0].err.Error(), "not marked") { + t.Fatalf("login error = %v, want an unmarked-name refusal", out[0].err) + } + }) + + t.Run("unset image skips", func(t *testing.T) { + cl := fake.NewClientBuilder().WithScheme(scheme).Build() + out := run(cl, "", "reg/lobby:1") + if out[0].skipped != "image not configured" { + t.Errorf("login skipped = %q, want %q", out[0].skipped, "image not configured") + } + }) +} diff --git a/cmd/felis/run.go b/cmd/felis/run.go index dc56de6..d0906e7 100644 --- a/cmd/felis/run.go +++ b/cmd/felis/run.go @@ -23,6 +23,7 @@ Commands: manifests Render the control-plane RBAC + NetworkPolicy install bundle as YAML apply Create a MinecraftServer CRD (direct K8s write; use -f server.json) setup Run host bootstrap + first-run setup console (TUI; requires root/sudo) + converge Fill in fields a newer desired spec added to already-installed system servers version Print the build stamp of this binary update Report which platform components have updates available breakGlass Open the local break-glass emergency console (TUI; requires root/sudo) @@ -52,6 +53,7 @@ var commands = map[string]func(args []string, stdout, stderr io.Writer) int{ "manifests": cmdManifests, "apply": cmdApply, "setup": cmdSetup, + "converge": cmdConverge, "breakGlass": cmdBreakGlass, "bootstrap-assets": cmdBootstrapAssets, "init-forwarding": cmdInitForwarding, diff --git a/cmd/felis/systemservers.go b/cmd/felis/systemservers.go index ebd80be..82db700 100644 --- a/cmd/felis/systemservers.go +++ b/cmd/felis/systemservers.go @@ -256,11 +256,31 @@ func buildSystemServerClient() (client.Client, error) { // setup can report it without the provisioner deciding on the output format. type systemServerOutcome struct { name string - created bool // true = we created it this run - updated bool // true = we refreshed an existing replica from the source - available bool // true = the required object now exists - skipped string // non-empty = why it was skipped (image unset / already exists) - err error // non-nil = create failed + created bool // true = we created it this run + updated bool // true = we refreshed an existing replica from the source + available bool // true = the required object now exists + skipped string // non-empty = why it was skipped (image unset / already exists) + err error // non-nil = create failed + changes []string // converge only: the fields this pass filled +} + +// systemServerPlan is one system service in the provisioner's table: its name, +// the image config gives it, and the pure builder for its desired CR. +type systemServerPlan struct { + name string + image string + build func(image, namespace string) (*v1alpha1.MinecraftServer, error) +} + +// systemServerPlans is the single description of the login+lobby pair, shared by +// ensureSystemServers (create-if-absent) and convergeSystemServers (field fill). +func systemServerPlans(loginImage, lobbyImage, apiBaseURL, rootDomain, panelHostname string) []systemServerPlan { + return []systemServerPlan{ + {name: naming.SystemLoginServer, image: loginImage, build: func(image, ns string) (*v1alpha1.MinecraftServer, error) { + return loginSystemServer(image, ns, apiBaseURL, rootDomain, panelHostname) + }}, + {name: naming.SystemLobbyServer, image: lobbyImage, build: lobbySystemServer}, + } } // ensureSystemServers idempotently creates the login and lobby system services. @@ -271,17 +291,7 @@ type systemServerOutcome struct { // K8s client and namespace; this function performs no signal-handler or client // setup of its own. func ensureSystemServers(ctx context.Context, cl client.Client, namespace, loginImage, lobbyImage, apiBaseURL, rootDomain, panelHostname string) []systemServerOutcome { - type plan struct { - name string - image string - build func(image, namespace string) (*v1alpha1.MinecraftServer, error) - } - plans := []plan{ - {name: naming.SystemLoginServer, image: loginImage, build: func(image, ns string) (*v1alpha1.MinecraftServer, error) { - return loginSystemServer(image, ns, apiBaseURL, rootDomain, panelHostname) - }}, - {name: naming.SystemLobbyServer, image: lobbyImage, build: lobbySystemServer}, - } + plans := systemServerPlans(loginImage, lobbyImage, apiBaseURL, rootDomain, panelHostname) outcomes := make([]systemServerOutcome, 0, len(plans)) for _, p := range plans { @@ -381,12 +391,7 @@ var derivedSystemEnv = map[string]bool{ // deliberate removal is indistinguishable from drift and re-adding it would fight the // operator every run. func refreshDerivedEnv(ctx context.Context, cl client.Client, existing, desired *v1alpha1.MinecraftServer) (bool, error) { - want := make(map[string]string, len(derivedSystemEnv)) - for _, e := range desired.Spec.Env { - if derivedSystemEnv[e.Name] { - want[e.Name] = e.Value - } - } + want := derivedEnvWanted(desired) changed := false for i, e := range existing.Spec.Env { @@ -404,6 +409,116 @@ func refreshDerivedEnv(ctx context.Context, cl client.Client, existing, desired return true, nil } +// derivedEnvWanted maps the derived env keys of desired onto their values. +func derivedEnvWanted(desired *v1alpha1.MinecraftServer) map[string]string { + want := make(map[string]string, len(derivedSystemEnv)) + for _, e := range desired.Spec.Env { + if derivedSystemEnv[e.Name] { + want[e.Name] = e.Value + } + } + return want +} + +// convergeSystemServers is the explicit convergence pass over already-installed +// system servers (#1). ensureSystemServers is create-if-absent by design — an +// existing CR is left alone so a re-run cannot clobber an operator's edits — and +// that leaves no path for a field the DESIRED spec gained after the install: +// spec.rcon (the lobby's write channel), spec.startup.healthHTTPPort (the login +// gate's readiness probe), or a config-derived env key that did not exist yet. +// Such fields sit at their zero value forever while re-running setup reports +// success, which is exactly the reported "configuration updates never reach an +// installed deployment" symptom. +// +// This pass fills exactly those zero-value fields and the config-derived env keys, +// and nothing else: a field already holding a non-zero value is the operator's and +// is never overwritten. It is an explicit command rather than an implicit step of +// setup because some fills need an ordering only the operator knows — enabling +// RCON or the HTTP readiness gate on a server whose image predates the listener +// would hold that server in Starting until it was marked Failed. Rebuild (or +// upgrade) the images first, then run this. +func convergeSystemServers(ctx context.Context, cl client.Client, namespace, loginImage, lobbyImage, apiBaseURL, rootDomain, panelHostname string) []systemServerOutcome { + outcomes := make([]systemServerOutcome, 0, 2) + for _, p := range systemServerPlans(loginImage, lobbyImage, apiBaseURL, rootDomain, panelHostname) { + if p.image == "" { + outcomes = append(outcomes, systemServerOutcome{name: p.name, skipped: "image not configured"}) + continue + } + desired, err := p.build(p.image, namespace) + if err != nil { + outcomes = append(outcomes, systemServerOutcome{name: p.name, err: err}) + continue + } + + var existing v1alpha1.MinecraftServer + switch err := cl.Get(ctx, client.ObjectKeyFromObject(desired), &existing); { + case apierrors.IsNotFound(err): + outcomes = append(outcomes, systemServerOutcome{name: p.name, + skipped: "not present — run `sudo felis setup` first"}) + continue + case err != nil: + outcomes = append(outcomes, systemServerOutcome{name: p.name, err: err}) + continue + } + if existing.Labels[v1alpha1.LabelSystemRole] != p.name { + outcomes = append(outcomes, systemServerOutcome{name: p.name, err: fmt.Errorf( + "existing MinecraftServer %s/%s is not marked as the Felis %q system role; refusing to converge it", + namespace, p.name, p.name, + )}) + continue + } + + var changes []string + if existing.Spec.Rcon == (v1alpha1.RconSpec{}) && desired.Spec.Rcon != (v1alpha1.RconSpec{}) { + existing.Spec.Rcon = desired.Spec.Rcon + changes = append(changes, "spec.rcon") + } + if existing.Spec.Startup.HealthHTTPPort == 0 && desired.Spec.Startup.HealthHTTPPort != 0 { + existing.Spec.Startup.HealthHTTPPort = desired.Spec.Startup.HealthHTTPPort + changes = append(changes, "spec.startup.healthHTTPPort") + } + changes = append(changes, convergeDerivedEnv(&existing, desired)...) + + if len(changes) == 0 { + outcomes = append(outcomes, systemServerOutcome{name: p.name, available: true, skipped: "already converged"}) + continue + } + if err := cl.Update(ctx, &existing); err != nil { + outcomes = append(outcomes, systemServerOutcome{name: p.name, err: fmt.Errorf("converge %s: %w", p.name, err)}) + continue + } + outcomes = append(outcomes, systemServerOutcome{name: p.name, available: true, updated: true, changes: changes}) + } + return outcomes +} + +// convergeDerivedEnv makes the config-derived env match the desired values: a key +// whose value drifted is overwritten, and a key missing entirely is added. This is +// the wider half of the same explicit pass — refreshDerivedEnv's present-only loop +// can never introduce a NEW key, which is how a derived key added after an install +// never reached it at all. +func convergeDerivedEnv(existing, desired *v1alpha1.MinecraftServer) []string { + want := derivedEnvWanted(desired) + var changes []string + present := make(map[string]bool, len(existing.Spec.Env)) + for i := range existing.Spec.Env { + e := &existing.Spec.Env[i] + present[e.Name] = true + if v, ok := want[e.Name]; ok && v != e.Value { + e.Value = v + changes = append(changes, "env "+e.Name) + } + } + for _, e := range desired.Spec.Env { + if !derivedSystemEnv[e.Name] || present[e.Name] { + continue + } + existing.Spec.Env = append(existing.Spec.Env, e) + changes = append(changes, "env "+e.Name) + } + return changes +} + // The login gate is a hard prerequisite of the Owner bind, so setup waits for it // rather than racing it. The ceiling covers a cold image pull on a fresh node; // the poll is fast enough that a warm start feels immediate. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 529e57b..6a756aa 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -710,6 +710,30 @@ deadline for the whole start, applied on the RCON-probe branch. --- +## 12b. A newer field never reaches an already-installed system server (`felis converge`) + +Provisioning is create-if-absent: `felis setup` never rewrites an existing +`login`/`lobby` `MinecraftServer` beyond the config-derived env it owns, so a +field the desired spec gained after your install sits absent forever — this is +how a deployment ends up with a lobby that has no `spec.rcon` (a dead console +and an online-player count that is always 0) and a login gate without +`spec.startup.healthHTTPPort`. `felis converge` is the explicit pass that fills +exactly those zero-valued fields (and re-adds a derived env key that is +missing). It never overwrites a value that already holds one — an operator's +RCON secretRef or tuning survives. + +``` +sudo felis converge +``` + +Run it **after the images are in place**. Enabling RCON, or the HTTP readiness +gate, on a server whose image predates the listener would hold that server in +`Starting` until the operator marks it `Failed` — that ordering is the reason +this is a command you run rather than something setup does on every re-run. +System servers that are already current report `already converged`. + +--- + ## 13. World PVC survives after I deleted the MinecraftServer This is expected. The world PVC is a StatefulSet `VolumeClaimTemplate`. There is