fix(felis): 编辑服务器时内存和 CPU 可各自单改互不清除，显示名可清空，内存显示真实上限 (d65cf3af) · Commits · FelisMC / Felis · GitLab

docs/openapi.yaml

+19 −3

Changes for docs/openapi.yaml: 19 added lines, 3 removed lines.
Original line number	Diff line number	Diff line
		@@ -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 }

internal/api/api_test.go

+17 −0

Changes for internal/api/api_test.go: 17 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -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
		}

internal/api/cluster.go

+10 −3

Changes for internal/api/cluster.go: 10 added lines, 3 removed lines.
Original line number	Diff line number	Diff line
		@@ -26,6 +26,9 @@ type ServerInfo struct {
		DisplayName string `json:"displayName,omitempty"`
		Image string `json:"image,omitempty"`
		JavaMemory string `json:"javaMemory,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
		@@ -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

internal/api/handlers_patch_test.go

+126 −1

Changes for internal/api/handlers_patch_test.go: 126 added lines, 1 removed line.
Original line number	Diff line number	Diff line
		@@ -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)
		}
		}

internal/api/handlers_user.go

+139 −25

Changes for internal/api/handlers_user.go: 139 added lines, 25 removed lines.
Original line number	Diff line number	Diff line
		@@ -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:
		@@ -718,7 +814,8 @@ type patchServerRequest struct {
		// 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"`
		// 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
		}
		if body.Memory != nil \|\| memMoved {
		patch.JavaMemory = &javaMemory
		}
		patch.Resources = &resources
		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