Merge pull request #53031 from vishh/device-id

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Make AllocateResponse artifacts global across all devices per container in device plugin API The current version of Device plugins returns artifacts (env vars, mounts, devices) per device, per container. This is not necessary and results in complex merging issues on kubelet side There can still be a conflict if the artifacts returned by device plugin conflicts with the pod spec. In that case, I'd recommend failing pods in kubelet. This is yet to be addressed. The go package name for device plugin apis is updated from `pkg/kubelet/apis/deviceplugin/v1alpha1` to `pkg/kubelet/apis/deviceplugin/v1alpha` (sub-version dropped) because we expect the alpha version to change until it graduates to beta and changing the go package everytime the actual alpha version changes is too tedious. ```release-note Device plugin Alpha API no longer supports returning artifacts per device as part of AllocateResponse. ``` TODO: - [x] Bump kubelet side API version - [X] Post an updated device plugin image that works with the new API version - [ ] Stabilize e2es (This PR needs to be merged since there is a dependency on the plugin side for vendoring)

Merge pull request #53031 from vishh/device-id
c3d47b68 · Kubernetes Submit Queue · GitHub · e292da81 · 16b0363b · c3d47b68
Commit c3d47b68 authored Oct 19, 2017 by Kubernetes Submit Queue Committed by GitHub Oct 19, 2017
22 changed files
--- a/hack/.golint_failures
+++ b/hack/.golint_failures
@@ -209,7 +209,7 @@ pkg/kubelet
 pkg/kubelet/apis
 pkg/kubelet/apis/cri/testing
 pkg/kubelet/apis/cri/v1alpha1/runtime
-pkg/kubelet/apis/deviceplugin/v1alpha1
+pkg/kubelet/apis/deviceplugin/v1alpha
 pkg/kubelet/apis/kubeletconfig
 pkg/kubelet/apis/kubeletconfig/v1alpha1
 pkg/kubelet/cadvisor

--- a/hack/update-generated-device-plugin-dockerized.sh
+++ b/hack/update-generated-device-plugin-dockerized.sh
@@ -19,8 +19,7 @@ set -o nounset
 set -o pipefail
 KUBE_ROOT="$(cd "$(dirname "${BASH_SOURCE}")/../" && pwd -P)"
-DEVICE_PLUGIN_ROOT="${KUBE_ROOT}/pkg/kubelet/apis/deviceplugin/v1alpha1/"
+DEVICE_PLUGIN_ROOT="${KUBE_ROOT}/pkg/kubelet/apis/deviceplugin/v1alpha/"
 source "${KUBE_ROOT}/hack/lib/protoc.sh"
 kube::protoc::generate_proto ${DEVICE_PLUGIN_ROOT}
--- a/hack/verify-generated-device-plugin.sh
+++ b/hack/verify-generated-device-plugin.sh
@@ -19,8 +19,8 @@ set -o nounset
 set -o pipefail
 KUBE_ROOT=$(dirname "${BASH_SOURCE}")/..
-DEVICE_PLUGIN_ROOT="${KUBE_ROOT}/pkg/kubelet/apis/deviceplugin/v1alpha1/"
 ERROR="Device plugin api is out of date. Please run hack/update-generated-device-plugin.sh"
+DEVICE_PLUGIN_ROOT="${KUBE_ROOT}/pkg/kubelet/apis/deviceplugin/v1alpha/"
 source "${KUBE_ROOT}/hack/lib/protoc.sh"
 kube::golang::setup_env

--- a/pkg/kubelet/apis/BUILD
+++ b/pkg/kubelet/apis/BUILD
@@ -26,7 +26,7 @@ filegroup(
    srcs = [
        ":package-srcs",
        "//pkg/kubelet/apis/cri:all-srcs",
-        "//pkg/kubelet/apis/deviceplugin/v1alpha1:all-srcs",
+        "//pkg/kubelet/apis/deviceplugin/v1alpha:all-srcs",
        "//pkg/kubelet/apis/kubeletconfig:all-srcs",
        "//pkg/kubelet/apis/stats/v1alpha1:all-srcs",
    ],

--- a/pkg/kubelet/apis/deviceplugin/v1alpha1/BUILD
+++ b/pkg/kubelet/apis/deviceplugin/v1alpha1/BUILD
@@ -11,7 +11,7 @@ go_library(
        "api.pb.go",
        "constants.go",
    ],
-    importpath = "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha1",
+    importpath = "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha",
    deps = [
        "//vendor/github.com/gogo/protobuf/gogoproto:go_default_library",
        "//vendor/github.com/gogo/protobuf/proto:go_default_library",

--- a/pkg/kubelet/apis/deviceplugin/v1alpha1/api.pb.go
+++ b/pkg/kubelet/apis/deviceplugin/v1alpha1/api.pb.go
--- a/pkg/kubelet/apis/deviceplugin/v1alpha1/api.proto
+++ b/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;
+  	// List of environment variable to be set in the container to access one of more devices.
-}
+	map<string, string> envs = 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;
 	// Mounts for the container.
-	repeated Mount mounts = 3;
+	repeated Mount mounts = 2;
-	// Devices for the container
+	// Devices for the container.
-	repeated DeviceSpec devices = 4;
+	repeated DeviceSpec devices = 3;
 }
 // Mount specifies a host volume to mount into a container.

--- a/pkg/kubelet/apis/deviceplugin/v1alpha1/constants.go
+++ b/pkg/kubelet/apis/deviceplugin/v1alpha1/constants.go
@@ -22,8 +22,8 @@ const (
 	// UnHealthy means that the device is unhealty
 	Unhealthy = "Unhealthy"
-	// Version is the API version
+	// Current version of the API supported by kubelet
-	Version = "0.1"
+	Version = "v1alpha2"
 	// DevicePluginPath is the folder the Device Plugin is expecting sockets to be on
 	// Only privileged pods have access to this path
 	// Note: Placeholder until we find a "standard path"

--- a/pkg/kubelet/cm/BUILD
+++ b/pkg/kubelet/cm/BUILD
@@ -34,7 +34,7 @@ go_library(
    deps = [
        "//pkg/features:go_default_library",
        "//pkg/kubelet/apis/cri:go_default_library",
-        "//pkg/kubelet/apis/deviceplugin/v1alpha1:go_default_library",
+        "//pkg/kubelet/apis/deviceplugin/v1alpha:go_default_library",
        "//pkg/kubelet/apis/kubeletconfig:go_default_library",
        "//pkg/kubelet/cadvisor:go_default_library",
        "//pkg/kubelet/cm/cpumanager:go_default_library",
@@ -94,7 +94,7 @@ go_test(
    importpath = "k8s.io/kubernetes/pkg/kubelet/cm",
    library = ":go_default_library",
    deps = [
-        "//pkg/kubelet/apis/deviceplugin/v1alpha1:go_default_library",
+        "//pkg/kubelet/apis/deviceplugin/v1alpha:go_default_library",
        "//pkg/util/mount:go_default_library",
        "//vendor/github.com/stretchr/testify/assert:go_default_library",
        "//vendor/k8s.io/api/core/v1:go_default_library",

--- a/pkg/kubelet/cm/container_manager_linux.go
+++ b/pkg/kubelet/cm/container_manager_linux.go
@@ -628,10 +628,26 @@ 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.
+		// Each Allocate response has the following artifacts.
-		for _, devRuntime := range resp.Spec {
+		// 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)
+				}
+				continue
+			}
+			envsMap[k] = v
+			opts.Envs = append(opts.Envs, kubecontainer.EnvVar{Name: k, Value: v})
+		}
 		// Updates RunContainerOptions.Devices.
-			for _, dev := range devRuntime.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 {
@@ -648,7 +664,7 @@ func (cm *containerManagerImpl) GetResources(pod *v1.Pod, container *v1.Containe
 			})
 		}
 		// Updates RunContainerOptions.Mounts.
-			for _, mount := range devRuntime.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 {
@@ -663,22 +679,10 @@ func (cm *containerManagerImpl) GetResources(pod *v1.Pod, container *v1.Containe
 				ContainerPath: mount.ContainerPath,
 				HostPath:      mount.HostPath,
 				ReadOnly:      mount.ReadOnly,
+				// TODO: This may need to be part of Device plugin API.
 				SELinuxRelabel: false,
 			})
 		}
-			// 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
-				}
-				envsMap[k] = v
-				opts.Envs = append(opts.Envs, kubecontainer.EnvVar{Name: k, Value: v})
-			}
-		}
 	}
 	return opts, nil
 }

--- a/pkg/kubelet/cm/device_plugin_handler.go
+++ b/pkg/kubelet/cm/device_plugin_handler.go
@@ -28,7 +28,7 @@ import (
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/util/sets"
-	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha1"
+	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha"
 	"k8s.io/kubernetes/pkg/kubelet/deviceplugin"
 )

--- a/pkg/kubelet/cm/device_plugin_handler_stub.go
+++ b/pkg/kubelet/cm/device_plugin_handler_stub.go
@@ -18,7 +18,7 @@ package cm
 import (
 	"k8s.io/api/core/v1"
-	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha1"
+	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha"
 )
 // A simple stub implementation for DevicePluginHandler.

--- a/pkg/kubelet/cm/device_plugin_handler_test.go
+++ b/pkg/kubelet/cm/device_plugin_handler_test.go
@@ -28,7 +28,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/uuid"
-	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha1"
+	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha"
 )
 func TestUpdateCapacity(t *testing.T) {
@@ -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
 }
@@ -164,6 +162,7 @@ func TestCheckpoint(t *testing.T) {
 }
 func TestPodContainerDeviceAllocation(t *testing.T) {
+	t.Skip("Skipping due to issue #54100")
 	flag.Set("alsologtostderr", fmt.Sprintf("%t", true))
 	var logLevel string
 	flag.StringVar(&logLevel, "logLevel", "4", "test")

--- a/pkg/kubelet/deviceplugin/BUILD
+++ b/pkg/kubelet/deviceplugin/BUILD
@@ -18,7 +18,7 @@ go_library(
    importpath = "k8s.io/kubernetes/pkg/kubelet/deviceplugin",
    deps = [
        "//pkg/api/v1/helper:go_default_library",
-        "//pkg/kubelet/apis/deviceplugin/v1alpha1:go_default_library",
+        "//pkg/kubelet/apis/deviceplugin/v1alpha:go_default_library",
        "//vendor/github.com/golang/glog:go_default_library",
        "//vendor/golang.org/x/net/context:go_default_library",
        "//vendor/google.golang.org/grpc:go_default_library",
@@ -49,7 +49,7 @@ go_test(
    importpath = "k8s.io/kubernetes/pkg/kubelet/deviceplugin",
    library = ":go_default_library",
    deps = [
-        "//pkg/kubelet/apis/deviceplugin/v1alpha1:go_default_library",
+        "//pkg/kubelet/apis/deviceplugin/v1alpha:go_default_library",
        "//vendor/github.com/stretchr/testify/require:go_default_library",
    ],
 )
--- a/pkg/kubelet/deviceplugin/device_plugin_stub.go
+++ b/pkg/kubelet/deviceplugin/device_plugin_stub.go
@@ -26,7 +26,7 @@ import (
 	"golang.org/x/net/context"
 	"google.golang.org/grpc"
-	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha1"
+	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha"
 )
 // Stub implementation for DevicePlugin.

--- a/pkg/kubelet/deviceplugin/endpoint.go
+++ b/pkg/kubelet/deviceplugin/endpoint.go
@@ -26,7 +26,7 @@ import (
 	"golang.org/x/net/context"
 	"google.golang.org/grpc"
-	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha1"
+	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha"
 )
 // endpoint maps to a single registered device plugin. It is responsible

--- a/pkg/kubelet/deviceplugin/endpoint_test.go
+++ b/pkg/kubelet/deviceplugin/endpoint_test.go
@@ -23,7 +23,7 @@ import (
 	"github.com/stretchr/testify/require"
-	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha1"
+	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha"
 )
 var (

--- a/pkg/kubelet/deviceplugin/manager.go
+++ b/pkg/kubelet/deviceplugin/manager.go
@@ -27,7 +27,7 @@ import (
 	"golang.org/x/net/context"
 	"google.golang.org/grpc"
-	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha1"
+	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha"
 )
 // ManagerImpl is the structure in charge of managing Device Plugins.

--- a/pkg/kubelet/deviceplugin/manager_test.go
+++ b/pkg/kubelet/deviceplugin/manager_test.go
@@ -23,7 +23,7 @@ import (
 	"github.com/stretchr/testify/require"
-	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha1"
+	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha"
 )
 const (

--- a/pkg/kubelet/deviceplugin/types.go
+++ b/pkg/kubelet/deviceplugin/types.go
@@ -17,7 +17,7 @@ limitations under the License.
 package deviceplugin
 import (
-	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha1"
+	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha"
 )
 // MonitorCallback is the function called when a device's health state changes,

--- a/pkg/kubelet/deviceplugin/utils.go
+++ b/pkg/kubelet/deviceplugin/utils.go
@@ -21,7 +21,7 @@ import (
 	"k8s.io/api/core/v1"
 	v1helper "k8s.io/kubernetes/pkg/api/v1/helper"
-	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha1"
+	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha"
 )
 func cloneDevice(d *pluginapi.Device) *pluginapi.Device {

--- a/pkg/kubelet/deviceplugin/utils_test.go
+++ b/pkg/kubelet/deviceplugin/utils_test.go
@@ -21,7 +21,7 @@ import (
 	"github.com/stretchr/testify/require"
-	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha1"
+	pluginapi "k8s.io/kubernetes/pkg/kubelet/apis/deviceplugin/v1alpha"
 )
 func TestCloneDevice(t *testing.T) {