Skip to content

Conversation

@elezar
Copy link
Member

@elezar elezar commented Oct 21, 2025

Ideally we would also switch away from using the nvidia-ctk CLI to create device nodes as part of this PR too.

@elezar elezar force-pushed the migrate-to-nvcdi-api branch 2 times, most recently from 52b9a64 to f9c0907 Compare October 21, 2025 18:07
// TODO: Instead of abusing CLI command we generate here, we should expose
// this API and use that instead. This would require refactoring in the
// toolkit.
cmd := devicenodes.NewCommand(log.StandardLogger())
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #262 for a follow-up.

@elezar elezar force-pushed the migrate-to-nvcdi-api branch from 8154723 to 3feb86b Compare October 21, 2025 18:42
}

return rcfg.Run()
return rcfg.Run(ctx)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we already ctx in the rcfg, err := reconfigure.New(ctx, clientset, migPartedBinary, opts) line. Can we access the context from the struct itself?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. You're right. I can use the context member.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@elezar elezar force-pushed the migrate-to-nvcdi-api branch from a1ee413 to 888a600 Compare October 22, 2025 08:06
Signed-off-by: Evan Lezar <[email protected]>
This change switches from a direct invocation of the nvidia-ctk cdi generate
command to using the nvcdi API. This matches what is done in the toolkit
container to generate a CDI spec for management devices.

Signed-off-by: Evan Lezar <[email protected]>
@elezar elezar force-pushed the migrate-to-nvcdi-api branch from 888a600 to b29c4d2 Compare October 22, 2025 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants