cubemaster: log score-plugin failures instead of silently skipping them

[dwin-gharibi]

Closes #1456.

Motivation

runScoreFilter swallowed score-plugin errors with a bare continue. A plugin failing on every call
looked exactly like a disabled one, and because totalPluginWeight only accumulates for plugins that
returned results, a persistently failing plugin silently drops out of the weighted average and skews
placement — with nothing in the log to explain it.

The filter stage already reports its failures (schedule.go:127), so this was an inconsistency
between the two stages.

What this changes

CubeMaster/pkg/scheduler/schedule.go — one line added to the existing error branch:

if tmpResult, err := f.Select(selCtx); err != nil {
    log.G(selCtx.Ctx).Warnf("runScoreFilter_skipped, selector: %s, err: %v", f.ID(), err)
    continue
} else {

f.ID() is part of the score.Selector interface (pkg/selector/score/init.go:21), so the plugin is
named in the log. The message shape follows the existing runFilter_failed convention. Warn level
matches runFilter.

Scoring behaviour is unchanged — this is observability only.

No comment changes.

Testing

No new test: the change adds a log line inside an existing branch and alters no logic, so there is no
new behaviour to pin. Existing coverage for the scoring path continues to pass.

$ docker run --rm ... -w /w/CubeMaster golang:1.26 go test -short ./pkg/scheduler/... ./pkg/selector/...
ok  .../pkg/scheduler          ok  .../pkg/scheduler/selctx
ok  .../pkg/selector/filter    ok  .../pkg/selector/score

CI gates checked locally:

• gofmt -l ./pkg/scheduler — clean (fmt-check).
• GOOS=linux go build ./pkg/scheduler/ — clean.

Related, not fixed here

parallelRunFilters discards the context returned by errgroup.WithContext
(schedule.go:139), so filter goroutines never observe cancellation when a sibling fails. Left out to
keep this PR to one concern; tracked with the scheduler filter-intersection issue.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[cubesandboxbot]

Non-blocking observations on this log line:

1. It only covers the err != nil path, but several built-in scorers return empty-success rather than an error on their silent paths — e.g. realTimeWeightedAverageScore.Select returns (nil, nil) at realtimescore.go:62-65 and :74-76, and affinityScore.Select returns an empty list at affinityscore.go:54-56. Those still look identical to a disabled plugin, which is the exact gap the PR motivation describes. Empty-success staying silent is probably intentional (no result isn't a failure), but it may be worth a brief comment or doc note so future readers don't grep runScoreFilter_skipped expecting to see them.
2. Since Disable() skips at line 187 without logging, _skipped could be misread as covering disabled plugins. runScoreFilter_failed would match the existing runFilter_failed shape more literally — though _skipped is arguably more accurate for a per-plugin skip. Just a naming nit, no change required.

The change itself is correct and the message shape/level are consistent with the codebase.

[cubesandboxbot]

Review: cubemaster: log score-plugin failures instead of silently skipping them (#1457)

Overall: Approve — correct, minimal, well-scoped observability change. No correctness issues found.

The PR adds a single Warnf log line to the err != nil branch of runScoreFilter in CubeMaster/pkg/scheduler/schedule.go, so a score plugin that fails no longer drops out of the weighted average silently. I verified every claim in the description against the base tree:

• f.ID() exists on the score.Selector interface (CubeMaster/pkg/selector/score/init.go:18-26) and is implemented by every built-in scorer, so the plugin is named in the log. ✅
• log.G(selCtx.Ctx).Warnf(...) matches the existing runFilter_failed pattern at schedule.go:127; the message shape and Warn level are consistent. ✅
• The PR's line references are accurate: schedule.go:127 (filter stage failure log) and schedule.go:139 (discarded errgroup.WithContext context). ✅
• Diff applies cleanly to the base; no import changes needed; types for %s/%v are correct; gofmt-clean. Compiles. ✅

Scoring behaviour is genuinely unchanged — this is observability only, as stated.

Non-blocking observations

1. The stated motivation is only partially closed by the error-path log. Several built-in scorers return empty-success rather than an error on their silent paths — realTimeWeightedAverageScore.Select returns (nil, nil) at realtimescore.go:62-65 and :74-76, and affinityScore.Select returns an empty list at affinityscore.go:54-56. Those still look exactly like a disabled plugin and remain unlogged. This is probably intentional (no result ≠ failure), but worth a one-line code comment so future readers don't grep runScoreFilter_skipped expecting coverage.
2. Naming nit. runScoreFilter_skipped vs. the existing runFilter_failed convention; _skipped could also be misread as covering the Disable() skip (line 187), which is not logged. Cosmetic only.
3. Log-per-attempt volume. A persistently failing plugin now emits one Warn per scheduling attempt. That is the point of the change, and Warn is the right level — just noting that at high scheduling churn the volume is worth being aware of.

On the test decision

No new test for a logging-only change is reasonable; existing scoring-path coverage is unaffected.

AI-generated review — not a human approval.