Make AllocateResponse artifacts global across all devices per container in device plugin API

pkg/kubelet/apis/deviceplugin/v1alpha1/api.proto

@@ -81,25 +81,21 @@ message AllocateRequest {
 	repeated string devicesIDs = 1;
 }
 
+// AllocateResponse includes the artifacts that needs to be injected into
+// a container for accessing 'deviceIDs' that were mentioned as part of
+// 'AllocateRequest'.
 // Failure Handling:
 // if Kubelet sends an allocation request for dev1 and dev2.
 // Allocation on dev1 succeeds but allocation on dev2 fails.
 // The Device plugin should send a ListAndWatch update and fail the
 // Allocation request
 message AllocateResponse {
-	repeated DeviceRuntimeSpec spec = 1;
-}
-
-// The list to be added to the CRI spec
-message DeviceRuntimeSpec {
-	// ID of the Device
-	string ID = 1;
-	// List of environment variable to set in the container.
-	map<string, string> envs = 2;
+  	// List of environment variable to be set in the container to access one of more devices.
+	map<string, string> envs = 1;
 	// Mounts for the container.
-	repeated Mount mounts = 3;
-	// Devices for the container
-	repeated DeviceSpec devices = 4;
+	repeated Mount mounts = 2;
+	// Devices for the container.
+	repeated DeviceSpec devices = 3;
 }
 
 // Mount specifies a host volume to mount into a container.

pkg/kubelet/cm/container_manager_linux.go

@@ -628,56 +628,60 @@ func (cm *containerManagerImpl) GetResources(pod *v1.Pod, container *v1.Containe
 	}
 	// Loops through AllocationResponses of all required extended resources.
 	for _, resp := range allocResps {
-		// Loops through runtime spec of all devices of the given resource.
-		for _, devRuntime := range resp.Spec {
-			// Updates RunContainerOptions.Devices.
-			for _, dev := range devRuntime.Devices {
-				if d, ok := devsMap[dev.ContainerPath]; ok {
-					glog.V(3).Infof("skip existing device %s %s", dev.ContainerPath, dev.HostPath)
-					if d != dev.HostPath {
-						glog.Errorf("Container device %s has conflicting mapping host devices: %s and %s",
-							dev.ContainerPath, d, dev.HostPath)
-					}
-					continue
+		// Each Allocate response has the following artifacts.
+		// Environment variables
+		// Mount points
+		// Device files
+		// These artifacts are per resource per container.
+		// Updates RunContainerOptions.Envs.
+		for k, v := range resp.Envs {
+			if e, ok := envsMap[k]; ok {
+				glog.V(3).Infof("skip existing envs %s %s", k, v)
+				if e != v {
+					glog.Errorf("Environment variable %s has conflicting setting: %s and %s", k, e, v)
 				}
-				devsMap[dev.ContainerPath] = dev.HostPath
-				opts.Devices = append(opts.Devices, kubecontainer.DeviceInfo{
-					PathOnHost:      dev.HostPath,
-					PathInContainer: dev.ContainerPath,
-					Permissions:     dev.Permissions,
-				})
+				continue
 			}
-			// Updates RunContainerOptions.Mounts.
-			for _, mount := range devRuntime.Mounts {
-				if m, ok := mountsMap[mount.ContainerPath]; ok {
-					glog.V(3).Infof("skip existing mount %s %s", mount.ContainerPath, mount.HostPath)
-					if m != mount.HostPath {
-						glog.Errorf("Container mount %s has conflicting mapping host mounts: %s and %s",
-							mount.ContainerPath, m, mount.HostPath)
-					}
-					continue
+			envsMap[k] = v
+			opts.Envs = append(opts.Envs, kubecontainer.EnvVar{Name: k, Value: v})
+		}
+
+		// Updates RunContainerOptions.Devices.
+		for _, dev := range resp.Devices {
+			if d, ok := devsMap[dev.ContainerPath]; ok {
+				glog.V(3).Infof("skip existing device %s %s", dev.ContainerPath, dev.HostPath)
+				if d != dev.HostPath {
+					glog.Errorf("Container device %s has conflicting mapping host devices: %s and %s",
+						dev.ContainerPath, d, dev.HostPath)
 				}
-				mountsMap[mount.ContainerPath] = mount.HostPath
-				opts.Mounts = append(opts.Mounts, kubecontainer.Mount{
-					Name:           mount.ContainerPath,
-					ContainerPath:  mount.ContainerPath,
-					HostPath:       mount.HostPath,
-					ReadOnly:       mount.ReadOnly,
-					SELinuxRelabel: false,
-				})
+				continue
 			}
-			// Updates RunContainerOptions.Envs.
-			for k, v := range devRuntime.Envs {
-				if e, ok := envsMap[k]; ok {
-					glog.V(3).Infof("skip existing envs %s %s", k, v)
-					if e != v {
-						glog.Errorf("Environment variable %s has conflicting setting: %s and %s", k, e, v)
-					}
-					continue
+			devsMap[dev.ContainerPath] = dev.HostPath
+			opts.Devices = append(opts.Devices, kubecontainer.DeviceInfo{
+				PathOnHost:      dev.HostPath,
+				PathInContainer: dev.ContainerPath,
+				Permissions:     dev.Permissions,
+			})
+		}
+		// Updates RunContainerOptions.Mounts.
+		for _, mount := range resp.Mounts {
+			if m, ok := mountsMap[mount.ContainerPath]; ok {
+				glog.V(3).Infof("skip existing mount %s %s", mount.ContainerPath, mount.HostPath)
+				if m != mount.HostPath {
+					glog.Errorf("Container mount %s has conflicting mapping host mounts: %s and %s",
+						mount.ContainerPath, m, mount.HostPath)
 				}
-				envsMap[k] = v
-				opts.Envs = append(opts.Envs, kubecontainer.EnvVar{Name: k, Value: v})
+				continue
 			}
+			mountsMap[mount.ContainerPath] = mount.HostPath
+			opts.Mounts = append(opts.Mounts, kubecontainer.Mount{
+				Name:          mount.ContainerPath,
+				ContainerPath: mount.ContainerPath,
+				HostPath:      mount.HostPath,
+				ReadOnly:      mount.ReadOnly,
+				// TODO: This may need to be part of Device plugin API.
+				SELinuxRelabel: false,
+			})
 		}
 	}
 	return opts, nil

pkg/kubelet/cm/device_plugin_handler_test.go

@@ -99,9 +99,8 @@ func (m *DevicePluginManagerTestStub) Allocate(resourceName string, devIds []str
 	for _, id := range devIds {
 		key := resourceName + id
 		fmt.Printf("Alloc device %q for resource %q\n", id, resourceName)
-		devRuntime := new(pluginapi.DeviceRuntimeSpec)
 		for _, dev := range m.devRuntimeDevices[key] {
-			devRuntime.Devices = append(devRuntime.Devices, &pluginapi.DeviceSpec{
+			resp.Devices = append(resp.Devices, &pluginapi.DeviceSpec{
 				ContainerPath: dev.value1,
 				HostPath:      dev.value2,
 				Permissions:   "mrw",
@@ -109,17 +108,16 @@ func (m *DevicePluginManagerTestStub) Allocate(resourceName string, devIds []str
 		}
 		for _, mount := range m.devRuntimeMounts[key] {
 			fmt.Printf("Add mount %q %q\n", mount.value1, mount.value2)
-			devRuntime.Mounts = append(devRuntime.Mounts, &pluginapi.Mount{
+			resp.Mounts = append(resp.Mounts, &pluginapi.Mount{
 				ContainerPath: mount.value1,
 				HostPath:      mount.value2,
 				ReadOnly:      true,
 			})
 		}
-		devRuntime.Envs = make(map[string]string)
+		resp.Envs = make(map[string]string)
 		for _, env := range m.devRuntimeEnvs[key] {
-			devRuntime.Envs[env.value1] = env.value2
+			resp.Envs[env.value1] = env.value2
 		}
-		resp.Spec = append(resp.Spec, devRuntime)
 	}
 	return resp, nil
 }