/kind bug

Description

The allocation-rate shard policy throws away its own default CPU range during Initialize, so a scheduler configured with that policy and no arguments block ends up filtering on the range [0.0, 0.0]. Utilization is rounded to two decimals before the comparison, so only nodes that round to 0.00, meaning under half a percent, survive the filter. The shard is left with almost no nodes even though every node in the cluster is healthy and unassigned.

arguments is optional. It is omitempty in the config type, the policy README marks only name as required, and the README's own example omits arguments on the warmup entry. allocation-rate is also DefaultPolicyName, so it is the policy most people will reach for first.

The sibling policies do this differently, which is what makes it look like a slip rather than intent. warmup passes a pointer to the struct field that already holds its default from New(), so an absent key leaves that default in place, and node-limit is written the same way. allocation-rate reads into fresh local variables that start at zero, then assigns them over the struct unconditionally, so the 1.0 set in New() can never survive.

I found this by reading the code. I have not hit it on a live cluster. I did reproduce it against the controller code by feeding a ConfigMap through ParseShardingConfig and running ShardingManager.CalculateShardAssignments, which is the same path the controller uses. Output is below.

Steps to reproduce the issue

1. Configure the sharding ConfigMap with a policy chain that leaves out the optional arguments block:

   schedulerConfigs:
     - name: volcano
       type: volcano
       policies:
         - name: allocation-rate
           weight: 1

2. Have a few nodes in the cluster with normal, non-zero CPU utilization.

3. Look at what the volcano shard gets assigned, either in the NodeShard object or in the controller log line that reports the assignment.

Observed: the shard only picks up nodes at 0.00 utilization. In the run below it took 1 node out of 4.

Expected: with no range configured the policy should use the default range from New(), which is [0.0, 1.0], and all 4 nodes should be eligible.

Evidence and production path

Permalinks are pinned to 26f0052d4.

New() sets the intended defaults, maxCPURounded: 1.0 and cpuRangeRounded: 1.0:

Initialize then reads into local zero values and overwrites those fields, so the defaults never apply:

GetFloat64 only writes through the pointer when the key is present, which is what lets a caller pre-seed a default:

The two siblings use it the intended way, passing their own already-defaulted fields:

Nothing upstream stops the config. validatePolicies only requires a non-empty name and a non-negative weight, and it never looks at whether arguments is present:

applyPolicyDefaults only synthesizes a chain when Policies is empty, so an explicit chain is passed through untouched:

and initializePolicies calls builder() then Initialize(ref.Arguments) with whatever the user wrote, including nothing:

Driving that path with the ConfigMap above, four nodes at 0.00, 0.20, 0.50 and 0.90 utilization, and the real ShardingManager, the controller's own log line reports:

sharding_manager.go:164] Scheduler volcano assigned 1 nodes via [allocation-rate(w=1)]: [node-idle]

Adding the range explicitly, and changing nothing else:

        arguments:
          minCPUUtil: 0.0
          maxCPUUtil: 1.0

sharding_manager.go:164] Scheduler volcano assigned 4 nodes via [allocation-rate(w=1)]: [node-hot node-busy node-light node-idle]

So the same nodes and the same policy differ only by whether the optional block was written out.

For completeness, the internal state either side of Initialize with an empty arguments map:

after New():             min=0.00 max=1.00 range=1.00
after Initialize(empty): min=0.00 max=0.00 range=0.00

and the sibling policy under the same conditions keeps its default, scoring a default-labelled node 1.0 with no arguments given.

The behavior arrived with the pluggable policy pipeline in 288d37cff and has not been touched since.

The fix looks like it is just reading into the struct fields the way the other two policies do, keeping the existing validation and rounding. Happy to send a PR with a test for the no-arguments case if that is the direction you want, or to do it differently if you would rather have the defaults expressed somewhere else.

What version of Volcano are you using?

v1.15.0, and master at 26f0052. v1.15.0 is the first release that contains it.