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

fixed #256. added runtimeClassName and fixed incorrect placement of nodeSelector #259

Conversation

stephanbertl
Copy link
Contributor

What this PR does / why we need it:

nodeSelector was incorrectly placed under the container. moved it to pod spec
added runtimeClassName to the pod spec to select specific GPU nodes.

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Verify the work you plan to merge addresses an existing issue (If not, open a new one) (required)
  • Check your branch with helm lint (required)
  • Update version in Chart.yaml according semver rules (required)
  • Substitute annotations section in Chart.yaml annotating implementations (useful for Artifecthub changelog) (required)
  • Update chart README using helm-docs (required)

Which issue(s) this PR fixes:

Fixes #256

Special notes for your reviewer:

nodeSelector was incorrectly placed under the container.
moved it to pod spec
added runtimeClassName to the pod spec to select specific GPU nodes.
@valeriano-manassero
Copy link
Contributor

Can you pls re-run helm-docs and update README.md ? CI is failing due to it. Ty.

@valeriano-manassero
Copy link
Contributor

CI still failing on helm-docs, this is really weird but it looks, there are some space/newline differences from helm-docs autogeneration script.

What version of helm-docs are you using to generate readme?

@stephanbertl
Copy link
Contributor Author

First tried helm-docs version 1.11.3 on windows
Now I am trying to use the same version on wsl. I am stuck here with windows currently.

@valeriano-manassero
Copy link
Contributor

Hang on, you are including in PR files from helm-docs also for other charts than just clearml-serving. I didn't noticed it immediately, pls remove these ones from PR and it should be good

@valeriano-manassero
Copy link
Contributor

Ok, I fixed them for you, let's see if CI si good now.

@valeriano-manassero valeriano-manassero merged commit fdbbe5b into clearml:main Nov 20, 2023
4 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.

[clearml-serving] Unable to specify runtimeClassName / nodeSelector for Triton Pod template
2 participants