diff --git a/cmd/felis/api.go b/cmd/felis/api.go index f5cd34f..2744a00 100644 --- a/cmd/felis/api.go +++ b/cmd/felis/api.go @@ -284,9 +284,10 @@ func cmdAPI(args []string, stdout, stderr io.Writer) int { rcfg = reaper.DefaultConfig() } + cluster := api.NewK8sCluster(cl, cfg.K8s.Namespace) a := &api.API{ Repo: repo, - Cluster: api.NewK8sCluster(cl, cfg.K8s.Namespace), + Cluster: cluster, Console: api.NewK8sConsole(cl, cfg.K8s.Namespace), Logs: api.NewK8sLogStreamer(clientset, cfg.K8s.Namespace), // Build-log stream (spec ยง16) is scoped to the BUILD namespace โ€” the same @@ -408,7 +409,7 @@ func cmdAPI(args []string, stdout, stderr io.Writer) int { // reconciles it, but this loop converges builds nobody is polling. go reconcileBuilds(ctx, builder, stderr) - if pruner := registryPruner(cfg, builder.Store, a.Cluster, stderr); pruner != nil { + if pruner := registryPruner(cfg, builder.Store, cluster, stderr); pruner != nil { go pruner.Loop(ctx, registryPruneInterval) } go reapRejectedContexts(ctx, submissions, stderr) @@ -683,13 +684,19 @@ type imageRefStore interface { type serverLister interface { ListServers(ctx context.Context) ([]api.ServerInfo, error) + PodImages(ctx context.Context) ([]string, error) } // inUseImageRefs lists every image reference the platform still depends on: the // whitelist (disabled rows too, an admin may enable them again), every server's -// spec, builds still running, and the images the control plane and the build -// Jobs run. Any source failing fails the whole list, so the pruner never decides -// on a partial view. +// spec, the images the game pods run, builds still running, and the images the +// control plane and the build Jobs run. Any source failing fails the whole list, +// so the pruner never decides on a partial view. +// +// The pods matter for the felis image: a running server keeps the one it started +// with across platform upgrades (operator.PodTemplateAnnotation), which after a +// few releases is no longer among the newest tags the pruner keeps anyway, and +// the pod needs it again whenever it is recreated. func inUseImageRefs(ctx context.Context, store imageRefStore, servers serverLister, static []string) ([]string, error) { refs := append([]string(nil), static...) images, err := store.ListImages(ctx) @@ -706,6 +713,11 @@ func inUseImageRefs(ctx context.Context, store imageRefStore, servers serverList for _, s := range srvs { refs = append(refs, s.Image) } + podImages, err := servers.PodImages(ctx) + if err != nil { + return nil, fmt.Errorf("game pods: %w", err) + } + refs = append(refs, podImages...) builds, err := store.ListUnfinishedBuilds(ctx) if err != nil { return nil, fmt.Errorf("running builds: %w", err) diff --git a/cmd/felis/api_test.go b/cmd/felis/api_test.go index f18c0b4..a9990ec 100644 --- a/cmd/felis/api_test.go +++ b/cmd/felis/api_test.go @@ -108,9 +108,14 @@ func (f fakeRefStore) ListUnfinishedBuilds(context.Context) ([]build.Build, erro return f.builds, nil } -type fakeServers []api.ServerInfo +type fakeServers struct { + list []api.ServerInfo + pods []string + podsErr error +} -func (f fakeServers) ListServers(context.Context) ([]api.ServerInfo, error) { return f, nil } +func (f fakeServers) ListServers(context.Context) ([]api.ServerInfo, error) { return f.list, nil } +func (f fakeServers) PodImages(context.Context) ([]string, error) { return f.pods, f.podsErr } // The registry pruner deletes whatever this list does not name, so every source of // a reference has to be in it, and a failing source must fail the list. @@ -120,7 +125,10 @@ func TestInUseImageRefsCoversEverySource(t *testing.T) { images: []build.Image{{ImageRef: reg + "modpacks/pack:*"}, {ImageRef: reg + "felis/paper:demo"}}, builds: []build.Build{{ImageRef: reg + "user-uploads/sub-9:latest"}}, } - servers := fakeServers{{Name: "s1", Image: reg + "felis/paper:demo@sha256:" + fmt.Sprintf("%064d", 1)}} + servers := fakeServers{ + list: []api.ServerInfo{{Name: "s1", Image: reg + "felis/paper:demo@sha256:" + fmt.Sprintf("%064d", 1)}}, + pods: []string{reg + "felis/felis:v1.0.0"}, + } got, err := inUseImageRefs(context.Background(), store, servers, []string{reg + "felis/felis:b60"}) if err != nil { t.Fatal(err) @@ -129,12 +137,19 @@ func TestInUseImageRefsCoversEverySource(t *testing.T) { reg + "felis/felis:b60", reg + "modpacks/pack:*", reg + "felis/paper:demo", reg + "felis/paper:demo@sha256:" + fmt.Sprintf("%064d", 1), + reg + "felis/felis:v1.0.0", reg + "user-uploads/sub-9:latest", } if fmt.Sprint(got) != fmt.Sprint(want) { t.Fatalf("refs = %v\nwant %v", got, want) } + servers.podsErr = errors.New("apiserver down") + if _, err := inUseImageRefs(context.Background(), store, servers, nil); err == nil { + t.Fatal("a failing pod list produced a reference list") + } + servers.podsErr = nil + store.err = errors.New("db down") if _, err := inUseImageRefs(context.Background(), store, servers, nil); err == nil { t.Fatal("a failing whitelist read produced a reference list") diff --git a/internal/api/k8scluster.go b/internal/api/k8scluster.go index 95507de..71bb068 100644 --- a/internal/api/k8scluster.go +++ b/internal/api/k8scluster.go @@ -67,6 +67,26 @@ func (k *K8sCluster) WorldVolumeExists(ctx context.Context, name string) (bool, return true, nil } +// PodImages lists the image of every container and init container of every pod +// in the namespace, for the registry pruner (cmd/felis inUseImageRefs). It is +// one list call on the pods:list grant the console already holds. +func (k *K8sCluster) PodImages(ctx context.Context) ([]string, error) { + var pods corev1.PodList + if err := k.c.List(ctx, &pods, client.InNamespace(k.namespace)); err != nil { + return nil, err + } + var images []string + for _, p := range pods.Items { + for _, c := range p.Spec.InitContainers { + images = append(images, c.Image) + } + for _, c := range p.Spec.Containers { + images = append(images, c.Image) + } + } + return images, nil +} + func (k *K8sCluster) GetBySubdomain(ctx context.Context, subdomain string) (*ServerInfo, error) { var list v1alpha1.MinecraftServerList if err := k.c.List(ctx, &list, client.InNamespace(k.namespace)); err != nil { diff --git a/internal/operator/reconciler.go b/internal/operator/reconciler.go index a85f856..e6ce7d9 100644 --- a/internal/operator/reconciler.go +++ b/internal/operator/reconciler.go @@ -10,6 +10,7 @@ import ( "crypto/rand" "crypto/sha256" "encoding/hex" + "encoding/json" "fmt" "time" @@ -61,6 +62,35 @@ func rconStamp(password []byte) string { return hex.EncodeToString(sum[:8]) } +// PodTemplateAnnotation stamps the StatefulSet with a fingerprint of the pod +// template the operator last wrote, taken with the felis image left out of the +// init containers. The installer tags that image by release, so every platform +// upgrade hands the operator a new one; rolling every running server onto it +// would restart each world under its players for an init step that has already +// run. While a server runs and the fingerprint still matches, its template is +// left as it is and the new image arrives with its next start (stop scales to +// zero, and a start writes the whole template). Any other change โ€” a spec edit, +// a new RCON password, a builder change in a new release โ€” moves the +// fingerprint and rolls the pod as before. +const PodTemplateAnnotation = "felis.lolicon.best/pod-template" + +// podTemplateStamp fingerprints tmpl for PodTemplateAnnotation. encoding/json +// writes map keys sorted, so the same template always hashes the same. +func podTemplateStamp(tmpl *corev1.PodTemplateSpec, felisImage string) (string, error) { + t := tmpl.DeepCopy() + for i := range t.Spec.InitContainers { + if felisImage != "" && t.Spec.InitContainers[i].Image == felisImage { + t.Spec.InitContainers[i].Image = "" + } + } + b, err := json.Marshal(t) + if err != nil { + return "", fmt.Errorf("fingerprint pod template: %w", err) + } + sum := sha256.Sum256(b) + return hex.EncodeToString(sum[:8]), nil +} + // Reconciler reconciles a MinecraftServer with its managed children. type Reconciler struct { client.Client @@ -536,19 +566,37 @@ func (r *Reconciler) rconPassword(ctx context.Context, server *v1alpha1.Minecraf // applyStatefulSet creates the StatefulSet or, if it exists, updates only its // mutable fields (StatefulSet selector/serviceName/volumeClaimTemplates are -// immutable and must not be re-sent). +// immutable and must not be re-sent). A running server whose template changed +// only in the felis image keeps its template (PodTemplateAnnotation). func (r *Reconciler) applyStatefulSet(ctx context.Context, desired *appsv1.StatefulSet) error { + stamp, err := podTemplateStamp(&desired.Spec.Template, r.FelisImage) + if err != nil { + return err + } + if desired.Annotations == nil { + desired.Annotations = map[string]string{} + } + desired.Annotations[PodTemplateAnnotation] = stamp + var existing appsv1.StatefulSet - err := r.Get(ctx, client.ObjectKeyFromObject(desired), &existing) + err = r.Get(ctx, client.ObjectKeyFromObject(desired), &existing) if apierrors.IsNotFound(err) { return r.Create(ctx, desired) } if err != nil { return err } + running := existing.Spec.Replicas != nil && *existing.Spec.Replicas > 0 existing.Labels = desired.Labels existing.Spec.Replicas = desired.Spec.Replicas + if running && existing.Annotations[PodTemplateAnnotation] == stamp { + return r.Update(ctx, &existing) + } existing.Spec.Template = desired.Spec.Template + if existing.Annotations == nil { + existing.Annotations = map[string]string{} + } + existing.Annotations[PodTemplateAnnotation] = stamp return r.Update(ctx, &existing) } diff --git a/internal/operator/reconciler_test.go b/internal/operator/reconciler_test.go index b30043e..0f8e401 100644 --- a/internal/operator/reconciler_test.go +++ b/internal/operator/reconciler_test.go @@ -1083,3 +1083,86 @@ func TestIdleAutoStopIsInertWithoutRcon(t *testing.T) { got, v1alpha1.DesiredRunning) } } + +const ( + felisV1 = "registry.felis.svc:5000/felis/felis:v1.0.0" + felisV2 = "registry.felis.svc:5000/felis/felis:v1.1.0" +) + +func initImages(t *testing.T, c client.Client, name string) []string { + t.Helper() + var images []string + for _, ic := range getSTS(t, c, name).Spec.Template.Spec.InitContainers { + images = append(images, ic.Image) + } + if len(images) == 0 { + t.Fatal("the StatefulSet runs no felis init container") + } + return images +} + +func expectInitImages(t *testing.T, c client.Client, want string) { + t.Helper() + for _, got := range initImages(t, c, "survival") { + if got != want { + t.Fatalf("init container image = %q, want %q", got, want) + } + } +} + +// TestFelisUpgradeLeavesARunningServerAlone: the installer moves the felis image +// on every release. A running server keeps the template it started with rather +// than restarting under its players; any other change still rolls it, and the +// roll carries the new image along. +func TestFelisUpgradeLeavesARunningServerAlone(t *testing.T) { + r, c := newReconciler(t, fakeProber{players: operator.PlayerCount{Online: 1, Max: 20, Known: true}}, runningServer(), rconSecret()) + r.FelisImage = felisV1 + reconcile(t, r, "survival") + markPodReady(t, c, "survival") + reconcile(t, r, "survival") + expectInitImages(t, c, felisV1) + stamp := getSTS(t, c, "survival").Annotations[operator.PodTemplateAnnotation] + if stamp == "" { + t.Fatal("the StatefulSet must carry the pod-template fingerprint") + } + + r.FelisImage = felisV2 + reconcile(t, r, "survival") + expectInitImages(t, c, felisV1) + if got := getSTS(t, c, "survival").Annotations[operator.PodTemplateAnnotation]; got != stamp { + t.Fatalf("fingerprint moved from %q to %q on an image-only change", stamp, got) + } + + server := getServer(t, c, "survival") + server.Spec.JavaMemory = "6G" + if err := c.Update(context.Background(), server); err != nil { + t.Fatalf("edit spec: %v", err) + } + reconcile(t, r, "survival") + expectInitImages(t, c, felisV2) + if got := getSTS(t, c, "survival").Annotations[operator.PodTemplateAnnotation]; got == stamp { + t.Fatal("a spec edit must move the fingerprint") + } +} + +// TestFelisUpgradeReachesAServerOnItsNextStart: a stopped server's next start +// writes the whole template, the new felis image included. +func TestFelisUpgradeReachesAServerOnItsNextStart(t *testing.T) { + r, c := newReconciler(t, fakeProber{players: operator.PlayerCount{Online: 1, Max: 20, Known: true}}, runningServer(), rconSecret()) + r.FelisImage = felisV1 + stopRunningServer(t, r, c) + markPodTerminated(t, c, "survival") + reconcile(t, r, "survival") + + r.FelisImage = felisV2 + server := getServer(t, c, "survival") + server.Spec.DesiredState = v1alpha1.DesiredRunning + if err := c.Update(context.Background(), server); err != nil { + t.Fatalf("flip desiredState: %v", err) + } + reconcile(t, r, "survival") + if sts := getSTS(t, c, "survival"); sts.Spec.Replicas == nil || *sts.Spec.Replicas != 1 { + t.Fatalf("replicas = %v, want 1 after start", sts.Spec.Replicas) + } + expectInitImages(t, c, felisV2) +}