Unverified Commit daf76022 authored by Lemon-miaow's avatar Lemon-miaow
Browse files

fix(cli): pin the reaper to its storage node; drop the stale uid-1000 note

Two things in the same surface. --reaper-node is the supported multi-node
answer: the rendered CronJob's pod gets a kubernetes.io/hostname selector, so
it reads the hostPath on the node that actually holds the worlds instead of
possibly scheduling where it is empty (naming a node without
--worlds-host-path is fail-loud). And the render note still told operators to
grant uid-1000 traverse / setfacl after #35 moved every world executor to
root+DAC_OVERRIDE — it now states that fact instead of the obsolete ritual.
parent a31eca65
Loading
Loading
Loading
Loading
+24 −13
Changes for cmd/felis/manifests.go: 24 added lines, 13 removed lines.
Original line number Diff line number Diff line
@@ -49,6 +49,7 @@ func cmdManifests(args []string, stdout, stderr io.Writer) int {
	backupPVC := fs.String("backup-pvc", "felis-backups", "name of the world-archive PVC this bundle renders in the Minecraft namespace and advertises to the backup/restore executors via FELIS_BACKUP_PVC (default: felis-backups; pass an empty value to render none, leaving backup/restore answering 503)")
	worldsHostPath := fs.String("worlds-host-path", "", "node directory the reaper reads worlds from: each world PVC resolves as <path>/<pvc>, or as the stock local-path directory <path>/<pv-name>_<ns>_<pvc-name> (k3s storage root: /var/lib/rancher/k3s/storage); enables the reaper CronJob (requires --archive-local-path and a non-empty --backup-pvc)")
	archiveLocalPath := fs.String("archive-local-path", "", "path the backup PVC is mounted at in the reaper CronJob; MUST equal felis.toml [archive] local_path")
	reaperNode := fs.String("reaper-node", "", "node that holds --worlds-host-path: pins the reaper CronJob's pod there via nodeSelector kubernetes.io/hostname (multi-node clusters need this, or the reaper may schedule where the hostPath is empty)")
	var velocityCIDRs multiFlag
	fs.Var(&velocityCIDRs, "velocity-cidr", "CIDR of a Velocity proxy host allowed to reach game port 25565 (repeatable, REQUIRED)")
	var packageCIDRs multiFlag
@@ -83,6 +84,14 @@ func cmdManifests(args []string, stdout, stderr io.Writer) int {
		fmt.Fprintf(stderr, "felis manifests: --panel-node-port must be in Kubernetes NodePort range 30000-32767 (got %d)\n", *panelNodePort)
		return 2
	}
	// The node pin exists only for the reaper's hostPath: naming a node without the
	// worlds root would be silently dropped (no CronJob renders), so fail loud like
	// the storage-trio check below.
	if *reaperNode != "" && *worldsHostPath == "" {
		fmt.Fprintln(stderr, "felis manifests: --reaper-node requires --worlds-host-path "+
			"(it pins the reaper CronJob, which renders only with the retention storage trio)")
		return 2
	}

	// Retention/reaper rendering is opt-in and needs a storage topology together:
	// where worlds live (to read+archive them), a backup PVC (to write archives
@@ -100,24 +109,25 @@ func cmdManifests(args []string, stdout, stderr io.Writer) int {
				"(the archive store; default felis-backups)")
			return 2
		}
		// The reaper WILL render. Three deployment preconditions this generator cannot
		// check would silently turn retention into a no-op (or a permission-denied
		// loop) if unmet — surface them as loudly as the fail-closed cases above, so
		// an operator is never left with a reaper that reaps nothing. (All three are
		// also in the WorldsHostPath flag/field docs, but nobody deploying from stdout
		// reads those.)
		// The reaper WILL render. Two deployment facts this generator cannot check
		// would silently turn retention into a no-op if unmet — surface them as
		// loudly as the fail-closed cases above, so an operator is never left with a
		// reaper that reaps nothing. (Both are also in the WorldsHostPath flag/field
		// docs, but nobody deploying from stdout reads those.)
		pin := "the CronJob sets NO nodeSelector: a single-node starter pins it to the worlds implicitly, but on a " +
			"multi-node cluster you MUST pass --reaper-node <name> (or add a nodeSelector) for the node holding the " +
			"worlds, or the reaper may schedule where the hostPath is empty"
		if *reaperNode != "" {
			pin = fmt.Sprintf("the CronJob is pinned to node %q via kubernetes.io/hostname — keep this pointed at the "+
				"node that actually holds the world volumes", *reaperNode)
		}
		fmt.Fprintf(stderr, "felis manifests: note: rendering the retention reaper CronJob (worlds hostPath %q). "+
			"Three preconditions are NOT verified here:\n"+
			"These points are NOT verified here:\n"+
			"  - the node's world volumes must actually live below %s: the reaper resolves a world as "+
			"%s/<pvc>, then as the stock local-path directory <path>/<pv-name>_<ns>_<pvc-name> (what k3s "+
			"writes under /var/lib/rancher/k3s/storage). Any other provisioner needs its volumes exposed as "+
			"<path>/<pvc>, or each candidate's archive fails and the world is preserved;\n"+
			"  - the reaper pod runs as uid 1000 and must be able to traverse %s (k3s ships its storage root "+
			"0700 root:root — the installer grants `setfacl -m u:1000:x` or o+x; a manual install must do the "+
			"same or every archive fails with permission denied and the world is preserved);\n"+
			"  - the CronJob sets NO nodeSelector: a single-node starter pins it to the worlds implicitly, but "+
			"on a multi-node cluster you MUST add a nodeSelector for the node holding the worlds, or the reaper "+
			"may schedule where the hostPath is empty.\n", *worldsHostPath, *worldsHostPath, *worldsHostPath, *worldsHostPath)
			"  - %s.\n", *worldsHostPath, *worldsHostPath, *worldsHostPath, pin)
	} else {
		fmt.Fprintln(stderr, "felis manifests: note: retention reaper CronJob not rendered "+
			"(pass --worlds-host-path and --archive-local-path — the archive PVC defaults to felis-backups — to enable it)")
@@ -134,6 +144,7 @@ func cmdManifests(args []string, stdout, stderr io.Writer) int {
		RegistryImage:      *registryImage,
		BackupPVC:          *backupPVC,
		WorldsHostPath:     *worldsHostPath,
		ReaperNode:         *reaperNode,
		ArchiveLocalPath:   *archiveLocalPath,
		VelocityCIDRs:      []string(velocityCIDRs),
		PackageSourceCIDRs: []string(packageCIDRs),
+38 −0
Changes for cmd/felis/manifests_test.go: 38 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -181,3 +181,41 @@ func TestManifestsRendersReaper(t *testing.T) {
		}
	}
}

// TestManifestsReaperNodePin: --reaper-node pins the rendered CronJob's pod via
// kubernetes.io/hostname and replaces the "no nodeSelector" hazard note with the
// pin confirmation; using it without the worlds root is a fail-loud 2.
func TestManifestsReaperNodePin(t *testing.T) {
	var out, errBuf bytes.Buffer
	code := run([]string{
		"manifests",
		"--felis-image", "registry.felis.svc:5000/felis:v1",
		"--velocity-cidr", "10.0.0.5/32",
		"--worlds-host-path", "/var/lib/felis/worlds",
		"--archive-local-path", "/backups",
		"--reaper-node", "node-a",
	}, &out, &errBuf)
	if code != 0 {
		t.Fatalf("exit code = %d, want 0; stderr=%q", code, errBuf.String())
	}
	for _, want := range []string{
		"kubernetes.io/hostname: node-a",
	} {
		if !strings.Contains(out.String(), want) {
			t.Errorf("pinned render missing %q", want)
		}
	}
	if !strings.Contains(errBuf.String(), "node-a") {
		t.Errorf("stderr must confirm the pin, got %q", errBuf.String())
	}

	var out2, err2 bytes.Buffer
	if code := run([]string{
		"manifests",
		"--felis-image", "registry.felis.svc:5000/felis:v1",
		"--velocity-cidr", "10.0.0.5/32",
		"--reaper-node", "node-a",
	}, &out2, &err2); code != 2 {
		t.Errorf("--reaper-node without --worlds-host-path: exit = %d, want 2", code)
	}
}
+9 −0
Changes for internal/platform/identities.go: 9 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -143,6 +143,15 @@ type Params struct {
	// The rendered CronJob is the correct K8s object; the actual tar depends on the
	// hosting node, which is not provable without a cluster.
	WorldsHostPath string
	// ReaperNode, when non-empty, pins the rendered reaper CronJob's pod to one node
	// via nodeSelector kubernetes.io/hostname — the multi-node answer to "the
	// hostPath root exists on every node but a world's directory lives on exactly
	// one". On a multi-node cluster the operator passes the node that holds the
	// world volumes (for the local-path starter: the node whose
	// /var/lib/rancher/k3s/storage carries them); leaving it empty keeps the
	// single-node behaviour (no selector). It is reaper-only and only meaningful
	// with WorldsHostPath set.
	ReaperNode string
	// ArchiveLocalPath is the path the backup PVC is mounted at inside the reaper
	// CronJob's pod, and MUST equal felis.toml's [archive] local_path. tarLocal writes
	// archive refs as absolute paths under [archive] local_path (internal/backup), and
+23 −11
Changes for internal/platform/workloads.go: 23 added lines, 11 removed lines.
Original line number Diff line number Diff line
@@ -38,11 +38,11 @@ import (
// (<pv-name>_<ns>_<pvc-name>, read from the live PVC), so pointing
// --worlds-host-path at /var/lib/rancher/k3s/storage works on a default install —
// see the WorldsHostPath field doc. Whether the tar finds a world still depends on
// the hosting node, and is not provable without a cluster. No nodeSelector is set:
// the single-node starter pins
// the worlds to one node implicitly; a multi-node deployment MUST add one (or the
// CronJob could schedule on a node where the hostPath is empty) — a hazard left on
// record here until multi-node retention is built.
// the hosting node, and is not provable without a cluster. No nodeSelector is set
// unless ReaperNode names one: the single-node starter pins the worlds to one node
// implicitly, while a multi-node deployment passes --reaper-node (rendered as a
// kubernetes.io/hostname selector) or the CronJob could schedule on a node where
// the hostPath is empty.
const (
	// configSecretName / serviceTokenSecretName are referenced BY NAME and NEVER
	// rendered into the bundle: felis.toml carries the database URL (a credential)
@@ -605,19 +605,31 @@ func reaperCronJob(p Params) *batchv1.CronJob {
					ActiveDeadlineSeconds: int64Ptr(reaperActiveDeadlineSeconds),
					Template: corev1.PodTemplateSpec{
						ObjectMeta: metav1.ObjectMeta{Labels: labels},
						Spec: corev1.PodSpec{
						Spec:       reaperPodSpec(p, container, volumes),
					},
				},
			},
		},
	}
}

// reaperPodSpec is the reaper Job's pod template. It lives apart from the CronJob
// literal only so the optional node pin is one visible branch: with ReaperNode
// set the pod carries a kubernetes.io/hostname selector, keeping the reaper on
// the node that actually holds the worlds hostPath on a multi-node cluster.
func reaperPodSpec(p Params, container corev1.Container, volumes []corev1.Volume) corev1.PodSpec {
	spec := corev1.PodSpec{
		ServiceAccountName: SAReaper,
		PriorityClassName:  controlPlanePriorityName,
		RestartPolicy:      corev1.RestartPolicyNever,
		SecurityContext:    reaperPodSecurityContext(),
		Containers:         []corev1.Container{container},
		Volumes:            volumes,
						},
					},
				},
			},
		},
	}
	if p.ReaperNode != "" {
		spec.NodeSelector = map[string]string{"kubernetes.io/hostname": p.ReaperNode}
	}
	return spec
}

// controlPlaneDeployment assembles a single-replica control-plane Deployment. The
+16 −0
Changes for internal/platform/workloads_test.go: 16 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -648,6 +648,22 @@ func TestReaperCronJob_Gating(t *testing.T) {
	}
}

// TestReaperCronJob_NodePin proves the optional multi-node pin: no selector by
// default (the single-node starter), and exactly the kubernetes.io/hostname
// selector when ReaperNode names the node holding the worlds hostPath.
func TestReaperCronJob_NodePin(t *testing.T) {
	ps, _ := cronPodSpec(t, reaperCronJob(reaperParams()))
	if ps.NodeSelector != nil {
		t.Errorf("NodeSelector = %v, want none without ReaperNode", ps.NodeSelector)
	}
	p := reaperParams()
	p.ReaperNode = "node-a"
	ps, _ = cronPodSpec(t, reaperCronJob(p))
	if got := ps.NodeSelector["kubernetes.io/hostname"]; got != "node-a" {
		t.Errorf("nodeSelector = %v, want kubernetes.io/hostname=node-a", ps.NodeSelector)
	}
}

// TestReaperCronJob_Shape pins the rendered CronJob: its scheduling guards, its
// run-as identity (felis-reaper WITH an auto-mounted token, because it legitimately
// calls the K8s API — unlike the weak Job/registry pods), the hardening, the