From 40ae7d3dfd3f615e253107f32ba493c0a211e337 Mon Sep 17 00:00:00 2001 From: Baylem Date: Tue, 10 Nov 2026 21:06:21 -0500 Subject: [PATCH 01/10] feat(operator): enforce game worker placement and tenant resource bounds Signed-off-by: Baylem --- .../templates/game-worker-admission.yaml | 59 +++++++ charts/gameplane/templates/operator.yaml | 3 + charts/gameplane/templates/tenant-quotas.yaml | 29 ++++ .../gameplane/values-production.example.yaml | 19 ++ charts/gameplane/values.yaml | 6 + docs/production-placement.md | 62 ++++++- operator/cmd/main.go | 8 + .../controller/game_workload_policy.go | 92 ++++++++++ .../controller/game_workload_policy_test.go | 151 ++++++++++++++++ .../controller/gameserver_controller.go | 31 +++- .../gameserver_template_revision.go | 14 ++ test/e2e/buckets.sh | 1 + test/e2e/game_worker_pool_e2e_test.go | 162 ++++++++++++++++++ test/e2e/specs.md | 6 + test/e2e/suite-components.json | 1 + 15 files changed, 634 insertions(+), 10 deletions(-) create mode 100644 charts/gameplane/templates/game-worker-admission.yaml create mode 100644 charts/gameplane/templates/tenant-quotas.yaml create mode 100644 operator/internal/controller/game_workload_policy.go create mode 100644 operator/internal/controller/game_workload_policy_test.go create mode 100644 test/e2e/game_worker_pool_e2e_test.go diff --git a/charts/gameplane/templates/game-worker-admission.yaml b/charts/gameplane/templates/game-worker-admission.yaml new file mode 100644 index 000000000..d26e1d886 --- /dev/null +++ b/charts/gameplane/templates/game-worker-admission.yaml @@ -0,0 +1,59 @@ +{{- if and (eq (include "gameplane.operatorEnabled" .) "true") (ne .Values.operator.gameWorkloadPolicy nil) }} +{{- $p := .Values.operator.gameWorkloadPolicy }} +{{- $key := required "gameWorkloadPolicy.poolLabel is required" $p.poolLabel | toJson }} +{{- $value := required "gameWorkloadPolicy.poolValue is required" $p.poolValue | toJson }} +{{- $name := printf "%s.%s.game-workers" .Release.Name .Release.Namespace }} +# An existing StatefulSet can recreate its Pod before the operator sees a +# changed policy. Enforce the scheduler constraint and bounds on that path too. +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicy +metadata: + name: {{ $name }} +spec: + failurePolicy: Fail + matchConstraints: + resourceRules: + - apiGroups: [""] + apiVersions: [v1] + operations: [CREATE] + resources: [pods] + - apiGroups: [""] + apiVersions: [v1] + operations: [UPDATE] + resources: [pods/resize] + validations: + - expression: >- + has(object.spec.nodeSelector) && {{ $key }} in object.spec.nodeSelector && + object.spec.nodeSelector[{{ $key }}] == {{ $value }} + message: "game pods must select the configured game worker pool" + - expression: >- + (request.operation != "CREATE" || !has(object.spec.nodeName) || object.spec.nodeName == "") && + (!has(object.spec.tolerations) || object.spec.tolerations.all(t, has(t.key) && t.key != "")) + message: "game pods must use the scheduler and may not tolerate every taint" + - expression: >- + object.spec.containers.exists(c, c.name == "game" && + has(c.resources) && has(c.resources.requests) && has(c.resources.limits) && + "cpu" in c.resources.requests && "memory" in c.resources.requests && "memory" in c.resources.limits && + quantity(string(c.resources.requests["cpu"])).compareTo(quantity({{ required "minCPURequest is required" $p.minCPURequest | toString | toJson }})) >= 0 && + quantity(string(c.resources.requests["cpu"])).compareTo(quantity({{ required "maxCPURequest is required" $p.maxCPURequest | toString | toJson }})) <= 0 && + quantity(string(c.resources.requests["memory"])).compareTo(quantity({{ required "minMemoryRequest is required" $p.minMemoryRequest | toString | toJson }})) >= 0 && + quantity(string(c.resources.limits["memory"])).compareTo(quantity({{ required "maxMemoryLimit is required" $p.maxMemoryLimit | toString | toJson }})) <= 0) + message: "game containers must meet the configured CPU request and memory request/limit bounds" +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicyBinding +metadata: + name: {{ $name }} +spec: + policyName: {{ $name }} + validationActions: [Deny] + matchResources: + namespaceSelector: + matchExpressions: + - key: kubernetes.io/metadata.name + operator: In + values: {{ include "gameplane.operatorGameNamespaces" . }} + objectSelector: + matchLabels: + app.kubernetes.io/name: gameplane-game +{{- end }} diff --git a/charts/gameplane/templates/operator.yaml b/charts/gameplane/templates/operator.yaml index 8e9427327..25e8edecb 100644 --- a/charts/gameplane/templates/operator.yaml +++ b/charts/gameplane/templates/operator.yaml @@ -377,6 +377,9 @@ spec: - --agent-client-key=/etc/gameplane/agent-client/tls.key - --agent-ca-secret-name={{ .Values.api.agentMTLS.caSecretRef.name }} - --game-namespaces={{ join "," $gameNamespaces }} + {{- if ne .Values.operator.gameWorkloadPolicy nil }} + - {{ printf "--game-workload-policy=%s" (mustToJson .Values.operator.gameWorkloadPolicy) | quote }} + {{- end }} - --control-plane-namespace={{ .Values.operator.controlPlaneNamespace | default .Release.Namespace }} - --agent-ca-secret-namespace={{ .Values.operator.agentCASecretNamespace | default .Release.Namespace }} - --module-namespace={{ .Values.operator.moduleNamespace | default .Release.Namespace }} diff --git a/charts/gameplane/templates/tenant-quotas.yaml b/charts/gameplane/templates/tenant-quotas.yaml new file mode 100644 index 000000000..8fe11e6bb --- /dev/null +++ b/charts/gameplane/templates/tenant-quotas.yaml @@ -0,0 +1,29 @@ +{{- if eq (include "gameplane.operatorEnabled" .) "true" }} +{{- $managed := include "gameplane.operatorGameNamespaces" . | fromJsonArray }} +{{- range $ns, $config := .Values.operator.tenantQuotas }} +{{- if not (has $ns $managed) }}{{ fail (printf "operator.tenantQuotas namespace %s must be a managed game namespace" $ns) }}{{ end }} +--- +apiVersion: v1 +kind: ResourceQuota +metadata: + name: {{ $.Release.Name }}-tenant-budget + namespace: {{ $ns }} + labels: {{- include "gameplane.labels" $ | nindent 4 }} +spec: + hard: {{- required "each operator.tenantQuotas entry requires hard resources" $config.hard | toYaml | nindent 4 }} +{{- with $config.defaultContainerMemoryLimit }} +--- +apiVersion: v1 +kind: LimitRange +metadata: + name: {{ $.Release.Name }}-container-defaults + namespace: {{ $ns }} + labels: {{- include "gameplane.labels" $ | nindent 4 }} +spec: + limits: + - type: Container + default: + memory: {{ . | quote }} +{{- end }} +{{- end }} +{{- end }} diff --git a/charts/gameplane/values-production.example.yaml b/charts/gameplane/values-production.example.yaml index 229448ac3..8671465d2 100644 --- a/charts/gameplane/values-production.example.yaml +++ b/charts/gameplane/values-production.example.yaml @@ -2,6 +2,25 @@ # applying this profile; see docs/production-placement.md. Never remove or # tolerate control-plane taints just to satisfy these selectors. operator: + gameWorkloadPolicy: + poolLabel: gameplane.local/pool + poolValue: games + minCPURequest: 100m + maxCPURequest: "4" + minMemoryRequest: 128Mi + maxMemoryLimit: 8Gi + # Budget for the default games namespace. Use a separate managed namespace + # and independently sized quota for each tenant in a shared installation. + tenantQuotas: + gameplane-games: + defaultContainerMemoryLimit: 512Mi + hard: + requests.cpu: "12" + requests.memory: 24Gi + limits.memory: 32Gi + pods: "24" + persistentvolumeclaims: "12" + requests.storage: 200Gi replicas: 2 leaderElect: true nodeSelector: &managementNodes { gameplane.local/pool: management } diff --git a/charts/gameplane/values.yaml b/charts/gameplane/values.yaml index 503c95481..94dff5beb 100644 --- a/charts/gameplane/values.yaml +++ b/charts/gameplane/values.yaml @@ -71,6 +71,12 @@ operator: # exist. The chart grants namespace-local workload permissions and includes # their Secrets in the operator cache. A Helm upgrade restarts the operator. gameNamespaces: [] + # Optional game worker label/NoSchedule taint and per-game tenant bounds. + # See values-production.example.yaml and docs/production-placement.md. + gameWorkloadPolicy: null + # ResourceQuota by managed game namespace. Each entry accepts hard and + # an optional defaultContainerMemoryLimit for containers without a limit. + tenantQuotas: {} # Empty uses the release namespace. Non-default namespaces must already exist; # provision the referenced Secrets there before upgrading. These settings move # the operator's API reads, not the API pod or its mounted mTLS client material. diff --git a/docs/production-placement.md b/docs/production-placement.md index 9bdd26dab..b6a6963d4 100644 --- a/docs/production-placement.md +++ b/docs/production-placement.md @@ -53,8 +53,9 @@ requests or limits, replacing the install agent object wholesale. An image-only agent override keeps install defaults. Kubernetes may omit empty maps in a template, so use explicit zero CPU/memory requests for a per-template zero reservation, or the install-level empty object to clear all defaults. Game -container resources continue to use the existing template defaults and complete -`GameServer.spec.resources` override. Game placement fields are unchanged. +container requests and limits merge per resource. For example, a server +overriding only `requests.cpu` inherits both memory values and any template +CPU limit. A tier with no CPU limit in either object keeps CPU bursting. Changing resources changes generated Pod templates. Existing game StatefulSets, sentinels and tunnels roll according to their normal reconciliation policy; @@ -106,7 +107,32 @@ kubectl label node game-a game-b gameplane.local/pool=games kubectl taint node game-a game-b gameplane.local/pool=games:NoSchedule ``` -Use existing GameServer placement fields for game workers: +The production example enables `operator.gameWorkloadPolicy`. The operator +adds the chosen pool selector and its exact `Equal` / `NoSchedule` toleration +to each game Pod. A server can narrow placement with additional node labels +or affinity. A conflicting pool selector or an extra toleration produces a +failed Ready condition. A pool with no schedulable workers leaves games Pending. +Keep permission to change pool labels and taints with cluster administrators. +With the NodeRestriction admission plugin, a label key such as +`gameplane.node-restriction.kubernetes.io/pool` also prevents a kubelet from +labeling itself as a game worker. Set the configured key and worker taint to match. + +The policy validates effective game resources before workload creation and +template adoption. The CPU request must fall between the configured bounds. +The memory request must meet its floor, and the memory limit must cover the +request and fit the configured ceiling. Set a CPU limit in the template or +server only for tiers that require one. Identify tiers by their measured +reservations and ceilings, since sidecar requests also influence Pod QoS. + +The chart installs a Kubernetes v1 ValidatingAdmissionPolicy with the profile. +It checks newly created game Pods and in-place resizes in the managed game +namespaces, including replacements from pre-upgrade StatefulSets. Existing +running Pods keep their allocation until replaced. Before enabling the profile, +inventory live requests, limits, selectors, tolerations and PVC topology. +Correct incompatible templates or server overrides and roll games during a +maintenance window. A `null` policy keeps administrator enforcement disabled. + +Without a policy, a server can request its own placement: ```yaml spec: @@ -127,6 +153,23 @@ Check API/receiver PVC node affinity before selecting management workers, and game PVC topology before backups or moving game pods. Restic scheduling and source preparation are separate from management placement. +## Tenant namespace quotas + +Provision a namespace for each tenant and add it to the operator's managed +game namespaces. Configure API/RBAC grants for the same tenant boundary. +Set `operator.tenantQuotas..hard` for `requests.cpu`, +`requests.memory`, `limits.memory`, `pods`, `persistentvolumeclaims` and +`requests.storage`. The production values illustrate a budget for the default +games namespace. Replace that key if you change `gamesNamespace`. Tenants +sharing a namespace share its quota. + +When using `limits.memory`, give supporting containers a memory limit. +`defaultContainerMemoryLimit` installs a LimitRange that supplies a default +for containers without an explicit memory limit. Size it for backup workers, +sidecars and initializers as well as games. A quota denial appears in +StatefulSet or Job events. Reserve quota for active backups, restore PVCs, +surge Pods and starts. Leave `limits.cpu` out of quotas for uncapped CPU tiers. + ## Sizing and controlled-drain checks Collect per-container CPU/memory, pod requests, Pending events, game tick/query @@ -136,6 +179,19 @@ the available Prometheus metrics. Include headroom for one failed management node, game startup, repository maintenance and replacement pods. No fixed server count follows from these request values. +Qualify each production tier on the intended node and storage classes. Record +the concurrent game count, tick/query latency p95 and p99, CPU throttling, +memory working sets and completed backup duration at steady state. Repeat +while a neighboring game allocates to its memory limit, a cold game starts and +the expected peak backup concurrency runs. Record the OOM termination and +verify the neighboring game stays within its latency target. Cordon and drain +one game worker and record replacement placement, PVC attachment, Pending +events and recovery time. For each eligible worker, compare remaining +allocatable CPU and memory after requests to the largest game start plus its +sidecars, and reserve capacity for the backup burst after one worker is lost. +Record each tier's accepted tenant quotas, node count and latency target from +that run before offering it to tenants. + In a disposable multi-worker environment, apply the example and verify every management pod's node labels. Temporarily set a nonexistent selector and verify Pending/FailedScheduling, then restore it. Cordon one management worker, evict diff --git a/operator/cmd/main.go b/operator/cmd/main.go index c52857639..4546777c8 100644 --- a/operator/cmd/main.go +++ b/operator/cmd/main.go @@ -163,6 +163,7 @@ func main() { backupCSICaptureTimeout time.Duration backupHardSameNode bool agentResourcesJSON string + gameWorkloadPolicyJSON string sentinelResourcesJSON string tunnelResourcesJSON string backupResourcesJSON string @@ -171,6 +172,7 @@ func main() { flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "Address the probe endpoint binds to.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election.") flag.StringVar(&agentResourcesJSON, "agent-resources", "", "JSON ResourceRequirements for agents and config init containers; default requests 25m/64Mi, {} clears defaults.") + flag.StringVar(&gameWorkloadPolicyJSON, "game-workload-policy", "", "JSON game worker pool and per-game resource bounds. Unset disables policy enforcement.") flag.StringVar(&sentinelResourcesJSON, "sentinel-resources", "", "JSON ResourceRequirements for sentinels; default requests 25m/32Mi, {} clears defaults.") flag.StringVar(&tunnelResourcesJSON, "tunnel-resources", "", "JSON ResourceRequirements for tunnel relays; default requests 25m/64Mi, {} clears defaults.") flag.StringVar(&backupResourcesJSON, "backup-resources", "", "JSON ResourceRequirements for restic backup/init, restore and cleanup workers; default backup/restore requests 100m/128Mi, {} clears defaults.") @@ -313,6 +315,11 @@ func main() { ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) setupLog := ctrl.Log.WithName("setup") + gameWorkloadPolicy, err := controller.ParseGameWorkloadPolicy(gameWorkloadPolicyJSON) + if err != nil { + setupLog.Error(err, "invalid --game-workload-policy") + os.Exit(1) + } agentResources, err := controller.ParseAuxiliaryResources(agentResourcesJSON) if err != nil { setupLog.Error(err, "invalid --agent-resources") @@ -498,6 +505,7 @@ func main() { ProbeIngressFromCIDRs: probeIngressFromCIDR, CaptureEnabled: captureEnabled, RestrictedWorkloads: restrictedWorkloads, + GameWorkloadPolicy: gameWorkloadPolicy, AgentTokenExpirationSeconds: agentTokenExpirationSeconds, DiagnosticsNamespace: diagnosticsNamespace, CaptureSidecarImage: captureSidecarImage, diff --git a/operator/internal/controller/game_workload_policy.go b/operator/internal/controller/game_workload_policy.go new file mode 100644 index 000000000..5db30913f --- /dev/null +++ b/operator/internal/controller/game_workload_policy.go @@ -0,0 +1,92 @@ +package controller + +import ( + "encoding/json" + "fmt" + "io" + "maps" + "strings" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/util/validation" + + gameplanev1alpha1 "github.com/GameplanePanel/gameplane/operator/api/v1alpha1" +) + +// GameWorkloadPolicy is configured by the cluster administrator. The pool +// label and NoSchedule taint use the same key/value pair. +type GameWorkloadPolicy struct { + PoolLabel string `json:"poolLabel"` + PoolValue string `json:"poolValue"` + MinCPURequest resource.Quantity `json:"minCPURequest"` + MaxCPURequest resource.Quantity `json:"maxCPURequest"` + MinMemoryRequest resource.Quantity `json:"minMemoryRequest"` + MaxMemoryLimit resource.Quantity `json:"maxMemoryLimit"` +} + +// ParseGameWorkloadPolicy rejects incomplete or unrecognized configuration at startup. +func ParseGameWorkloadPolicy(raw string) (*GameWorkloadPolicy, error) { + if raw == "" { + return nil, nil + } + var policy GameWorkloadPolicy + decoder := json.NewDecoder(strings.NewReader(raw)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&policy); err != nil { + return nil, fmt.Errorf("decode game workload policy: %w", err) + } + if decoder.Decode(new(any)) != io.EOF { + return nil, fmt.Errorf("expected a single game workload policy object") + } + if len(validation.IsQualifiedName(policy.PoolLabel)) != 0 || policy.PoolValue == "" || len(validation.IsValidLabelValue(policy.PoolValue)) != 0 { + return nil, fmt.Errorf("poolLabel and poolValue must identify a valid, nonempty worker label") + } + if policy.MinCPURequest.Sign() <= 0 || policy.MaxCPURequest.Cmp(policy.MinCPURequest) < 0 || + policy.MinMemoryRequest.Sign() <= 0 || policy.MaxMemoryLimit.Cmp(policy.MinMemoryRequest) < 0 { + return nil, fmt.Errorf("CPU and memory bounds require positive minimums and maximums at least as large") + } + return &policy, nil +} + +func (p *GameWorkloadPolicy) validate(gs *gameplanev1alpha1.GameServer, tmpl *gameplanev1alpha1.GameTemplate) error { + res := effectiveResources(gs, tmpl) + for name, request := range res.Requests { + if limit, ok := res.Limits[name]; ok && request.Cmp(limit) > 0 { + return fmt.Errorf("effective resources.requests.%s exceeds resources.limits.%s", name, name) + } + } + if p == nil { + return nil + } + if value, found := gs.Spec.NodeSelector[p.PoolLabel]; found && value != p.PoolValue { + return fmt.Errorf("spec.nodeSelector conflicts with the required game worker pool %s=%s", p.PoolLabel, p.PoolValue) + } + for _, t := range gs.Spec.Tolerations { + if t.Key != p.PoolLabel || t.Value != p.PoolValue || (t.Operator != corev1.TolerationOpEqual && t.Operator != "") || + t.Effect != corev1.TaintEffectNoSchedule || t.TolerationSeconds != nil { + return fmt.Errorf("spec.tolerations may only tolerate %s=%s:NoSchedule", p.PoolLabel, p.PoolValue) + } + } + cpu := res.Requests.Cpu() + if cpu.Cmp(p.MinCPURequest) < 0 || cpu.Cmp(p.MaxCPURequest) > 0 { + return fmt.Errorf("effective resources.requests.cpu must be between %s and %s", p.MinCPURequest.String(), p.MaxCPURequest.String()) + } + request, limit := res.Requests.Memory(), res.Limits.Memory() + if request.Cmp(p.MinMemoryRequest) < 0 || limit.IsZero() || limit.Cmp(p.MaxMemoryLimit) > 0 { + return fmt.Errorf("effective resources.requests.memory must be at least %s, and resources.limits.memory must cover the request and stay within %s", p.MinMemoryRequest.String(), p.MaxMemoryLimit.String()) + } + return nil +} + +func (p *GameWorkloadPolicy) placement(gs *gameplanev1alpha1.GameServer) (map[string]string, []corev1.Toleration) { + if p == nil { + return gs.Spec.NodeSelector, gs.Spec.Tolerations + } + selector := map[string]string{} + maps.Copy(selector, gs.Spec.NodeSelector) + selector[p.PoolLabel] = p.PoolValue + return selector, []corev1.Toleration{{ + Key: p.PoolLabel, Operator: corev1.TolerationOpEqual, Value: p.PoolValue, Effect: corev1.TaintEffectNoSchedule, + }} +} diff --git a/operator/internal/controller/game_workload_policy_test.go b/operator/internal/controller/game_workload_policy_test.go new file mode 100644 index 000000000..ff1150bf1 --- /dev/null +++ b/operator/internal/controller/game_workload_policy_test.go @@ -0,0 +1,151 @@ +package controller + +import ( + "strings" + "testing" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + gameplanev1alpha1 "github.com/GameplanePanel/gameplane/operator/api/v1alpha1" +) + +const gamePolicyJSON = `{"poolLabel":"gameplane.local/pool","poolValue":"games","minCPURequest":"100m","maxCPURequest":"4","minMemoryRequest":"128Mi","maxMemoryLimit":"8Gi"}` + +func TestGameWorkloadPolicyConfiguration(t *testing.T) { + if p, err := ParseGameWorkloadPolicy(""); err != nil || p != nil { + t.Fatalf("absent policy: %v %v", p, err) + } + for _, raw := range []string{"{}", "null", gamePolicyJSON + "{}", strings.Replace(gamePolicyJSON, "poolValue", "typo", 1), + strings.Replace(gamePolicyJSON, "gameplane.local/pool", "bad key", 1), + strings.Replace(gamePolicyJSON, "100m", "8", 1), strings.Replace(gamePolicyJSON, "128Mi", "0", 1)} { + if _, err := ParseGameWorkloadPolicy(raw); err == nil { + t.Errorf("accepted invalid policy %s", raw) + } + } +} + +func gamePolicyFixture(t *testing.T) (*GameWorkloadPolicy, *gameplanev1alpha1.GameServer, *gameplanev1alpha1.GameTemplate) { + t.Helper() + p, err := ParseGameWorkloadPolicy(gamePolicyJSON) + if err != nil { + t.Fatal(err) + } + tmpl := &gameplanev1alpha1.GameTemplate{} + tmpl.Spec.Resources = corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("250m"), corev1.ResourceMemory: resource.MustParse("256Mi")}, + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + } + return p, &gameplanev1alpha1.GameServer{}, tmpl +} + +func TestGamePartialResourceOverrideAndPlacement(t *testing.T) { + p, gs, tmpl := gamePolicyFixture(t) + gs.Spec.Resources = &corev1.ResourceRequirements{Requests: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("500m")}} + gs.Spec.NodeSelector = map[string]string{"rack": "west"} + gs.Spec.Tolerations = []corev1.Toleration{{Key: p.PoolLabel, Value: p.PoolValue, Effect: corev1.TaintEffectNoSchedule}} + if err := p.validate(gs, tmpl); err != nil { + t.Fatal(err) + } + res := effectiveResources(gs, tmpl) + if res.Requests.Cpu().String() != "500m" || res.Requests.Memory().String() != "256Mi" || res.Limits.Memory().String() != "1Gi" || !res.Limits.Cpu().IsZero() { + t.Fatalf("partial override discarded template bounds or capped CPU bursts: %+v", res) + } + if tmpl.Spec.Resources.Requests.Cpu().String() != "250m" || gs.Spec.Resources.Limits != nil { + t.Fatal("merge mutated its inputs") + } + selector, tolerations := p.placement(gs) + if selector["rack"] != "west" || selector[p.PoolLabel] != p.PoolValue || len(gs.Spec.NodeSelector) != 1 || + len(tolerations) != 1 || tolerations[0].Operator != corev1.TolerationOpEqual { + t.Fatalf("worker placement: %v %v", selector, tolerations) + } + var disabled *GameWorkloadPolicy + sel, tol := disabled.placement(gs) + if len(sel) != 1 || sel["rack"] != "west" || len(tol) != 1 || tol[0] != gs.Spec.Tolerations[0] { + t.Fatalf("legacy placement changed: %v %v", sel, tol) + } +} + +func TestGameWorkloadPolicyRejectsOutOfBoundsInputs(t *testing.T) { + for _, field := range []string{"pool", "tolerations", "missing memory limit", "memory ceiling", "memory reservation", "CPU floor", "CPU cap", "CPU limit"} { + t.Run(field, func(t *testing.T) { + p, gs, tmpl := gamePolicyFixture(t) + switch field { + case "pool": + gs.Spec.NodeSelector = map[string]string{p.PoolLabel: "management"} + case "tolerations": + gs.Spec.Tolerations = []corev1.Toleration{{Operator: corev1.TolerationOpExists}} + case "missing memory limit": + delete(tmpl.Spec.Resources.Limits, corev1.ResourceMemory) + case "memory ceiling": + tmpl.Spec.Resources.Limits[corev1.ResourceMemory] = resource.MustParse("9Gi") + case "memory reservation": + tmpl.Spec.Resources.Requests[corev1.ResourceMemory] = resource.MustParse("2Gi") + case "CPU floor": + tmpl.Spec.Resources.Requests[corev1.ResourceCPU] = resource.MustParse("50m") + case "CPU cap": + tmpl.Spec.Resources.Requests[corev1.ResourceCPU] = resource.MustParse("5") + case "CPU limit": + tmpl.Spec.Resources.Limits[corev1.ResourceCPU] = resource.MustParse("50m") + } + if err := p.validate(gs, tmpl); err == nil { + t.Fatal("accepted invalid input") + } + }) + } +} + +func TestGameWorkloadPolicyRendersMergedResourcesAndWorkerPlacement(t *testing.T) { + r, gs, effective := revisionFixture(t) + p, _, tmpl := gamePolicyFixture(t) + r.GameWorkloadPolicy = p + effective.Spec.Resources = tmpl.Spec.Resources + gs.Spec.Resources = &corev1.ResourceRequirements{Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}} + if err := r.Update(t.Context(), gs); err != nil { + t.Fatal(err) + } + if err := r.reconcileStatefulSet(t.Context(), gs, effective, nil, &materializedConfig{}, 1); err != nil { + t.Fatal(err) + } + var ss appsv1.StatefulSet + if err := r.Get(t.Context(), client.ObjectKeyFromObject(gs), &ss); err != nil { + t.Fatal(err) + } + pod := ss.Spec.Template.Spec + game := pod.Containers[0] + if game.Name != gameContainerName || game.Resources.Requests.Cpu().String() != "250m" || + game.Resources.Limits.Memory().String() != "2Gi" || !game.Resources.Limits.Cpu().IsZero() || + pod.NodeSelector[p.PoolLabel] != p.PoolValue || len(pod.Tolerations) != 1 || pod.Tolerations[0].Key != p.PoolLabel { + t.Fatalf("wrong game resources or placement: %+v / %+v / %+v", game.Resources, pod.NodeSelector, pod.Tolerations) + } +} + +func TestStoredGameCannotCreateWorkloadOutsideWorkerPolicy(t *testing.T) { + r, gs, _ := revisionFixture(t) + p, _, _ := gamePolicyFixture(t) + r.GameWorkloadPolicy = p + gs.Spec.NodeSelector = map[string]string{p.PoolLabel: "management"} + if err := r.Update(t.Context(), gs); err != nil { + t.Fatal(err) + } + key := client.ObjectKeyFromObject(gs) + if _, err := r.Reconcile(t.Context(), ctrl.Request{NamespacedName: key}); err != nil { + t.Fatal(err) + } + if err := r.Get(t.Context(), key, gs); err != nil { + t.Fatal(err) + } + ready := meta.FindStatusCondition(gs.Status.Conditions, "Ready") + if ready == nil || ready.Status != metav1.ConditionFalse || !strings.Contains(ready.Message, "spec.nodeSelector") { + t.Fatalf("missing actionable policy failure: %+v", gs.Status) + } + if err := r.Get(t.Context(), key, &appsv1.StatefulSet{}); !apierrors.IsNotFound(err) { + t.Fatalf("unsafe workload: %v", err) + } +} diff --git a/operator/internal/controller/gameserver_controller.go b/operator/internal/controller/gameserver_controller.go index 536ae7bbb..a8c849702 100644 --- a/operator/internal/controller/gameserver_controller.go +++ b/operator/internal/controller/gameserver_controller.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "maps" "net/netip" "path" "sort" @@ -178,6 +179,7 @@ type GameServerReconciler struct { // RestrictedWorkloads applies the opt-in PSA-compatible game profile. RestrictedWorkloads bool + GameWorkloadPolicy *GameWorkloadPolicy AgentTokenExpirationSeconds int64 DiagnosticsNamespace string @@ -321,6 +323,9 @@ func (r *GameServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, r.setPhase(ctx, &gs, err.Error()) } } + if err := r.GameWorkloadPolicy.validate(&gs, &tmpl); err != nil { + return ctrl.Result{}, r.setPhase(ctx, &gs, err.Error()) + } mc, err := materializeConfig(&gs, &tmpl) if err != nil { return ctrl.Result{}, r.setPhase(ctx, &gs, @@ -1838,8 +1843,7 @@ func (r *GameServerReconciler) reconcileStatefulSet( // Assign unconditionally so clearing spec.nodeSelector also clears the // pod template's selector (nil resets it); the previous nil-guard left // a removed scheduling pin active on the StatefulSet. - ss.Spec.Template.Spec.NodeSelector = gs.Spec.NodeSelector - ss.Spec.Template.Spec.Tolerations = gs.Spec.Tolerations + ss.Spec.Template.Spec.NodeSelector, ss.Spec.Template.Spec.Tolerations = r.GameWorkloadPolicy.placement(gs) // Do not change a live Pod's template merely because its node was // recorded while a graceful stop is still saving the game. Apply the // endpoint pin at zero replicas, then retain it on every future wake. @@ -2294,15 +2298,28 @@ func buildConfigInitContainer(image string, tmpl *gameplanev1alpha1.GameTemplate } } -// effectiveResources resolves the game container's compute resources: -// the template's defaults, replaced wholesale by spec.resources when set. +// Each explicit request or limit overrides that resource's template value. +// An unconfigured CPU limit stays absent for Burstable games. func effectiveResources( gs *gameplanev1alpha1.GameServer, tmpl *gameplanev1alpha1.GameTemplate, ) corev1.ResourceRequirements { - if gs.Spec.Resources != nil { - return *gs.Spec.Resources + resources := tmpl.Spec.Resources.DeepCopy() + if override := gs.Spec.Resources; override != nil { + resources.Requests = mergeResourceList(resources.Requests, override.Requests) + resources.Limits = mergeResourceList(resources.Limits, override.Limits) + if override.Claims != nil { + resources.Claims = override.DeepCopy().Claims + } + } + return *resources +} + +func mergeResourceList(defaults, override corev1.ResourceList) corev1.ResourceList { + if defaults == nil { + return override.DeepCopy() } - return tmpl.Spec.Resources + maps.Copy(defaults, override.DeepCopy()) + return defaults } // buildGameContainer constructs the primary game server container definition. diff --git a/operator/internal/controller/gameserver_template_revision.go b/operator/internal/controller/gameserver_template_revision.go index 7003625f1..f03157137 100644 --- a/operator/internal/controller/gameserver_template_revision.go +++ b/operator/internal/controller/gameserver_template_revision.go @@ -201,6 +201,9 @@ func (r *GameServerReconciler) reconcileTemplateRevision(ctx context.Context, gs if _, err := resolveVersion(gs, target); err != nil { return nil, false, err } + if err := r.GameWorkloadPolicy.validate(gs, target); err != nil { + return nil, false, err + } if r.RestrictedWorkloads { if err := validateRestrictedTemplate(target); err != nil { return nil, false, err @@ -252,6 +255,9 @@ func (r *GameServerReconciler) reconcileTemplateRevision(ctx context.Context, gs if _, err := resolveVersion(gs, &installed); err != nil { return nil, false, err } + if err := r.GameWorkloadPolicy.validate(gs, &installed); err != nil { + return nil, false, err + } if r.RestrictedWorkloads { if err := validateRestrictedTemplate(&installed); err != nil { return nil, false, err @@ -310,6 +316,9 @@ func (r *GameServerReconciler) initializeTemplateBaseline(ctx context.Context, g if err := validateTemplatePasswordTransition(gs, nil, target); err != nil { return err } + if err := r.GameWorkloadPolicy.validate(gs, target); err != nil { + return err + } var ss appsv1.StatefulSet err = r.wipeReader().Get(ctx, client.ObjectKeyFromObject(gs), &ss) if err != nil && !apierrors.IsNotFound(err) { @@ -342,6 +351,11 @@ func (r *GameServerReconciler) initializeTemplateBaseline(ctx context.Context, g } func (r *GameServerReconciler) preserveBaselinePodTemplate(ctx context.Context, gs *gameplanev1alpha1.GameServer, ss *appsv1.StatefulSet) (bool, error) { + // A pre-upgrade template may predate the required pool or the merged + // resource bounds. Re-render it before scaling the workload. + if r.GameWorkloadPolicy != nil { + return false, nil + } pin := gs.Annotations[effectiveTemplateAnnotation] if pin == "" || gs.Annotations[baselineTemplateAnnotation] != pin || string(ss.UID) != gs.Annotations[baselineWorkloadAnnotation] || ss.Spec.Template.Annotations[templateRevisionAnnotation] != "" { return false, nil diff --git a/test/e2e/buckets.sh b/test/e2e/buckets.sh index c4a7afdec..4d3a32bf0 100755 --- a/test/e2e/buckets.sh +++ b/test/e2e/buckets.sh @@ -441,6 +441,7 @@ EOF bucket_management_placement() { cat <<'EOF' TestManagementPlacementAndDisruption +TestGameWorkerPoolAndQuota EOF } diff --git a/test/e2e/game_worker_pool_e2e_test.go b/test/e2e/game_worker_pool_e2e_test.go new file mode 100644 index 000000000..2bf49b073 --- /dev/null +++ b/test/e2e/game_worker_pool_e2e_test.go @@ -0,0 +1,162 @@ +//go:build e2e + +package e2e + +import ( + "context" + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + policyv1 "k8s.io/api/policy/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// Exercise the exact rendered quota and Pod admission policy behind a game +// StatefulSet. This isolated bucket owns two disposable Kind workers. +func TestGameWorkerPoolAndQuota(t *testing.T) { + e, ctx := envInstance, t.Context() + ns := fmt.Sprintf("game-pool-%d", time.Now().UnixNano()) + key := "gameplane.local/" + ns + k := e.K8s.CoreV1() + nodes, err := k.Nodes().List(ctx, metav1.ListOptions{}) + if err != nil { + t.Fatal(err) + } + var workers []string + for _, node := range nodes.Items { + if !node.Spec.Unschedulable && len(node.Spec.Taints) == 0 { + workers = append(workers, node.Name) + } + } + if len(workers) < 2 { + t.Fatal("need two schedulable workers") + } + kubectl := func(args ...string) { + t.Helper() + if out, err := e.Kubectl(ctx, args...); err != nil { + t.Fatalf("kubectl %v: %v: %s", args, err, out) + } + } + labelWorker := func(name string) { + kubectl("label", "node", name, key+"=games") + t.Cleanup(func() { _, _ = e.Kubectl(context.Background(), "label", "node", name, key+"-") }) + kubectl("taint", "node", name, key+"=games:NoSchedule") + t.Cleanup(func() { _, _ = e.Kubectl(context.Background(), "taint", "node", name, key+":NoSchedule-") }) + } + labelWorker(workers[0]) + kubectl("create", "namespace", ns) + t.Cleanup(func() { _ = k.Namespaces().Delete(context.Background(), ns, metav1.DeleteOptions{}) }) + chart, err := findRepoRel("charts/gameplane") + if err != nil { + t.Fatal(err) + } + values := map[string]any{"gamesNamespace": ns, "operator": map[string]any{ + "gameWorkloadPolicy": map[string]string{"poolLabel": key, "poolValue": "games", "minCPURequest": "10m", + "maxCPURequest": "1", "minMemoryRequest": "8Mi", "maxMemoryLimit": "32Mi"}, + "tenantQuotas": map[string]any{ns: map[string]any{"hard": map[string]string{ + "pods": "1", "requests.cpu": "1", "requests.memory": "32Mi", "limits.memory": "32Mi", + "persistentvolumeclaims": "1", "requests.storage": "1Gi", + }}}, + }} + data, err := json.Marshal(values) + if err != nil { + t.Fatal(err) + } + path := filepath.Join(t.TempDir(), "values.json") + if err := os.WriteFile(path, data, 0o600); err != nil { + t.Fatal(err) + } + rendered, err := exec.CommandContext(ctx, "helm", "template", ns, chart, "-n", ns, "-f", path, + "--show-only", "templates/game-worker-admission.yaml", "--show-only", "templates/tenant-quotas.yaml").CombinedOutput() + if err != nil { + t.Fatalf("helm: %v: %s", err, rendered) + } + t.Cleanup(func() { + _, _ = e.KubectlWithStdin(context.Background(), string(rendered), "delete", "--ignore-not-found", "-f", "-") + }) + if out, err := e.KubectlWithStdin(ctx, string(rendered), "apply", "-f", "-"); err != nil { + t.Fatalf("apply worker policy: %v: %s", err, out) + } + + pods := k.Pods(ns) + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "game", Labels: map[string]string{"app.kubernetes.io/name": "gameplane-game"}}, + Spec: corev1.PodSpec{RestartPolicy: corev1.RestartPolicyNever, NodeSelector: map[string]string{key: "management"}, + Tolerations: []corev1.Toleration{{Key: key, Operator: corev1.TolerationOpEqual, Value: "games", Effect: corev1.TaintEffectNoSchedule}}, + Containers: []corev1.Container{{Name: "game", Image: "busybox:1.38.0", Command: []string{"sleep", "600"}, + Resources: corev1.ResourceRequirements{Requests: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("10m"), corev1.ResourceMemory: resource.MustParse("16Mi")}, + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("32Mi")}}}}}} + e.Eventually(t, time.Minute, func() (bool, string) { + _, err := pods.Create(ctx, pod, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}) + return err != nil && strings.Contains(err.Error(), "must select the configured game worker pool"), fmt.Sprint(err) + }) + pod.Spec.NodeSelector[key] = "games" + oversized := pod.DeepCopy() + oversized.Spec.Containers[0].Resources.Limits[corev1.ResourceMemory] = resource.MustParse("64Mi") + if _, err := pods.Create(ctx, oversized, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err == nil { + t.Fatal("admission accepted an oversized game") + } + if _, err := pods.Create(ctx, pod, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } + e.Eventually(t, 2*time.Minute, func() (bool, string) { + live, err := pods.Get(ctx, pod.Name, metav1.GetOptions{}) + if err != nil { + return false, err.Error() + } + return live.Spec.NodeName == workers[0] && live.Status.Phase == corev1.PodRunning, fmt.Sprint(live.Status.Phase) + }) + second := pod.DeepCopy() + second.Name = "replacement" + if _, err := pods.Create(ctx, second, metav1.CreateOptions{}); !apierrors.IsForbidden(err) || !strings.Contains(err.Error(), "exceeded quota") { + t.Fatalf("second game should exceed the tenant quota: %v", err) + } + kubectl("cordon", workers[0]) + t.Cleanup(func() { _, _ = e.Kubectl(context.Background(), "uncordon", workers[0]) }) + zero := int64(0) + if err := pods.EvictV1(ctx, &policyv1.Eviction{ObjectMeta: metav1.ObjectMeta{Name: pod.Name, Namespace: ns}, + DeleteOptions: &metav1.DeleteOptions{GracePeriodSeconds: &zero}}); err != nil { + t.Fatal(err) + } + second.Spec.Containers[0].Command = []string{"awk", `BEGIN { s="x"; while(length(s)<1048576) s=s s; for(i=0;;i++) a[i]=s i }`} + e.Eventually(t, time.Minute, func() (bool, string) { + _, err := pods.Create(ctx, second, metav1.CreateOptions{}) + if err != nil && !apierrors.IsForbidden(err) { + t.Fatalf("create replacement: %v", err) + } + return err == nil, fmt.Sprint(err) + }) + e.Eventually(t, time.Minute, func() (bool, string) { + live, err := pods.Get(ctx, second.Name, metav1.GetOptions{}) + if err != nil { + return false, err.Error() + } + for _, cond := range live.Status.Conditions { + if cond.Type == corev1.PodScheduled && cond.Reason == "Unschedulable" && live.Spec.NodeName == "" { + return true, "" + } + } + return false, fmt.Sprint(live.Status.Conditions) + }) + labelWorker(workers[1]) + e.Eventually(t, 3*time.Minute, func() (bool, string) { + live, err := pods.Get(ctx, second.Name, metav1.GetOptions{}) + if err != nil { + return false, err.Error() + } + for _, status := range live.Status.ContainerStatuses { + if ended := status.State.Terminated; ended != nil { + return live.Spec.NodeName == workers[1] && ended.Reason == "OOMKilled", fmt.Sprintf("node=%s reason=%s", live.Spec.NodeName, ended.Reason) + } + } + return false, fmt.Sprint(live.Status.Phase) + }) +} diff --git a/test/e2e/specs.md b/test/e2e/specs.md index 9caad3502..d5edf297e 100644 --- a/test/e2e/specs.md +++ b/test/e2e/specs.md @@ -134,6 +134,12 @@ a cordon, readiness of the surviving replica, and singleton API eviction. Node labels and cordons are restored in cleanup. This validates scheduler/PDB behavior, not application HA or storage migration. +`TestGameWorkerPoolAndQuota` applies the production chart's game Pod +admission and tenant quota to an isolated namespace. A second Pod exceeds its +quota. When the test cordons the only game worker and evicts its game Pod, the +replacement stays Pending until a second worker joins the pool. Its allocating +process must terminate with `OOMKilled` at the configured memory limit. + ### Backup preparation acceptance The `backup-preparation` bucket also uses the `management` Kind profile and zero diff --git a/test/e2e/suite-components.json b/test/e2e/suite-components.json index 803e2917f..c4fc7c3a5 100644 --- a/test/e2e/suite-components.json +++ b/test/e2e/suite-components.json @@ -24,6 +24,7 @@ { "tests": "^TestMonitoring_DiscoveryPerformanceAndPolicy$", "components": ["operator", "api", "agent", "tunnel"] }, { "tests": "^TestRestrictedWorkloads_", "components": ["operator", "api", "agent", "capture-sidecar"] }, { "tests": "^TestManagementPlacementAndDisruption$", "components": ["operator", "api", "agent"] }, + { "tests": "^TestGameWorkerPoolAndQuota$", "components": ["operator"] }, { "tests": "^TestHelmInstall_", "components": ["*"] }, { "tests": "^TestCRD_Validation_", "components": ["operator"] }, { "tests": "^TestGameServer_", "components": ["operator", "api", "agent"] }, From 26ed28899da027fa3e58d8745afa67a61e14cfe7 Mon Sep 17 00:00:00 2001 From: Baylem Date: Tue, 10 Nov 2026 21:11:11 -0500 Subject: [PATCH 02/10] chore(chart): group tenant policy with workload resource settings Signed-off-by: Baylem --- charts/gameplane/templates/operator.yaml | 6 +++--- charts/gameplane/values.yaml | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/charts/gameplane/templates/operator.yaml b/charts/gameplane/templates/operator.yaml index 25e8edecb..c2995d351 100644 --- a/charts/gameplane/templates/operator.yaml +++ b/charts/gameplane/templates/operator.yaml @@ -377,9 +377,6 @@ spec: - --agent-client-key=/etc/gameplane/agent-client/tls.key - --agent-ca-secret-name={{ .Values.api.agentMTLS.caSecretRef.name }} - --game-namespaces={{ join "," $gameNamespaces }} - {{- if ne .Values.operator.gameWorkloadPolicy nil }} - - {{ printf "--game-workload-policy=%s" (mustToJson .Values.operator.gameWorkloadPolicy) | quote }} - {{- end }} - --control-plane-namespace={{ .Values.operator.controlPlaneNamespace | default .Release.Namespace }} - --agent-ca-secret-namespace={{ .Values.operator.agentCASecretNamespace | default .Release.Namespace }} - --module-namespace={{ .Values.operator.moduleNamespace | default .Release.Namespace }} @@ -412,6 +409,9 @@ spec: {{- if .Values.operator.agentLogLevel }} - --agent-log-level={{ .Values.operator.agentLogLevel }} {{- end }} + {{- if ne .Values.operator.gameWorkloadPolicy nil }} + - {{ printf "--game-workload-policy=%s" (mustToJson .Values.operator.gameWorkloadPolicy) | quote }} + {{- end }} {{- if and .Values.operator.gameDataStorage .Values.operator.gameDataStorage.storageClassName }} - --game-data-storage-class={{ .Values.operator.gameDataStorage.storageClassName }} {{- end }} diff --git a/charts/gameplane/values.yaml b/charts/gameplane/values.yaml index 94dff5beb..42fffe9dd 100644 --- a/charts/gameplane/values.yaml +++ b/charts/gameplane/values.yaml @@ -71,12 +71,6 @@ operator: # exist. The chart grants namespace-local workload permissions and includes # their Secrets in the operator cache. A Helm upgrade restarts the operator. gameNamespaces: [] - # Optional game worker label/NoSchedule taint and per-game tenant bounds. - # See values-production.example.yaml and docs/production-placement.md. - gameWorkloadPolicy: null - # ResourceQuota by managed game namespace. Each entry accepts hard and - # an optional defaultContainerMemoryLimit for containers without a limit. - tenantQuotas: {} # Empty uses the release namespace. Non-default namespaces must already exist; # provision the referenced Secrets there before upgrading. These settings move # the operator's API reads, not the API pod or its mounted mTLS client material. @@ -91,6 +85,12 @@ operator: resources: requests: { cpu: 100m, memory: 64Mi } limits: { cpu: 500m, memory: 256Mi } + # Optional game worker label/NoSchedule taint and per-game tenant bounds. + # See values-production.example.yaml and docs/production-placement.md. + gameWorkloadPolicy: null + # ResourceQuota by managed game namespace. Each entry accepts hard and + # an optional defaultContainerMemoryLimit for containers without a limit. + tenantQuotas: {} # Complete auxiliary ResourceRequirements overrides. Null/missing uses binary # defaults, including on --reuse-values upgrades. {} (or requests: {}, limits: # {}) clears defaults. No new CPU/memory limits are set by default. From 3a59a6c3058be80a5cba6425967bac7e7d222afa Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 00:30:33 -0500 Subject: [PATCH 03/10] test(e2e): measure neighbor ticks under a tenant leak and backup Signed-off-by: Baylem --- .github/workflows/ci.yaml | 2 +- docs/production-placement.md | 9 ++ test/e2e/game_worker_capacity_e2e_test.go | 116 ++++++++++++++++++++++ test/e2e/game_worker_pool_e2e_test.go | 58 +++++++++-- 4 files changed, 177 insertions(+), 8 deletions(-) create mode 100644 test/e2e/game_worker_capacity_e2e_test.go diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 8ce0fa057..7bc7c0222 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -1864,7 +1864,7 @@ jobs: job_timeout: 60 # Boots two-node kind, installs the chart and exercises repeated Hostport sleep/wake handoffs. - bucket: management-placement parallel: 1 - test_timeout: 15m + test_timeout: 30m job_timeout: 60 # Boots management kind, installs the chart and checks placement/disruption behavior. - bucket: backup-preparation parallel: 1 diff --git a/docs/production-placement.md b/docs/production-placement.md index b6a6963d4..8f3e3f3ee 100644 --- a/docs/production-placement.md +++ b/docs/production-placement.md @@ -192,6 +192,15 @@ sidecars, and reserve capacity for the backup burst after one worker is lost. Record each tier's accepted tenant quotas, node count and latency target from that run before offering it to tenants. +The disposable `management-placement` CI bucket runs a Minecraft Java +1.21.4 flat world alongside the replacement tenant Pod and a Restic worker +on the sole remaining eligible node. It records Minecraft's own `tick query` +p95/p99, the worker's allocatable and requested resources, the tenant OOM +exit and the time to complete and verify the backup. It checks the neighbor's +Pod UID and container restart counts across the event. The small fixture's +100 ms p95 and 250 ms p99 budgets apply to this CI run. Qualify production +tiers with representative player counts, worlds and storage. + In a disposable multi-worker environment, apply the example and verify every management pod's node labels. Temporarily set a nonexistent selector and verify Pending/FailedScheduling, then restore it. Cordon one management worker, evict diff --git a/test/e2e/game_worker_capacity_e2e_test.go b/test/e2e/game_worker_capacity_e2e_test.go new file mode 100644 index 000000000..775b8bc16 --- /dev/null +++ b/test/e2e/game_worker_capacity_e2e_test.go @@ -0,0 +1,116 @@ +//go:build e2e + +package e2e + +import ( + "context" + "fmt" + "regexp" + "strconv" + "strings" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var tickPercentiles = regexp.MustCompile(`P95:\s*([0-9.]+)\s*ms\s+P99:\s*([0-9.]+)\s*ms`) + +func sampleNeighborTicks(ctx context.Context, t *testing.T, ns, pod, phase string) { + t.Helper() + out, err := envInstance.Kubectl(ctx, "exec", "-n", ns, pod, "-c", "game", "--", "rcon-cli", "tick", "query") + if err != nil { + t.Fatalf("query Minecraft tick durations at %s: %v %s", phase, err, out) + } + match := tickPercentiles.FindStringSubmatch(string(out)) + if len(match) != 3 { + t.Fatalf("Minecraft did not report tick percentiles at %s: %s", phase, out) + } + p95, err95 := strconv.ParseFloat(match[1], 64) + p99, err99 := strconv.ParseFloat(match[2], 64) + t.Logf("%s: Minecraft p95=%s ms p99=%s ms", phase, match[1], match[2]) + if err95 != nil || err99 != nil || p95 > 100 || p99 > 250 { + t.Fatalf("neighbor exceeded the disposable CI fixture's 100 ms p95 / 250 ms p99 budget at %s", phase) + } +} + +func recordWorkerCapacity(ctx context.Context, t *testing.T, worker string) { + t.Helper() + node, err := envInstance.K8s.CoreV1().Nodes().Get(ctx, worker, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + for _, cond := range node.Status.Conditions { + if (cond.Type == corev1.NodeMemoryPressure || cond.Type == corev1.NodeDiskPressure) && cond.Status != corev1.ConditionFalse { + t.Fatalf("worker %s reported %s=%s", worker, cond.Type, cond.Status) + } + } + out, err := envInstance.Kubectl(ctx, "describe", "node", worker) + _, allocated, found := strings.Cut(string(out), "Allocated resources:") + if err != nil || !found { + t.Fatalf("read scheduler requests on %s: %v", worker, err) + } + allocated, _, _ = strings.Cut(allocated, "Events:") + t.Logf("worker=%s allocatable CPU=%s memory=%s; allocated resources:%s", worker, + node.Status.Allocatable.Cpu(), node.Status.Allocatable.Memory(), allocated) +} + +func startPlacementBackup(ctx context.Context, t *testing.T, ns, poolKey, worker string) func() { + t.Helper() + pods := envInstance.K8s.CoreV1().Pods(ns) + const name = "placement-restic" + work := `export RESTIC_REPOSITORY=/tmp/repo RESTIC_PASSWORD=disposable +mkdir /tmp/world +restic init +for i in $(seq 1 120); do + dd if=/dev/urandom of=/tmp/world/chunk bs=1M count=2 2>/dev/null + restic backup -q /tmp/world + touch /tmp/snapshot-complete + test ! -e /tmp/finish || break + sleep 1 +done +restic check --read-data` + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: name}, Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, NodeSelector: map[string]string{poolKey: "games"}, + Tolerations: []corev1.Toleration{{Key: poolKey, Operator: corev1.TolerationOpEqual, Value: "games", Effect: corev1.TaintEffectNoSchedule}}, + Containers: []corev1.Container{{Name: "backup", Image: "restic/restic:0.19.1", Command: []string{"sh", "-ec", work}, + Resources: corev1.ResourceRequirements{Requests: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("100m"), corev1.ResourceMemory: resource.MustParse("128Mi")}, + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("512Mi")}}}}, + }} + if _, err := pods.Create(ctx, pod, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + cleanup, cancel := context.WithTimeout(context.WithoutCancel(ctx), 15*time.Second) + defer cancel() + _ = pods.Delete(cleanup, name, metav1.DeleteOptions{}) + }) + started := time.Now() + envInstance.Eventually(t, 3*time.Minute, func() (bool, string) { + live, err := pods.Get(ctx, name, metav1.GetOptions{}) + if err != nil { + return false, err.Error() + } + if live.Status.Phase != corev1.PodRunning || live.Spec.NodeName != worker { + return false, fmt.Sprint(live.Status.Phase, live.Spec.NodeName) + } + out, err := envInstance.Kubectl(ctx, "exec", "-n", ns, name, "--", "sh", "-ec", "test -f /tmp/snapshot-complete") + return err == nil, string(out) + }) + return func() { + t.Helper() + if out, err := envInstance.Kubectl(ctx, "exec", "-n", ns, name, "--", "touch", "/tmp/finish"); err != nil { + t.Fatalf("finish restic load: %v %s", err, out) + } + envInstance.Eventually(t, 3*time.Minute, func() (bool, string) { + live, err := pods.Get(ctx, name, metav1.GetOptions{}) + if err != nil { + return false, err.Error() + } + return live.Status.Phase == corev1.PodSucceeded, fmt.Sprint(live.Status.Phase) + }) + t.Logf("restic backup and repository read verification completed on %s in %s", worker, time.Since(started)) + } +} diff --git a/test/e2e/game_worker_pool_e2e_test.go b/test/e2e/game_worker_pool_e2e_test.go index 2bf49b073..1dd0fb37e 100644 --- a/test/e2e/game_worker_pool_e2e_test.go +++ b/test/e2e/game_worker_pool_e2e_test.go @@ -40,21 +40,45 @@ func TestGameWorkerPoolAndQuota(t *testing.T) { if len(workers) < 2 { t.Fatal("need two schedulable workers") } + neighbor := minecraftBotSpec() + neighbor.Template = "e2e-placement-neighbor" + neighbor.Env["PAUSE_WHEN_EMPTY_SECONDS"] = "0" + gameName, gameNS := createGameBotServer(t, e, neighbor) + gamePod, err := k.Pods(gameNS).Get(ctx, gameName+"-0", metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + // Lose the other worker. The leak, real Restic workload and neighboring + // Minecraft world must share the surviving worker throughout the burst. + if gamePod.Spec.NodeName == workers[0] { + workers[0], workers[1] = workers[1], workers[0] + } + if gamePod.Spec.NodeName != workers[1] { + t.Fatalf("neighbor scheduled outside the two workers: %s", gamePod.Spec.NodeName) + } + sampleNeighborTicks(ctx, t, gameNS, gamePod.Name, "baseline") kubectl := func(args ...string) { t.Helper() if out, err := e.Kubectl(ctx, args...); err != nil { t.Fatalf("kubectl %v: %v: %s", args, err, out) } } + cleanup := func(f func(context.Context)) { + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 15*time.Second) + defer cancel() + f(ctx) + }) + } labelWorker := func(name string) { kubectl("label", "node", name, key+"=games") - t.Cleanup(func() { _, _ = e.Kubectl(context.Background(), "label", "node", name, key+"-") }) + cleanup(func(ctx context.Context) { _, _ = e.Kubectl(ctx, "label", "node", name, key+"-") }) kubectl("taint", "node", name, key+"=games:NoSchedule") - t.Cleanup(func() { _, _ = e.Kubectl(context.Background(), "taint", "node", name, key+":NoSchedule-") }) + cleanup(func(ctx context.Context) { _, _ = e.Kubectl(ctx, "taint", "node", name, key+":NoSchedule-") }) } labelWorker(workers[0]) kubectl("create", "namespace", ns) - t.Cleanup(func() { _ = k.Namespaces().Delete(context.Background(), ns, metav1.DeleteOptions{}) }) + cleanup(func(ctx context.Context) { _ = k.Namespaces().Delete(ctx, ns, metav1.DeleteOptions{}) }) chart, err := findRepoRel("charts/gameplane") if err != nil { t.Fatal(err) @@ -80,8 +104,8 @@ func TestGameWorkerPoolAndQuota(t *testing.T) { if err != nil { t.Fatalf("helm: %v: %s", err, rendered) } - t.Cleanup(func() { - _, _ = e.KubectlWithStdin(context.Background(), string(rendered), "delete", "--ignore-not-found", "-f", "-") + cleanup(func(ctx context.Context) { + _, _ = e.KubectlWithStdin(ctx, string(rendered), "delete", "--ignore-not-found", "-f", "-") }) if out, err := e.KubectlWithStdin(ctx, string(rendered), "apply", "-f", "-"); err != nil { t.Fatalf("apply worker policy: %v: %s", err, out) @@ -120,13 +144,14 @@ func TestGameWorkerPoolAndQuota(t *testing.T) { t.Fatalf("second game should exceed the tenant quota: %v", err) } kubectl("cordon", workers[0]) - t.Cleanup(func() { _, _ = e.Kubectl(context.Background(), "uncordon", workers[0]) }) + cleanup(func(ctx context.Context) { _, _ = e.Kubectl(ctx, "uncordon", workers[0]) }) zero := int64(0) if err := pods.EvictV1(ctx, &policyv1.Eviction{ObjectMeta: metav1.ObjectMeta{Name: pod.Name, Namespace: ns}, DeleteOptions: &metav1.DeleteOptions{GracePeriodSeconds: &zero}}); err != nil { t.Fatal(err) } - second.Spec.Containers[0].Command = []string{"awk", `BEGIN { s="x"; while(length(s)<1048576) s=s s; for(i=0;;i++) a[i]=s i }`} + second.Spec.Containers[0].Command = []string{"sh", "-ec", `until test -e /tmp/leak; do sleep 1; done +exec awk 'BEGIN { s="x"; while(length(s)<1048576) s=s s; for(i=0;;i++) { a[i]=s i; system("sleep 0.25") } }'`} e.Eventually(t, time.Minute, func() (bool, string) { _, err := pods.Create(ctx, second, metav1.CreateOptions{}) if err != nil && !apierrors.IsForbidden(err) { @@ -147,6 +172,14 @@ func TestGameWorkerPoolAndQuota(t *testing.T) { return false, fmt.Sprint(live.Status.Conditions) }) labelWorker(workers[1]) + stopBackup := startPlacementBackup(ctx, t, gameNS, key, workers[1]) + e.Eventually(t, time.Minute, func() (bool, string) { + live, err := pods.Get(ctx, second.Name, metav1.GetOptions{}) + return err == nil && live.Spec.NodeName == workers[1] && live.Status.Phase == corev1.PodRunning, fmt.Sprint(err) + }) + recordWorkerCapacity(ctx, t, workers[1]) + kubectl("exec", "-n", ns, second.Name, "--", "touch", "/tmp/leak") + sampleNeighborTicks(ctx, t, gameNS, gamePod.Name, "start, leak and backup after worker loss") e.Eventually(t, 3*time.Minute, func() (bool, string) { live, err := pods.Get(ctx, second.Name, metav1.GetOptions{}) if err != nil { @@ -159,4 +192,15 @@ func TestGameWorkerPoolAndQuota(t *testing.T) { } return false, fmt.Sprint(live.Status.Phase) }) + sampleNeighborTicks(ctx, t, gameNS, gamePod.Name, "after tenant OOM") + stopBackup() + after, err := k.Pods(gameNS).Get(ctx, gamePod.Name, metav1.GetOptions{}) + if err != nil || after.UID != gamePod.UID || after.Spec.NodeName != workers[1] { + t.Fatalf("neighbor replaced during isolation drill: %v", err) + } + for _, status := range after.Status.ContainerStatuses { + if status.RestartCount != 0 { + t.Fatalf("neighbor %s restarted during isolation drill", status.Name) + } + } } From 84acab9e1d9cd0e3224bf4c2bfa6589cc2d7eab6 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 03:35:36 -0500 Subject: [PATCH 04/10] test(operator): allocate tenant memory in one process through OOM Signed-off-by: Baylem --- test/e2e/game_worker_pool_e2e_test.go | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/test/e2e/game_worker_pool_e2e_test.go b/test/e2e/game_worker_pool_e2e_test.go index 1dd0fb37e..990601682 100644 --- a/test/e2e/game_worker_pool_e2e_test.go +++ b/test/e2e/game_worker_pool_e2e_test.go @@ -150,8 +150,21 @@ func TestGameWorkerPoolAndQuota(t *testing.T) { DeleteOptions: &metav1.DeleteOptions{GracePeriodSeconds: &zero}}); err != nil { t.Fatal(err) } - second.Spec.Containers[0].Command = []string{"sh", "-ec", `until test -e /tmp/leak; do sleep 1; done -exec awk 'BEGIN { s="x"; while(length(s)<1048576) s=s s; for(i=0;;i++) { a[i]=s i; system("sleep 0.25") } }'`} + // Touch anonymous pages in one process. Forking a sleep helper at the + // tenant's ceiling can fail before the kernel OOM-kills the allocator. + second.Spec.Containers[0].Image = "python:3.14-alpine" + second.Spec.Containers[0].Command = []string{"python", "-c", ` +import mmap, os, time +while not os.path.exists("/tmp/leak"): + time.sleep(0.25) +held = [] +while True: + block = bytearray(1024 * 1024) + for page in range(0, len(block), mmap.PAGESIZE): + block[page] = 1 + held.append(block) + time.sleep(0.25) +`} e.Eventually(t, time.Minute, func() (bool, string) { _, err := pods.Create(ctx, second, metav1.CreateOptions{}) if err != nil && !apierrors.IsForbidden(err) { @@ -187,7 +200,11 @@ exec awk 'BEGIN { s="x"; while(length(s)<1048576) s=s s; for(i=0;;i++) { a[i]=s } for _, status := range live.Status.ContainerStatuses { if ended := status.State.Terminated; ended != nil { - return live.Spec.NodeName == workers[1] && ended.Reason == "OOMKilled", fmt.Sprintf("node=%s reason=%s", live.Spec.NodeName, ended.Reason) + if live.Spec.NodeName != workers[1] || ended.Reason != "OOMKilled" { + logs, _ := e.Kubectl(ctx, "logs", "-n", ns, second.Name, "-c", "game", "--tail=10") + t.Fatalf("tenant stopped on %s: reason=%s exit=%d message=%s logs=%s", live.Spec.NodeName, ended.Reason, ended.ExitCode, ended.Message, logs) + } + return true, "" } } return false, fmt.Sprint(live.Status.Phase) From d5f6fbf0c3f9bdecd8b614dd2762a4b21010174a Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 04:01:41 -0500 Subject: [PATCH 05/10] fix(operator): allow safe suspension after worker policy rejection Signed-off-by: Baylem --- .../controller/game_workload_policy.go | 65 ++++++++++ .../controller/game_workload_policy_test.go | 91 ++++++++++++++ .../controller/gameserver_controller.go | 6 +- .../gameserver_template_revision.go | 6 +- operator/specs.md | 14 ++- test/e2e/game_worker_capacity_e2e_test.go | 3 +- test/e2e/game_worker_operator_e2e_test.go | 117 ++++++++++++++++++ test/e2e/game_worker_pool_e2e_test.go | 5 + test/e2e/specs.md | 6 +- 9 files changed, 305 insertions(+), 8 deletions(-) create mode 100644 test/e2e/game_worker_operator_e2e_test.go diff --git a/operator/internal/controller/game_workload_policy.go b/operator/internal/controller/game_workload_policy.go index 5db30913f..bd0419e8b 100644 --- a/operator/internal/controller/game_workload_policy.go +++ b/operator/internal/controller/game_workload_policy.go @@ -1,15 +1,21 @@ package controller import ( + "context" "encoding/json" "fmt" "io" "maps" "strings" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" gameplanev1alpha1 "github.com/GameplanePanel/gameplane/operator/api/v1alpha1" ) @@ -25,6 +31,65 @@ type GameWorkloadPolicy struct { MaxMemoryLimit resource.Quantity `json:"maxMemoryLimit"` } +// A rejected rollout must retain the effective template for the running +// game's stop sequence. The rejected target is never rendered or executed. +type workloadPolicyRejection struct{ error } + +func (r *GameServerReconciler) reconcileRejectedGameWorkload(ctx context.Context, gs *gameplanev1alpha1.GameServer, + effective *gameplanev1alpha1.GameTemplate, rejection error, +) (ctrl.Result, error) { + var result ctrl.Result + if gs.Spec.Suspend { + key := client.ObjectKeyFromObject(gs) + var live gameplanev1alpha1.GameServer + if err := r.wipeReader().Get(ctx, key, &live); err != nil { + return result, client.IgnoreNotFound(err) + } + if !live.Spec.Suspend || live.UID != gs.UID || live.Generation != gs.Generation || + live.Annotations[effectiveTemplateAnnotation] != gs.Annotations[effectiveTemplateAnnotation] { + return ctrl.Result{RequeueAfter: reconcileRetryInterval}, nil + } + var ss appsv1.StatefulSet + err := r.wipeReader().Get(ctx, key, &ss) + if err != nil && !apierrors.IsNotFound(err) { + return result, err + } + if err == nil && metav1.IsControlledBy(&ss, &live) && (ss.Spec.Replicas == nil || *ss.Spec.Replicas != 0) { + if live.Annotations[effectiveTemplateAnnotation] == "" { + // A legacy workload has no verified template pin. Use its own + // Kubernetes termination grace instead of sending commands from + // catalog content that may never have run on this server. + effective = &gameplanev1alpha1.GameTemplate{} + } + op, held, err := lifecycleOwner(&live) + if err != nil { + return result, err + } + var replicas int32 + if held && op.Kind == "Rollout" && op.Baseline && len(op.Recovery) == 0 { + // Baseline normally preserves the existing workload. An explicit + // stop can still drain it while the new baseline is inadmissible. + replicas, result.RequeueAfter, err = r.softStop(ctx, &live, effective) + } else { + replicas, result.RequeueAfter, err = r.desiredReplicas(ctx, &live, effective, idleAwake) + } + if err != nil { + return requeueOnConflict(result, err) + } + if replicas == 0 { + base := ss.DeepCopy() + ss.Spec.Replicas = &replicas + // Pin the original StatefulSet resourceVersion and preserve every + // field of the existing Pod template, including its revision. + if err := r.Patch(ctx, &ss, client.MergeFromWithOptions(base, client.MergeFromWithOptimisticLock{})); err != nil { + return requeueOnConflict(result, err) + } + } + } + } + return result, r.setPhase(ctx, gs, rejection.Error()) +} + // ParseGameWorkloadPolicy rejects incomplete or unrecognized configuration at startup. func ParseGameWorkloadPolicy(raw string) (*GameWorkloadPolicy, error) { if raw == "" { diff --git a/operator/internal/controller/game_workload_policy_test.go b/operator/internal/controller/game_workload_policy_test.go index ff1150bf1..8e0fcac4f 100644 --- a/operator/internal/controller/game_workload_policy_test.go +++ b/operator/internal/controller/game_workload_policy_test.go @@ -1,6 +1,7 @@ package controller import ( + "reflect" "strings" "testing" @@ -149,3 +150,93 @@ func TestStoredGameCannotCreateWorkloadOutsideWorkerPolicy(t *testing.T) { t.Fatalf("unsafe workload: %v", err) } } + +func TestRejectedWorkloadOnlyStopsItsOwnedStatefulSet(t *testing.T) { + for _, tc := range []struct { + name string + legacy bool + suspended bool + foreign bool + want int32 + }{ + {name: "pinned and running", want: 1}, + {name: "pinned and suspended", suspended: true}, + {name: "unadmitted baseline and suspended", legacy: true, suspended: true}, + {name: "foreign StatefulSet", suspended: true, foreign: true, want: 1}, + } { + t.Run(tc.name, func(t *testing.T) { + r, gs, effective := revisionFixture(t) + if err := r.reconcileStatefulSet(t.Context(), gs, effective, nil, &materializedConfig{}, 1); err != nil { + t.Fatal(err) + } + key := client.ObjectKeyFromObject(gs) + var ss appsv1.StatefulSet + if err := r.Get(t.Context(), key, &ss); err != nil { + t.Fatal(err) + } + if tc.foreign { + ss.OwnerReferences[0].UID = "another-server" + if err := r.Update(t.Context(), &ss); err != nil { + t.Fatal(err) + } + } + original := ss.Spec.DeepCopy() + if tc.legacy { + delete(gs.Annotations, effectiveTemplateAnnotation) + } + r.GameWorkloadPolicy, _, _ = gamePolicyFixture(t) + gs.Spec.NodeSelector = map[string]string{r.GameWorkloadPolicy.PoolLabel: "invalid"} + gs.Spec.Suspend = tc.suspended + if err := r.Update(t.Context(), gs); err != nil { + t.Fatal(err) + } + if _, err := r.Reconcile(t.Context(), ctrl.Request{NamespacedName: key}); err != nil { + t.Fatal(err) + } + if err := r.Get(t.Context(), key, &ss); err != nil { + t.Fatal(err) + } + original.Replicas = &tc.want + if !reflect.DeepEqual(ss.Spec, *original) { + t.Fatalf("expected existing workload preserved with %d replicas, got %+v", tc.want, ss.Spec) + } + }) + } +} + +func TestRejectedRolloutDrainsWithThePinnedStopContract(t *testing.T) { + r, gs, old := revisionFixture(t) + old.Spec.RCON = &gameplanev1alpha1.RCONSpec{Protocol: "source"} + old.Spec.Capabilities = &gameplanev1alpha1.CapabilitiesSpec{Lifecycle: &gameplanev1alpha1.LifecycleSpec{Stop: []string{"save", "stop"}}} + pin, err := r.saveTemplateSnapshot(t.Context(), gs, old) + if err != nil { + t.Fatal(err) + } + gs.Annotations[effectiveTemplateAnnotation] = pin + gs.Spec.TemplateRollout.Policy = "Automatic" + gs.Spec.Suspend = true + if err := r.Update(t.Context(), gs); err != nil { + t.Fatal(err) + } + one := int32(1) + ss := &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{Name: gs.Name, Namespace: gs.Namespace, OwnerReferences: []metav1.OwnerReference{ + *metav1.NewControllerRef(gs, gameplanev1alpha1.GroupVersion.WithKind("GameServer")), + }}, + Spec: appsv1.StatefulSetSpec{Replicas: &one}, Status: appsv1.StatefulSetStatus{ReadyReplicas: 1, Replicas: 1}, + } + if err := r.Create(t.Context(), ss); err != nil { + t.Fatal(err) + } + stopper := &revisionStopper{} + r.AgentClient = stopper + r.GameWorkloadPolicy, _, _ = gamePolicyFixture(t) + key := client.ObjectKeyFromObject(gs) + result, err := r.Reconcile(t.Context(), ctrl.Request{NamespacedName: key}) + if err != nil || result.RequeueAfter <= 0 || stopper.calls != 1 { + t.Fatalf("pinned graceful stop: calls=%d requeue=%v err=%v", stopper.calls, result.RequeueAfter, err) + } + if err := r.Get(t.Context(), key, ss); err != nil || *ss.Spec.Replicas != 1 { + t.Fatalf("failed to preserve the running world during grace: %v", err) + } +} diff --git a/operator/internal/controller/gameserver_controller.go b/operator/internal/controller/gameserver_controller.go index a8c849702..7a1a8630b 100644 --- a/operator/internal/controller/gameserver_controller.go +++ b/operator/internal/controller/gameserver_controller.go @@ -297,6 +297,10 @@ func (r *GameServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) // services and every other template-derived resource. effective, changed, err := r.reconcileTemplateRevision(ctx, &gs) if err != nil { + var rejected workloadPolicyRejection + if errors.As(err, &rejected) { + return r.reconcileRejectedGameWorkload(ctx, &gs, effective, rejected) + } if apierrors.IsConflict(err) { return requeueOnConflict(ctrl.Result{}, err) } @@ -324,7 +328,7 @@ func (r *GameServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) } } if err := r.GameWorkloadPolicy.validate(&gs, &tmpl); err != nil { - return ctrl.Result{}, r.setPhase(ctx, &gs, err.Error()) + return r.reconcileRejectedGameWorkload(ctx, &gs, &tmpl, err) } mc, err := materializeConfig(&gs, &tmpl) if err != nil { diff --git a/operator/internal/controller/gameserver_template_revision.go b/operator/internal/controller/gameserver_template_revision.go index f03157137..91e6f28b6 100644 --- a/operator/internal/controller/gameserver_template_revision.go +++ b/operator/internal/controller/gameserver_template_revision.go @@ -202,7 +202,7 @@ func (r *GameServerReconciler) reconcileTemplateRevision(ctx context.Context, gs return nil, false, err } if err := r.GameWorkloadPolicy.validate(gs, target); err != nil { - return nil, false, err + return effective, false, workloadPolicyRejection{err} } if r.RestrictedWorkloads { if err := validateRestrictedTemplate(target); err != nil { @@ -256,7 +256,7 @@ func (r *GameServerReconciler) reconcileTemplateRevision(ctx context.Context, gs return nil, false, err } if err := r.GameWorkloadPolicy.validate(gs, &installed); err != nil { - return nil, false, err + return effective, false, workloadPolicyRejection{err} } if r.RestrictedWorkloads { if err := validateRestrictedTemplate(&installed); err != nil { @@ -317,7 +317,7 @@ func (r *GameServerReconciler) initializeTemplateBaseline(ctx context.Context, g return err } if err := r.GameWorkloadPolicy.validate(gs, target); err != nil { - return err + return workloadPolicyRejection{err} } var ss appsv1.StatefulSet err = r.wipeReader().Get(ctx, client.ObjectKeyFromObject(gs), &ss) diff --git a/operator/specs.md b/operator/specs.md index ad0254e01..32435b3e8 100644 --- a/operator/specs.md +++ b/operator/specs.md @@ -332,7 +332,19 @@ No new limits are set. Forget/prune retains 50m/128Mi and its existing 512Mi memory limit when no override is configured. Explicit `{}` clears defaults. Builders deep-copy resource maps. Template agent requests/limits, when declared, replace install agent defaults; image-only overrides keep those defaults. -Game resources and GameServer placement retain their existing semantics. +For the game container, each GameServer request or limit overrides the matching +resource in the template. Omitted quantities retain the template value. The +operator rejects effective requests above their limits. The optional +`--game-workload-policy` JSON flag (`operator.gameWorkloadPolicy` in Helm) +bounds CPU requests and requires a memory reservation and ceiling. It adds +the configured game worker selector and exact NoSchedule toleration and +rejects conflicting selectors or broader tolerations. These checks run for +direct and stored GameServers and before template revision adoption. An +explicit suspend on a rejected configuration gracefully drains an existing +owned StatefulSet using its pinned template, subject to lifecycle ownership. +A legacy workload without a pin uses its Kubernetes termination grace. +Only its replica count can change. Correct the reported policy violation +before starting or rolling out the game. Chart placement and optional PDBs are documented in [`docs/production-placement.md`](../docs/production-placement.md). diff --git a/test/e2e/game_worker_capacity_e2e_test.go b/test/e2e/game_worker_capacity_e2e_test.go index 775b8bc16..65c0f083b 100644 --- a/test/e2e/game_worker_capacity_e2e_test.go +++ b/test/e2e/game_worker_capacity_e2e_test.go @@ -64,11 +64,10 @@ func startPlacementBackup(ctx context.Context, t *testing.T, ns, poolKey, worker work := `export RESTIC_REPOSITORY=/tmp/repo RESTIC_PASSWORD=disposable mkdir /tmp/world restic init -for i in $(seq 1 120); do +while test ! -e /tmp/finish; do dd if=/dev/urandom of=/tmp/world/chunk bs=1M count=2 2>/dev/null restic backup -q /tmp/world touch /tmp/snapshot-complete - test ! -e /tmp/finish || break sleep 1 done restic check --read-data` diff --git a/test/e2e/game_worker_operator_e2e_test.go b/test/e2e/game_worker_operator_e2e_test.go new file mode 100644 index 000000000..1baef8165 --- /dev/null +++ b/test/e2e/game_worker_operator_e2e_test.go @@ -0,0 +1,117 @@ +//go:build e2e + +package e2e + +import ( + "context" + "encoding/json" + "fmt" + "slices" + "strings" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// Exercise the controller across a policy rollout on the isolated placement +// cluster. Direct CR writes also cover installations without the panel API. +func checkGameServerWorkerPolicy(t *testing.T, worker string) { + e, ctx := envInstance, t.Context() + const ns, system = "gameplane-games", "gameplane-system" + const template, legacy, valid, refused = "e2e-pool-template", "e2e-pool-legacy", "e2e-pool-valid", "e2e-pool-refused" + const key = "gameplane.local/e2e-operator-worker" + kubectl := func(ctx context.Context, args ...string) string { + t.Helper() + out, err := e.Kubectl(ctx, args...) + if err != nil { + t.Fatalf("kubectl %v: %v: %s", args, err, out) + } + return string(out) + } + pods, sets := e.K8s.CoreV1().Pods(ns), e.K8s.AppsV1().StatefulSets(ns) + applyBusyboxTemplate(t, template) + kubectl(ctx, "patch", "gametemplate", template, "--type=merge", "-p", + `{"spec":{"resources":{"requests":{"cpu":"25m","memory":"16Mi"},"limits":{"memory":"32Mi"}}}}`) + applyBusyboxGameServer(t, ns, legacy, template) + kubectl(ctx, "-n", ns, "patch", "gameserver", legacy, "--type=merge", "-p", + `{"spec":{"resources":{"limits":{"memory":"64Mi"}}}}`) + e.Eventually(t, 3*time.Minute, func() (bool, string) { + pod, err := pods.Get(ctx, legacy+"-0", metav1.GetOptions{}) + if err != nil { + return false, err.Error() + } + game := pod.Spec.Containers[0] + return pod.Status.Phase == corev1.PodRunning && game.Name == "game" && + game.Resources.Limits.Memory().String() == "64Mi", fmt.Sprint(pod.Status.Phase, game.Resources) + }) + before, err := sets.Get(ctx, legacy, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + kubectl(ctx, "label", "node", worker, key+"=games") + t.Cleanup(func() { kubectl(context.Background(), "label", "node", worker, key+"-") }) + dep, err := e.K8s.AppsV1().Deployments(system).Get(ctx, "gameplane-operator", metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + manager := dep.Spec.Template.Spec.Containers[0] + if manager.Name != "manager" { + t.Fatalf("unexpected operator container %q", manager.Name) + } + setArgs := func(ctx context.Context, args []string) { + t.Helper() + data, err := json.Marshal(args) + if err != nil { + t.Fatal(err) + } + patch := fmt.Sprintf(`{"spec":{"template":{"spec":{"containers":[{"name":"manager","args":%s}]}}}}`, data) + kubectl(ctx, "-n", system, "patch", "deployment/gameplane-operator", "--type=strategic", "-p", patch) + kubectl(ctx, "-n", system, "rollout", "status", "deployment/gameplane-operator", "--timeout=3m") + } + t.Cleanup(func() { setArgs(context.Background(), manager.Args) }) + policy := `--game-workload-policy={"poolLabel":"` + key + `","poolValue":"games","minCPURequest":"10m","maxCPURequest":"1","minMemoryRequest":"8Mi","maxMemoryLimit":"32Mi"}` + setArgs(ctx, append(slices.Clone(manager.Args), policy)) + waitRejection := func(name, reason string) { + t.Helper() + e.Eventually(t, time.Minute, func() (bool, string) { + m := kubectl(ctx, "-n", ns, "get", "gameserver", name, "-o", + `jsonpath={.status.conditions[?(@.type=="Ready")].message}`) + return strings.Contains(m, reason), m + }) + } + waitRejection(legacy, "resources.limits.memory") + kubectl(ctx, "-n", ns, "patch", "gameserver", legacy, "--type=merge", "-p", `{"spec":{"suspend":true}}`) + e.Eventually(t, 2*time.Minute, func() (bool, string) { + ss, err := sets.Get(ctx, legacy, metav1.GetOptions{}) + if err != nil { + return false, err.Error() + } + _, podErr := pods.Get(ctx, legacy+"-0", metav1.GetOptions{}) + return ss.UID == before.UID && ss.Spec.Replicas != nil && *ss.Spec.Replicas == 0 && apierrors.IsNotFound(podErr) && + ss.Spec.Template.Spec.Containers[0].Resources.Limits.Memory().String() == "64Mi", fmt.Sprint(ss.Status, podErr) + }) + applyBusyboxTemplate(t, refused) + applyBusyboxGameServer(t, ns, refused, refused) + waitRejection(refused, "resources.requests.cpu") + if _, err := sets.Get(ctx, refused, metav1.GetOptions{}); !apierrors.IsNotFound(err) { + t.Fatalf("rejected GameServer produced a StatefulSet or lookup failed: %v", err) + } + applyBusyboxGameServer(t, ns, valid, template) + kubectl(ctx, "-n", ns, "patch", "gameserver", valid, "--type=merge", "-p", + `{"spec":{"resources":{"requests":{"cpu":"50m"}}}}`) + e.Eventually(t, 3*time.Minute, func() (bool, string) { + pod, err := pods.Get(ctx, valid+"-0", metav1.GetOptions{}) + if err != nil { + return false, err.Error() + } + game := pod.Spec.Containers[0] + match := corev1.Toleration{Key: key, Value: "games", Operator: corev1.TolerationOpEqual, Effect: corev1.TaintEffectNoSchedule} + return pod.Status.Phase == corev1.PodRunning && pod.Spec.NodeName == worker && pod.Spec.NodeSelector[key] == "games" && + slices.Contains(pod.Spec.Tolerations, match) && game.Name == "game" && game.Resources.Requests.Cpu().String() == "50m" && + game.Resources.Requests.Memory().String() == "16Mi" && game.Resources.Limits.Memory().String() == "32Mi", + fmt.Sprint(pod.Spec.NodeName, pod.Status.Phase, game.Resources) + }) +} diff --git a/test/e2e/game_worker_pool_e2e_test.go b/test/e2e/game_worker_pool_e2e_test.go index 990601682..06542a151 100644 --- a/test/e2e/game_worker_pool_e2e_test.go +++ b/test/e2e/game_worker_pool_e2e_test.go @@ -40,6 +40,11 @@ func TestGameWorkerPoolAndQuota(t *testing.T) { if len(workers) < 2 { t.Fatal("need two schedulable workers") } + if !t.Run("GameServer policy reconciliation", func(t *testing.T) { + checkGameServerWorkerPolicy(t, workers[0]) + }) { + return + } neighbor := minecraftBotSpec() neighbor.Template = "e2e-placement-neighbor" neighbor.Env["PAUSE_WHEN_EMPTY_SECONDS"] = "0" diff --git a/test/e2e/specs.md b/test/e2e/specs.md index d5edf297e..aa0578dce 100644 --- a/test/e2e/specs.md +++ b/test/e2e/specs.md @@ -134,7 +134,11 @@ a cordon, readiness of the surviving replica, and singleton API eviction. Node labels and cordons are restored in cleanup. This validates scheduler/PDB behavior, not application HA or storage migration. -`TestGameWorkerPoolAndQuota` applies the production chart's game Pod +`TestGameWorkerPoolAndQuota` first enables the operator worker policy with a +legacy server running. It checks that the rejected server can stop, that a +directly submitted out-of-bounds server gets no StatefulSet, and that a +valid partial override preserves template memory bounds on the worker Pod. +It then applies the production chart's game Pod admission and tenant quota to an isolated namespace. A second Pod exceeds its quota. When the test cordons the only game worker and evicts its game Pod, the replacement stays Pending until a second worker joins the pool. Its allocating From 9486f5b4e51edc63649c4c485719f3cd2c6f54fb Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 05:33:04 -0500 Subject: [PATCH 06/10] fix(operator): retain policy parse errors and isolate worker fixtures Signed-off-by: Baylem --- .../controller/game_workload_policy.go | 4 +- .../controller/game_workload_policy_test.go | 6 +++ test/e2e/game_worker_capacity_e2e_test.go | 7 ++-- test/e2e/game_worker_operator_e2e_test.go | 37 ++++++++++++++----- test/e2e/game_worker_pool_e2e_test.go | 4 +- 5 files changed, 42 insertions(+), 16 deletions(-) diff --git a/operator/internal/controller/game_workload_policy.go b/operator/internal/controller/game_workload_policy.go index bd0419e8b..c975ad450 100644 --- a/operator/internal/controller/game_workload_policy.go +++ b/operator/internal/controller/game_workload_policy.go @@ -101,8 +101,10 @@ func ParseGameWorkloadPolicy(raw string) (*GameWorkloadPolicy, error) { if err := decoder.Decode(&policy); err != nil { return nil, fmt.Errorf("decode game workload policy: %w", err) } - if decoder.Decode(new(any)) != io.EOF { + if err := decoder.Decode(new(any)); err == nil { return nil, fmt.Errorf("expected a single game workload policy object") + } else if err != io.EOF { + return nil, fmt.Errorf("decode trailing game workload policy data: %w", err) } if len(validation.IsQualifiedName(policy.PoolLabel)) != 0 || policy.PoolValue == "" || len(validation.IsValidLabelValue(policy.PoolValue)) != 0 { return nil, fmt.Errorf("poolLabel and poolValue must identify a valid, nonempty worker label") diff --git a/operator/internal/controller/game_workload_policy_test.go b/operator/internal/controller/game_workload_policy_test.go index 8e0fcac4f..e385574f9 100644 --- a/operator/internal/controller/game_workload_policy_test.go +++ b/operator/internal/controller/game_workload_policy_test.go @@ -1,6 +1,8 @@ package controller import ( + "encoding/json" + "errors" "reflect" "strings" "testing" @@ -30,6 +32,10 @@ func TestGameWorkloadPolicyConfiguration(t *testing.T) { t.Errorf("accepted invalid policy %s", raw) } } + var syntax *json.SyntaxError + if _, err := ParseGameWorkloadPolicy(gamePolicyJSON + "!"); !errors.As(err, &syntax) { + t.Fatalf("trailing JSON syntax error lost: %v", err) + } } func gamePolicyFixture(t *testing.T) (*GameWorkloadPolicy, *gameplanev1alpha1.GameServer, *gameplanev1alpha1.GameTemplate) { diff --git a/test/e2e/game_worker_capacity_e2e_test.go b/test/e2e/game_worker_capacity_e2e_test.go index 65c0f083b..65176dd72 100644 --- a/test/e2e/game_worker_capacity_e2e_test.go +++ b/test/e2e/game_worker_capacity_e2e_test.go @@ -60,7 +60,6 @@ func recordWorkerCapacity(ctx context.Context, t *testing.T, worker string) { func startPlacementBackup(ctx context.Context, t *testing.T, ns, poolKey, worker string) func() { t.Helper() pods := envInstance.K8s.CoreV1().Pods(ns) - const name = "placement-restic" work := `export RESTIC_REPOSITORY=/tmp/repo RESTIC_PASSWORD=disposable mkdir /tmp/world restic init @@ -71,16 +70,18 @@ while test ! -e /tmp/finish; do sleep 1 done restic check --read-data` - pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: name}, Spec: corev1.PodSpec{ + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{GenerateName: "placement-restic-"}, Spec: corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, NodeSelector: map[string]string{poolKey: "games"}, Tolerations: []corev1.Toleration{{Key: poolKey, Operator: corev1.TolerationOpEqual, Value: "games", Effect: corev1.TaintEffectNoSchedule}}, Containers: []corev1.Container{{Name: "backup", Image: "restic/restic:0.19.1", Command: []string{"sh", "-ec", work}, Resources: corev1.ResourceRequirements{Requests: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("100m"), corev1.ResourceMemory: resource.MustParse("128Mi")}, Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("512Mi")}}}}, }} - if _, err := pods.Create(ctx, pod, metav1.CreateOptions{}); err != nil { + created, err := pods.Create(ctx, pod, metav1.CreateOptions{}) + if err != nil { t.Fatal(err) } + name := created.Name t.Cleanup(func() { cleanup, cancel := context.WithTimeout(context.WithoutCancel(ctx), 15*time.Second) defer cancel() diff --git a/test/e2e/game_worker_operator_e2e_test.go b/test/e2e/game_worker_operator_e2e_test.go index 1baef8165..085b67445 100644 --- a/test/e2e/game_worker_operator_e2e_test.go +++ b/test/e2e/game_worker_operator_e2e_test.go @@ -18,11 +18,11 @@ import ( // Exercise the controller across a policy rollout on the isolated placement // cluster. Direct CR writes also cover installations without the panel API. -func checkGameServerWorkerPolicy(t *testing.T, worker string) { +func checkGameServerWorkerPolicy(t *testing.T, worker, prefix string) { e, ctx := envInstance, t.Context() const ns, system = "gameplane-games", "gameplane-system" - const template, legacy, valid, refused = "e2e-pool-template", "e2e-pool-legacy", "e2e-pool-valid", "e2e-pool-refused" - const key = "gameplane.local/e2e-operator-worker" + template, legacy, valid, refused := prefix+"-template", prefix+"-legacy", prefix+"-valid", prefix+"-refused" + key := "gameplane.local/" + prefix kubectl := func(ctx context.Context, args ...string) string { t.Helper() out, err := e.Kubectl(ctx, args...) @@ -61,19 +61,36 @@ func checkGameServerWorkerPolicy(t *testing.T, worker string) { if manager.Name != "manager" { t.Fatalf("unexpected operator container %q", manager.Name) } - setArgs := func(ctx context.Context, args []string) { + for _, arg := range manager.Args { + if strings.HasPrefix(arg, "--game-workload-policy") { + t.Fatalf("operator already has a worker policy; run in the isolated placement cluster") + } + } + setArgs := func(ctx context.Context, current, next []string) { t.Helper() - data, err := json.Marshal(args) + const path = "/spec/template/spec/containers/0/args" + data, err := json.Marshal([]map[string]any{ + {"op": "test", "path": path, "value": current}, + {"op": "replace", "path": path, "value": next}, + }) if err != nil { t.Fatal(err) } - patch := fmt.Sprintf(`{"spec":{"template":{"spec":{"containers":[{"name":"manager","args":%s}]}}}}`, data) - kubectl(ctx, "-n", system, "patch", "deployment/gameplane-operator", "--type=strategic", "-p", patch) - kubectl(ctx, "-n", system, "rollout", "status", "deployment/gameplane-operator", "--timeout=3m") + kubectl(ctx, "-n", system, "patch", "deployment/gameplane-operator", "--type=json", "-p", string(data)) } - t.Cleanup(func() { setArgs(context.Background(), manager.Args) }) policy := `--game-workload-policy={"poolLabel":"` + key + `","poolValue":"games","minCPURequest":"10m","maxCPURequest":"1","minMemoryRequest":"8Mi","maxMemoryLimit":"32Mi"}` - setArgs(ctx, append(slices.Clone(manager.Args), policy)) + installed := append(slices.Clone(manager.Args), policy) + setArgs(ctx, manager.Args, installed) + awaitRollout := func(ctx context.Context) { + kubectl(ctx, "-n", system, "rollout", "status", "deployment/gameplane-operator", "--timeout=3m") + } + t.Cleanup(func() { + cleanup, cancel := context.WithTimeout(context.Background(), 4*time.Minute) + defer cancel() + setArgs(cleanup, installed, manager.Args) + awaitRollout(cleanup) + }) + awaitRollout(ctx) waitRejection := func(name, reason string) { t.Helper() e.Eventually(t, time.Minute, func() (bool, string) { diff --git a/test/e2e/game_worker_pool_e2e_test.go b/test/e2e/game_worker_pool_e2e_test.go index 06542a151..bbfa7ca67 100644 --- a/test/e2e/game_worker_pool_e2e_test.go +++ b/test/e2e/game_worker_pool_e2e_test.go @@ -41,12 +41,12 @@ func TestGameWorkerPoolAndQuota(t *testing.T) { t.Fatal("need two schedulable workers") } if !t.Run("GameServer policy reconciliation", func(t *testing.T) { - checkGameServerWorkerPolicy(t, workers[0]) + checkGameServerWorkerPolicy(t, workers[0], ns) }) { return } neighbor := minecraftBotSpec() - neighbor.Template = "e2e-placement-neighbor" + neighbor.Template = ns + "-neighbor" neighbor.Env["PAUSE_WHEN_EMPTY_SECONDS"] = "0" gameName, gameNS := createGameBotServer(t, e, neighbor) gamePod, err := k.Pods(gameNS).Get(ctx, gameName+"-0", metav1.GetOptions{}) From a2def5e84ecf0da57137a9734ecf8dc502337557 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 07:47:14 -0500 Subject: [PATCH 07/10] fix(e2e): use Kubernetes clients for structured worker policy inputs Signed-off-by: Baylem --- test/e2e/game_worker_operator_e2e_test.go | 28 +++++++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/test/e2e/game_worker_operator_e2e_test.go b/test/e2e/game_worker_operator_e2e_test.go index 085b67445..d09c68daa 100644 --- a/test/e2e/game_worker_operator_e2e_test.go +++ b/test/e2e/game_worker_operator_e2e_test.go @@ -14,6 +14,8 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" ) // Exercise the controller across a policy rollout on the isolated placement @@ -53,7 +55,8 @@ func checkGameServerWorkerPolicy(t *testing.T, worker, prefix string) { } kubectl(ctx, "label", "node", worker, key+"=games") t.Cleanup(func() { kubectl(context.Background(), "label", "node", worker, key+"-") }) - dep, err := e.K8s.AppsV1().Deployments(system).Get(ctx, "gameplane-operator", metav1.GetOptions{}) + deployments := e.K8s.AppsV1().Deployments(system) + dep, err := deployments.Get(ctx, "gameplane-operator", metav1.GetOptions{}) if err != nil { t.Fatal(err) } @@ -76,7 +79,9 @@ func checkGameServerWorkerPolicy(t *testing.T, worker, prefix string) { if err != nil { t.Fatal(err) } - kubectl(ctx, "-n", system, "patch", "deployment/gameplane-operator", "--type=json", "-p", string(data)) + if _, err := deployments.Patch(ctx, dep.Name, types.JSONPatchType, data, metav1.PatchOptions{}); err != nil { + t.Fatalf("patch operator arguments: %v", err) + } } policy := `--game-workload-policy={"poolLabel":"` + key + `","poolValue":"games","minCPURequest":"10m","maxCPURequest":"1","minMemoryRequest":"8Mi","maxMemoryLimit":"32Mi"}` installed := append(slices.Clone(manager.Args), policy) @@ -94,9 +99,22 @@ func checkGameServerWorkerPolicy(t *testing.T, worker, prefix string) { waitRejection := func(name, reason string) { t.Helper() e.Eventually(t, time.Minute, func() (bool, string) { - m := kubectl(ctx, "-n", ns, "get", "gameserver", name, "-o", - `jsonpath={.status.conditions[?(@.type=="Ready")].message}`) - return strings.Contains(m, reason), m + gs, err := e.Dyn.Resource(gameServerGVR).Namespace(ns).Get(ctx, name, metav1.GetOptions{}) + if err != nil { + return false, err.Error() + } + conditions, _, err := unstructured.NestedSlice(gs.Object, "status", "conditions") + if err != nil { + return false, err.Error() + } + for _, raw := range conditions { + c, _ := raw.(map[string]any) + if c["type"] == "Ready" && c["status"] == "False" { + message, _ := c["message"].(string) + return strings.Contains(message, reason), message + } + } + return false, "waiting for Ready=False" }) } waitRejection(legacy, "resources.limits.memory") From a5a4d17535607669890ba9a523bc19ff25c54c35 Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 09:31:19 -0500 Subject: [PATCH 08/10] test(operator): verify tenant OOM kills in the Pod memory cgroup Signed-off-by: Baylem --- test/e2e/game_worker_capacity_e2e_test.go | 52 +++++++++++++++++++++++ test/e2e/game_worker_pool_e2e_test.go | 43 ++++++++++++++----- test/e2e/specs.md | 7 ++- 3 files changed, 91 insertions(+), 11 deletions(-) diff --git a/test/e2e/game_worker_capacity_e2e_test.go b/test/e2e/game_worker_capacity_e2e_test.go index 65176dd72..feb95196e 100644 --- a/test/e2e/game_worker_capacity_e2e_test.go +++ b/test/e2e/game_worker_capacity_e2e_test.go @@ -4,7 +4,9 @@ package e2e import ( "context" + "encoding/hex" "fmt" + "os/exec" "regexp" "strconv" "strings" @@ -18,6 +20,56 @@ import ( var tickPercentiles = regexp.MustCompile(`P95:\s*([0-9.]+)\s*ms\s+P99:\s*([0-9.]+)\s*ms`) +// The Pod parent survives a container restart. Its hierarchical memory.events +// retains the original leaf's limit hits and OOM kills after containerd +// removes that leaf. This test Pod has just one application container. +type tenantMemoryEvents struct { + group string + max, oom, kills uint64 +} + +func readTenantMemoryEvents(ctx context.Context, t *testing.T, pod *corev1.Pod) tenantMemoryEvents { + t.Helper() + if envInstance.Context != "kind-"+envInstance.ClusterName || !strings.HasPrefix(pod.Spec.NodeName, envInstance.ClusterName+"-") { + t.Fatal("memory qualification requires the disposable Kind worker") + } + if len(pod.Status.ContainerStatuses) != 1 || pod.Status.ContainerStatuses[0].Name != "game" { + t.Fatalf("unexpected tenant containers: %+v", pod.Status.ContainerStatuses) + } + id, ok := strings.CutPrefix(pod.Status.ContainerStatuses[0].ContainerID, "containerd://") + rawID, err := hex.DecodeString(id) + if !ok || err != nil || len(rawID) != 32 { + t.Fatalf("invalid containerd game container ID: %q", id) + } + const read = ` +set -eu +leaf=$(find /sys/fs/cgroup -xdev -ignore_readdir_race -type d \( -name "$1" -o -name "cri-containerd-$1.scope" \) -print) +test -d "$leaf" || { echo "missing unique cgroup for $1" >&2; exit 1; } +group=${leaf%/*} +systemd_uid=$(printf '%s' "$2" | tr '-' '_') +case ${group##*/} in + "pod$2"|*"-pod$systemd_uid.slice") ;; + *) echo "container $1 has unexpected Pod parent $group for $2" >&2; exit 1 ;; +esac +awk -v group="$group" -v limit="$(cat "$leaf/memory.max")" ' + { value[$1]=$2 } + END { + if (!("max" in value && "oom" in value && "oom_kill" in value)) exit 1 + print group, limit, value["max"], value["oom"], value["oom_kill"] + }' "$group/memory.events" +` + out, err := exec.CommandContext(ctx, "docker", "exec", pod.Spec.NodeName, "sh", "-ec", read, "sh", id, string(pod.UID)).CombinedOutput() + if err != nil { + t.Fatalf("read tenant cgroup on %s: %v: %s", pod.Spec.NodeName, err, out) + } + var snapshot tenantMemoryEvents + var limit int64 + if _, err := fmt.Sscan(string(out), &snapshot.group, &limit, &snapshot.max, &snapshot.oom, &snapshot.kills); err != nil || limit != 32<<20 { + t.Fatalf("expected counters and a 32 MiB game leaf on %s: %q (%v)", pod.Spec.NodeName, out, err) + } + return snapshot +} + func sampleNeighborTicks(ctx context.Context, t *testing.T, ns, pod, phase string) { t.Helper() out, err := envInstance.Kubectl(ctx, "exec", "-n", ns, pod, "-c", "game", "--", "rcon-cli", "tick", "query") diff --git a/test/e2e/game_worker_pool_e2e_test.go b/test/e2e/game_worker_pool_e2e_test.go index bbfa7ca67..f54a10035 100644 --- a/test/e2e/game_worker_pool_e2e_test.go +++ b/test/e2e/game_worker_pool_e2e_test.go @@ -157,6 +157,9 @@ func TestGameWorkerPoolAndQuota(t *testing.T) { } // Touch anonymous pages in one process. Forking a sleep helper at the // tenant's ceiling can fail before the kernel OOM-kills the allocator. + // OnFailure keeps the Pod cgroup alive for postmortem counters. The new + // container's writable layer has no /tmp/leak, so it stays idle. + second.Spec.RestartPolicy = corev1.RestartPolicyOnFailure second.Spec.Containers[0].Image = "python:3.14-alpine" second.Spec.Containers[0].Command = []string{"python", "-c", ` import mmap, os, time @@ -191,11 +194,19 @@ while True: }) labelWorker(workers[1]) stopBackup := startPlacementBackup(ctx, t, gameNS, key, workers[1]) + var leaking *corev1.Pod e.Eventually(t, time.Minute, func() (bool, string) { live, err := pods.Get(ctx, second.Name, metav1.GetOptions{}) - return err == nil && live.Spec.NodeName == workers[1] && live.Status.Phase == corev1.PodRunning, fmt.Sprint(err) + if err != nil { + return false, err.Error() + } + leaking = live + return live.Spec.NodeName == workers[1] && len(live.Status.ContainerStatuses) == 1 && + live.Status.ContainerStatuses[0].State.Running != nil && live.Status.ContainerStatuses[0].RestartCount == 0, fmt.Sprint(live.Status.Phase) }) recordWorkerCapacity(ctx, t, workers[1]) + before := readTenantMemoryEvents(ctx, t, leaking) + originalID := leaking.Status.ContainerStatuses[0].ContainerID kubectl("exec", "-n", ns, second.Name, "--", "touch", "/tmp/leak") sampleNeighborTicks(ctx, t, gameNS, gamePod.Name, "start, leak and backup after worker loss") e.Eventually(t, 3*time.Minute, func() (bool, string) { @@ -203,16 +214,28 @@ while True: if err != nil { return false, err.Error() } - for _, status := range live.Status.ContainerStatuses { - if ended := status.State.Terminated; ended != nil { - if live.Spec.NodeName != workers[1] || ended.Reason != "OOMKilled" { - logs, _ := e.Kubectl(ctx, "logs", "-n", ns, second.Name, "-c", "game", "--tail=10") - t.Fatalf("tenant stopped on %s: reason=%s exit=%d message=%s logs=%s", live.Spec.NodeName, ended.Reason, ended.ExitCode, ended.Message, logs) - } - return true, "" - } + if live.UID != leaking.UID || live.Spec.NodeName != leaking.Spec.NodeName { + t.Fatalf("leaking Pod changed identity or node: %s on %s", live.UID, live.Spec.NodeName) + } + if len(live.Status.ContainerStatuses) != 1 { + return false, "waiting for game container status" + } + status := live.Status.ContainerStatuses[0] + ended := status.LastTerminationState.Terminated + if status.State.Running == nil || ended == nil { + return false, fmt.Sprint(status.State) + } + if status.RestartCount != 1 || ended.ContainerID != originalID || status.ContainerID == originalID || + ended.ExitCode != 137 || (ended.Reason != "OOMKilled" && ended.Reason != "Error") { + t.Fatalf("unexpected game restart: %+v, previous termination: %+v", status, ended) + } + after := readTenantMemoryEvents(ctx, t, live) + if after.group != before.group || after.max <= before.max || after.oom <= before.oom || after.kills <= before.kills { + t.Fatalf("missing local limit/OOM/kill increments on %s: before=%+v after=%+v", live.Spec.NodeName, before, after) } - return false, fmt.Sprint(live.Status.Phase) + t.Logf("tenant %s container %s exited %d (%s) at 32 MiB; Pod cgroup counters before=%+v after=%+v", + live.UID, originalID, ended.ExitCode, ended.Reason, before, after) + return true, "" }) sampleNeighborTicks(ctx, t, gameNS, gamePod.Name, "after tenant OOM") stopBackup() diff --git a/test/e2e/specs.md b/test/e2e/specs.md index aa0578dce..0936453e0 100644 --- a/test/e2e/specs.md +++ b/test/e2e/specs.md @@ -142,7 +142,12 @@ It then applies the production chart's game Pod admission and tenant quota to an isolated namespace. A second Pod exceeds its quota. When the test cordons the only game worker and evicts its game Pod, the replacement stays Pending until a second worker joins the pool. Its allocating -process must terminate with `OOMKilled` at the configured memory limit. +process runs in a verified 32 MiB container cgroup. On the disposable Kind +worker, the test measures the Pod's hierarchical cgroup-v2 `memory.events` +before pressure and after a single container restart. The same Pod must record +increases in `max`, `oom` and `oom_kill`. The preceding container ID must match +the terminated game with exit 137. These counters distinguish a local limit +kill even when the runtime records the generic termination reason `Error`. ### Backup preparation acceptance From 64a412dcf99e90d67ac32b7643b6bb35e19ffb5e Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 17:22:17 -0500 Subject: [PATCH 09/10] fix(operator): drain legacy games before enforcing worker placement Signed-off-by: Baylem --- .../unreleased/903-game-worker-policy.md | 8 ++ docs/production-placement.md | 4 + .../controller/game_workload_policy.go | 3 + .../controller/game_workload_policy_test.go | 128 ++++++++++++++++++ .../controller/gameserver_controller.go | 30 +++- .../gameserver_template_revision.go | 98 +++++++++++--- operator/specs.md | 6 + test/e2e/game_worker_capacity_e2e_test.go | 4 + test/e2e/game_worker_pool_e2e_test.go | 7 +- 9 files changed, 267 insertions(+), 21 deletions(-) create mode 100644 changelog/unreleased/903-game-worker-policy.md diff --git a/changelog/unreleased/903-game-worker-policy.md b/changelog/unreleased/903-game-worker-policy.md new file mode 100644 index 000000000..f7b57605c --- /dev/null +++ b/changelog/unreleased/903-game-worker-policy.md @@ -0,0 +1,8 @@ +### Game worker placement and resource limits + +Administrators can reserve game workers with a node label and taint, bound CPU +requests and memory reservations and limits, and set per-namespace quotas. +GameServer overrides retain the template quantities for resources they omit. +When the policy takes over a legacy game, the operator drains the existing Pod +before applying placement and resource changes. A verified running revision +uses its saved stop sequence. An unversioned game uses its Pod termination grace. diff --git a/docs/production-placement.md b/docs/production-placement.md index 8f3e3f3ee..e2a33315c 100644 --- a/docs/production-placement.md +++ b/docs/production-placement.md @@ -113,6 +113,10 @@ to each game Pod. A server can narrow placement with additional node labels or affinity. A conflicting pool selector or an extra toleration produces a failed Ready condition. A pool with no schedulable workers leaves games Pending. Keep permission to change pool labels and taints with cluster administrators. +Enabling the policy for a legacy game can briefly interrupt play. The operator +drains its existing Pod before moving it to the configured pool. Games with +a verified running template use its stop sequence. Unversioned games use the +termination grace already configured on their Pods. With the NodeRestriction admission plugin, a label key such as `gameplane.node-restriction.kubernetes.io/pool` also prevents a kubelet from labeling itself as a game worker. Set the configured key and worker taint to match. diff --git a/operator/internal/controller/game_workload_policy.go b/operator/internal/controller/game_workload_policy.go index c975ad450..a85b5c100 100644 --- a/operator/internal/controller/game_workload_policy.go +++ b/operator/internal/controller/game_workload_policy.go @@ -35,6 +35,7 @@ type GameWorkloadPolicy struct { // game's stop sequence. The rejected target is never rendered or executed. type workloadPolicyRejection struct{ error } +// reconcileRejectedGameWorkload permits a suspended, owned server to drain while its requested configuration is invalid. func (r *GameServerReconciler) reconcileRejectedGameWorkload(ctx context.Context, gs *gameplanev1alpha1.GameServer, effective *gameplanev1alpha1.GameTemplate, rejection error, ) (ctrl.Result, error) { @@ -116,6 +117,7 @@ func ParseGameWorkloadPolicy(raw string) (*GameWorkloadPolicy, error) { return &policy, nil } +// validate checks the merged game resources and placement before the controller can create or change a workload. func (p *GameWorkloadPolicy) validate(gs *gameplanev1alpha1.GameServer, tmpl *gameplanev1alpha1.GameTemplate) error { res := effectiveResources(gs, tmpl) for name, request := range res.Requests { @@ -146,6 +148,7 @@ func (p *GameWorkloadPolicy) validate(gs *gameplanev1alpha1.GameServer, tmpl *ga return nil } +// placement combines the server's node selectors with the administrator's dedicated worker-pool taint and label. func (p *GameWorkloadPolicy) placement(gs *gameplanev1alpha1.GameServer) (map[string]string, []corev1.Toleration) { if p == nil { return gs.Spec.NodeSelector, gs.Spec.Tolerations diff --git a/operator/internal/controller/game_workload_policy_test.go b/operator/internal/controller/game_workload_policy_test.go index e385574f9..d3be5a50b 100644 --- a/operator/internal/controller/game_workload_policy_test.go +++ b/operator/internal/controller/game_workload_policy_test.go @@ -21,6 +21,7 @@ import ( const gamePolicyJSON = `{"poolLabel":"gameplane.local/pool","poolValue":"games","minCPURequest":"100m","maxCPURequest":"4","minMemoryRequest":"128Mi","maxMemoryLimit":"8Gi"}` +// TestGameWorkloadPolicyConfiguration rejects malformed, ambiguous and incomplete administrator policies. func TestGameWorkloadPolicyConfiguration(t *testing.T) { if p, err := ParseGameWorkloadPolicy(""); err != nil || p != nil { t.Fatalf("absent policy: %v %v", p, err) @@ -38,6 +39,7 @@ func TestGameWorkloadPolicyConfiguration(t *testing.T) { } } +// gamePolicyFixture provides a valid resource tier with no CPU limit for burst capacity. func gamePolicyFixture(t *testing.T) (*GameWorkloadPolicy, *gameplanev1alpha1.GameServer, *gameplanev1alpha1.GameTemplate) { t.Helper() p, err := ParseGameWorkloadPolicy(gamePolicyJSON) @@ -52,6 +54,7 @@ func gamePolicyFixture(t *testing.T) (*GameWorkloadPolicy, *gameplanev1alpha1.Ga return p, &gameplanev1alpha1.GameServer{}, tmpl } +// TestGamePartialResourceOverrideAndPlacement checks that an override leaves independent resource and placement settings intact. func TestGamePartialResourceOverrideAndPlacement(t *testing.T) { p, gs, tmpl := gamePolicyFixture(t) gs.Spec.Resources = &corev1.ResourceRequirements{Requests: corev1.ResourceList{corev1.ResourceCPU: resource.MustParse("500m")}} @@ -79,6 +82,7 @@ func TestGamePartialResourceOverrideAndPlacement(t *testing.T) { } } +// TestGameWorkloadPolicyRejectsOutOfBoundsInputs exercises each rejected quantity and scheduling constraint. func TestGameWorkloadPolicyRejectsOutOfBoundsInputs(t *testing.T) { for _, field := range []string{"pool", "tolerations", "missing memory limit", "memory ceiling", "memory reservation", "CPU floor", "CPU cap", "CPU limit"} { t.Run(field, func(t *testing.T) { @@ -108,6 +112,7 @@ func TestGameWorkloadPolicyRejectsOutOfBoundsInputs(t *testing.T) { } } +// TestGameWorkloadPolicyRendersMergedResourcesAndWorkerPlacement inspects the actual game container and Pod specification. func TestGameWorkloadPolicyRendersMergedResourcesAndWorkerPlacement(t *testing.T) { r, gs, effective := revisionFixture(t) p, _, tmpl := gamePolicyFixture(t) @@ -133,6 +138,7 @@ func TestGameWorkloadPolicyRendersMergedResourcesAndWorkerPlacement(t *testing.T } } +// TestStoredGameCannotCreateWorkloadOutsideWorkerPolicy checks direct-CR rejection without creating a StatefulSet. func TestStoredGameCannotCreateWorkloadOutsideWorkerPolicy(t *testing.T) { r, gs, _ := revisionFixture(t) p, _, _ := gamePolicyFixture(t) @@ -157,6 +163,7 @@ func TestStoredGameCannotCreateWorkloadOutsideWorkerPolicy(t *testing.T) { } } +// TestRejectedWorkloadOnlyStopsItsOwnedStatefulSet verifies that a stop preserves the old template and respects ownership. func TestRejectedWorkloadOnlyStopsItsOwnedStatefulSet(t *testing.T) { for _, tc := range []struct { name string @@ -210,6 +217,7 @@ func TestRejectedWorkloadOnlyStopsItsOwnedStatefulSet(t *testing.T) { } } +// TestRejectedRolloutDrainsWithThePinnedStopContract keeps the game running during its verified stop grace. func TestRejectedRolloutDrainsWithThePinnedStopContract(t *testing.T) { r, gs, old := revisionFixture(t) old.Spec.RCON = &gameplanev1alpha1.RCONSpec{Protocol: "source"} @@ -246,3 +254,123 @@ func TestRejectedRolloutDrainsWithThePinnedStopContract(t *testing.T) { t.Fatalf("failed to preserve the running world during grace: %v", err) } } + +// A catalog snapshot saved during migration is not proof of the legacy Pod's stop contract. +func TestWorkerPolicyDrainsLegacyPodBeforeFirstPin(t *testing.T) { + for _, contract := range []string{"unversioned", "verified", "previously pinned baseline"} { + t.Run(contract, func(t *testing.T) { + ctx := t.Context() + r, gs, catalog := revisionFixture(t) + policy, _, valid := gamePolicyFixture(t) + r.GameWorkloadPolicy = policy + catalog.Spec.Resources = valid.Spec.Resources + catalog.Spec.RCON = &gameplanev1alpha1.RCONSpec{Protocol: "source"} + catalog.Spec.Capabilities = &gameplanev1alpha1.CapabilitiesSpec{Lifecycle: &gameplanev1alpha1.LifecycleSpec{Stop: []string{"save", "stop"}}} + oldRevision, err := r.saveTemplateSnapshot(ctx, gs, catalog) + if err != nil { + t.Fatal(err) + } + catalog.Spec.Image = "example.invalid/game:current" + if contract == "verified" { + // Only the old, verified contract has a stop sequence in this case. + catalog.Spec.Capabilities = nil + } + if err := r.Update(ctx, catalog); err != nil { + t.Fatal(err) + } + delete(gs.Annotations, effectiveTemplateAnnotation) + if contract == "previously pinned baseline" { + pin, err := r.saveTemplateSnapshot(ctx, gs, catalog) + if err != nil { + t.Fatal(err) + } + gs.Annotations[effectiveTemplateAnnotation] = pin + gs.Annotations[baselineTemplateAnnotation] = pin + gs.Annotations[baselineWorkloadAnnotation] = "legacy" + } + if err := r.Update(ctx, gs); err != nil { + t.Fatal(err) + } + one := int32(1) + ss := &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{Name: gs.Name, Namespace: gs.Namespace, UID: "legacy", OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(gs, gameplanev1alpha1.GroupVersion.WithKind("GameServer"))}}, + Spec: appsv1.StatefulSetSpec{Replicas: &one, Template: corev1.PodTemplateSpec{Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "game", Image: "example.invalid/game:legacy"}}}}}, + Status: appsv1.StatefulSetStatus{Replicas: 1, ReadyReplicas: 1}, + } + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: gs.Name + "-0", Namespace: gs.Namespace, OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(ss, appsv1.SchemeGroupVersion.WithKind("StatefulSet"))}}, + Status: corev1.PodStatus{ContainerStatuses: []corev1.ContainerStatus{{Name: "game", State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}}}}, + } + if contract == "verified" { + pod.Annotations = map[string]string{templateRevisionAnnotation: oldRevision} + } + for _, obj := range []client.Object{ss, pod} { + if err := r.Create(ctx, obj); err != nil { + t.Fatal(err) + } + } + stopper := &revisionStopper{} + r.AgentClient = stopper + original := ss.Spec.Template.DeepCopy() + key := client.ObjectKeyFromObject(gs) + step := func() { + t.Helper() + if err := r.Get(ctx, key, gs); err != nil { + t.Fatal(err) + } + tmpl, changed, err := r.reconcileTemplateRevision(ctx, gs) + if err != nil { + t.Fatal(err) + } + if !changed { + replicas, _, err := r.desiredReplicas(ctx, gs, tmpl, idleAwake) + if err != nil { + t.Fatal(err) + } + if err := r.reconcileStatefulSet(ctx, gs, tmpl, nil, &materializedConfig{}, replicas); err != nil { + t.Fatal(err) + } + } + if err := r.Get(ctx, key, ss); err != nil { + t.Fatal(err) + } + } + for range 4 { + step() + if !reflect.DeepEqual(*original, ss.Spec.Template) { + t.Fatal("worker policy replaced the live legacy Pod template before drain") + } + } + if contract == "verified" { + if stopper.calls != 1 || *ss.Spec.Replicas != 1 { + t.Fatalf("expected one verified stop and a running game during save: calls=%d replicas=%d", stopper.calls, *ss.Spec.Replicas) + } + } else if stopper.calls != 0 || *ss.Spec.Replicas != 0 { + t.Fatalf("expected Kubernetes termination without catalog commands: calls=%d replicas=%d", stopper.calls, *ss.Spec.Replicas) + } + ss.Status.ReadyReplicas = 0 + if err := r.Status().Update(ctx, ss); err != nil { + t.Fatal(err) + } + step() + ss.Status.Replicas, ss.Status.ObservedGeneration = 0, ss.Generation + if err := r.Status().Update(ctx, ss); err != nil { + t.Fatal(err) + } + step() + if *ss.Spec.Replicas != 0 || !reflect.DeepEqual(*original, ss.Spec.Template) { + t.Fatal("zero replicas did not fence the still-present legacy Pod") + } + if err := r.Delete(ctx, pod); err != nil { + t.Fatal(err) + } + for range 5 { + step() + } + if *ss.Spec.Replicas != 1 || ss.Spec.Template.Spec.NodeSelector[r.GameWorkloadPolicy.PoolLabel] != r.GameWorkloadPolicy.PoolValue || + !validTemplateRevision(ss.Spec.Template.Annotations[templateRevisionAnnotation]) || ss.Spec.Template.Spec.Containers[0].Image != catalog.Spec.Image { + t.Fatal("drained workload did not resume on its policy-compliant revision") + } + }) + } +} diff --git a/operator/internal/controller/gameserver_controller.go b/operator/internal/controller/gameserver_controller.go index 7a1a8630b..321aa5f55 100644 --- a/operator/internal/controller/gameserver_controller.go +++ b/operator/internal/controller/gameserver_controller.go @@ -1300,7 +1300,11 @@ func (r *GameServerReconciler) desiredReplicas( } if op, held, err := lifecycleOwner(&live); err != nil { return 0, 0, err - } else if held && op.Kind == "Rollout" && !op.Baseline && len(op.Recovery) == 0 && op.Token != live.Annotations[effectiveTemplateAnnotation] { + } else if held && op.Kind == "Rollout" && len(op.Recovery) == 0 && + ((op.Baseline && r.GameWorkloadPolicy != nil) || (!op.Baseline && op.Token != live.Annotations[effectiveTemplateAnnotation])) { + if op.Baseline && live.Annotations[lifecycleOperationAnnotation] != gs.Annotations[lifecycleOperationAnnotation] { + return 1, restartDrainPoll, nil + } replicas, poll, err := r.softStop(ctx, gs, tmpl) if poll == 0 { poll = restartDrainPoll @@ -1455,6 +1459,19 @@ func selectStopTransport(tmpl *gameplanev1alpha1.GameTemplate) stopTransport { func (r *GameServerReconciler) softStop( ctx context.Context, gs *gameplanev1alpha1.GameServer, tmpl *gameplanev1alpha1.GameTemplate, ) (int32, time.Duration, error) { + if r.GameWorkloadPolicy != nil && gs.Annotations[lifecycleOperationAnnotation] != "" { + op, _, err := lifecycleOwner(gs) + if err != nil { + return 0, 0, err + } + if op.Kind == "Rollout" && op.Baseline { + // This also applies when a Hostport node disappears during migration. + tmpl, err = r.baselineStopTemplate(ctx, gs) + if err != nil { + return 0, 0, err + } + } + } declared := tmpl.Spec.Capabilities != nil && tmpl.Spec.Capabilities.Lifecycle != nil && len(tmpl.Spec.Capabilities.Lifecycle.Stop) > 0 @@ -1724,6 +1741,16 @@ func (r *GameServerReconciler) reconcileStatefulSet( } } } + if held && op.Baseline && r.GameWorkloadPolicy != nil && ss.ResourceVersion != "" { + // A stale pre-admission decision may stamp the owner but cannot + // change replicas. During the drain, never raise replicas or roll + // the live Pod's template, including on previously pinned baselines. + if live.Annotations[lifecycleOperationAnnotation] == gs.Annotations[lifecycleOperationAnnotation] && + (ss.Spec.Replicas == nil || actualReplicas < *ss.Spec.Replicas) { + ss.Spec.Replicas = &actualReplicas + } + return nil + } preserveBaseline, err := r.preserveBaselinePodTemplate(ctx, &live, ss) if err != nil { return err @@ -2318,6 +2345,7 @@ func effectiveResources( return *resources } +// mergeResourceList owns its defaults map and copies each override quantity before replacing its template value. func mergeResourceList(defaults, override corev1.ResourceList) corev1.ResourceList { if defaults == nil { return override.DeepCopy() diff --git a/operator/internal/controller/gameserver_template_revision.go b/operator/internal/controller/gameserver_template_revision.go index 91e6f28b6..860cc3acb 100644 --- a/operator/internal/controller/gameserver_template_revision.go +++ b/operator/internal/controller/gameserver_template_revision.go @@ -184,6 +184,14 @@ func (r *GameServerReconciler) reconcileTemplateRevision(ctx context.Context, gs return effective, true, err } if op.Baseline { + if r.GameWorkloadPolicy != nil { + waiting, err := r.workerPolicyBaselineNeedsDrain(ctx, gs) + if waiting || err != nil { + // Continue to desiredReplicas with the durable owner held. The + // StatefulSet may scale down but must keep its Pod template. + return effective, false, err + } + } return effective, true, r.initializeTemplateBaseline(ctx, gs, op) } if op.Token != pin { @@ -239,27 +247,35 @@ func (r *GameServerReconciler) reconcileTemplateRevision(ctx context.Context, gs if held { return effective, false, nil } - if desired != "" && desired != pin { + // An earlier operator may have recorded a first pin while preserving the + // original, unversioned Pod. Migrate that baseline under the same fence + // when the worker policy is enabled later, even with OnRequest updates. + migrateBaseline := r.GameWorkloadPolicy != nil && pin != "" && gs.Annotations[baselineTemplateAnnotation] == pin + if (desired != "" && desired != pin) || migrateBaseline { + target, revision := &installed, desired + if migrateBaseline { + target, revision = effective, pin + } policy := gs.Spec.TemplateRollout - admit := pin == "" || policy == nil || policy.Policy == "" || policy.Policy == "Automatic" || policy.RequestedRevision == desired - if gs.Annotations[cancelTemplateRolloutAnnotation] == desired { + admit := pin == "" || migrateBaseline || policy == nil || policy.Policy == "" || policy.Policy == "Automatic" || policy.RequestedRevision == revision + if gs.Annotations[cancelTemplateRolloutAnnotation] == revision { admit = false } if admit { - if err := validateTemplatePasswordTransition(gs, effective, &installed); err != nil { + if err := validateTemplatePasswordTransition(gs, effective, target); err != nil { return nil, false, err } - if _, err := materializeConfig(gs, &installed); err != nil { + if _, err := materializeConfig(gs, target); err != nil { return nil, false, err } - if _, err := resolveVersion(gs, &installed); err != nil { + if _, err := resolveVersion(gs, target); err != nil { return nil, false, err } - if err := r.GameWorkloadPolicy.validate(gs, &installed); err != nil { + if err := r.GameWorkloadPolicy.validate(gs, target); err != nil { return effective, false, workloadPolicyRejection{err} } if r.RestrictedWorkloads { - if err := validateRestrictedTemplate(&installed); err != nil { + if err := validateRestrictedTemplate(target); err != nil { return nil, false, err } } @@ -268,11 +284,11 @@ func (r *GameServerReconciler) reconcileTemplateRevision(ctx context.Context, gs if err != nil || pending { return effective, false, err } - revision, err := r.saveTemplateSnapshot(ctx, gs, &installed) + revision, err := r.saveTemplateSnapshot(ctx, gs, target) if err != nil { return nil, false, err } - want := lifecycleIdentity{Kind: "Rollout", Name: gs.Name, UID: gs.UID, TargetUID: gs.UID, Generation: gs.Generation, Token: revision, Baseline: pin == ""} + want := lifecycleIdentity{Kind: "Rollout", Name: gs.Name, UID: gs.UID, TargetUID: gs.UID, Generation: gs.Generation, Token: revision, Baseline: pin == "" || migrateBaseline} claimed, err := claimLifecycleOperation(ctx, r.Client, r.wipeReader(), gs, want, r.OperationQuiescer) return effective, claimed, err } @@ -280,6 +296,44 @@ func (r *GameServerReconciler) reconcileTemplateRevision(ctx context.Context, gs return effective, false, r.pruneTemplateSnapshots(ctx, gs) } +// workerPolicyBaselineNeedsDrain waits for the fenced StatefulSet and all world writers. +// A new server or an already-replaced historical baseline can render immediately. +func (r *GameServerReconciler) workerPolicyBaselineNeedsDrain(ctx context.Context, gs *gameplanev1alpha1.GameServer) (bool, error) { + var ss appsv1.StatefulSet + if err := r.wipeReader().Get(ctx, client.ObjectKeyFromObject(gs), &ss); err != nil { + if !apierrors.IsNotFound(err) { + return false, err + } + // A deleted StatefulSet can leave a terminating Pod that still holds the world. + gone, err := r.templateWritersGone(ctx, gs, nil) + return !gone, err + } + if !metav1.IsControlledBy(&ss, gs) { + return false, fmt.Errorf("baseline workload belongs to another server") + } + if gs.Annotations[effectiveTemplateAnnotation] != "" && (string(ss.UID) != gs.Annotations[baselineWorkloadAnnotation] || ss.Spec.Template.Annotations[templateRevisionAnnotation] != "") { + return false, nil + } + drained, err := r.templateWorkloadDrained(ctx, gs) + return !drained, err +} + +// baselineStopTemplate uses only an owned running Pod and its verified snapshot. +// An unversioned game retains its Kubernetes termination grace and receives no catalog commands. +func (r *GameServerReconciler) baselineStopTemplate(ctx context.Context, gs *gameplanev1alpha1.GameServer) (*gameplanev1alpha1.GameTemplate, error) { + revision, err := observedTemplateRevision(ctx, r.wipeReader(), gs) + if err != nil { + return nil, err + } + if revision != "" { + tmpl, err := loadTemplateSnapshot(ctx, r.wipeReader(), gs, revision) + if !apierrors.IsNotFound(err) { + return tmpl, err + } + } + return &gameplanev1alpha1.GameTemplate{}, nil +} + // Baseline captures render inputs rather than comparing reconstructed PodSpecs: // API defaults/admission fields must not turn feature installation into a fleet // rollout. These fields deliberately exclude power intent, service-only config, @@ -330,7 +384,7 @@ func (r *GameServerReconciler) initializeTemplateBaseline(ctx context.Context, g } // A real spec edit after admission must render normally. Recording // its new inputs against the old workload would silently swallow it. - if gs.Generation == op.Generation { + if r.GameWorkloadPolicy == nil && gs.Generation == op.Generation { inputs, err := r.baselineRenderInputs(gs, op.Token) if err != nil { return err @@ -340,6 +394,12 @@ func (r *GameServerReconciler) initializeTemplateBaseline(ctx context.Context, g gs.Annotations[baselineInputsAnnotation] = inputs } } + if r.GameWorkloadPolicy != nil { + delete(gs.Annotations, baselineTemplateAnnotation) + delete(gs.Annotations, baselineWorkloadAnnotation) + delete(gs.Annotations, baselineInputsAnnotation) + delete(gs.Annotations, stopRequestedAtAnnotation) + } if err := pruneEmptyProtectedConfigKeys(gs); err != nil { return err } @@ -351,15 +411,14 @@ func (r *GameServerReconciler) initializeTemplateBaseline(ctx context.Context, g } func (r *GameServerReconciler) preserveBaselinePodTemplate(ctx context.Context, gs *gameplanev1alpha1.GameServer, ss *appsv1.StatefulSet) (bool, error) { - // A pre-upgrade template may predate the required pool or the merged - // resource bounds. Re-render it before scaling the workload. - if r.GameWorkloadPolicy != nil { - return false, nil - } pin := gs.Annotations[effectiveTemplateAnnotation] if pin == "" || gs.Annotations[baselineTemplateAnnotation] != pin || string(ss.UID) != gs.Annotations[baselineWorkloadAnnotation] || ss.Spec.Template.Annotations[templateRevisionAnnotation] != "" { return false, nil } + if r.GameWorkloadPolicy != nil { + // The migration clears these baseline markers only after draining. + return true, nil + } inputs, err := r.baselineRenderInputs(gs, pin) if err != nil || inputs != gs.Annotations[baselineInputsAnnotation] { return false, err @@ -470,13 +529,18 @@ func (r *GameServerReconciler) templateWorkloadDrained(ctx context.Context, gs * if ss.Annotations[lifecycleOperationAnnotation] != gs.Annotations[lifecycleOperationAnnotation] || ss.Spec.Replicas == nil || *ss.Spec.Replicas != 0 || ss.Status.Replicas != 0 || ss.Status.ObservedGeneration < ss.Generation { return false, nil } + return r.templateWritersGone(ctx, gs, &ss) +} + +// templateWritersGone also sees terminating or orphaned Pods that can still hold the world volume. +func (r *GameServerReconciler) templateWritersGone(ctx context.Context, gs *gameplanev1alpha1.GameServer, ss *appsv1.StatefulSet) (bool, error) { var pods corev1.PodList if err := r.wipeReader().List(ctx, &pods, client.InNamespace(gs.Namespace)); err != nil { return false, err } for i := range pods.Items { pod := &pods.Items[i] - if pod.Name == gs.Name+"-0" || metav1.IsControlledBy(pod, &ss) || (liveRestorePod(pod) && podMountsRestoreTarget(pod, gs.Name)) { + if pod.Name == gs.Name+"-0" || (ss != nil && metav1.IsControlledBy(pod, ss)) || (liveRestorePod(pod) && podMountsRestoreTarget(pod, gs.Name)) { return false, nil } } diff --git a/operator/specs.md b/operator/specs.md index 32435b3e8..3b9998f59 100644 --- a/operator/specs.md +++ b/operator/specs.md @@ -345,6 +345,12 @@ owned StatefulSet using its pinned template, subject to lifecycle ownership. A legacy workload without a pin uses its Kubernetes termination grace. Only its replica count can change. Correct the reported policy violation before starting or rolling out the game. +When the worker policy takes over a legacy StatefulSet, a Rollout owner +keeps its Pod template intact until the StatefulSet observes zero replicas +and the old Pod and world-volume writers have gone. A running game with +a verified Pod revision uses its own saved stop sequence. An unversioned +Pod uses its existing Kubernetes termination grace. The operator then +starts the policy-compliant template according to the server's power intent. Chart placement and optional PDBs are documented in [`docs/production-placement.md`](../docs/production-placement.md). diff --git a/test/e2e/game_worker_capacity_e2e_test.go b/test/e2e/game_worker_capacity_e2e_test.go index feb95196e..7b51a0cad 100644 --- a/test/e2e/game_worker_capacity_e2e_test.go +++ b/test/e2e/game_worker_capacity_e2e_test.go @@ -28,6 +28,7 @@ type tenantMemoryEvents struct { max, oom, kills uint64 } +// readTenantMemoryEvents identifies the exact container leaf and reads its Pod's hierarchical memory counters on a disposable Kind worker. func readTenantMemoryEvents(ctx context.Context, t *testing.T, pod *corev1.Pod) tenantMemoryEvents { t.Helper() if envInstance.Context != "kind-"+envInstance.ClusterName || !strings.HasPrefix(pod.Spec.NodeName, envInstance.ClusterName+"-") { @@ -70,6 +71,7 @@ awk -v group="$group" -v limit="$(cat "$leaf/memory.max")" ' return snapshot } +// sampleNeighborTicks records live Minecraft tick durations against the disposable worker's latency budget. func sampleNeighborTicks(ctx context.Context, t *testing.T, ns, pod, phase string) { t.Helper() out, err := envInstance.Kubectl(ctx, "exec", "-n", ns, pod, "-c", "game", "--", "rcon-cli", "tick", "query") @@ -88,6 +90,7 @@ func sampleNeighborTicks(ctx context.Context, t *testing.T, ns, pod, phase strin } } +// recordWorkerCapacity checks worker pressure and records the requests Kubernetes has scheduled. func recordWorkerCapacity(ctx context.Context, t *testing.T, worker string) { t.Helper() node, err := envInstance.K8s.CoreV1().Nodes().Get(ctx, worker, metav1.GetOptions{}) @@ -109,6 +112,7 @@ func recordWorkerCapacity(ctx context.Context, t *testing.T, worker string) { node.Status.Allocatable.Cpu(), node.Status.Allocatable.Memory(), allocated) } +// startPlacementBackup keeps Restic copying on the game worker until the caller releases it for repository verification. func startPlacementBackup(ctx context.Context, t *testing.T, ns, poolKey, worker string) func() { t.Helper() pods := envInstance.K8s.CoreV1().Pods(ns) diff --git a/test/e2e/game_worker_pool_e2e_test.go b/test/e2e/game_worker_pool_e2e_test.go index f54a10035..b03739f96 100644 --- a/test/e2e/game_worker_pool_e2e_test.go +++ b/test/e2e/game_worker_pool_e2e_test.go @@ -92,7 +92,7 @@ func TestGameWorkerPoolAndQuota(t *testing.T) { "gameWorkloadPolicy": map[string]string{"poolLabel": key, "poolValue": "games", "minCPURequest": "10m", "maxCPURequest": "1", "minMemoryRequest": "8Mi", "maxMemoryLimit": "32Mi"}, "tenantQuotas": map[string]any{ns: map[string]any{"hard": map[string]string{ - "pods": "1", "requests.cpu": "1", "requests.memory": "32Mi", "limits.memory": "32Mi", + "pods": "1", "requests.cpu": "1", "requests.memory": "32Mi", "limits.memory": "64Mi", "persistentvolumeclaims": "1", "requests.storage": "1Gi", }}}, }} @@ -130,8 +130,9 @@ func TestGameWorkerPoolAndQuota(t *testing.T) { pod.Spec.NodeSelector[key] = "games" oversized := pod.DeepCopy() oversized.Spec.Containers[0].Resources.Limits[corev1.ResourceMemory] = resource.MustParse("64Mi") - if _, err := pods.Create(ctx, oversized, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err == nil { - t.Fatal("admission accepted an oversized game") + if _, err := pods.Create(ctx, oversized, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err == nil || + !strings.Contains(err.Error(), "game containers must meet the configured CPU request and memory request/limit bounds") { + t.Fatalf("expected worker-policy admission to reject the oversized game: %v", err) } if _, err := pods.Create(ctx, pod, metav1.CreateOptions{}); err != nil { t.Fatal(err) From c3445519a32b595c363b640564eaccf0f22a83ae Mon Sep 17 00:00:00 2001 From: Baylem Date: Wed, 11 Nov 2026 17:29:05 -0500 Subject: [PATCH 10/10] fix(operator): fence recreated workloads while legacy writers drain Signed-off-by: Baylem --- .../controller/game_workload_policy_test.go | 21 +++++++++++ .../controller/gameserver_controller.go | 5 +++ .../gameserver_template_revision.go | 37 +++++++------------ 3 files changed, 39 insertions(+), 24 deletions(-) diff --git a/operator/internal/controller/game_workload_policy_test.go b/operator/internal/controller/game_workload_policy_test.go index d3be5a50b..1e2fcb27c 100644 --- a/operator/internal/controller/game_workload_policy_test.go +++ b/operator/internal/controller/game_workload_policy_test.go @@ -361,6 +361,27 @@ func TestWorkerPolicyDrainsLegacyPodBeforeFirstPin(t *testing.T) { if *ss.Spec.Replicas != 0 || !reflect.DeepEqual(*original, ss.Spec.Template) { t.Fatal("zero replicas did not fence the still-present legacy Pod") } + if contract == "previously pinned baseline" { + // The StatefulSet can disappear while its old Pod still owns the + // world. Race the owner with a stale scale-up decision on recreate. + if err := r.Delete(ctx, ss); err != nil { + t.Fatal(err) + } + stale := gs.DeepCopy() + delete(stale.Annotations, lifecycleOperationAnnotation) + if err := r.reconcileStatefulSet(ctx, stale, catalog, nil, &materializedConfig{}, 1); err != nil { + t.Fatal(err) + } + for range 3 { + if err := r.Get(ctx, key, ss); err != nil { + t.Fatal(err) + } + if *ss.Spec.Replicas != 0 { + t.Fatalf("replacement escaped the baseline fence while the old writer survived: replicas=%d", *ss.Spec.Replicas) + } + step() + } + } if err := r.Delete(ctx, pod); err != nil { t.Fatal(err) } diff --git a/operator/internal/controller/gameserver_controller.go b/operator/internal/controller/gameserver_controller.go index 321aa5f55..5e475b7c1 100644 --- a/operator/internal/controller/gameserver_controller.go +++ b/operator/internal/controller/gameserver_controller.go @@ -1670,6 +1670,11 @@ func (r *GameServerReconciler) reconcileStatefulSet( ss.Annotations = map[string]string{} } ss.Annotations[lifecycleOperationAnnotation] = lifecycleOperationJSON(op) + if op.Kind == "Rollout" && op.Baseline && r.GameWorkloadPolicy != nil && ss.ResourceVersion == "" { + // A stale scale-up can race recreation while the old world + // writer is terminating. Create the fenced replacement stopped. + actualReplicas, replicas = 0, 0 + } if op.InitialStartup { actualReplicas = replicas delete(ss.Annotations, restoreGuardAnnotation) diff --git a/operator/internal/controller/gameserver_template_revision.go b/operator/internal/controller/gameserver_template_revision.go index 860cc3acb..ba249c556 100644 --- a/operator/internal/controller/gameserver_template_revision.go +++ b/operator/internal/controller/gameserver_template_revision.go @@ -185,8 +185,8 @@ func (r *GameServerReconciler) reconcileTemplateRevision(ctx context.Context, gs } if op.Baseline { if r.GameWorkloadPolicy != nil { - waiting, err := r.workerPolicyBaselineNeedsDrain(ctx, gs) - if waiting || err != nil { + drained, err := r.templateWorkloadDrained(ctx, gs) + if !drained || err != nil { // Continue to desiredReplicas with the durable owner held. The // StatefulSet may scale down but must keep its Pod template. return effective, false, err @@ -296,28 +296,6 @@ func (r *GameServerReconciler) reconcileTemplateRevision(ctx context.Context, gs return effective, false, r.pruneTemplateSnapshots(ctx, gs) } -// workerPolicyBaselineNeedsDrain waits for the fenced StatefulSet and all world writers. -// A new server or an already-replaced historical baseline can render immediately. -func (r *GameServerReconciler) workerPolicyBaselineNeedsDrain(ctx context.Context, gs *gameplanev1alpha1.GameServer) (bool, error) { - var ss appsv1.StatefulSet - if err := r.wipeReader().Get(ctx, client.ObjectKeyFromObject(gs), &ss); err != nil { - if !apierrors.IsNotFound(err) { - return false, err - } - // A deleted StatefulSet can leave a terminating Pod that still holds the world. - gone, err := r.templateWritersGone(ctx, gs, nil) - return !gone, err - } - if !metav1.IsControlledBy(&ss, gs) { - return false, fmt.Errorf("baseline workload belongs to another server") - } - if gs.Annotations[effectiveTemplateAnnotation] != "" && (string(ss.UID) != gs.Annotations[baselineWorkloadAnnotation] || ss.Spec.Template.Annotations[templateRevisionAnnotation] != "") { - return false, nil - } - drained, err := r.templateWorkloadDrained(ctx, gs) - return !drained, err -} - // baselineStopTemplate uses only an owned running Pod and its verified snapshot. // An unversioned game retains its Kubernetes termination grace and receives no catalog commands. func (r *GameServerReconciler) baselineStopTemplate(ctx context.Context, gs *gameplanev1alpha1.GameServer) (*gameplanev1alpha1.GameTemplate, error) { @@ -521,6 +499,17 @@ func observedTemplateRevision(ctx context.Context, reader client.Reader, gs *gam func (r *GameServerReconciler) templateWorkloadDrained(ctx context.Context, gs *gameplanev1alpha1.GameServer) (bool, error) { var ss appsv1.StatefulSet if err := r.wipeReader().Get(ctx, client.ObjectKeyFromObject(gs), &ss); err != nil { + if apierrors.IsNotFound(err) && r.GameWorkloadPolicy != nil { + op, held, err := lifecycleOwner(gs) + if err != nil { + return false, err + } + if held && op.Kind == "Rollout" && op.Baseline { + // A new server has no workload to fence. A deleted StatefulSet + // can leave a terminating Pod that still holds the world. + return r.templateWritersGone(ctx, gs, nil) + } + } return false, client.IgnoreNotFound(err) } if !metav1.IsControlledBy(&ss, gs) {