Enable Nvidia NIM as an application #2959
Conversation
openshift-ci
bot
added
the
needs-ok-to-test
|
Hi @xieshenzh. Thanks for your PR. I'm waiting for a opendatahub-io member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@andrewballantyne Please take a look this PR which adds NIM as application. Thanks. |
| @@ -12,3 +12,4 @@ resources: | |||
| - ./pachyderm | |||
| - ./watson-x | |||
| - ./rhoai | |||
| - ./nvidia-nim | |||
There was a problem hiding this comment.
Do we know if this is a partner connection? I'd imagine this is only for RHOAI and not for ODH by default 🤔
If so, is it for Managed or Self Managed or both?
We are reworking our manifest folder but currently it's hard to navigate. Answers to these questions will help where to put it. I'd imagine though, you want to find the anaconda stuff and place this effort next to that.
There was a problem hiding this comment.
On a side note, we may want to look at not directly including it (in the kustomization of where you want to deploy it) and manually installing it for the short term while we work out everything else.
We don't have a flag for disabling manifest files unfortunately. But we could build an overlay that includes it and you could use the DSC devFlags 🤔 Give me a moment to see if I can pen that together.
There was a problem hiding this comment.
@mpaulgreen Could you please answer Andrew's questions? Thanks.
There was a problem hiding this comment.
@xieshenzh Please reach out to PM for this. I think it will be Adam. Create this query in Pending Questions doc.
There was a problem hiding this comment.
Okay, so we just reworked the manifest files directory to make it easier to maintain (you got some conflicts) -- Do we have an answer on this front yet where PM wants this inclusion?
There was a problem hiding this comment.
@andrewballantyne I have rebased.
We haven't got the answers to the questions in this PR.
But it was confirmed earlier by Adam: NIM will be a base offering, not an add-on.
|
/ok-to-test |
openshift-ci
bot
added
ok-to-test
|
You have conflicts, @xieshenzh. Please rebase (not merge) -- we want to get down to 1 commit when we are done and it will be easier in the long run if you rebase. |
xieshenzh
force-pushed
the
nim-app-1
branch
4 times, most recently
from
8684e4a
to
227c920
Compare
xieshenzh
force-pushed
the
nim-app-1
branch
from
227c920
to
108b588
Compare
There was a problem hiding this comment.
You'll need to add this next to the anaconda-ce-validator-cron.yaml in the ./kustomization.yaml otherwise it won't be deployed to the cluster.
There was a problem hiding this comment.
Sure, I've added the yamls to the kustomization files.
Previously, the lines were removed to unblock the PR: #2959 (comment)
xieshenzh
force-pushed
the
nim-app-1
branch
from
108b588
to
86482d0
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andrewballantyne The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andrewballantyne The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
xieshenzh
force-pushed
the
nim-app-1
branch
from
25485c5
to
86482d0
Compare
|
/retest |
|
/test images |
|
Miscommunication with @xieshenzh -- he will merge his other PRs together (and not with this one)... approving again based on my approval message. |
xieshenzh
force-pushed
the
nim-app-1
branch
from
86482d0
to
7c548a4
Compare
xieshenzh
force-pushed
the
nim-app-1
branch
from
7c548a4
to
3dab16e
Compare
|
/lgtm @xieshenzh lets not do anymore updates and get this in. Seems it missed the boat on Thursday. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2959 +/- ##
==========================================
+ Coverage 85.26% 85.32% +0.05%
==========================================
Files 1270 1270
Lines 27900 27900
Branches 7422 7422
==========================================
+ Hits 23790 23805 +15
+ Misses 4110 4095 -15 see 6 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Description
Create an OdhApplication CR to add a tile for the Nvidia NIM application.
Support enabling the NIM application with a Nvidia ngc api key.
Create a CronJob to validate the api key and enable the NIM application.
How Has This Been Tested?
Negative scenario:
Positive scenario:
Test Impact
No impact on existing code
Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
main