Skip to content
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

Remove unnused setting in daemon claim config #274

Merged
merged 1 commit into from
Mar 10, 2025

Conversation

klueska
Copy link
Collaborator

@klueska klueska commented Mar 10, 2025

The compute domain Daemon no longer needs to store it's own reference to the NumNodes in the compute domain. The NumNodes field of the domain itself is queried via the compute domain ID.

Copy link

copy-pr-bot bot commented Mar 10, 2025

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.

@klueska
Copy link
Collaborator Author

klueska commented Mar 10, 2025

/ok to test

@ArangoGutierrez ArangoGutierrez requested a review from Copilot March 10, 2025 10:01

Choose a reason for hiding this comment

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

PR Overview

This PR removes the unused "numNodes" setting from the compute domain daemon configuration. Key changes include:

  • Removing the numNodes parameter from the YAML template.
  • Deleting the line that sets the numNodes field in the resource claim template creation.
  • Eliminating the numNodes field and its associated validation from the daemon configuration API.

Reviewed Changes

File Description
templates/compute-domain-daemon-claim-template.tmpl.yaml Removed the numNodes parameter from the template
cmd/compute-domain-controller/resourceclaimtemplate.go Removed the assignment of daemonConfig.NumNodes
api/nvidia.com/resource/v1beta1/computedomainconfig.go Removed the NumNodes field and validation logic

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

Copy link
Contributor

@jgehrcke jgehrcke left a comment

Choose a reason for hiding this comment

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

Thanks. I suppose we want that in the release, yes? Should we work with a milestone to express clear intent for release management? (edit: created a milestone)

@jgehrcke jgehrcke added this to the v25.3.0 milestone Mar 10, 2025
@klueska klueska merged commit cdbd444 into NVIDIA:main Mar 10, 2025
13 checks passed
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.

3 participants