stacklok / stacklok/toolhive

Replace custom ResourceList with corev1.ResourceRequirements

Open
#4,546 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

api breaking-change go kubernetes operator tech-debt
Dominant language
Go
Stars
2.2k
Forks
300
Avg merge
1d 15h
Merged PRs (30d)
184

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.Quantity validation 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 test

Notes

  • corev1.ResourceRequirements uses resource.Quantity which serializes as strings in JSON (e.g., "500m", "64Mi"), so the user-facing YAML format is similar.
  • The JSON field names change from cpu/memory to a map structure — this is a breaking schema change acceptable for v1alpha1.
  • Ensure corev1 import exists in each file (it likely already does).

Generated with Claude Code

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the resource fields and custom type definitions in cmd/thv-operator/api/v1alpha1/mcpserver_types.go, mcpremoteproxy_types.go, and embeddingserver_types.go, then search cmd/thv-operator/controllers/ for BuildResourceRequirements callers. Review cmd/thv-operator/pkg/controllerutil/resources.go and update affected tests using corev1.ResourceRequirements and resource.MustParse(). Run task gen, task crdref-gen, task lint-fix, task lint, and task test; done means generated files and all tests pass without the conversion function.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, kubernetes
Domain
infrastructure
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.