enflame: resource name flags use the same variable

[Yashbhu]

I noticed that these two flags in pkg/device/enflame/config.go are using the same variable:

--enflame-drs-gcu-resource-name
--enflame-vgcu-resource-name

Both are bound to EnflameResourceNameDRSGCU.

So if both flags are passed, the last one wins. tor example, this:

--enflame-drs-gcu-resource-name=custom-a --enflame-vgcu-resource-name=custom-b

gives custom-b, while reversing them gives custom-a.

This is because both flags are registered on the same FlagSet and Go's flag package doesn't care that they're pointing to the same variable.

This could be an issue for users migrating from the old VGCU flag since the final resource name can change just based on argument order.

I also noticed that:

--enflame-vgcu-legacy-resource-name
--enflame-vgcu-percentage-resource-name

seem to be unused. They're assigned in InitEnflameDevice, but I couldn't find anywhere reading those values afterwards.

I think we should define which flag takes precedence here and add a test for it. The unused flags probably also need to be checked to see if they're still needed.

No Enflame hardware should be needed since this can be tested at the config/flag level.

[Yashbhu]

\assign i want to work on this issue

[moezdil]

\assign i want to work on this issue

hi, did you start? is it still an issue, have you tested?

[Ggrryta]

Hi @Yashbhu, are you still working on this? If not I'd like to take it over.

/assign

I had a look at config.go, both flags still point to EnflameResourceNameDRSGCU.
My plan is to give the legacy flag its own variable and add a test for it.

[moezdil]

Hi @Yashbhu, are you still working on this? If not I'd like to take it over.

/assign

I had a look at config.go, both flags still point to EnflameResourceNameDRSGCU. My plan is to give the legacy flag its own variable and add a test for it.

is it still an issue? have you tested it?

[Ggrryta]

Yes, still an issue on master. Confirmed with tests:

1. Flag order decides the resource name:

--enflame-drs-gcu-resource-name=custom-a --enflame-vgcu-resource-name=custom-b
-> EnflameResourceNameDRSGCU = "custom-b"
--enflame-vgcu-resource-name=custom-b --enflame-drs-gcu-resource-name=custom-a
-> EnflameResourceNameDRSGCU = "custom-a"

2. Downstream impact: a pod requesting the legacy name is silently skipped.
   With EnflameResourceNameDRSGCU = "enflame.com/drs-gcu" and a container
   requesting enflame.com/vgcu: 2:

MutateAdmission -> (false, nil) // no error, no indication of the mismatch
GenerateResourceRequests -> Nums=0

Same container with the name matching -> mutated=true, Nums=1, Memreq=2.
So the pod stays unmanaged — nothing in the logs points at the name
mismatch — and it can only schedule if the node happens to advertise
the requested name, which it won't in the migration case.
The yaml path (InitEnflameDevice) has a deterministic fallback
(drs-gcu > vgcu > default); only the flag path is order-dependent.

I'll send the PR with the fix (separate variable for the legacy flag, same
fallback order as the yaml path) and include both tests.

[Ggrryta]

One correction to my earlier comment: I traced the startup path while
writing the fix. In the scheduler itself these flag values never survive —
cobra parses them first, then start() -> InitDevices() -> InitEnflameDevice()
overwrites the variable from the yaml config, which helm always sets. So the
"pod gets skipped" scenario is not reachable via flag order in a standard
scheduler deployment.

The double binding is still a real bug for anything that parses the flags
and reads EnflameResourceNameDRSGCU directly, and either way the flags were
completely dead in-tree before (yaml or hard default always won). The fix
binds the legacy flag to its own variable and lets the flags act as a
fallback when yaml is silent.

[moezdil]

bcs of inactivity of issue owner it was closed.
it can be re-open if needed.