[lenamonj]

Description

An empty deviceListStrategy list in the config file is well-formed YAML, passes NewDeviceListStrategies, and UpdateFromCLIFlags leaves the non-nil pointer alone, so the envvar default never replaces it. AllCDIEnabled() is then vacuously true over the empty set, Allocate takes the all-CDI early return, and the container starts with no device list, no mounts, no CDI devices and no annotations: no GPU access, and no error anywhere.

NewDeviceListStrategies now rejects an empty set with an error naming the accepted strategies, which validateFlags surfaces as invalid --device-list-strategy option. AllCDIEnabled() returns false unless at least one CDI strategy is enabled, so a set with nothing enabled cannot take the all-CDI early return either; every set with at least one strategy enabled gets the same result as before.

Checklist

• No secrets, sensitive information, or unrelated changes
• Lint checks passing (make lint)
• Unit tests passing (make test)
• Go mod artifacts in-sync (make check-modules)
• Third-party notices in-sync (make check-third-party-notices)
• Test cases are added for new code paths

Testing

Three unit tests in api/config/v1/strategy_test.go: NewDeviceListStrategies over valid, unknown, empty and nil inputs, AllCDIEnabled() over a set with nothing enabled, and the config-file path, where NewConfig gives an omitted deviceListStrategy the envvar default and leaves an explicit empty list for NewDeviceListStrategies to reject. All three fail on main.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[ssalagam]

Thanks for the contribution. We will review the PR soon.

[abrarshivani]

@lenamonj Thanks for contributing this. The fix looks good overall. I left a few comments, PTAL.

[lenamonj]

d72e5c4 covers both. AllCDIEnabled() now returns false unless at least one CDI strategy is enabled, so a set with nothing enabled no longer takes the all-CDI early return; every set with at least one strategy enabled keeps its old result. TestDeviceListStrategyFromConfigFile builds the config through NewConfig with the device-list-strategy flag at its default: an omitted deviceListStrategy comes back as envvar, an explicit empty list survives to NewDeviceListStrategies and is rejected there, and the earlier test that unmarshalled the config directly is folded into the table.

[cdesiniotis]

@lenamonj thanks for the contribution. Can you squash your commits and make sure they are signed-off and cryptographically signed? Will trigger the GHAs after that. Thanks.

[abrarshivani]

LGTM

[lenamonj]

Squashed into one commit, 864cddc, signed off and signed. Its tree is identical to d72e5c4, the head both approvals were given on.

[abrarshivani]

/ok to test 864cddc