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

Add support for v1beta1 device plugin API. #55

Merged
merged 2 commits into from
Feb 23, 2018

Conversation

jiayingz
Copy link
Collaborator

@jiayingz jiayingz commented Feb 17, 2018

Other than unit tests, also ran GPUDevicePlugin e2e test with and without PR kubernetes/kubernetes#58282 to make sure the gke gpu device plugin works with both v1alpha kubelet and v1beta1 kubelet.

Makefile Outdated

build:
go install ${REPO}/pkg/gpu/nvidia
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need to install

Dockerfile Outdated
@@ -15,6 +15,7 @@
FROM golang:1.9-alpine as builder
WORKDIR /go/src/github.com/GoogleCloudPlatform/container-engine-accelerators
COPY . .
RUN go install github.com/GoogleCloudPlatform/container-engine-accelerators/pkg/gpu/nvidia
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this necessary?

@vishh
Copy link
Collaborator

vishh commented Feb 22, 2018

Fix the use_alpha logic as discussed offline and then this is good to be merged. Feel free to self-merge.

@jiayingz jiayingz force-pushed the v1beta1 branch 3 times, most recently from f24568e to 7a0fe87 Compare February 22, 2018 23:45
)

var (
hostPathPrefix = flag.String("host-path", "/home/kubernetes/bin/nvidia", "Path on the host that contains nvidia libraries. This will be mounted inside the container as '-container-path'")
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Flags should be in injected to libraries.

@vishh
Copy link
Collaborator

vishh commented Feb 23, 2018

LGTM

@jiayingz
Copy link
Collaborator Author

Thanks a lot for the quick review!

@jiayingz jiayingz merged commit 70a1113 into GoogleCloudPlatform:master Feb 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants