Add default-resource-class-name config option

[gempesaw]

Background

We really like using resource_classes from #668 to be able to vary CPU/memory requests based off a single agent template, but currently if a pipeline step doesn't specify a resource_class: foo, they don't get any resource requests at all. I was hoping perhaps specifying resources in pod-spec-patch would be the fallback/default in that case, but pod-spec-patch's resources override anything from resource_class:

applyResourceClass happens around 524:

agent-stack-k8s/internal/controller/scheduler/scheduler.go

Lines 524 to 527 in 4beedd0

	// Handle resource_class tag
	if err := w.applyResourceClass(podSpec, tags); err != nil {
	return nil, err
	}

and then later on the pod-spec-patch and plugin spec patches are applied on top:

agent-stack-k8s/internal/controller/scheduler/scheduler.go

Lines 739 to 742 in 4beedd0

	// Allow podSpec to be overridden by the controller configuration and the k8s plugin
	// Patch from the controller is applied first
	if w.cfg.PodSpecPatch != nil {
	patched, err := PatchPodSpec(podSpec, w.cfg.PodSpecPatch, w.cfg.DefaultCommandParams, inputs.k8sPlugin, w.cfg.AllowPodSpecPatchUnsafeCmdMod)

agent-stack-k8s/internal/controller/scheduler/scheduler.go

Lines 753 to 755 in 4beedd0

	// If present, patch from the k8s plugin is applied second.
	if inputs.k8sPlugin != nil && inputs.k8sPlugin.PodSpecPatch != nil {
	patched, err := PatchPodSpec(podSpec, inputs.k8sPlugin.PodSpecPatch, w.cfg.DefaultCommandParams, inputs.k8sPlugin, w.cfg.AllowPodSpecPatchUnsafeCmdMod)

it'd be nice to have a default that applies when a step doesn't remember to specify a resource_class

Changes

Adds a new default-resource-class-name config option that references a named resource class from resource-classes. When a job doesn't have a resource_class agent tag, the default kicks in; jobs that explicitly set resource_class still take priority.

usage looks like:

resource-classes:
  small:
    resource:
      requests:
        cpu: "500m"
        memory: "512Mi"
  large:
    resource:
      requests:
        cpu: "2"
        memory: "4Gi"

default-resource-class-name: "small"

I also added resource-classes to the helm schema since it seemed to be missing?

Testing

• Unit tests for applyResourceClass covering the various combinations (no tag/no default, tag specified, default fallback, tag overrides default, error cases)
• Integration test that verifies the default resource class actually gets applied by checking cgroup limits

[moskyb]

this looks really good! i've been wanting to add this feature myself ever since we added resource classes, so thank you!

one note: it might be worth adding a validation in controller.ParseAndValidateconfig to ensure that the default resource class is included in the list of resource classes provided, as it would significantly shorten the loop on fixing errors like missspelling the default resource class or similar.

[gempesaw]

@moskyb

adding a validation in controller.ParseAndValidateconfig to ensure that the default resource class is included in the list of resource classes provided

oo yea that's a great idea, tyvm b6087f9

[moskyb]

looks great! thanks so much @gempesaw