feature: support nvidia-container 2.0 to enable GPU access #2029

CodeJuan · 2018-07-31T11:15:25Z

Ⅰ. Describe what this PR did

Make pouch natively support GPU devices

Ⅱ. Does this pull request fix one issue?

fixes #824 #2033

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Prerequisites:

An instance with a NVIDIA GPU
The appropriate GPU driver is installed

Create container with NvidiaConfig.

curl --unix-socket /run/pouchd.sock -X POST http:/containers/create -H "Content-Type: application/json" -d '{"Image": "centos:7", "HostConfig":{"Devices": [],"NvidiaConfig":{"NvidiaDriverCapabilities":"all","NvidiaVisibleDevices":"0"}},"Cmd": ["ping","127.0.0.1"]}'

Start container.
Exec nvidia-smi in container.
If nvidia-smi return error NVIDIA-SMI couldn't find libnvidia-ml.so library in your system. Please make sure that the NVIDIA Display Driver is properly installed and present in your system, then add a link to libnvidia-ml.so

ln -s  /usr/lib64/libnvidia-cfg.so.1 /usr/lib64/libnvidia-cfg.so
ln -s /usr/lib64/libnvidia-cfg.so.387.26 /usr/lib64/libnvidia-cfg.so.1
ln -s  /usr/lib64/libnvidia-ml.so.1 /usr/lib64/libnvidia-ml.so
ln -s /usr/lib64/libnvidia-ml.so.387.26 /usr/lib64/libnvidia-ml.so.1

Ⅴ. Special notes for reviews

codecov-io · 2018-08-01T02:22:52Z

Codecov Report

Merging #2029 into master will increase coverage by 5.27%.
The diff coverage is 62.9%.

@@            Coverage Diff             @@
##           master    #2029      +/-   ##
==========================================
+ Coverage   58.64%   63.91%   +5.27%     
==========================================
  Files         206      206              
  Lines       15840    15902      +62     
==========================================
+ Hits         9289    10164     +875     
+ Misses       5413     4468     -945     
- Partials     1138     1270     +132

Flag	Coverage Δ
#criv1alpha1test	`33.17% <17.74%> (-0.03%)`	⬇️
#criv1alpha2test	`33.77% <17.74%> (?)`
#integrationtest	`37.93% <35.48%> (-0.01%)`	⬇️
#unittest	`23.84% <48.38%> (+0.11%)`	⬆️

Impacted Files	Coverage Δ
daemon/mgr/spec_hook.go	`24.56% <26.66%> (+0.75%)`	⬆️
daemon/mgr/spec_process.go	`65.65% <44.44%> (-2.13%)`	⬇️
daemon/mgr/container_validation.go	`43.13% <81.57%> (+12.7%)`	⬆️
daemon/logger/jsonfile/utils.go	`71.54% <0%> (-1.63%)`	⬇️
ctrd/image.go	`77.29% <0%> (+2.18%)`	⬆️
cri/stream/runtime.go	`68.23% <0%> (+5.88%)`	⬆️
cri/stream/remotecommand/httpstream.go	`44.04% <0%> (+9.32%)`	⬆️
cri/criservice.go	`64.7% <0%> (+24.99%)`	⬆️
cri/v1alpha2/cri_utils.go	`82.75% <0%> (+25.94%)`	⬆️
cri/v1alpha2/cri_wrapper.go	`53.87% <0%> (+53.87%)`	⬆️
... and 5 more

allencloud · 2018-08-01T06:04:00Z

daemon/mgr/container_utils.go

+		return nil
+	}
+
+	if err := vlidateNvidiaDriver(r); err != nil {


s/validate/vlidate?

allencloud · 2018-08-01T06:05:08Z

daemon/mgr/container_utils.go

+	return nil
+}
+
+func stringInSlice(a string, list []string) bool {


You do not need to add this function, this has been covered in utils.StringInSlice.

allencloud · 2018-08-01T06:06:46Z

daemon/mgr/spec_hook.go

+		Path: path,
+		Args: append(args, "prestart"),
+	}
+	spec.s.Hooks.Prestart = append(spec.s.Hooks.Prestart, nvidiaPrestart)


If we already have a prestart hook, will the nvidia prestart take effect here? Just double check this.

Sure, prestart commands are executed after the container namespaces are created.

allencloud · 2018-08-01T06:09:06Z

How to use this? Do we need to support this in pouch CLI?
I only see this is supported in API daemon side. @CodeJuan

CodeJuan · 2018-08-02T02:29:37Z

@allencloud
#2033
I will do it by next Monday.

fuweid · 2018-08-07T05:46:07Z

daemon/mgr/container_utils.go

@@ -5,14 +5,14 @@ import (
 	"strconv"
 	"strings"

+	"github.com/opencontainers/selinux/go-selinux/label"


move the third party to next to the pouch group.

fuweid · 2018-08-07T05:47:09Z

daemon/mgr/container_validation.go


 	"github.com/alibaba/pouch/apis/types"
 	"github.com/alibaba/pouch/daemon/logger/syslog"
 	"github.com/alibaba/pouch/pkg/system"
-
-	"github.com/sirupsen/logrus"
+	"github.com/alibaba/pouch/pkg/utils"


in pouch, we use three groups to manage the packages like:

"os" "strconv" "strings" "github.com/alibaba/pouch/apis/types" "github.com/alibaba/pouch/daemon/logger/syslog" "github.com/alibaba/pouch/pkg/system" "github.com/alibaba/pouch/pkg/utils" "github.com/sirupsen/logrus"

fuweid

Please add UT for the validation part.

fuweid · 2018-08-08T05:54:25Z

daemon/mgr/container_validation.go

+		return nil
+	}
+
+	supportedDrivers := []string{"compute", "compat32", "graphics", "utility", "video", "display"}


could we use map global var to hold the supportedDrivers, like

var supportedNvidiaDrivers = map[string]struct{}{ "compute": struct{}, .... }

therefore, we don't need to allocate new memory in the utils.StringInSlice.

fuweid

LGTM, I will merge after rebase the master

fuweid · 2018-08-10T01:42:56Z

hack/package/rpm/build.sh

@@ -69,6 +74,24 @@ function build_pouch()
    popd
 }

+# install nvidia-container-runtime
+function build_nvidia_runtime(){


next time, I will rewrite the package folder

Signed-off-by: codejuan <xh@decbug.com>

allencloud · 2018-08-10T06:37:49Z

Thanks for your great work. I will merge this. @CodeJuan

allencloud · 2018-08-13T01:42:51Z

Do we have a document to help users use GPU with pouchcontainer? @CodeJuan

pouchrobot added the size/XL label Jul 31, 2018

CodeJuan force-pushed the nvidia2 branch from 1d74a21 to f590d96 Compare July 31, 2018 11:20

allencloud changed the title ~~support nvidia-container 2.0~~ feature: support nvidia-container 2.0 Aug 1, 2018

pouchrobot added the kind/feature label Aug 1, 2018

CodeJuan force-pushed the nvidia2 branch from f590d96 to a85898d Compare August 1, 2018 01:47

allencloud reviewed Aug 1, 2018

View reviewed changes

daemon/mgr/container_utils.go Outdated

return nil

}

if err := vlidateNvidiaDriver(r); err != nil {

Copy link

Collaborator

allencloud Aug 1, 2018

Choose a reason for hiding this comment

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

s/validate/vlidate?

allencloud reviewed Aug 1, 2018

View reviewed changes

CodeJuan force-pushed the nvidia2 branch 2 times, most recently from d7d928c to 36d9682 Compare August 2, 2018 02:20

pouchrobot added conflict/needs-rebase and removed conflict/needs-rebase labels Aug 3, 2018

CodeJuan force-pushed the nvidia2 branch from 36d9682 to 49a2f4e Compare August 6, 2018 13:17

pouchrobot removed the conflict/needs-rebase label Aug 6, 2018

CodeJuan force-pushed the nvidia2 branch from 49a2f4e to 780e7e7 Compare August 6, 2018 14:31

CodeJuan requested a review from fuweid August 7, 2018 05:28

fuweid reviewed Aug 7, 2018

View reviewed changes

CodeJuan force-pushed the nvidia2 branch from 780e7e7 to d8df8dc Compare August 7, 2018 06:11

fuweid suggested changes Aug 7, 2018

View reviewed changes

CodeJuan force-pushed the nvidia2 branch from d8df8dc to 23a3c8f Compare August 8, 2018 02:42

fuweid reviewed Aug 8, 2018

View reviewed changes

CodeJuan force-pushed the nvidia2 branch 2 times, most recently from 2b3ef3f to 05ebcbe Compare August 8, 2018 11:43

pouchrobot added the conflict/needs-rebase label Aug 9, 2018

fuweid reviewed Aug 10, 2018

View reviewed changes

CodeJuan force-pushed the nvidia2 branch from 05ebcbe to 438fa24 Compare August 10, 2018 02:23

pouchrobot removed the conflict/needs-rebase label Aug 10, 2018

feature: add nvidia-container 2.0

cf12087

Signed-off-by: codejuan <xh@decbug.com>

CodeJuan force-pushed the nvidia2 branch from 438fa24 to cf12087 Compare August 10, 2018 02:46

allencloud changed the title ~~feature: support nvidia-container 2.0~~ feature: support nvidia-container 2.0 to enable GPU access Aug 10, 2018

allencloud merged commit a77e0da into AliyunContainerService:master Aug 10, 2018

pouchrobot mentioned this pull request Aug 11, 2018

Weekly Report of PouchContainer from 2018-08-04 to 2018-08-11 #2076

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: support nvidia-container 2.0 to enable GPU access #2029

feature: support nvidia-container 2.0 to enable GPU access #2029

CodeJuan commented Jul 31, 2018 •

edited

Loading

codecov-io commented Aug 1, 2018 •

edited

Loading

allencloud Aug 1, 2018

allencloud Aug 1, 2018

allencloud Aug 1, 2018 •

edited

Loading

CodeJuan Aug 2, 2018

allencloud commented Aug 1, 2018 •

edited

Loading

CodeJuan commented Aug 2, 2018

fuweid Aug 7, 2018

fuweid Aug 7, 2018 •

edited

Loading

fuweid left a comment

fuweid Aug 8, 2018

fuweid left a comment

fuweid Aug 10, 2018

allencloud commented Aug 10, 2018

allencloud commented Aug 13, 2018

feature: support nvidia-container 2.0 to enable GPU access #2029

feature: support nvidia-container 2.0 to enable GPU access #2029

Conversation

CodeJuan commented Jul 31, 2018 • edited Loading

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

codecov-io commented Aug 1, 2018 • edited Loading

Codecov Report

allencloud Aug 1, 2018

Choose a reason for hiding this comment

allencloud Aug 1, 2018

Choose a reason for hiding this comment

allencloud Aug 1, 2018 • edited Loading

Choose a reason for hiding this comment

CodeJuan Aug 2, 2018

Choose a reason for hiding this comment

allencloud commented Aug 1, 2018 • edited Loading

CodeJuan commented Aug 2, 2018

fuweid Aug 7, 2018

Choose a reason for hiding this comment

fuweid Aug 7, 2018 • edited Loading

Choose a reason for hiding this comment

fuweid left a comment

Choose a reason for hiding this comment

fuweid Aug 8, 2018

Choose a reason for hiding this comment

fuweid left a comment

Choose a reason for hiding this comment

fuweid Aug 10, 2018

Choose a reason for hiding this comment

allencloud commented Aug 10, 2018

allencloud commented Aug 13, 2018

CodeJuan commented Jul 31, 2018 •

edited

Loading

codecov-io commented Aug 1, 2018 •

edited

Loading

allencloud Aug 1, 2018 •

edited

Loading

allencloud commented Aug 1, 2018 •

edited

Loading

fuweid Aug 7, 2018 •

edited

Loading