Skip to content

Add option to set desired GPU count #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 6, 2025
Merged

Conversation

hlts2
Copy link
Member

@hlts2 hlts2 commented Mar 6, 2025

This PR modifies the application to allow setting the desired GPU count via an functional option.
Since having a variable in the New function about gpu count makes it look like a required parameter, it is better to set it via an option instead.

Summary

If the desired GPU count is not set via environment variables, it defaults to 0.
When the desired GPU count is 0, GPU count checking is skipped.
This enables node-agent to run in both GPU and non-GPU environments.

Behavior

  • GPU environments:

    • The existing two conditions for checks remain unchanged:
      • The node enters the NotReady state.
      • The number of available GPUs per node falls below a configured threshold.
  • Non-GPU environments:

    • GPU count checking is skipped.
    • Only the NotReady state condition is evaluated.

This enhancement improves flexibility by supporting both GPU-equipped and non-GPU environments seamlessly.

This is not urgent, so it's fine to include it with the next release.

@hlts2 hlts2 marked this pull request as ready for review March 6, 2025 05:23
@hlts2 hlts2 self-assigned this Mar 6, 2025
@hlts2 hlts2 requested review from johndietz and jokestax March 6, 2025 05:23
@hlts2
Copy link
Member Author

hlts2 commented Mar 6, 2025

@johndietz Thank you for your review🙏I will merge this PR🚀

@hlts2 hlts2 merged commit 40fbb90 into main Mar 6, 2025
1 check passed
@hlts2 hlts2 deleted the fix/add-option-gpu-count branch March 6, 2025 18:50
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