From d65cf3af30bd2cd3d244332d27195b4834b54dc4 Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Sat, 26 Sep 2026 22:22:02 +0800 Subject: [PATCH] =?UTF-8?q?fix(felis):=20=E7=BC=96=E8=BE=91=E6=9C=8D?= =?UTF-8?q?=E5=8A=A1=E5=99=A8=E6=97=B6=E5=86=85=E5=AD=98=E5=92=8C=20CPU=20?= =?UTF-8?q?=E5=8F=AF=E5=90=84=E8=87=AA=E5=8D=95=E6=94=B9=E4=BA=92=E4=B8=8D?= =?UTF-8?q?=E6=B8=85=E9=99=A4=EF=BC=8C=E6=98=BE=E7=A4=BA=E5=90=8D=E5=8F=AF?= =?UTF-8?q?=E6=B8=85=E7=A9=BA=EF=BC=8C=E5=86=85=E5=AD=98=E6=98=BE=E7=A4=BA?= =?UTF-8?q?=E7=9C=9F=E5=AE=9E=E4=B8=8A=E9=99=90?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/openapi.yaml | 22 ++- internal/api/api_test.go | 17 ++ internal/api/cluster.go | 17 +- internal/api/handlers_patch_test.go | 127 ++++++++++++- internal/api/handlers_user.go | 172 +++++++++++++++--- internal/api/k8scluster.go | 13 +- internal/api/k8scluster_test.go | 81 +++++++++ panel/dev/mockApi.ts | 3 +- panel/src/components/ConfirmFooter.tsx | 6 +- .../src/components/EditServerDialog.test.tsx | 124 +++++++++++++ panel/src/components/EditServerDialog.tsx | 38 +++- panel/src/i18n/resources/en-US/servers.json | 3 +- panel/src/i18n/resources/zh-CN/servers.json | 3 +- panel/src/lib/api.ts | 3 + panel/src/lib/openapi.gen.ts | 6 + panel/src/lib/types.ts | 3 + panel/src/pages/ServerConsole.test.tsx | 20 +- panel/src/pages/ServerConsole.tsx | 2 +- 18 files changed, 600 insertions(+), 60 deletions(-) create mode 100644 panel/src/components/EditServerDialog.test.tsx diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 141062e..5b2983d 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -472,7 +472,12 @@ components: playersMax: { type: integer, format: int32 } displayName: { type: string } image: { type: string } - javaMemory: { type: string } + javaMemory: + type: string + description: The JVM heap (-Xmx) derived from the memory limit. + memory: + type: string + description: The pod memory limit (the server's memory as an admin picks it), a Kubernetes quantity such as 4Gi. storageSize: { type: string } cpu: { type: string } idleStopSeconds: @@ -5773,7 +5778,9 @@ paths: type: object description: Only the supplied fields are patched; an empty patch is rejected. properties: - displayName: { type: string } + displayName: + type: string + description: Trimmed. An empty name clears it, and the server goes by its name again. autostartPolicy: { type: string } image: type: string @@ -5788,12 +5795,21 @@ paths: version, whose chunk upgrades the old one cannot read. Without it an image that would move the server is refused with 409 image_change_unconfirmed. The audit row records image_from/image_to. - memory: { type: string } + memory: + type: string + description: >- + Sets the memory limit and request together, as create does, and + re-derives the JVM heap. The CPU limit and request are kept. storage: type: string description: Rejected with 400 storage_immutable — present for a clear error, not mutation. resources: type: object + description: >- + Single fields of the pod block, laid over what the server has: a + field left out keeps its value, so each may be sent alone. An empty + cpu or cpuRequest removes that limit or request; the memory ceiling + cannot be emptied. A request above its limit is a 400. properties: cpu: { type: string } cpuRequest: { type: string } diff --git a/internal/api/api_test.go b/internal/api/api_test.go index ea40359..0b25ed2 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -15,6 +15,7 @@ import ( "time" "felis.lolicon.best/internal/apis/felis/v1alpha1" + corev1 "k8s.io/api/core/v1" ) const testRoot = "mc.example.net" // neutral; never a deployment domain @@ -1845,6 +1846,22 @@ func (c *fakeCluster) PatchServerSpec(_ context.Context, n string, p ServerSpecP if p.IdleStopSeconds != nil { info.IdleStopSeconds = *p.IdleStopSeconds } + if p.DisplayName != nil { + info.DisplayName = *p.DisplayName + } + if p.JavaMemory != nil { + info.JavaMemory = *p.JavaMemory + } + if p.Resources != nil { + info.Resources = *p.Resources.DeepCopy() + info.Memory, info.CPU = "", "" + if q, ok := p.Resources.Limits[corev1.ResourceMemory]; ok { + info.Memory = q.String() + } + if q, ok := p.Resources.Limits[corev1.ResourceCPU]; ok { + info.CPU = q.String() + } + } return nil } diff --git a/internal/api/cluster.go b/internal/api/cluster.go index c7d16bf..bba4e3a 100644 --- a/internal/api/cluster.go +++ b/internal/api/cluster.go @@ -26,8 +26,11 @@ type ServerInfo struct { DisplayName string `json:"displayName,omitempty"` Image string `json:"image,omitempty"` JavaMemory string `json:"javaMemory,omitempty"` - StorageSize string `json:"storageSize,omitempty"` - CPU string `json:"cpu,omitempty"` + // Memory is the pod memory limit (the §22 ceiling) — what an admin picks as + // the server's memory. JavaMemory is the heap derived from it. + Memory string `json:"memory,omitempty"` + StorageSize string `json:"storageSize,omitempty"` + CPU string `json:"cpu,omitempty"` // IdleStopSeconds is how long the server may sit empty before idle // auto-stop scales it down; 0 means it never idles out. IdleStopSeconds int32 `json:"idleStopSeconds"` @@ -43,6 +46,9 @@ type ServerInfo struct { // restart backoff and may yet come up on its own. AutoRestarts int32 `json:"autoRestarts,omitempty"` StartGaveUp bool `json:"startGaveUp,omitempty"` + // Resources is the spec's pod resource block. It stays off the wire; a spec + // patch reads it so the fields the admin left out keep their values. + Resources corev1.ResourceRequirements `json:"-"` } // CreateServerInput is the validated, structured create-server form (spec §15). @@ -77,9 +83,10 @@ type ServerSpecPatch struct { DisplayName *string AutostartPolicy *v1alpha1.AutostartPolicy Image *string - // JavaMemory is the re-derived JVM heap string; Resources carries the matching - // pod block whose memory limit is the non-zero §22 ceiling. They move together - // (felis-api resolves both from the same form) or both stay nil. + // Resources is the whole pod block after the patch — the current spec with the + // admin's changes laid over it — whose memory limit is the non-zero §22 + // ceiling. JavaMemory is the heap re-derived from that ceiling; it is set only + // when the memory moved, so a CPU-only patch leaves the heap alone. JavaMemory *string Resources *corev1.ResourceRequirements // IdleStopSeconds sets idle auto-stop: 0 turns it off, anything else is the diff --git a/internal/api/handlers_patch_test.go b/internal/api/handlers_patch_test.go index 73a9746..96a6d61 100644 --- a/internal/api/handlers_patch_test.go +++ b/internal/api/handlers_patch_test.go @@ -3,6 +3,7 @@ package api import ( "net/http" "net/http/httptest" + "strings" "testing" "felis.lolicon.best/internal/apis/felis/v1alpha1" @@ -210,7 +211,8 @@ func TestPatchServerRejections(t *testing.T) { wantCode: http.StatusBadRequest, wantErr: "bad_request", }, { - name: "resources without memory", + // The seeded server has no pod block, so there is no ceiling to keep. + name: "resources on a server with no memory ceiling", body: `{"resources":{"cpu":"2"}}`, wantCode: http.StatusBadRequest, wantErr: "bad_request", }, @@ -309,3 +311,126 @@ func TestPatchServerIdleStop(t *testing.T) { } } } + +// seedResources gives survival the pod block create would have written: a 4Gi +// ceiling, a 1-core CPU limit and a 500m CPU request. +func seedResources(cl *fakeCluster) { + cl.byName["survival"].JavaMemory = "3072M" + cl.byName["survival"].Resources = corev1.ResourceRequirements{ + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("4Gi"), corev1.ResourceCPU: resource.MustParse("1")}, + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("4Gi"), corev1.ResourceCPU: resource.MustParse("500m")}, + } +} + +func wantQuantity(t *testing.T, what string, list corev1.ResourceList, key corev1.ResourceName, want string) { + t.Helper() + got, ok := list[key] + if want == "" { + if ok { + t.Errorf("%s = %s, want none", what, got.String()) + } + return + } + if !ok || got.Cmp(resource.MustParse(want)) != 0 { + t.Errorf("%s = %s (present %v), want %s", what, got.String(), ok, want) + } +} + +// The edit dialog sends only the field the admin changed. Each patch lays that one +// field over the server's pod block and keeps the rest. +func TestPatchServerEditsOneResource(t *testing.T) { + t.Run("cpu only keeps the memory and the heap", func(t *testing.T) { + api, repo, cl, _ := newPatchAPI() + seedResources(cl) + repo.byName["survival"].OwnerID = "u1" + repo.quota["u1"] = true + + w := patchSurvival(api, `{"resources":{"cpu":"2"}}`) + if w.Code != http.StatusOK { + t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String()) + } + p := cl.patched["survival"] + if p.Resources == nil { + t.Fatal("a cpu patch must carry the pod block") + } + wantQuantity(t, "memory limit", p.Resources.Limits, corev1.ResourceMemory, "4Gi") + wantQuantity(t, "cpu limit", p.Resources.Limits, corev1.ResourceCPU, "2") + wantQuantity(t, "memory request", p.Resources.Requests, corev1.ResourceMemory, "4Gi") + wantQuantity(t, "cpu request", p.Resources.Requests, corev1.ResourceCPU, "500m") + if p.JavaMemory != nil { + t.Errorf("heap = %q, want untouched by a cpu patch", *p.JavaMemory) + } + if got := repo.resourceUpdates["survival"]; got.CPUMilli != 2000 || got.MemoryMB != 4096 { + t.Errorf("resource cache = %+v, want cpu 2000 / mem 4096", got) + } + if body := w.Body.String(); !strings.Contains(body, `"patched":["resources"]`) { + t.Errorf("response = %s, want patched [resources]", body) + } + }) + + t.Run("memory only keeps the cpu limit and request", func(t *testing.T) { + api, _, cl, _ := newPatchAPI() + seedResources(cl) + + w := patchSurvival(api, `{"memory":"8Gi"}`) + if w.Code != http.StatusOK { + t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String()) + } + p := cl.patched["survival"] + wantQuantity(t, "memory limit", p.Resources.Limits, corev1.ResourceMemory, "8Gi") + wantQuantity(t, "cpu limit", p.Resources.Limits, corev1.ResourceCPU, "1") + wantQuantity(t, "memory request", p.Resources.Requests, corev1.ResourceMemory, "8Gi") + wantQuantity(t, "cpu request", p.Resources.Requests, corev1.ResourceCPU, "500m") + if p.JavaMemory == nil || *p.JavaMemory != "6144M" { + t.Errorf("heap = %v, want 6144M derived from 8Gi", p.JavaMemory) + } + }) + + t.Run("an empty cpu removes the limit", func(t *testing.T) { + api, _, cl, _ := newPatchAPI() + seedResources(cl) + + w := patchSurvival(api, `{"resources":{"cpu":""}}`) + if w.Code != http.StatusOK { + t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String()) + } + p := cl.patched["survival"] + wantQuantity(t, "cpu limit", p.Resources.Limits, corev1.ResourceCPU, "") + wantQuantity(t, "memory limit", p.Resources.Limits, corev1.ResourceMemory, "4Gi") + }) + + for _, c := range []struct{ name, body string }{ + {"the memory ceiling cannot be emptied", `{"resources":{"memory":""}}`}, + {"a cpu request above the kept limit", `{"resources":{"cpuRequest":"2"}}`}, + {"a memory ceiling below the kept request", `{"resources":{"memory":"2Gi"}}`}, + } { + t.Run(c.name, func(t *testing.T) { + api, _, cl, _ := newPatchAPI() + seedResources(cl) + + w := patchSurvival(api, c.body) + if w.Code != http.StatusBadRequest || decodeErr(t, w) != "bad_request" { + t.Fatalf("code = %d (%s), want 400 bad_request", w.Code, w.Body.String()) + } + if len(cl.patched) != 0 { + t.Errorf("a rejected patch must not write a spec, got %+v", cl.patched) + } + }) + } +} + +// Clearing the display name in the dialog sends an empty one; the server then +// goes by its name again. Blank space counts as empty. +func TestPatchServerClearsDisplayName(t *testing.T) { + api, _, cl, _ := newPatchAPI() + cl.byName["survival"].DisplayName = "Survival Realm" + + w := patchSurvival(api, `{"displayName":" "}`) + if w.Code != http.StatusOK { + t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String()) + } + p := cl.patched["survival"] + if p.DisplayName == nil || *p.DisplayName != "" { + t.Fatalf("patched displayName = %v, want an empty one", p.DisplayName) + } +} diff --git a/internal/api/handlers_user.go b/internal/api/handlers_user.go index f3dc37a..730328c 100644 --- a/internal/api/handlers_user.go +++ b/internal/api/handlers_user.go @@ -617,17 +617,8 @@ func resolveResources(memory string, rr *resourceRequest) (string, corev1.Resour } } - // A request that exceeds its limit is rejected by Kubernetes; fail fast here - // with a clear 400 instead of letting the CRD write bounce. - if memReq, memLim := requests[corev1.ResourceMemory], limits[corev1.ResourceMemory]; memReq.Cmp(memLim) > 0 { - return "", corev1.ResourceRequirements{}, newError(http.StatusBadRequest, "bad_request", - "memory request %s exceeds limit %s", memReq.String(), memLim.String()) - } - if cpuReq, hasReq := requests[corev1.ResourceCPU]; hasReq { - if cpuLim, hasLim := limits[corev1.ResourceCPU]; hasLim && cpuReq.Cmp(cpuLim) > 0 { - return "", corev1.ResourceRequirements{}, newError(http.StatusBadRequest, "bad_request", - "cpu request %s exceeds limit %s", cpuReq.String(), cpuLim.String()) - } + if err := checkResourceBounds(limits, requests); err != nil { + return "", corev1.ResourceRequirements{}, err } // §22 fail-closed: never hand the operator a CRD without a concrete memory @@ -642,6 +633,111 @@ func resolveResources(memory string, rr *resourceRequest) (string, corev1.Resour return deriveJavaHeap(memLim), corev1.ResourceRequirements{Limits: limits, Requests: requests}, nil } +// checkResourceBounds refuses a request above its limit. Kubernetes rejects one +// too; failing here gives a clear 400 instead of a bounced CRD write. +func checkResourceBounds(limits, requests corev1.ResourceList) error { + if memReq, hasReq := requests[corev1.ResourceMemory]; hasReq { + if memLim, hasLim := limits[corev1.ResourceMemory]; hasLim && memReq.Cmp(memLim) > 0 { + return newError(http.StatusBadRequest, "bad_request", + "memory request %s exceeds limit %s", memReq.String(), memLim.String()) + } + } + if cpuReq, hasReq := requests[corev1.ResourceCPU]; hasReq { + if cpuLim, hasLim := limits[corev1.ResourceCPU]; hasLim && cpuReq.Cmp(cpuLim) > 0 { + return newError(http.StatusBadRequest, "bad_request", + "cpu request %s exceeds limit %s", cpuReq.String(), cpuLim.String()) + } + } + return nil +} + +// patchResourceRequest is the resources block of a patch. A field left out keeps +// what the server has. An empty cpu or cpuRequest removes that limit or request; +// the memory ceiling can be changed but never removed (§22). +type patchResourceRequest struct { + CPU *string `json:"cpu,omitempty"` + CPURequest *string `json:"cpuRequest,omitempty"` + Memory *string `json:"memory,omitempty"` + MemoryRequest *string `json:"memoryRequest,omitempty"` +} + +// mergeResources lays a patch's memory and resources over the server's current +// pod block, so a field the admin left out keeps its value: a CPU-only patch +// keeps the memory, a memory-only patch keeps the CPU limit. The top-level +// memory sets the memory limit and request together, as create does; the block +// then overrides single fields. It returns the heap derived from the final +// ceiling and whether the ceiling moved, which is when the heap must follow. +func mergeResources(cur corev1.ResourceRequirements, memory *string, rr *patchResourceRequest) (string, bool, corev1.ResourceRequirements, error) { + out := *cur.DeepCopy() + if out.Limits == nil { + out.Limits = corev1.ResourceList{} + } + if out.Requests == nil { + out.Requests = corev1.ResourceList{} + } + fail := func(err error) (string, bool, corev1.ResourceRequirements, error) { + return "", false, corev1.ResourceRequirements{}, err + } + // set parses a present field onto one list entry. An empty value removes the + // entry where that is allowed and is refused where it is not. + set := func(list corev1.ResourceList, key corev1.ResourceName, v *string, field string, removable bool) error { + if v == nil { + return nil + } + if *v == "" { + if !removable { + return newError(http.StatusBadRequest, "bad_request", "%s cannot be empty", field) + } + delete(list, key) + return nil + } + q, err := parsePositiveQuantity(*v, field) + if err != nil { + return err + } + list[key] = q + return nil + } + + if err := set(out.Limits, corev1.ResourceMemory, memory, "memory", false); err != nil { + return fail(err) + } + if memory != nil { + out.Requests[corev1.ResourceMemory] = out.Limits[corev1.ResourceMemory] + } + if rr != nil { + for _, f := range []struct { + list corev1.ResourceList + key corev1.ResourceName + v *string + field string + removable bool + }{ + {out.Limits, corev1.ResourceMemory, rr.Memory, "resources.memory", false}, + {out.Requests, corev1.ResourceMemory, rr.MemoryRequest, "resources.memoryRequest", false}, + {out.Limits, corev1.ResourceCPU, rr.CPU, "resources.cpu", true}, + {out.Requests, corev1.ResourceCPU, rr.CPURequest, "resources.cpuRequest", true}, + } { + if err := set(f.list, f.key, f.v, f.field, f.removable); err != nil { + return fail(err) + } + } + } + if err := checkResourceBounds(out.Limits, out.Requests); err != nil { + return fail(err) + } + + // §22 fail-closed: a server that somehow has no ceiling gets none invented + // here; the admin has to pick the memory. + memLim, ok := out.Limits[corev1.ResourceMemory] + if !ok || memLim.IsZero() { + return fail(newError(http.StatusBadRequest, "bad_request", + "this server has no memory ceiling; set memory in the same patch")) + } + curLim := cur.Limits[corev1.ResourceMemory] + return deriveJavaHeap(memLim), memLim.Cmp(curLim) != 0, out, nil +} + // deriveJavaHeap converts the pod memory ceiling into a JVM max-heap string // (JavaMemory → JAVA_MEMORY → -Xmx). Two reasons the raw K8s quantity cannot be // forwarded as-is: @@ -716,9 +812,10 @@ type patchServerRequest struct { // whatever Minecraft version it carries. Chunks a newer version has upgraded // cannot be read by the older one again, so without it an image change that // would actually move the server is refused (image_change_unconfirmed). - ConfirmImageChange bool `json:"confirmImageChange,omitempty"` - Memory *string `json:"memory,omitempty"` - Resources *resourceRequest `json:"resources,omitempty"` + ConfirmImageChange bool `json:"confirmImageChange,omitempty"` + Memory *string `json:"memory,omitempty"` + // Resources overrides single fields of the pod block; see patchResourceRequest. + Resources *patchResourceRequest `json:"resources,omitempty"` // IdleStopSeconds sets idle auto-stop: 0 turns it off, otherwise the server // stops after that many seconds with nobody online (60 to 86400). IdleStopSeconds *int32 `json:"idleStopSeconds,omitempty"` @@ -777,9 +874,22 @@ func (a *API) handlePatchServer(w http.ResponseWriter, r *http.Request) { changed := []string{} // imageFrom is the image a confirmed image change replaced, for the audit row. var imageFrom string + // current reads the server once, for the fields that are checked or merged + // against what it has now. + var cur *ServerInfo + current := func() (*ServerInfo, error) { + if cur != nil { + return cur, nil + } + info, err := a.Cluster.GetServer(r.Context(), name) + cur = info + return info, err + } if body.DisplayName != nil { - patch.DisplayName = body.DisplayName + // An empty name is allowed: the panel then shows the server's name. + displayName := strings.TrimSpace(*body.DisplayName) + patch.DisplayName = &displayName changed = append(changed, "displayName") } @@ -840,7 +950,7 @@ func (a *API) handlePatchServer(w http.ResponseWriter, r *http.Request) { writeError(w, r, err) return } - info, err := a.Cluster.GetServer(r.Context(), name) + info, err := current() if err != nil { a.writeLookupError(w, r, err) return @@ -862,33 +972,37 @@ func (a *API) handlePatchServer(w http.ResponseWriter, r *http.Request) { } } - // Memory and the resource overrides move together: resolveResources derives the - // JVM heap and the §22 non-zero ceiling from the FINAL memory limit, and the - // override block is meaningless without that base. A resources-only patch has no - // base ceiling to widen (this endpoint does not read the current spec back), so - // it is rejected rather than guessed. + // Memory and the resource overrides are laid over the server's current pod + // block (mergeResources), so each may come alone and whatever the admin left + // out keeps its value. The heap follows the ceiling whenever memory is picked + // or the ceiling moves. var ( newResources corev1.ResourceRequirements resUpdated bool ) - if body.Memory != nil { - javaMemory, resources, err := resolveResources(*body.Memory, body.Resources) + if body.Memory != nil || body.Resources != nil { + info, err := current() + if err != nil { + a.writeLookupError(w, r, err) + return + } + javaMemory, memMoved, resources, err := mergeResources(info.Resources, body.Memory, body.Resources) if err != nil { writeError(w, r, err) return } - patch.JavaMemory = &javaMemory + if body.Memory != nil || memMoved { + patch.JavaMemory = &javaMemory + } patch.Resources = &resources - changed = append(changed, "memory") + if body.Memory != nil { + changed = append(changed, "memory") + } if body.Resources != nil { changed = append(changed, "resources") } newResources = resources resUpdated = true - } else if body.Resources != nil { - writeError(w, r, newError(http.StatusBadRequest, "bad_request", - "resources overrides require memory to be set in the same patch")) - return } // Resource-cache consistency + quota enforcement (spec §9.3 / §22): every diff --git a/internal/api/k8scluster.go b/internal/api/k8scluster.go index f1919d1..009789a 100644 --- a/internal/api/k8scluster.go +++ b/internal/api/k8scluster.go @@ -437,11 +437,12 @@ func (k *K8sCluster) PatchServerSpec(ctx context.Context, name string, p ServerS // serverInfo projects a MinecraftServer onto the API's lifecycle view. func serverInfo(ms *v1alpha1.MinecraftServer) *ServerInfo { - var cpuStr string - if ms.Spec.Resources.Limits != nil { - if limit, ok := ms.Spec.Resources.Limits[corev1.ResourceCPU]; ok { - cpuStr = limit.String() - } + var cpuStr, memStr string + if limit, ok := ms.Spec.Resources.Limits[corev1.ResourceCPU]; ok { + cpuStr = limit.String() + } + if limit, ok := ms.Spec.Resources.Limits[corev1.ResourceMemory]; ok { + memStr = limit.String() } return &ServerInfo{ @@ -458,6 +459,7 @@ func serverInfo(ms *v1alpha1.MinecraftServer) *ServerInfo { DisplayName: ms.Spec.DisplayName, Image: ms.Spec.Image, JavaMemory: ms.Spec.JavaMemory, + Memory: memStr, StorageSize: ms.Spec.Storage.Size, CPU: cpuStr, IdleStopSeconds: idleStopSeconds(ms), @@ -466,6 +468,7 @@ func serverInfo(ms *v1alpha1.MinecraftServer) *ServerInfo { LegacyForwarding: ms.Labels[v1alpha1.LabelForwarding] == v1alpha1.ForwardingLegacy, AutoRestarts: ms.Status.AutoRestarts, StartGaveUp: v1alpha1.StartGaveUp(&ms.Status), + Resources: *ms.Spec.Resources.DeepCopy(), } } diff --git a/internal/api/k8scluster_test.go b/internal/api/k8scluster_test.go index f8f0ea5..aa39a36 100644 --- a/internal/api/k8scluster_test.go +++ b/internal/api/k8scluster_test.go @@ -10,6 +10,8 @@ import ( "felis.lolicon.best/internal/apis/felis/v1alpha1" "felis.lolicon.best/internal/naming" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -320,3 +322,82 @@ func TestServerInfoCarriesStartGaveUp(t *testing.T) { t.Fatalf("spent: startGaveUp=%v JSON %s, want true", spent.StartGaveUp, b) } } + +// An admin edits one resource at a time. The view hands the handler the whole +// pod block, the handler lays the change over it, and the merge patch keeps +// everything the admin left alone: memory-only keeps the CPU limit, CPU-only +// keeps the memory, and an emptied CPU field really drops the limit. +func TestResourcePatchesKeepWhatTheyLeaveOut(t *testing.T) { + scheme := runtime.NewScheme() + if err := v1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("scheme: %v", err) + } + ms := testServer("survival", "survival") + ms.Spec.JavaMemory = "3072M" + ms.Spec.Resources = corev1.ResourceRequirements{ + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("4Gi"), corev1.ResourceCPU: resource.MustParse("2")}, + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("4Gi"), corev1.ResourceCPU: resource.MustParse("500m")}, + } + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(ms).Build() + k := NewK8sCluster(c, "minecraft") + ctx := context.Background() + str := func(s string) *string { return &s } + // edit is one PATCH /servers/survival: read the view, merge, write. + edit := func(memory *string, rr *patchResourceRequest) *ServerInfo { + t.Helper() + info, err := k.GetServer(ctx, "survival") + if err != nil { + t.Fatalf("GetServer: %v", err) + } + heap, moved, res, err := mergeResources(info.Resources, memory, rr) + if err != nil { + t.Fatalf("merge: %v", err) + } + p := ServerSpecPatch{Resources: &res} + if memory != nil || moved { + p.JavaMemory = &heap + } + if err := k.PatchServerSpec(ctx, "survival", p); err != nil { + t.Fatalf("patch: %v", err) + } + after, err := k.GetServer(ctx, "survival") + if err != nil { + t.Fatalf("GetServer: %v", err) + } + return after + } + + info, err := k.GetServer(ctx, "survival") + if err != nil { + t.Fatalf("GetServer: %v", err) + } + if info.Memory != "4Gi" || info.CPU != "2" || info.JavaMemory != "3072M" { + t.Fatalf("view = memory %q cpu %q heap %q, want 4Gi 2 3072M", info.Memory, info.CPU, info.JavaMemory) + } + b, err := json.Marshal(info) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(b), "requests") || strings.Contains(string(b), "Resources") { + t.Fatalf("the pod block reached the wire: %s", b) + } + + after := edit(str("8Gi"), nil) + cpuReq := after.Resources.Requests[corev1.ResourceCPU] + if after.Memory != "8Gi" || after.CPU != "2" || cpuReq.String() != "500m" || after.JavaMemory != "6144M" { + t.Fatalf("memory-only: memory %q cpu %q cpuRequest %s heap %q, want 8Gi 2 500m 6144M", + after.Memory, after.CPU, cpuReq.String(), after.JavaMemory) + } + + after = edit(nil, &patchResourceRequest{CPU: str("3")}) + memReq := after.Resources.Requests[corev1.ResourceMemory] + if after.CPU != "3" || after.Memory != "8Gi" || memReq.String() != "8Gi" || after.JavaMemory != "6144M" { + t.Fatalf("cpu-only: cpu %q memory %q memoryRequest %s heap %q, want 3 8Gi 8Gi 6144M", + after.CPU, after.Memory, memReq.String(), after.JavaMemory) + } + + after = edit(nil, &patchResourceRequest{CPU: str("")}) + if _, has := after.Resources.Limits[corev1.ResourceCPU]; has || after.CPU != "" || after.Memory != "8Gi" { + t.Fatalf("cleared cpu: limits %v, want the CPU limit gone and memory 8Gi kept", after.Resources.Limits) + } +} diff --git a/panel/dev/mockApi.ts b/panel/dev/mockApi.ts index 831faae..7d3d6e8 100644 --- a/panel/dev/mockApi.ts +++ b/panel/dev/mockApi.ts @@ -256,7 +256,8 @@ function initialState(): MockState { playersOnline: 12, playersMax: 20, autostartPolicy: "public", - javaMemory: "4Gi", + memory: "4Gi", + javaMemory: "3072M", storageSize: "20Gi", // Pinned the way felis-api stores it: the tag it was created from plus the // digest that tag named then. diff --git a/panel/src/components/ConfirmFooter.tsx b/panel/src/components/ConfirmFooter.tsx index 9405a36..cb0ae1b 100644 --- a/panel/src/components/ConfirmFooter.tsx +++ b/panel/src/components/ConfirmFooter.tsx @@ -29,10 +29,12 @@ export function ConfirmFooter({ return ( {children} - - diff --git a/panel/src/components/EditServerDialog.test.tsx b/panel/src/components/EditServerDialog.test.tsx new file mode 100644 index 0000000..1c416e9 --- /dev/null +++ b/panel/src/components/EditServerDialog.test.tsx @@ -0,0 +1,124 @@ +// @vitest-environment jsdom +import { describe, it, expect, vi, beforeEach, beforeAll } from "vitest"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { EditServerDialog } from "./EditServerDialog"; + +const calls = vi.hoisted(() => ({ listImages: vi.fn(), patchServer: vi.fn() })); +vi.mock("@/lib/api", async (importActual) => { + const actual = await importActual(); + return { ...actual, api: { ...actual.api, ...calls } }; +}); + +// Radix Select opens with pointer capture and scrolls the picked item into view, +// neither of which jsdom implements. +beforeAll(() => { + Element.prototype.hasPointerCapture ??= () => false; + Element.prototype.releasePointerCapture ??= () => {}; + Element.prototype.scrollIntoView ??= () => {}; +}); + +const onUpdated = vi.fn(); + +beforeEach(() => { + calls.listImages.mockReset(); + calls.patchServer.mockReset(); + onUpdated.mockReset(); + calls.listImages.mockResolvedValue([]); + calls.patchServer.mockResolvedValue({ name: "survival", patched: [] }); +}); + +async function openDialog() { + const user = userEvent.setup(); + render( + , + ); + await user.click(screen.getByRole("button", { name: /Edit Server Config/ })); + await screen.findByRole("dialog"); + return user; +} + +const save = () => screen.getByRole("button", { name: "Save Config" }) as HTMLButtonElement; +const cpuInput = () => screen.getByLabelText("CPU Limit") as HTMLInputElement; + +describe("EditServerDialog request body", () => { + it("sends the CPU alone when only the CPU changed", async () => { + const user = await openDialog(); + await user.clear(cpuInput()); + await user.type(cpuInput(), "2"); + await user.click(save()); + + expect(calls.patchServer).toHaveBeenCalledWith("survival", { resources: { cpu: "2" } }); + expect(onUpdated).toHaveBeenCalledTimes(1); + }); + + it("sends the memory alone when only the memory changed", async () => { + const user = await openDialog(); + await user.click(screen.getByRole("combobox", { name: "Memory" })); + await user.click(await screen.findByRole("option", { name: "8Gi" })); + await user.click(save()); + + expect(calls.patchServer).toHaveBeenCalledWith("survival", { memory: "8Gi" }); + }); + + it("clears the display name with an empty one", async () => { + const user = await openDialog(); + await user.clear(screen.getByLabelText("Display name (optional)")); + await user.click(save()); + + expect(calls.patchServer).toHaveBeenCalledWith("survival", { displayName: "" }); + }); + + it("lifts the CPU limit when the field is emptied", async () => { + const user = await openDialog(); + await user.clear(cpuInput()); + await user.click(save()); + + expect(calls.patchServer).toHaveBeenCalledWith("survival", { resources: { cpu: "" } }); + }); + + it("treats surrounding space as no change", async () => { + const user = await openDialog(); + await user.type(cpuInput(), " "); + await user.type(screen.getByLabelText("Display name (optional)"), " "); + + expect(save().disabled).toBe(true); + }); + + it("refuses a CPU value the server cannot take", async () => { + const user = await openDialog(); + for (const bad of ["two", "0", "1.5m", "-1"]) { + await user.clear(cpuInput()); + await user.type(cpuInput(), bad); + expect(screen.getByText("Enter cores (1, 1.5) or millicores (500m), above zero."), bad).toBeTruthy(); + expect(cpuInput().getAttribute("aria-invalid"), bad).toBe("true"); + expect(save().disabled, bad).toBe(true); + } + for (const good of ["1.5", "500m", "4"]) { + await user.clear(cpuInput()); + await user.type(cpuInput(), good); + expect(screen.queryByText(/millicores/), good).toBeNull(); + expect(save().disabled, good).toBe(false); + } + expect(calls.patchServer).not.toHaveBeenCalled(); + }); + + it("can be cancelled with nothing changed", async () => { + const user = await openDialog(); + expect(save().disabled).toBe(true); + const cancel = screen.getByRole("button", { name: "Cancel" }) as HTMLButtonElement; + expect(cancel.disabled).toBe(false); + await user.click(cancel); + expect(screen.queryByRole("dialog")).toBeNull(); + }); +}); diff --git a/panel/src/components/EditServerDialog.tsx b/panel/src/components/EditServerDialog.tsx index 84a5cb7..2268c4c 100644 --- a/panel/src/components/EditServerDialog.tsx +++ b/panel/src/components/EditServerDialog.tsx @@ -27,6 +27,14 @@ import { InlineError } from "@/components/MessageLine"; const MEMORY_OPTIONS = ["2Gi", "4Gi", "6Gi", "8Gi"]; +/** cpuValid accepts what the API takes as a CPU limit: a positive number of + * cores ("2", "1.5") or millicores ("500m"). Empty is valid too: no limit. */ +function cpuValid(cpu: string): boolean { + if (cpu === "") return true; + const m = /^(\d+(?:\.\d+)?)(m?)$/.exec(cpu); + return m !== null && Number(m[1]) > 0 && (m[2] === "" || !m[1].includes(".")); +} + /** Idle auto-stop presets in seconds; "0" is Never. The server default is 600. */ const IDLE_OPTIONS = ["0", "300", "600", "900", "1800", "3600", "7200"]; @@ -141,18 +149,22 @@ export function EditServerDialog({ const enabledImages = (images.data ?? []).filter((i) => i.enabled); - // Checks if form state has mutated from initial values + // Text fields count as what they send: surrounding space is no change, and an + // emptied field is one (it clears the name, or lifts the CPU limit). + const displayName = form.displayName.trim(); + const cpu = form.cpu.trim(); + const cpuOk = cpuValid(cpu); const hasChanges = - form.displayName !== currentDisplayName || + displayName !== currentDisplayName || form.autostartPolicy !== currentPolicy || form.image !== currentImage || form.memory !== currentMemory || - form.cpu !== currentCpu || + cpu !== currentCpu || form.idleStop !== currentIdleStop; const imageChanged = form.image !== currentImage; const pinnedBuild = splitImageRef(currentImage).short; - const canSubmit = hasChanges && !submitting && (!imageChanged || imageConfirmed); + const canSubmit = hasChanges && cpuOk && !submitting && (!imageChanged || imageConfirmed); async function submit() { setSubmitting(true); @@ -160,8 +172,8 @@ export function EditServerDialog({ try { const payload: Parameters[1] = {}; - if (form.displayName !== currentDisplayName) { - payload.displayName = form.displayName.trim() || undefined; + if (displayName !== currentDisplayName) { + payload.displayName = displayName; } if (form.autostartPolicy !== currentPolicy) { payload.autostartPolicy = form.autostartPolicy; @@ -170,13 +182,12 @@ export function EditServerDialog({ payload.image = form.image; payload.confirmImageChange = imageConfirmed; } + // Memory and CPU each go alone; the API keeps whatever is not sent. if (form.memory !== currentMemory) { payload.memory = form.memory; } - if (form.cpu !== currentCpu) { - payload.resources = { - cpu: form.cpu.trim(), - }; + if (cpu !== currentCpu) { + payload.resources = { cpu }; } if (form.idleStop !== currentIdleStop) { payload.idleStopSeconds = Number(form.idleStop); @@ -324,7 +335,14 @@ export function EditServerDialog({ placeholder={t("edit_server_cpu_placeholder")} value={form.cpu} onChange={(e) => set("cpu", e.target.value)} + aria-invalid={!cpuOk} + aria-describedby={cpuOk ? undefined : "es-cpu-error"} /> + {!cpuOk && ( +

+ {t("edit_server_cpu_invalid")} +

+ )}
diff --git a/panel/src/i18n/resources/en-US/servers.json b/panel/src/i18n/resources/en-US/servers.json index 526be74..15e0775 100644 --- a/panel/src/i18n/resources/en-US/servers.json +++ b/panel/src/i18n/resources/en-US/servers.json @@ -178,7 +178,8 @@ "luckperms_rcon_output": "RCON Console Output", "luckperms_clear_history": "Clear history", "edit_server_cpu": "CPU Limit", - "edit_server_cpu_placeholder": "e.g. 1, 2, 500m", + "edit_server_cpu_placeholder": "e.g. 1, 1.5, 500m — empty for no limit", + "edit_server_cpu_invalid": "Enter cores (1, 1.5) or millicores (500m), above zero.", "owned_filter_mine": "me", "edit_server_idle": "Idle auto-stop", "edit_server_idle_hint": "Stops the server after this long with nobody online, freeing memory and CPU. The next player to join wakes it.", diff --git a/panel/src/i18n/resources/zh-CN/servers.json b/panel/src/i18n/resources/zh-CN/servers.json index 8b38fb1..1dc68a2 100644 --- a/panel/src/i18n/resources/zh-CN/servers.json +++ b/panel/src/i18n/resources/zh-CN/servers.json @@ -178,7 +178,8 @@ "luckperms_rcon_output": "RCON 控制台输出", "luckperms_clear_history": "清除历史记录", "edit_server_cpu": "CPU 限制", - "edit_server_cpu_placeholder": "例如 1, 2, 500m", + "edit_server_cpu_placeholder": "例如 1、1.5、500m，留空不限制", + "edit_server_cpu_invalid": "填核数（1、1.5）或毫核（500m），要大于 0。", "owned_filter_mine": "我", "edit_server_idle": "空闲自动停服", "edit_server_idle_hint": "没人在线达到这个时长就自动停服，省下内存和 CPU；玩家下次进服时自动唤醒。", diff --git a/panel/src/lib/api.ts b/panel/src/lib/api.ts index 28e207e..4f93eac 100644 --- a/panel/src/lib/api.ts +++ b/panel/src/lib/api.ts @@ -553,6 +553,7 @@ export const api = rejectingSync({ ), patchServer: (name: string, req: { + /** Trimmed server-side; an empty one clears it. */ displayName?: string; autostartPolicy?: AutostartPolicy; image?: string; @@ -562,6 +563,8 @@ export const api = rejectingSync({ memory?: string; /** Idle auto-stop: 0 turns it off, else seconds empty before the stop (60–86400). */ idleStopSeconds?: number; + /** Single fields laid over the server's pod block: a field left out keeps its + * value, and an empty cpu or cpuRequest removes that limit or request. */ resources?: { cpu?: string; cpuRequest?: string; diff --git a/panel/src/lib/openapi.gen.ts b/panel/src/lib/openapi.gen.ts index f8b7b36..0929eed 100644 --- a/panel/src/lib/openapi.gen.ts +++ b/panel/src/lib/openapi.gen.ts @@ -2339,7 +2339,10 @@ export interface components { playersMax: number; displayName?: string; image?: string; + /** @description The JVM heap (-Xmx) derived from the memory limit. */ javaMemory?: string; + /** @description The pod memory limit (the server's memory as an admin picks it), a Kubernetes quantity such as 4Gi. */ + memory?: string; storageSize?: string; cpu?: string; /** @@ -7685,15 +7688,18 @@ export interface operations { requestBody: { content: { "application/json": { + /** @description Trimmed. An empty name clears it, and the server goes by its name again. */ displayName?: string; autostartPolicy?: string; /** @description Re-admitted against the whitelist (a pinned name:tag@sha256:… ref is admitted by its name:tag) and pinned like create does. A pin equal to the current image is no change; any other needs confirmImageChange. */ image?: string; /** @description Acknowledges that the new image opens the world with its Minecraft version, whose chunk upgrades the old one cannot read. Without it an image that would move the server is refused with 409 image_change_unconfirmed. The audit row records image_from/image_to. */ confirmImageChange?: boolean; + /** @description Sets the memory limit and request together, as create does, and re-derives the JVM heap. The CPU limit and request are kept. */ memory?: string; /** @description Rejected with 400 storage_immutable — present for a clear error, not mutation. */ storage?: string; + /** @description Single fields of the pod block, laid over what the server has: a field left out keeps its value, so each may be sent alone. An empty cpu or cpuRequest removes that limit or request; the memory ceiling cannot be emptied. A request above its limit is a 400. */ resources?: { cpu?: string; cpuRequest?: string; diff --git a/panel/src/lib/types.ts b/panel/src/lib/types.ts index 42ada1a..f590ab6 100644 --- a/panel/src/lib/types.ts +++ b/panel/src/lib/types.ts @@ -59,7 +59,10 @@ export interface ServerStatus { playersMax: number; displayName?: string; image?: string; + /** The JVM heap derived from `memory`. */ javaMemory?: string; + /** The pod memory limit — the server's memory as an admin picks it (e.g. "4Gi"). */ + memory?: string; storageSize?: string; cpu?: string; /** Seconds empty before idle auto-stop; 0 when the server never idles out. */ diff --git a/panel/src/pages/ServerConsole.test.tsx b/panel/src/pages/ServerConsole.test.tsx index 342b914..a9f522c 100644 --- a/panel/src/pages/ServerConsole.test.tsx +++ b/panel/src/pages/ServerConsole.test.tsx @@ -1,10 +1,11 @@ // @vitest-environment jsdom import { describe, it, expect, vi, beforeEach } from "vitest"; import { render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import { MemoryRouter, Route, Routes } from "react-router-dom"; import { ServerConsole } from "./ServerConsole"; -const calls = vi.hoisted(() => ({ status: vi.fn(), myServers: vi.fn() })); +const calls = vi.hoisted(() => ({ status: vi.fn(), myServers: vi.fn(), listImages: vi.fn() })); vi.mock("@/lib/tier", () => ({ useTier: () => ({ loading: false, @@ -31,6 +32,8 @@ beforeEach(() => { calls.status.mockReset(); calls.myServers.mockReset(); calls.myServers.mockResolvedValue([]); + calls.listImages.mockReset(); + calls.listImages.mockResolvedValue([]); }); function renderConsole() { @@ -76,3 +79,18 @@ describe("ServerConsole failed start", () => { expect(screen.queryByRole("button", { name: /Retry start/ })).toBeNull(); }); }); + +describe("ServerConsole edit dialog", () => { + it("offers the server's memory limit as the current memory, not the JVM heap", async () => { + calls.status.mockResolvedValue( + status({ phase: "Stopped", desiredState: "Stopped", memory: "4Gi", javaMemory: "3072M", cpu: "2" }), + ); + renderConsole(); + + await userEvent.setup().click(await screen.findByRole("button", { name: /Edit Server Config/ })); + const memory = await screen.findByRole("combobox", { name: "Memory" }); + expect(within(memory).getByText("4Gi")).toBeTruthy(); + expect(screen.queryByText("3072M")).toBeNull(); + expect((screen.getByLabelText("CPU Limit") as HTMLInputElement).value).toBe("2"); + }); +}); diff --git a/panel/src/pages/ServerConsole.tsx b/panel/src/pages/ServerConsole.tsx index 0e05ac7..efe8cbe 100644 --- a/panel/src/pages/ServerConsole.tsx +++ b/panel/src/pages/ServerConsole.tsx @@ -308,7 +308,7 @@ export function ServerConsole() { currentDisplayName={data.displayName} currentPolicy={data.autostartPolicy as AutostartPolicy} currentImage={data.image} - currentMemory={data.javaMemory} + currentMemory={data.memory} currentStorage={data.storageSize} currentCpu={data.cpu} currentIdleStopSeconds={data.idleStopSeconds}