Skip to content

Scheduler exits on a wrong-typed plugin argument value: framework.Get uses klog.Fatalf #5749

Description

@bhuvan-somisetty

Description

framework.Get[T] calls klog.Fatalf when a plugin argument in the scheduler ConfigMap cannot be decoded into the expected type. Since klog.Fatalf terminates the process, a single mistyped argument value in volcano-scheduler-configmap kills the running scheduler and keeps it in CrashLoopBackOff until the ConfigMap is fixed. Nothing else is scheduled in the cluster meanwhile.

This contradicts two behaviours the scheduler already has:

  1. Config reload is designed to be non-fatal. When the ConfigMap cannot be read or unmarshalled, loadSchedulerConf logs an error and keeps the previously loaded configuration instead of exiting (scheduler.go#L186-L204). A bad argument value bypasses that safety net, because the YAML itself is valid — arguments are map[string]interface{}, so the type error only surfaces later, when plugins are built.
  2. Every other argument getter tolerates a bad value. GetInt, GetFloat64, GetBool and GetString log a warning and leave the caller's default in place (arguments.go#L37-L127). Only the generic Get[T] is fatal (arguments.go#L134-L146):
err := mapstructure.Decode(argv, &result)
if err != nil {
    klog.Fatalf("Could not parse argument for key %s to type %T: %v", key, result, err)
}

So binpack.weight: "10" only produces a warning, while resourceStrategyFitWeight: "10" takes the scheduler down. Same class of mistake, very different blast radius.

The callers are already written to cope with a failed lookup — calculateWeight in resource-strategy-fit falls back to DefaultResourceStrategyFitPluginWeight when Get returns false — but they never get the chance, because the helper exits the process first.

The arguments currently read through Get[T]:

Steps to reproduce the issue

  1. Run a cluster with a healthy volcano-scheduler and the resource-strategy-fit plugin enabled.
  2. Edit volcano-scheduler-configmap and quote the weight — a very common habit in ConfigMaps, and the way many other Kubernetes components accept numbers:
actions: "enqueue, allocate, backfill"
tiers:
- plugins:
  - name: priority
  - name: gang
- plugins:
  - name: resource-strategy-fit
    arguments:
      resourceStrategyFitWeight: "10"     # quoted on purpose
      resources:
        nvidia.com/gpu:
          type: MostAllocated
          weight: 2
  1. Wait for kubelet to project the updated ConfigMap into the scheduler pod. The file watcher reloads the config, and the next scheduling cycle (default every 1s) rebuilds the plugins and the process exits.

The extender variant is reachable the same way, and is easier to hit by accident because extender.managedResources expects a YAML list while the neighbouring binpack.resources documents a comma-separated string:

  - name: extender
    arguments:
      extender.urlPrefix: http://127.0.0.1:8081
      extender.managedResources: nvidia.com/gpu, nvidia.com/gpumem   # binpack-style, not a list

Evidence and production path

I found this by reading the code, and I have reproduced the decode failure that triggers the fatal path, not the pod restart in a live cluster. The full path from the user action to process exit:

  1. The user edits volcano-scheduler-configmap. watchSchedulerConf picks up the file change and calls loadSchedulerConf (scheduler.go#L227-L230).
  2. UnmarshalSchedulerConf succeeds — Arguments is map[string]interface{}, so "10" is a perfectly valid value at this stage. The new tiers are stored under pc.mutex (scheduler.go#L196-L208). The graceful "keep previous configuration" fallback is therefore never reached.
  3. runOnce calls framework.OpenSession, which rebuilds every plugin from its arguments on every scheduling cycle (framework.go#L42-L54).
  4. The plugin builder calls framework.Get[int](args, "resourceStrategyFitWeight"), mapstructure.Decode returns an error, and klog.Fatalf exits the process (status 255). The pod restarts, reads the same ConfigMap, and dies again.

mapstructure v1.5.0 is used with the default (non weakly-typed) config, so all of these ordinary-looking values fail to decode. Output from a standalone program against github.com/mitchellh/mapstructure v1.5.0, the version in go.mod:

int<-string:     '' expected type 'int', got unconvertible type 'string', value: '10'
float64<-string: '' expected type 'float64', got unconvertible type 'string', value: '0.5'
map<-string:     '' expected a map, got 'string'
slice<-string:   '': source data must be an array or slice, got string

To be clear about the scope: the documented examples decode correctly. I checked that the sra/proportional/resources blocks from the resource-strategy-fit doc comment decode cleanly even though gopkg.in/yaml.v2 hands over map[interface{}]interface{}, so this is not a problem with the documented configuration — only with a value whose type the plugin does not expect.

A secondary, quieter problem in the same helper: mapstructure.Decode(1.5, &i) succeeds and yields 1, so volumebinding.weight: 1.5 is silently truncated with no warning at all.

Describe the results you received and expected

Received: the scheduler process exits on a mistyped plugin argument value, and stays in CrashLoopBackOff while the bad ConfigMap is in place. Cluster-wide scheduling stops.

Expected: a bad argument value should never terminate the scheduler. Get[T] should behave like the other getters in the same file — log a warning, return (zero, false) so the caller keeps its default — and scheduling should continue with the last good/default value.

Suggested fix, which I would like to work on:

  • Replace klog.Fatalf in Get[T] with klog.Warningf (or klog.ErrorS) and return result, false, matching GetInt/GetBool/GetString/GetFloat64.
  • Fix the callers that currently discard the boolean (proportional, sra at resource_strategy_fit.go#L176 and #L188) so a failed decode falls back to the documented default instead of a zero-valued config.
  • Add unit tests in pkg/scheduler/framework/arguments_test.go covering a wrong-typed value for each Get[T] shape (scalar, slice, map, struct), asserting no exit and that defaults survive.
  • Optionally reject the argument at config-load time in UnmarshalSchedulerConf instead, so the existing "keep previous configuration" path handles it and the operator gets one clear error. Happy to go this way if maintainers prefer validation over lenient defaults — it is a larger change, since argument types are only known to the plugins.

I can send a PR for this if the direction looks right.

What version of Volcano are you using?

Present on current master (2a5d61c). The fatal path was introduced in f2b4f66 and is in every release since v1.12.0, up to and including v1.14.0.

Any other relevant information

Not version specific — it is reachable on any Kubernetes version, with any of the plugins listed above enabled, as soon as a plugin argument value has a type the plugin does not expect.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions