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
Port deviceManager to windows container manager to enable GPU access #80917
Conversation
Welcome @aarnaud! |
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
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/test-infra repository. I understand the commands that are listed here. |
Hi @aarnaud. Thanks for your PR. I'm waiting for a kubernetes 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/test-infra repository. |
01edb84
to
4ce09a7
Compare
@derekwaynecarr @sjenning @RenaudWasTaken |
I hope this PR would be merge. @thomacos Until merge you can find some build here : https://github.com/aarnaud/k8s-directx-device-plugin/releases |
That's great! Nevertheless, let's hope this PR finds its way into the official releases, too. |
Also depends of your windows version you have to edit
There is a lite issue for Linux Socket on Windows Tested on windows server 2019 with docker 19.03 |
Works like charm! |
hi, this PR is ok for you? when can i expect to be merged |
/cc |
@yujuhong or @dchen1107 can you please approve? |
Would sending a candy box to @yujuhong and @dchen1107 help getting their attention? |
@klueska |
The PR itself is pretty straightforward and isolated to a windows-specific file. My only hesitations in approving are therefore:
From @ddebroy 's comment above is seems that people are generally OK with the lack of tests so long as #88554 is worked on soon. I am OK with this too. Can someone from |
hi @klueska , sig-windows approves this. Deep Debroy is the TL for sig-windows and i am the co-chair for sig-windows |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aarnaud, klueska, michmike, thomacos 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 |
/retest Review the full test history for this PR. Silence the bot with an |
@klueska Thank you for the quick action! |
@aarnaud: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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/test-infra repository. I understand the commands that are listed here. |
/retest Review the full test history for this PR. Silence the bot with an |
Opened #93263 with a revert of this since it fails windows kubelet startup |
revert is merged, please verify azure CI jobs pass as part of reintroducing this |
So it seems to me that going back to the separate default paths for Windows is the natural right choice:
So what are the next steps? |
/reopen |
@thomacos: You can't reopen an issue/PR unless you authored it or you are a collaborator. In response to this:
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/test-infra repository. |
@aarnaud, what do you think about the following? Obviously this PR won't get into 1.19 anymore. But Code Thaw is today. So I would suggest to aim for getting the fixed version approved and merged as soon as possible before it gets forgotten again. |
@aarnaud: Failed to re-open PR: state cannot be changed. The pull request cannot be reopened. In response to this:
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/test-infra repository. |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Port the devicemanager to Windows node to allow device plugins.
An important device plugin (not part of this PR) is the directx device plugin for GPU access: https://github.com/aarnaud/k8s-directx-device-plugin
This is required for machine learning, high-performance computing and computer graphics under Windows using the GPU.
Does this PR introduce a user-facing change?: