-
Notifications
You must be signed in to change notification settings - Fork 201
Replace custom ResourceList with corev1.ResourceRequirements #4546
Description
Summary
The operator defines custom ResourceRequirements and ResourceList types that only support cpu and memory as plain strings. The standard corev1.ResourceRequirements supports GPU, ephemeral storage, custom resources, and uses resource.Quantity for proper schema validation. A manual conversion function bridges the gap at runtime.
Current custom types (cmd/thv-operator/api/v1alpha1/mcpserver_types.go, lines ~401-421)
type ResourceRequirements struct {
Limits ResourceList `json:"limits,omitempty"`
Requests ResourceList `json:"requests,omitempty"`
}
type ResourceList struct {
CPU string `json:"cpu,omitempty"`
Memory string `json:"memory,omitempty"`
}Fields using the custom type
| CRD | File | Line |
|---|---|---|
| MCPServer | mcpserver_types.go |
~204 |
| MCPRemoteProxy | mcpremoteproxy_types.go |
~103 |
| EmbeddingServer | embeddingserver_types.go |
~80 |
Conversion function to remove
cmd/thv-operator/pkg/controllerutil/resources.go (lines 20-46) — BuildResourceRequirements() manually converts the custom type to corev1.ResourceRequirements using resource.MustParse(). This function can be deleted once native types are used.
Why this matters
- Only CPU and memory are supported — no GPU, ephemeral storage, or custom resources
- Values are plain strings with no
resource.Quantityvalidation at the schema level - The conversion function adds unnecessary code and a potential panic via
MustParse() - Kubernetes conventions prefer upstream types for standard resource concepts
- Adding new resource types later would require a CRD schema change
What to change
1. Replace type definitions (cmd/thv-operator/api/v1alpha1/mcpserver_types.go)
Delete the ResourceRequirements and ResourceList custom type definitions (lines ~401-421). Replace the Resources field in all three CRDs with corev1.ResourceRequirements:
// Before
Resources ResourceRequirements `json:"resources,omitempty"`
// After
Resources corev1.ResourceRequirements `json:"resources,omitempty"`Update in:
mcpserver_types.go(~line 204)mcpremoteproxy_types.go(~line 103)embeddingserver_types.go(~line 80)
2. Remove conversion function
Delete BuildResourceRequirements() from cmd/thv-operator/pkg/controllerutil/resources.go. Update all callers in the controllers to use the CRD field directly instead of calling the converter.
Search for BuildResourceRequirements in cmd/thv-operator/controllers/ to find all call sites.
3. Update tests
Test files constructing the custom ResourceList{CPU: "500m", Memory: "64Mi"} need updating to use corev1.ResourceRequirements with resource.MustParse() values.
4. Regenerate and verify
task gen # Regenerates CRD manifests and deepcopy
task crdref-gen # Regenerates CRD API docs
task lint-fix
task lint
task testNotes
corev1.ResourceRequirementsusesresource.Quantitywhich serializes as strings in JSON (e.g.,"500m","64Mi"), so the user-facing YAML format is similar.- The JSON field names change from
cpu/memoryto a map structure — this is a breaking schema change acceptable forv1alpha1. - Ensure
corev1import exists in each file (it likely already does).
Generated with Claude Code