From 24ac71f9b22f7b5c1e7d4800e360e7ea1185aea6 Mon Sep 17 00:00:00 2001 From: thxCode Date: Thu, 8 Oct 2026 12:31:56 +0800 Subject: [PATCH 1/2] feat(modeldeployment): configure per-role shared memory Add a positive shmSize quantity with a 16Gi rendered default. Preserve explicit mounts and existing Elastic members. Update generated APIs, render checks and the owning guides. Signed-off-by: thxCode --- api/worker/v1alpha1/generated.pb.go | 55 ++++++++++ api/worker/v1alpha1/generated.proto | 8 ++ api/worker/v1alpha1/model_deployment.go | 8 ++ api/worker/v1alpha1/zz_generated.crds.go | 14 +++ api/worker/v1alpha1/zz_generated.deepcopy.go | 5 + api/worker/zz_generated.openapi.go | 8 +- docs/modules/model-deployment/deployment.md | 27 ++++- docs/modules/model-deployment/elastic-ep.md | 13 +-- .../worker/v1alpha1/modeldeploymentrole.go | 14 +++ .../model_deployment_elastic_realization.go | 34 ------ .../model_deployment_elastic_workload_test.go | 100 +++++++++++++++--- .../worker/model_deployment_render.go | 13 +++ .../worker/model_deployment_render_test.go | 51 +++++---- .../webhooks/worker/model_deployment.go | 3 + .../webhooks/worker/model_deployment_test.go | 30 ++++++ ...ache-and-model-deployment-consolidation.md | 7 ++ specs/2026-10-01-s2-elastic-ep-resize.md | 15 ++- 17 files changed, 323 insertions(+), 82 deletions(-) diff --git a/api/worker/v1alpha1/generated.pb.go b/api/worker/v1alpha1/generated.pb.go index acc1f8927..cb4555878 100644 --- a/api/worker/v1alpha1/generated.pb.go +++ b/api/worker/v1alpha1/generated.pb.go @@ -6299,6 +6299,20 @@ func (m *ModelDeploymentRole) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l + if m.ShmSize != nil { + { + size, err := m.ShmSize.MarshalToSizedBuffer(dAtA[:i]) + if err != nil { + return 0, err + } + i -= size + i = encodeVarintGenerated(dAtA, i, uint64(size)) + } + i-- + dAtA[i] = 0x1 + i-- + dAtA[i] = 0x9a + } if m.ElasticEP != nil { { size, err := m.ElasticEP.MarshalToSizedBuffer(dAtA[:i]) @@ -11418,6 +11432,10 @@ func (m *ModelDeploymentRole) Size() (n int) { l = m.ElasticEP.Size() n += 2 + l + sovGenerated(uint64(l)) } + if m.ShmSize != nil { + l = m.ShmSize.Size() + n += 2 + l + sovGenerated(uint64(l)) + } return n } @@ -14117,6 +14135,7 @@ func (this *ModelDeploymentRole) String() string { `Topology:` + strings.Replace(this.Topology.String(), "ModelDeploymentRoleTopology", "ModelDeploymentRoleTopology", 1) + `,`, `TerminationGracePeriodSeconds:` + valueToStringGenerated(this.TerminationGracePeriodSeconds) + `,`, `ElasticEP:` + strings.Replace(this.ElasticEP.String(), "ModelDeploymentRoleElasticEP", "ModelDeploymentRoleElasticEP", 1) + `,`, + `ShmSize:` + strings.Replace(fmt.Sprintf("%v", this.ShmSize), "Quantity", "resource.Quantity", 1) + `,`, `}`, }, "") return s @@ -34547,6 +34566,42 @@ func (m *ModelDeploymentRole) Unmarshal(dAtA []byte) error { return err } iNdEx = postIndex + case 19: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field ShmSize", wireType) + } + var msglen int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowGenerated + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + msglen |= int(b&0x7F) << shift + if b < 0x80 { + break + } + } + if msglen < 0 { + return ErrInvalidLengthGenerated + } + postIndex := iNdEx + msglen + if postIndex < 0 { + return ErrInvalidLengthGenerated + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + if m.ShmSize == nil { + m.ShmSize = &resource.Quantity{} + } + if err := m.ShmSize.Unmarshal(dAtA[iNdEx:postIndex]); err != nil { + return err + } + iNdEx = postIndex default: iNdEx = preIndex skippy, err := skipGenerated(dAtA[iNdEx:]) diff --git a/api/worker/v1alpha1/generated.proto b/api/worker/v1alpha1/generated.proto index f63b11e0e..928438d5c 100644 --- a/api/worker/v1alpha1/generated.proto +++ b/api/worker/v1alpha1/generated.proto @@ -3796,6 +3796,14 @@ message ModelDeploymentRole { // +listType=atomic repeated ModelDeploymentAdditionalVolume additionalVolumes = 13; + // ShmSize limits the memory-backed /dev/shm volume in each role Pod. It must be positive. + // Omission renders 16Gi. An explicit /dev/shm mount in AdditionalVolumes takes precedence. + // Used shared memory counts toward the container's memory limit; this is not a reservation. + // Existing elastic members keep their mounts; new and replacement Pods use the current value. + // + // +optional + optional .k8s.io.apimachinery.pkg.api.resource.Quantity shmSize = 19; + // Command replaces the whole argv, which is the TAKE-OVER tier: the user owns the whole // command line, the operator synthesizes no engine argument and no client environment, the // role is marked unmanaged and CacheAttached goes to Unknown. Arguments fold into Command; diff --git a/api/worker/v1alpha1/model_deployment.go b/api/worker/v1alpha1/model_deployment.go index d940889ee..5b37bf6ba 100644 --- a/api/worker/v1alpha1/model_deployment.go +++ b/api/worker/v1alpha1/model_deployment.go @@ -468,6 +468,14 @@ type ModelDeploymentRole struct { // +listType=atomic AdditionalVolumes []ModelDeploymentAdditionalVolume `json:"additionalVolumes,omitempty" protobuf:"bytes,13,rep,name=additionalVolumes"` // nolint: lll + // ShmSize limits the memory-backed /dev/shm volume in each role Pod. It must be positive. + // Omission renders 16Gi. An explicit /dev/shm mount in AdditionalVolumes takes precedence. + // Used shared memory counts toward the container's memory limit; this is not a reservation. + // Existing elastic members keep their mounts; new and replacement Pods use the current value. + // + // +optional + ShmSize *resource.Quantity `json:"shmSize,omitempty" protobuf:"bytes,19,opt,name=shmSize"` + // Command replaces the whole argv, which is the TAKE-OVER tier: the user owns the whole // command line, the operator synthesizes no engine argument and no client environment, the // role is marked unmanaged and CacheAttached goes to Unknown. Arguments fold into Command; diff --git a/api/worker/v1alpha1/zz_generated.crds.go b/api/worker/v1alpha1/zz_generated.crds.go index 27b3a93c5..86bcd6413 100644 --- a/api/worker/v1alpha1/zz_generated.crds.go +++ b/api/worker/v1alpha1/zz_generated.crds.go @@ -5060,6 +5060,20 @@ func crd_gpustack_api_worker_v1alpha1_ModelDeployment() *v1.CustomResourceDefini }, Nullable: true, }, + "shmSize": { + Description: "ShmSize limits the memory-backed /dev/shm volume in each role Pod. It must be positive.\nOmission renders 16Gi. An explicit /dev/shm mount in AdditionalVolumes takes precedence.\nUsed shared memory counts toward the container's memory limit; this is not a reservation.\nExisting elastic members keep their mounts; new and replacement Pods use the current value.", + Pattern: `^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$`, + AnyOf: []v1.JSONSchemaProps{ + { + Type: "integer", + }, + { + Type: "string", + }, + }, + Nullable: true, + XIntOrString: true, + }, "size": { Description: "ReplicaSize is how many Pods form ONE serving instance. Those Pods are fate-sharing: they\nstart together, they are replaced together, and none of them serves alone — the instance,\nnot the Pod, is the unit that appears and disappears.\nTHIS NUMBER IS FIXED AT CREATION AND CANNOT BE CHANGED. An instance's size is the shape of the\ninstance, not a dial on it: the Pods a running instance is made of are not the Pods a different\nsize asks for. Scaling is what replicas is for, and it leaves every running instance alone. To\nserve at a different size, create a deployment that declares it.\nABOVE ONE, THE PODS OF AN INSTANCE NEED EACH OTHER'S ADDRESSES, so an instance of several Pods\nis rendered with stable names and publishes the first Pod's address, this instance's size and\neach Pod's own rank to every container. What an engine does with those facts -- which\nparallelism it turns on, and over how many ranks -- stays the author's to say.\nTHE GO IDENTIFIER IS NOT Size BECAUSE gogo protobuf generates a Size() method on this type and\nGo forbids a field and a method sharing a name; the API field is size.\nTHE UPPER BOUND IS THE SAME LIMIT REPLICAS CARRIES, AND IT MULTIPLIES WITH IT: this number is\nhow many Pods one instance is rendered as, so a pass renders replicas times this many before it\nwrites any of them. It is set far above the sizes an accelerator topology makes sense at, and\nfar below the range that turns one accepted field value into an out-of-memory worker.", Type: "integer", diff --git a/api/worker/v1alpha1/zz_generated.deepcopy.go b/api/worker/v1alpha1/zz_generated.deepcopy.go index a5cca751d..568c0406e 100644 --- a/api/worker/v1alpha1/zz_generated.deepcopy.go +++ b/api/worker/v1alpha1/zz_generated.deepcopy.go @@ -2679,6 +2679,11 @@ func (in *ModelDeploymentRole) DeepCopyInto(out *ModelDeploymentRole) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.ShmSize != nil { + in, out := &in.ShmSize, &out.ShmSize + x := (*in).DeepCopy() + *out = &x + } if in.Command != nil { in, out := &in.Command, &out.Command *out = make([]string, len(*in)) diff --git a/api/worker/zz_generated.openapi.go b/api/worker/zz_generated.openapi.go index a057ecf5b..99d7d1493 100644 --- a/api/worker/zz_generated.openapi.go +++ b/api/worker/zz_generated.openapi.go @@ -9964,6 +9964,12 @@ func schema_gpustack_api_worker_v1alpha1_ModelDeploymentRole(ref common.Referenc }, }, }, + "shmSize": { + SchemaProps: spec.SchemaProps{ + Description: "ShmSize limits the memory-backed /dev/shm volume in each role Pod. It must be positive. Omission renders 16Gi. An explicit /dev/shm mount in AdditionalVolumes takes precedence. Used shared memory counts toward the container's memory limit; this is not a reservation. Existing elastic members keep their mounts; new and replacement Pods use the current value.", + Ref: ref(resource.Quantity{}.OpenAPIModelName()), + }, + }, "command": { VendorExtensible: spec.VendorExtensible{ Extensions: spec.Extensions{ @@ -10054,7 +10060,7 @@ func schema_gpustack_api_worker_v1alpha1_ModelDeploymentRole(ref common.Referenc }, }, Dependencies: []string{ - v1alpha1.ModelDeploymentAdditionalVolume{}.OpenAPIModelName(), v1alpha1.ModelDeploymentEnvVar{}.OpenAPIModelName(), v1alpha1.ModelDeploymentPort{}.OpenAPIModelName(), v1alpha1.ModelDeploymentRoleElasticEP{}.OpenAPIModelName(), v1alpha1.ModelDeploymentRoleResources{}.OpenAPIModelName(), v1alpha1.ModelDeploymentRoleTopology{}.OpenAPIModelName(), corev1.LocalObjectReference{}.OpenAPIModelName()}, + v1alpha1.ModelDeploymentAdditionalVolume{}.OpenAPIModelName(), v1alpha1.ModelDeploymentEnvVar{}.OpenAPIModelName(), v1alpha1.ModelDeploymentPort{}.OpenAPIModelName(), v1alpha1.ModelDeploymentRoleElasticEP{}.OpenAPIModelName(), v1alpha1.ModelDeploymentRoleResources{}.OpenAPIModelName(), v1alpha1.ModelDeploymentRoleTopology{}.OpenAPIModelName(), corev1.LocalObjectReference{}.OpenAPIModelName(), resource.Quantity{}.OpenAPIModelName()}, } } diff --git a/docs/modules/model-deployment/deployment.md b/docs/modules/model-deployment/deployment.md index e41e28d56..53a603834 100644 --- a/docs/modules/model-deployment/deployment.md +++ b/docs/modules/model-deployment/deployment.md @@ -256,7 +256,7 @@ Without one, users patch the rendered Pod and the reconcile loop silently overwr | Tier | Field | Semantics | |---|---|---| | append | `roles[].extraArgs`, `roles[].env` | appended **after** the operator-synthesized arguments; a key the operator owns is refused, never merged | -| overlay | the role's own Pod fields — `image`, `imagePullPolicy`, `imagePullSecrets`, `privileged`, `ports`, `additionalVolumes`, `terminationGracePeriodSeconds` | the operator renders first, then merges this overlay on top | +| overlay | the role's own Pod fields — `image`, `imagePullPolicy`, `imagePullSecrets`, `privileged`, `ports`, `additionalVolumes`, `shmSize`, `terminationGracePeriodSeconds` | the operator renders first, then merges this overlay on top | | take over | `roles[].command` | the user owns the whole argv; the operator synthesizes **no** engine argument and **no** client environment | A role's Pod fields sit on the role itself, unlike the `Instance` that keeps its pod shape inside an @@ -296,6 +296,27 @@ That is one instance of a wider exposure, not its boundary. The boundary is stat [#168](https://github.com/gpustack/gpustack-operator/issues/168), whose own void conditions include a webhook-level one, so nothing here should be read as a claim about how that issue can be closed. +### Shared memory + +`roles[].shmSize` sets the capacity limit of `/dev/shm` in each role Pod. It accepts a positive Kubernetes +quantity, such as `32Gi`. Omission renders a `16Gi` memory-backed `EmptyDir`. +This applies to both engines, take-over roles, and the Elastic Ray head and GPU members. +Each Pod has its own volume. Only the main container mounts it. + +An explicit `/dev/shm` mount in `additionalVolumes` takes precedence, including when `shmSize` is set. +The operator preserves that mount's capacity and backing. +The default is a project capacity choice, not a universal engine requirement. + +The capacity limit does not reserve RAM or increase the Pod's memory request. +Used shared memory counts toward the main container's memory limit. +Budget memory for both shared memory and the engine heap. +Ray's object store may need more than `16Gi`; size the volume for the configured workload. +Kubelet lowers the mount size when the Pod memory limit or node allocatable memory is smaller. + +Changing the rendered capacity follows ordinary role rollout behavior. +Existing Elastic members keep their mounts; new or replacement Pods use the current value. +Upgrading the operator adds the default mount to ordinary roles and triggers their recreate rollout. + ## Operator-owned keys Ownership is per (engine, key): a key one engine owns is an ordinary user argument on another. @@ -433,7 +454,7 @@ appearing as an unattributable `ImagePullBackOff`. Changing `replicas` adds or removes instances and nothing more: the survivors are not restarted, do not reload their weights and keep their cached blocks. What still replaces **every** instance of the role is an edit that changes what a replica's Pod renders: `image`, `extraArgs`, `env`, `ports`, -`additionalVolumes`, `terminationGracePeriodSeconds`. A change to `size` is not on that list because +`additionalVolumes`, `shmSize`, `terminationGracePeriodSeconds`. A change to `size` is not on that list because it cannot be made: see `roles[].size` above. Such an edit **deletes and recreates** the role's replicas, one replica per role per pass, waited @@ -521,7 +542,7 @@ editable; the table below lists both groups and the scheduling exceptions. | the set of roles, and each role's `name` and `kind` | `roles[].replicas` | | `roles[].size` | | | `roles[].instanceType` | `roles[].extraArgs`, `roles[].env` | -| `roles[].resources` | the role's own Pod fields — `image`, `imagePullPolicy`, `imagePullSecrets`, `privileged`, `ports`, `additionalVolumes`, `terminationGracePeriodSeconds` | +| `roles[].resources` | the role's own Pod fields — `image`, `imagePullPolicy`, `imagePullSecrets`, `privileged`, `ports`, `additionalVolumes`, `shmSize`, `terminationGracePeriodSeconds` | | `roles[].command` | labels and annotations | `roles[].resources` is frozen because changing it renegotiates scheduling, much like deleting and diff --git a/docs/modules/model-deployment/elastic-ep.md b/docs/modules/model-deployment/elastic-ep.md index e28177505..54510d3b2 100644 --- a/docs/modules/model-deployment/elastic-ep.md +++ b/docs/modules/model-deployment/elastic-ep.md @@ -8,7 +8,7 @@ The engine reconfigures while Kubernetes, Kueue, Ray and accelerator accounting - [What the profile means](#what-the-profile-means) - [Requirements](#requirements) -- [Shared memory for GPU members](#shared-memory-for-gpu-members) +- [Shared memory](#shared-memory) - [A minimal manifest](#a-minimal-manifest) - [Changing the width](#changing-the-width) - [What you see while it runs](#what-you-see-while-it-runs) @@ -70,14 +70,15 @@ The profile increases the number of GPU members. It does not resize the underlyi cloud node elasticity, and it does not patch anything upstream in the engine. It drives the engine through the resize interface the engine already exposes. -## Shared memory for GPU members +## Shared memory -Each Elastic GPU member gets a separate memory-backed `EmptyDir` mounted at `/dev/shm`. -Its size limit is 512 MiB. This includes the master and workers. The CPU head has no default mount. -The volume is isolated per Pod and uses the Pod's memory allocation. +The CPU head, master and GPU workers each get a separate memory-backed `EmptyDir` at `/dev/shm`. +The role's `shmSize` sets its capacity; omission renders `16Gi`. +The volume is isolated per Pod and uses the main container's memory allocation. An explicit `/dev/shm` mount takes precedence. Its capacity and backing remain the user's responsibility. -Sufficiency at 512 MiB is not established. A workload that needs more must declare its own `/dev/shm` mount. +Changing `shmSize` affects new or replacement members; existing members keep their mounts. +See [Shared memory](deployment.md#shared-memory) for precedence and memory budgeting. ## A minimal manifest diff --git a/pkg/kubeclients/applyconfiguration/worker/v1alpha1/modeldeploymentrole.go b/pkg/kubeclients/applyconfiguration/worker/v1alpha1/modeldeploymentrole.go index d9cbfcff0..70c2edc71 100644 --- a/pkg/kubeclients/applyconfiguration/worker/v1alpha1/modeldeploymentrole.go +++ b/pkg/kubeclients/applyconfiguration/worker/v1alpha1/modeldeploymentrole.go @@ -6,6 +6,7 @@ import ( workerv1alpha1 "gpustack.ai/gpustack/api/worker/v1alpha1" corev1 "gpustack.ai/gpustack/pkg/kubeclients/applyconfiguration/core/v1" v1 "k8s.io/api/core/v1" + resource "k8s.io/apimachinery/pkg/api/resource" ) // ModelDeploymentRoleApplyConfiguration represents a declarative configuration of the ModelDeploymentRole type for use @@ -128,6 +129,11 @@ type ModelDeploymentRoleApplyConfiguration struct { Ports []ModelDeploymentPortApplyConfiguration `json:"ports,omitempty"` // AdditionalVolumes are volumes mounted into the container alongside the operator's own. AdditionalVolumes []ModelDeploymentAdditionalVolumeApplyConfiguration `json:"additionalVolumes,omitempty"` + // ShmSize limits the memory-backed /dev/shm volume in each role Pod. It must be positive. + // Omission renders 16Gi. An explicit /dev/shm mount in AdditionalVolumes takes precedence. + // Used shared memory counts toward the container's memory limit; this is not a reservation. + // Existing elastic members keep their mounts; new and replacement Pods use the current value. + ShmSize *resource.Quantity `json:"shmSize,omitempty"` // Command replaces the whole argv, which is the TAKE-OVER tier: the user owns the whole // command line, the operator synthesizes no engine argument and no client environment, the // role is marked unmanaged and CacheAttached goes to Unknown. Arguments fold into Command; @@ -317,6 +323,14 @@ func (b *ModelDeploymentRoleApplyConfiguration) WithAdditionalVolumes(values ... return b } +// WithShmSize sets the ShmSize field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the ShmSize field is set to the value of the last call. +func (b *ModelDeploymentRoleApplyConfiguration) WithShmSize(value resource.Quantity) *ModelDeploymentRoleApplyConfiguration { + b.ShmSize = &value + return b +} + // WithCommand adds the given value to the Command field in the declarative configuration // and returns the receiver, so that objects can be build by chaining "With" function invocations. // If called multiple times, values provided by each call will be appended to the Command field. diff --git a/pkg/worker/controllers/worker/model_deployment_elastic_realization.go b/pkg/worker/controllers/worker/model_deployment_elastic_realization.go index a330e16c5..83a9cb4b5 100644 --- a/pkg/worker/controllers/worker/model_deployment_elastic_realization.go +++ b/pkg/worker/controllers/worker/model_deployment_elastic_realization.go @@ -9,7 +9,6 @@ import ( core "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/resource" meta "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" ctrllog "sigs.k8s.io/controller-runtime/pkg/log" @@ -19,12 +18,6 @@ import ( "gpustack.ai/gpustack/pkg/systemmeta" ) -const ( - modelDeploymentElasticSharedMemoryPath = "/dev/shm" - modelDeploymentElasticSharedMemoryVolume = "elastic-shm" - modelDeploymentElasticSharedMemorySize = "512Mi" -) - // ModelDeploymentElasticTensorParallelSize reads the fixed TP degree from extraArgs. // Non-elastic roles retain their ordinary accelerator default of one. func ModelDeploymentElasticTensorParallelSize(role *workercore.ModelDeploymentRole) (int, error) { @@ -138,7 +131,6 @@ func (r *ModelDeploymentReconciler) renderModelDeploymentElasticPods(ctx context c.LivenessProbe = nil c.Lifecycle = nil } - modelDeploymentElasticSharedMemory(pod) pod.Annotations[modelDeploymentPodSpecHashAnnotation] = modelDeploymentPodSpecHash(pod) } head := gpu.DeepCopy() @@ -176,32 +168,6 @@ func (r *ModelDeploymentReconciler) renderModelDeploymentElasticPods(ctx context return desired, nil } -// modelDeploymentElasticSharedMemory gives each GPU member an isolated shared-memory volume. -// Preserve an explicit mount, whose backing and capacity remain the user's responsibility. -func modelDeploymentElasticSharedMemory(pod *core.Pod) { - if len(pod.Spec.Containers) == 0 { - return - } - container := &pod.Spec.Containers[0] - for _, m := range container.VolumeMounts { - if m.MountPath == modelDeploymentElasticSharedMemoryPath { - return - } - } - size := resource.MustParse(modelDeploymentElasticSharedMemorySize) - pod.Spec.Volumes = append(pod.Spec.Volumes, core.Volume{ - Name: modelDeploymentElasticSharedMemoryVolume, - VolumeSource: core.VolumeSource{EmptyDir: &core.EmptyDirVolumeSource{ - Medium: core.StorageMediumMemory, - SizeLimit: &size, - }}, - }) - container.VolumeMounts = append(container.VolumeMounts, core.VolumeMount{ - Name: modelDeploymentElasticSharedMemoryVolume, - MountPath: modelDeploymentElasticSharedMemoryPath, - }) -} - func renderModelDeploymentElasticHeadService(md *workercore.ModelDeployment) *core.Service { name := modelDeploymentElasticHeadName(md) svc := &core.Service{ diff --git a/pkg/worker/controllers/worker/model_deployment_elastic_workload_test.go b/pkg/worker/controllers/worker/model_deployment_elastic_workload_test.go index 16618f5ae..73d5f48dd 100644 --- a/pkg/worker/controllers/worker/model_deployment_elastic_workload_test.go +++ b/pkg/worker/controllers/worker/model_deployment_elastic_workload_test.go @@ -15,6 +15,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" meta "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" ctrlcli "sigs.k8s.io/controller-runtime/pkg/client" kueue "sigs.k8s.io/kueue/apis/kueue/v1beta2" kueuepodconst "sigs.k8s.io/kueue/pkg/controller/jobs/pod/constants" @@ -404,17 +405,25 @@ func TestElasticTerminatingMemberWorkloadIsReleased(t *testing.T) { } } -func TestElasticGPUMembersCarryIsolatedSharedMemory(t *testing.T) { +func TestModelDeploymentRolesSharedMemory(t *testing.T) { for _, tc := range []struct { name string additional []workercore.ModelDeploymentAdditionalVolume ordinary bool + engine string + shmSize string + takeOver bool wantEmptyDir string }{ - {name: "default isolated emptyDir", wantEmptyDir: "512Mi"}, - {name: "ordinary render is unchanged", ordinary: true}, + {name: "elastic default", wantEmptyDir: "16Gi"}, + {name: "elastic custom", shmSize: "32Gi", wantEmptyDir: "32Gi"}, + {name: "ordinary vllm default", ordinary: true, wantEmptyDir: "16Gi"}, + {name: "ordinary sglang default", ordinary: true, engine: workercore.ModelDeploymentEngineSGLang, wantEmptyDir: "16Gi"}, + {name: "ordinary custom", ordinary: true, shmSize: "32Gi", wantEmptyDir: "32Gi"}, + {name: "take over default", ordinary: true, takeOver: true, wantEmptyDir: "16Gi"}, { - name: "explicit mount is preserved", + name: "explicit mount overrides custom size", + shmSize: "32Gi", additional: []workercore.ModelDeploymentAdditionalVolume{{ MountPath: "/dev/shm", ConfigMap: &core.LocalObjectReference{Name: "user-shm"}, @@ -429,6 +438,15 @@ func TestElasticGPUMembersCarryIsolatedSharedMemory(t *testing.T) { md.Spec.Roles[0].ElasticEP = &workercore.ModelDeploymentRoleElasticEP{Width: 2, HeadInstanceType: "cpu"} } md.Spec.Roles[0].AdditionalVolumes = tc.additional + if tc.engine != "" { + md.Spec.Engine.Name = tc.engine + } + if tc.shmSize != "" { + md.Spec.Roles[0].ShmSize = ptr.To(resource.MustParse(tc.shmSize)) + } + if tc.takeOver { + md.Spec.Roles[0].Command = []string{"custom-server"} + } cpu := newRenderInstanceType(func(it *worker.InstanceType) { it.Name = "cpu"; it.Spec.Acceleratable = false }) cli := newModelDeploymentClient(md, newRenderInstanceType(), cpu) r := &ModelDeploymentReconciler{Client: cli, APIReader: cli} @@ -451,18 +469,9 @@ func TestElasticGPUMembersCarryIsolatedSharedMemory(t *testing.T) { shmNames = append(shmNames, m.Name) } } - if tc.ordinary { - require.Empty(t, shmNames, "%s is an ordinary render, not an elastic member", pod.Name) - continue - } - if modelDeploymentPodRole(pod) == modelDeploymentElasticHeadName(md) { - // Explicit role mounts also reach the CPU head. Only the default must stay absent. - if tc.additional == nil { - require.Empty(t, shmNames, "the CPU head gains no shared memory of its own") - } - continue + if modelDeploymentPodRole(pod) != modelDeploymentElasticHeadName(md) { + members++ } - members++ require.Len(t, shmNames, 1, "%s must mount /dev/shm exactly once", pod.Name) var backing *core.Volume for j := range pod.Spec.Volumes { @@ -495,6 +504,67 @@ func TestElasticGPUMembersCarryIsolatedSharedMemory(t *testing.T) { } } +func TestElasticSharedMemoryEditRetainsExistingMembers(t *testing.T) { + ctx := context.Background() + f := newElasticConvergenceFixture(t) + pods := new(core.PodList) + require.NoError(t, f.reconciler.Client.List(ctx, pods)) + original := make(map[string]types.UID, len(pods.Items)) + for _, pod := range pods.Items { + original[pod.Name] = pod.UID + } + require.Len(t, original, 3) + assertRetained := func() { + t.Helper() + for name, uid := range original { + index := slices.IndexFunc(pods.Items, func(pod core.Pod) bool { return pod.Name == name }) + require.NotEqual(t, -1, index, "original Pod %s must remain", name) + pod := &pods.Items[index] + require.Equal(t, uid, pod.UID, name) + for _, volume := range pod.Spec.Volumes { + if volume.EmptyDir != nil && volume.EmptyDir.Medium == core.StorageMediumMemory { + require.NotNil(t, volume.EmptyDir.SizeLimit) + require.Zero(t, volume.EmptyDir.SizeLimit.Cmp(resource.MustParse("16Gi")), name) + } + } + } + } + + f.md.Spec.Roles[0].ShmSize = ptr.To(resource.MustParse("32Gi")) + _, err := f.reconciler.convergeModelDeployment(ctx, f.md) + require.NoError(t, err) + require.NoError(t, f.reconciler.Client.List(ctx, pods)) + require.Len(t, pods.Items, 3) + assertRetained() + + f.md.Spec.Roles[0].ElasticEP.Width = 3 + _, err = f.reconciler.convergeModelDeployment(ctx, f.md) + require.NoError(t, err) + require.NoError(t, f.reconciler.Client.List(ctx, pods)) + require.Len(t, pods.Items, 4) + assertRetained() + newMembers := 0 + for _, pod := range pods.Items { + want := "32Gi" + if uid, exists := original[pod.Name]; exists { + require.Equal(t, uid, pod.UID) + want = "16Gi" + } else { + newMembers++ + } + found := false + for _, volume := range pod.Spec.Volumes { + if volume.EmptyDir != nil && volume.EmptyDir.Medium == core.StorageMediumMemory { + found = true + require.NotNil(t, volume.EmptyDir.SizeLimit) + require.Zero(t, volume.EmptyDir.SizeLimit.Cmp(resource.MustParse(want)), pod.Name) + } + } + require.True(t, found, pod.Name) + } + require.Equal(t, 1, newMembers) +} + func TestElasticUnknownBootstrapRetainsMembers(t *testing.T) { for _, raw := range []string{"", "bad", "1", "65"} { t.Run("boot="+raw, func(t *testing.T) { diff --git a/pkg/worker/controllers/worker/model_deployment_render.go b/pkg/worker/controllers/worker/model_deployment_render.go index 5cf0640cd..a41a9b0db 100644 --- a/pkg/worker/controllers/worker/model_deployment_render.go +++ b/pkg/worker/controllers/worker/model_deployment_render.go @@ -490,6 +490,19 @@ func renderModelDeploymentPodTemplate(ctx context.Context, in ModelDeploymentRen vols = append(vols, in.Connector.Volumes...) mounts = append(mounts, in.Connector.VolumeMounts...) } + if !slices.ContainsFunc(mounts, func(m core.VolumeMount) bool { return m.MountPath == "/dev/shm" }) { + size := resource.MustParse("16Gi") + if role.ShmSize != nil { + size = role.ShmSize.DeepCopy() + } + vols = append(vols, core.Volume{ + Name: "model-shm", + VolumeSource: core.VolumeSource{EmptyDir: &core.EmptyDirVolumeSource{ + Medium: core.StorageMediumMemory, SizeLimit: &size, + }}, + }) + mounts = append(mounts, core.VolumeMount{Name: "model-shm", MountPath: "/dev/shm"}) + } probePath := modelDeploymentProbePath probeScheme := scheme diff --git a/pkg/worker/controllers/worker/model_deployment_render_test.go b/pkg/worker/controllers/worker/model_deployment_render_test.go index 1fb06ada3..a7b1fa899 100644 --- a/pkg/worker/controllers/worker/model_deployment_render_test.go +++ b/pkg/worker/controllers/worker/model_deployment_render_test.go @@ -827,8 +827,12 @@ func TestRenderModelDeploymentPod_TakeOver(t *testing.T) { _, ok := envValue(pod, "MOONCAKE_CONFIG_PATH") assert.False(t, ok, "the operator's client environment points at a file this argv never reads") - assert.Empty(t, pod.Spec.Volumes, "and the file itself is not mounted either") - assert.Empty(t, pod.Spec.Containers[0].VolumeMounts, "nor is it mounted into the container") + for _, volume := range pod.Spec.Volumes { + assert.NotEqual(t, inject.ConfigVolumeName, volume.Name, "the client file is not mounted") + } + for _, mount := range pod.Spec.Containers[0].VolumeMounts { + assert.NotEqual(t, inject.ConfigMountPath, mount.MountPath, "the client file is not mounted") + } for _, port := range pod.Spec.Containers[0].Ports { assert.NotContains(t, []int32{5557, 5558}, port.ContainerPort, "a take-over role gets no publisher port") @@ -1542,7 +1546,7 @@ func TestRenderModelDeploymentPod_ClientConfigMount(t *testing.T) { }) require.NoError(t, err) - require.Len(t, pod.Spec.Volumes, 1) + require.Len(t, pod.Spec.Volumes, 2) require.NotNil(t, pod.Spec.Volumes[0].DownwardAPI, "the file is a projection of an annotation, not a ConfigMap: there is no second object") require.Len(t, pod.Spec.Volumes[0].DownwardAPI.Items, 1) @@ -1554,20 +1558,24 @@ func TestRenderModelDeploymentPod_ClientConfigMount(t *testing.T) { "the projection reads this annotation, so an absent or empty one mounts an empty file") mounts := pod.Spec.Containers[0].VolumeMounts - require.Len(t, mounts, 1) + require.Len(t, mounts, 2) assert.Equal(t, inject.ConfigMountPath, mounts[0].MountPath) assert.True(t, mounts[0].ReadOnly) } // TestRenderModelDeploymentPod_NoClientConfigWithoutOne pins the state a deployment is in before its -// Binding has been resolved: a replica renders and runs, with no mount and no volume, rather than -// referencing a ConfigMap nothing has written. +// Binding has been resolved: a replica renders and runs without a client configuration mount, +// rather than referencing a ConfigMap nothing has written. func TestRenderModelDeploymentPod_NoClientConfigWithoutOne(t *testing.T) { md := newRenderDeployment() pod := renderOne(t, md, newRenderInstanceType()) - assert.Empty(t, pod.Spec.Volumes) - assert.Empty(t, pod.Spec.Containers[0].VolumeMounts) + for _, volume := range pod.Spec.Volumes { + assert.NotEqual(t, inject.ConfigVolumeName, volume.Name) + } + for _, mount := range pod.Spec.Containers[0].VolumeMounts { + assert.NotEqual(t, inject.ConfigMountPath, mount.MountPath) + } } // TestModelDeploymentPodSpecHash_IsDeterministic states the property the whole rollout rests on: two @@ -1668,6 +1676,12 @@ func TestModelDeploymentPodSpecHash_MovesWithEveryRenderedInput(t *testing.T) { name: "image", mutate: func(md *workercore.ModelDeployment) { md.Spec.Roles[0].Image = "vllm/vllm-openai:v0.26.0" }, }, + { + name: "shared memory size", + mutate: func(md *workercore.ModelDeployment) { + md.Spec.Roles[0].ShmSize = ptr.To(resource.MustParse("32Gi")) + }, + }, { name: "model", mutate: func(md *workercore.ModelDeployment) { md.Spec.Model.Name = "Qwen/Qwen3-32B" }, @@ -2189,22 +2203,23 @@ func TestRenderModelDeploymentPod_Probes(t *testing.T) { // selector names the Pods that answer the API at every role size; once when each managed Pod // began declaring its metrics listener scheme; and once when every Pod whose command line the // operator builds began carrying the drain hook and the termination grace it is budgeted against, -// which left the take-over case's digest where it was. Every digest here moved by intent +// which left the take-over case's digest where it was. The shared-memory volume then changed +// every role's Pod, including take-over roles. Every digest here moved by intent // rather than by drift. Each case renders ORDINAL ZERO -- the composite's zero value -- which is the // one ordinal a digest can name without the table growing a dimension. To re-baseline after an // intended rendering change: empty the table, run this case, and pin the digests the failures // print. func TestRenderModelDeploymentPod_SerializedOutputIsPinnedToThePreSplitRender(t *testing.T) { pinned := map[string]string{ - "a sole server role": "bbd82155b1d685e6906f885fdbebf266890117cafa9d1f58e089e6d678c27ec4", - "a sole server role with a synthesized cache connector": "42565b50fb7e640fa5d6a1a23c7a6d1920fa98370a3ca2d2080c2bb7e7a0a7d8", - "a take-over role carrying a connector it must be given no part of": "1984284b5fbc1e7245e71bb5ea8c3ed1daccef316d724e398952fa637c4c67cd", - "a direct decoder with a native routing sidecar": "f7c36762d7ae4034f5a3642e0670d75a5fde2d05a151126ee82fa121fc11de2f", - "a direct decoder with a classic routing sidecar": "4c6b12bd46308005d3d1fccad37d7523525c2993500a7a40d1822c2ee84b6c83", - "a role naming no image, synthesized from the observed hardware": "c6fbaa790a7663c4d9f601ba5d2183e8cb049b410e168ba05acc62b633b2d5f8", - "a TLS-listening role with declared ports, privileges, a runtime class and a volume": "a576d65292f1abd652bb02aa5670707da9d6acdc122d5bc990de42e00387cd1c", - "the prefill role of a two-role deployment": "108810fcd9c5b7b417bf9fdb3b91af0d8f6031458c45eeffa22083b105cdc5aa", - "the decode role of a two-role deployment": "6d5e95c247bf376093f8d2263fc2469983bae32ad30c2f23422a3971f8454dc2", + "a sole server role": "f32a43254d561da431bd5f1107f97ffc5e3c9382b88db0478edc811a22178392", + "a sole server role with a synthesized cache connector": "52aff227f37f8ca8d4c311b4008b7650a2823ee0668a9f693ceb14837f8643d6", + "a take-over role carrying a connector it must be given no part of": "93e925e2bfaa5ab9e152a685a336d1b8031d8904bfb54b240a483f0a517e9a54", + "a direct decoder with a native routing sidecar": "9708da967a83ff20e04944fa38033d8fceca8de0379d9a6d7e7847312c997813", + "a direct decoder with a classic routing sidecar": "ed7d954b5f334dc1993caab86d7e2fa0f53458eda1db3006dcd2e861ca464a6b", + "a role naming no image, synthesized from the observed hardware": "30d7c6cd55c58ac4a9a502396be0b291a5f9a8872967be0b4dfd430448abcac1", + "a TLS-listening role with declared ports, privileges, a runtime class and a volume": "3b9148d73e337285192aea35aa995813747fc2ebfefda19d107548b788c11791", + "the prefill role of a two-role deployment": "7eef707d65e3c6307219d33612e56de8173d9a7b4ece67ff4239f21cc788ba82", + "the decode role of a two-role deployment": "cc1217e6e9b9de5e61b0c01aca76666126f24176804151f772b640e13d5c5a77", } // newPinnedInput builds the render input the way the reconciler does: the deployment and its diff --git a/pkg/worker/webhooks/worker/model_deployment.go b/pkg/worker/webhooks/worker/model_deployment.go index 9cee29397..75cf29c08 100644 --- a/pkg/worker/webhooks/worker/model_deployment.go +++ b/pkg/worker/webhooks/worker/model_deployment.go @@ -1639,6 +1639,9 @@ func validateModelDeploymentRoles(md *workercore.ModelDeployment) field.ErrorLis rolesPath := field.NewPath("spec", "roles") for i := range md.Spec.Roles { role, rolePath := &md.Spec.Roles[i], rolesPath.Index(i) + if role.ShmSize != nil && role.ShmSize.Sign() <= 0 { + errs = append(errs, field.Invalid(rolePath.Child("shmSize"), role.ShmSize.String(), "must be greater than zero")) + } errs = append(errs, validateModelDeploymentRoleExtraArgs(md.Spec.Engine.Name, role, rolePath)...) errs = append(errs, validateModelDeploymentRoleParallelWidth(md.Spec.Engine.Name, role, rolePath)...) diff --git a/pkg/worker/webhooks/worker/model_deployment_test.go b/pkg/worker/webhooks/worker/model_deployment_test.go index 51e684285..844ad2571 100644 --- a/pkg/worker/webhooks/worker/model_deployment_test.go +++ b/pkg/worker/webhooks/worker/model_deployment_test.go @@ -1741,6 +1741,36 @@ func TestValidateModelDeploymentRoleParallelWidth_DoesNotRefuseTwice(t *testing. // THE SOURCELESS CASE IS PAIRED WITH ONE PER SOURCE, because the rule is a three-way disjunction: an // implementation testing only configMap accepts the same table, and each accepting case is what says // the other two sources still satisfy it. +func TestValidateModelDeploymentRoleShmSize(t *testing.T) { + for _, tc := range []struct { + name string + size string + refused bool + }{ + {name: "omitted"}, + {name: "binary", size: "16Gi"}, + {name: "decimal", size: "500M"}, + {name: "zero", size: "0", refused: true}, + {name: "negative", size: "-1Gi", refused: true}, + } { + t.Run(tc.name, func(t *testing.T) { + md := modelDeployment(workercore.ModelDeploymentEngineVLLM) + if tc.size != "" { + md.Spec.Roles[0].ShmSize = ptr.To(resource.MustParse(tc.size)) + } + errs := validateModelDeploymentRoles(md) + if !tc.refused { + require.Empty(t, errs) + + return + } + require.Len(t, errs, 1) + require.Equal(t, "spec.roles[0].shmSize", errs[0].Field) + require.Contains(t, errs[0].Detail, "must be greater than zero") + }) + } +} + func TestValidateModelDeploymentRoleAdditionalVolumes(t *testing.T) { volumes := func(avs ...workercore.ModelDeploymentAdditionalVolume) *workercore.ModelDeployment { return modelDeployment(workercore.ModelDeploymentEngineVLLM, diff --git a/specs/2026-09-18-kvcache-and-model-deployment-consolidation.md b/specs/2026-09-18-kvcache-and-model-deployment-consolidation.md index 5755e880f..f09231bbd 100644 --- a/specs/2026-09-18-kvcache-and-model-deployment-consolidation.md +++ b/specs/2026-09-18-kvcache-and-model-deployment-consolidation.md @@ -616,6 +616,13 @@ together with the role's own `InstanceType`. role. `template.env` merges into `roles[].env`. `template.resources` is deleted outright, and `template` itself ceases to exist. +The role also owns `shmSize`, an optional positive Kubernetes quantity. +Ordinary rendering supplies a per-Pod memory-backed `/dev/shm` volume, defaulting to `16Gi`. +An explicit `/dev/shm` additional volume takes precedence. The limit does not reserve RAM. +Only the main container mounts it; used pages count toward that container's memory limit. +Changing the rendered capacity changes the Pod hash and follows recreate rollout. +Elastic members use the same rendering path but retain existing Pods during a capacity edit. + - **`resources` is deleted rather than moved.** It has no behavior but refusal, which its own comment states. The refusal it produced is replaced by strict decoding's unknown-field error, which is a worse message for a better reason: a field kept solely to improve one error message is a promise diff --git a/specs/2026-10-01-s2-elastic-ep-resize.md b/specs/2026-10-01-s2-elastic-ep-resize.md index 3caf23c7d..7ff0cc88d 100644 --- a/specs/2026-10-01-s2-elastic-ep-resize.md +++ b/specs/2026-10-01-s2-elastic-ep-resize.md @@ -320,13 +320,18 @@ whole-GPU admission only; Ray/elastic realization **NOT-established**). layers are derived from objects that realization created (negative: a fixture that only counts Pods cannot satisfy any four-layer check). -**Per-member shared memory is a realization prerequisite.** +**Shared memory is a realization prerequisite.** **MEASURED** — the pinned native profile failed before inference: its `/dev/shm` buffer required 160 MiB, with 64 MiB available. -Each GPU member, including the master, therefore renders a separate memory-backed `EmptyDir` at `/dev/shm`, limited to 512 MiB. -The realization loop adds it after ordinary rendering and before the Pod spec hash. +Ordinary role rendering provides each Pod with a memory-backed `EmptyDir` at `/dev/shm`. +`roles[].shmSize` is an optional positive Kubernetes quantity. Omission renders `16Gi`. +The common path covers ordinary roles, take-over roles, Elastic GPU members and the CPU head. +The volume enters the Pod spec hash. Its capacity does not reserve RAM or raise the memory request. +Only the main container mounts it; used shared memory counts toward that container's memory limit. An explicit `/dev/shm` mount takes precedence; its capacity and backing remain the user's responsibility. -CPU head and ordinary rendering remain unchanged. No host path is introduced. -**NOT-established** — 512 MiB covers the observed buffer requirement; full engine sufficiency still needs physical validation. +An ordinary capacity change follows recreate rollout. Existing Elastic members remain unchanged; +new or replacement members use the current role value. No host path is introduced. +The default replaces the earlier fixed 512 MiB member volume. It is a project choice, not an upstream engine guarantee. +**NOT-established** — full engine sufficiency and the dynamic TP>1 failure's root cause still need physical validation. No native resize pass is claimed. ### Feature 2 — Engine resize lifecycle with failure and recovery contract (E7 / E10 / T08) From 1e368ee9884e9d52f368eb42c5c18d6ab63eda77 Mon Sep 17 00:00:00 2001 From: thxCode Date: Thu, 8 Oct 2026 12:44:23 +0800 Subject: [PATCH 2/2] fixup! feat(modeldeployment): configure per-role shared memory Signed-off-by: thxCode --- docs/modules/model-deployment/elastic-ep.md | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/docs/modules/model-deployment/elastic-ep.md b/docs/modules/model-deployment/elastic-ep.md index 54510d3b2..11a796b58 100644 --- a/docs/modules/model-deployment/elastic-ep.md +++ b/docs/modules/model-deployment/elastic-ep.md @@ -73,12 +73,8 @@ through the resize interface the engine already exposes. ## Shared memory The CPU head, master and GPU workers each get a separate memory-backed `EmptyDir` at `/dev/shm`. -The role's `shmSize` sets its capacity; omission renders `16Gi`. -The volume is isolated per Pod and uses the main container's memory allocation. - -An explicit `/dev/shm` mount takes precedence. Its capacity and backing remain the user's responsibility. -Changing `shmSize` affects new or replacement members; existing members keep their mounts. -See [Shared memory](deployment.md#shared-memory) for precedence and memory budgeting. +Changing the role's `shmSize` affects new or replacement members; existing members keep their mounts. +See [Shared memory](deployment.md#shared-memory) for configuration, precedence and memory budgeting. ## A minimal manifest