Unverified Commit e31c8a22 authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub

Merge pull request #60318 from jiayingz/api-change

Automatic merge from submit-queue (batch tested with PRs 59159, 60318, 60079, 59371, 57415). 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>. Made a couple API changes to deviceplugin/v1beta1 to avoid future incompatible API changes: - Add GetDevicePluginOptions rpc call. This is needed when we switch from Registration service to probe-based plugin watcher. - Change AllocateRequest and AllocateResponse to allow device requests from multiple containers in a pod. Currently only made mechanical change on the devicemanager and test code to cope with the API but still issues an Allocate call per container. We can modify the devicemanager in 1.11 to issue a single Allocate call per pod. The change will also facilitate incremental API change to communicate pod level information through Allocate rpc if there is such future need. **What this PR does / why we need it**: Made a couple API changes to deviceplugin/v1beta1 to avoid future incompatible API changes. **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Fixes https://github.com/kubernetes/kubernetes/issues/59370 **Special notes for your reviewer**: **Release note**: ```release-note ```
parents 0394ffba 07beac60
...@@ -46,6 +46,10 @@ message Empty { ...@@ -46,6 +46,10 @@ message Empty {
// DevicePlugin is the service advertised by Device Plugins // DevicePlugin is the service advertised by Device Plugins
service DevicePlugin { service DevicePlugin {
// GetDevicePluginOptions returns options to be communicated with Device
// Manager
rpc GetDevicePluginOptions(Empty) returns (DevicePluginOptions) {}
// ListAndWatch returns a stream of List of Devices // ListAndWatch returns a stream of List of Devices
// Whenever a Device state change or a Device disapears, ListAndWatch // Whenever a Device state change or a Device disapears, ListAndWatch
// returns the new list // returns the new list
...@@ -102,6 +106,10 @@ message PreStartContainerResponse { ...@@ -102,6 +106,10 @@ message PreStartContainerResponse {
// - Allocate allows Device Plugin to run device specific operations on // - Allocate allows Device Plugin to run device specific operations on
// the Devices requested // the Devices requested
message AllocateRequest { message AllocateRequest {
repeated ContainerAllocateRequest container_requests = 1;
}
message ContainerAllocateRequest {
repeated string devicesIDs = 1; repeated string devicesIDs = 1;
} }
...@@ -114,6 +122,10 @@ message AllocateRequest { ...@@ -114,6 +122,10 @@ message AllocateRequest {
// The Device plugin should send a ListAndWatch update and fail the // The Device plugin should send a ListAndWatch update and fail the
// Allocation request // Allocation request
message AllocateResponse { message AllocateResponse {
repeated ContainerAllocateResponse container_responses = 1;
}
message ContainerAllocateResponse {
// List of environment variable to be set in the container to access one of more devices. // List of environment variable to be set in the container to access one of more devices.
map<string, string> envs = 1; map<string, string> envs = 1;
// Mounts for the container. // Mounts for the container.
......
...@@ -130,6 +130,11 @@ func (m *Stub) Register(kubeletEndpoint, resourceName string, preStartContainerF ...@@ -130,6 +130,11 @@ func (m *Stub) Register(kubeletEndpoint, resourceName string, preStartContainerF
return nil return nil
} }
// GetDevicePluginOptions returns DevicePluginOptions settings for the device plugin.
func (m *Stub) GetDevicePluginOptions(ctx context.Context, e *pluginapi.Empty) (*pluginapi.DevicePluginOptions, error) {
return &pluginapi.DevicePluginOptions{}, nil
}
// PreStartContainer resets the devices received // PreStartContainer resets the devices received
func (m *Stub) PreStartContainer(ctx context.Context, r *pluginapi.PreStartContainerRequest) (*pluginapi.PreStartContainerResponse, error) { func (m *Stub) PreStartContainer(ctx context.Context, r *pluginapi.PreStartContainerRequest) (*pluginapi.PreStartContainerResponse, error) {
log.Printf("PreStartContainer, %+v", r) log.Printf("PreStartContainer, %+v", r)
......
...@@ -179,7 +179,9 @@ func (e *endpointImpl) run() { ...@@ -179,7 +179,9 @@ func (e *endpointImpl) run() {
// allocate issues Allocate gRPC call to the device plugin. // allocate issues Allocate gRPC call to the device plugin.
func (e *endpointImpl) allocate(devs []string) (*pluginapi.AllocateResponse, error) { func (e *endpointImpl) allocate(devs []string) (*pluginapi.AllocateResponse, error) {
return e.client.Allocate(context.Background(), &pluginapi.AllocateRequest{ return e.client.Allocate(context.Background(), &pluginapi.AllocateRequest{
DevicesIDs: devs, ContainerRequests: []*pluginapi.ContainerAllocateRequest{
{DevicesIDs: devs},
},
}) })
} }
......
...@@ -130,24 +130,27 @@ func TestAllocate(t *testing.T) { ...@@ -130,24 +130,27 @@ func TestAllocate(t *testing.T) {
defer ecleanup(t, p, e) defer ecleanup(t, p, e)
resp := new(pluginapi.AllocateResponse) resp := new(pluginapi.AllocateResponse)
resp.Devices = append(resp.Devices, &pluginapi.DeviceSpec{ contResp := new(pluginapi.ContainerAllocateResponse)
contResp.Devices = append(contResp.Devices, &pluginapi.DeviceSpec{
ContainerPath: "/dev/aaa", ContainerPath: "/dev/aaa",
HostPath: "/dev/aaa", HostPath: "/dev/aaa",
Permissions: "mrw", Permissions: "mrw",
}) })
resp.Devices = append(resp.Devices, &pluginapi.DeviceSpec{ contResp.Devices = append(contResp.Devices, &pluginapi.DeviceSpec{
ContainerPath: "/dev/bbb", ContainerPath: "/dev/bbb",
HostPath: "/dev/bbb", HostPath: "/dev/bbb",
Permissions: "mrw", Permissions: "mrw",
}) })
resp.Mounts = append(resp.Mounts, &pluginapi.Mount{ contResp.Mounts = append(contResp.Mounts, &pluginapi.Mount{
ContainerPath: "/container_dir1/file1", ContainerPath: "/container_dir1/file1",
HostPath: "host_dir1/file1", HostPath: "host_dir1/file1",
ReadOnly: true, ReadOnly: true,
}) })
resp.ContainerResponses = append(resp.ContainerResponses, contResp)
p.SetAllocFunc(func(r *pluginapi.AllocateRequest, devs map[string]pluginapi.Device) (*pluginapi.AllocateResponse, error) { p.SetAllocFunc(func(r *pluginapi.AllocateRequest, devs map[string]pluginapi.Device) (*pluginapi.AllocateResponse, error) {
return resp, nil return resp, nil
}) })
......
...@@ -643,6 +643,8 @@ func (m *ManagerImpl) allocateContainerResources(pod *v1.Pod, container *v1.Cont ...@@ -643,6 +643,8 @@ func (m *ManagerImpl) allocateContainerResources(pod *v1.Pod, container *v1.Cont
} }
devs := allocDevices.UnsortedList() devs := allocDevices.UnsortedList()
// TODO: refactor this part of code to just append a ContainerAllocationRequest
// in a passed in AllocateRequest pointer, and issues a single Allocate call per pod.
glog.V(3).Infof("Making allocation request for devices %v for device plugin %s", devs, resource) glog.V(3).Infof("Making allocation request for devices %v for device plugin %s", devs, resource)
resp, err := e.allocate(devs) resp, err := e.allocate(devs)
metrics.DevicePluginAllocationLatency.WithLabelValues(resource).Observe(metrics.SinceInMicroseconds(startRPCTime)) metrics.DevicePluginAllocationLatency.WithLabelValues(resource).Observe(metrics.SinceInMicroseconds(startRPCTime))
...@@ -657,7 +659,7 @@ func (m *ManagerImpl) allocateContainerResources(pod *v1.Pod, container *v1.Cont ...@@ -657,7 +659,7 @@ func (m *ManagerImpl) allocateContainerResources(pod *v1.Pod, container *v1.Cont
// Update internal cached podDevices state. // Update internal cached podDevices state.
m.mutex.Lock() m.mutex.Lock()
m.podDevices.insert(podUID, contName, resource, allocDevices, resp) m.podDevices.insert(podUID, contName, resource, allocDevices, resp.ContainerResponses[0])
m.mutex.Unlock() m.mutex.Unlock()
} }
......
...@@ -279,8 +279,8 @@ func constructDevices(devices []string) sets.String { ...@@ -279,8 +279,8 @@ func constructDevices(devices []string) sets.String {
return ret return ret
} }
func constructAllocResp(devices, mounts, envs map[string]string) *pluginapi.AllocateResponse { func constructAllocResp(devices, mounts, envs map[string]string) *pluginapi.ContainerAllocateResponse {
resp := &pluginapi.AllocateResponse{} resp := &pluginapi.ContainerAllocateResponse{}
for k, v := range devices { for k, v := range devices {
resp.Devices = append(resp.Devices, &pluginapi.DeviceSpec{ resp.Devices = append(resp.Devices, &pluginapi.DeviceSpec{
HostPath: k, HostPath: k,
...@@ -458,7 +458,7 @@ func getTestManager(tmpDir string, activePods ActivePodsFunc, testRes []TestReso ...@@ -458,7 +458,7 @@ func getTestManager(tmpDir string, activePods ActivePodsFunc, testRes []TestReso
if res.resourceName == "domain2.com/resource2" { if res.resourceName == "domain2.com/resource2" {
testManager.endpoints[res.resourceName] = &MockEndpoint{ testManager.endpoints[res.resourceName] = &MockEndpoint{
allocateFunc: func(devs []string) (*pluginapi.AllocateResponse, error) { allocateFunc: func(devs []string) (*pluginapi.AllocateResponse, error) {
resp := new(pluginapi.AllocateResponse) resp := new(pluginapi.ContainerAllocateResponse)
resp.Envs = make(map[string]string) resp.Envs = make(map[string]string)
for _, dev := range devs { for _, dev := range devs {
switch dev { switch dev {
...@@ -469,7 +469,9 @@ func getTestManager(tmpDir string, activePods ActivePodsFunc, testRes []TestReso ...@@ -469,7 +469,9 @@ func getTestManager(tmpDir string, activePods ActivePodsFunc, testRes []TestReso
resp.Envs["key2"] = "val3" resp.Envs["key2"] = "val3"
} }
} }
return resp, nil resps := new(pluginapi.AllocateResponse)
resps.ContainerResponses = append(resps.ContainerResponses, resp)
return resps, nil
}, },
} }
} }
...@@ -774,8 +776,10 @@ func TestDevicePreStartContainer(t *testing.T) { ...@@ -774,8 +776,10 @@ func TestDevicePreStartContainer(t *testing.T) {
as.Contains(initializedDevs, "dev2") as.Contains(initializedDevs, "dev2")
as.Equal(len(initializedDevs), len(res1.devs)) as.Equal(len(initializedDevs), len(res1.devs))
expectedResp, err := allocateStubFunc()([]string{"dev1", "dev2"}) expectedResps, err := allocateStubFunc()([]string{"dev1", "dev2"})
as.Nil(err) as.Nil(err)
as.Equal(1, len(expectedResps.ContainerResponses))
expectedResp := expectedResps.ContainerResponses[0]
as.Equal(len(runContainerOpts.Devices), len(expectedResp.Devices)) as.Equal(len(runContainerOpts.Devices), len(expectedResp.Devices))
as.Equal(len(runContainerOpts.Mounts), len(expectedResp.Mounts)) as.Equal(len(runContainerOpts.Mounts), len(expectedResp.Mounts))
as.Equal(len(runContainerOpts.Envs), len(expectedResp.Envs)) as.Equal(len(runContainerOpts.Envs), len(expectedResp.Envs))
...@@ -783,7 +787,7 @@ func TestDevicePreStartContainer(t *testing.T) { ...@@ -783,7 +787,7 @@ func TestDevicePreStartContainer(t *testing.T) {
func allocateStubFunc() func(devs []string) (*pluginapi.AllocateResponse, error) { func allocateStubFunc() func(devs []string) (*pluginapi.AllocateResponse, error) {
return func(devs []string) (*pluginapi.AllocateResponse, error) { return func(devs []string) (*pluginapi.AllocateResponse, error) {
resp := new(pluginapi.AllocateResponse) resp := new(pluginapi.ContainerAllocateResponse)
resp.Envs = make(map[string]string) resp.Envs = make(map[string]string)
for _, dev := range devs { for _, dev := range devs {
switch dev { switch dev {
...@@ -822,6 +826,8 @@ func allocateStubFunc() func(devs []string) (*pluginapi.AllocateResponse, error) ...@@ -822,6 +826,8 @@ func allocateStubFunc() func(devs []string) (*pluginapi.AllocateResponse, error)
resp.Envs["key1"] = "val1" resp.Envs["key1"] = "val1"
} }
} }
return resp, nil resps := new(pluginapi.AllocateResponse)
resps.ContainerResponses = append(resps.ContainerResponses, resp)
return resps, nil
} }
} }
...@@ -28,7 +28,7 @@ type deviceAllocateInfo struct { ...@@ -28,7 +28,7 @@ type deviceAllocateInfo struct {
// deviceIds contains device Ids allocated to this container for the given resourceName. // deviceIds contains device Ids allocated to this container for the given resourceName.
deviceIds sets.String deviceIds sets.String
// allocResp contains cached rpc AllocateResponse. // allocResp contains cached rpc AllocateResponse.
allocResp *pluginapi.AllocateResponse allocResp *pluginapi.ContainerAllocateResponse
} }
type resourceAllocateInfo map[string]deviceAllocateInfo // Keyed by resourceName. type resourceAllocateInfo map[string]deviceAllocateInfo // Keyed by resourceName.
...@@ -43,7 +43,7 @@ func (pdev podDevices) pods() sets.String { ...@@ -43,7 +43,7 @@ func (pdev podDevices) pods() sets.String {
return ret return ret
} }
func (pdev podDevices) insert(podUID, contName, resource string, devices sets.String, resp *pluginapi.AllocateResponse) { func (pdev podDevices) insert(podUID, contName, resource string, devices sets.String, resp *pluginapi.ContainerAllocateResponse) {
if _, podExists := pdev[podUID]; !podExists { if _, podExists := pdev[podUID]; !podExists {
pdev[podUID] = make(containerDevices) pdev[podUID] = make(containerDevices)
} }
...@@ -168,7 +168,7 @@ func (pdev podDevices) fromCheckpointData(data []podDevicesCheckpointEntry) { ...@@ -168,7 +168,7 @@ func (pdev podDevices) fromCheckpointData(data []podDevicesCheckpointEntry) {
for _, devID := range entry.DeviceIDs { for _, devID := range entry.DeviceIDs {
devIDs.Insert(devID) devIDs.Insert(devID)
} }
allocResp := &pluginapi.AllocateResponse{} allocResp := &pluginapi.ContainerAllocateResponse{}
err := allocResp.Unmarshal(entry.AllocResp) err := allocResp.Unmarshal(entry.AllocResp)
if err != nil { if err != nil {
glog.Errorf("Can't unmarshal allocResp for %v %v %v: %v", entry.PodUID, entry.ContainerName, entry.ResourceName, err) glog.Errorf("Can't unmarshal allocResp for %v %v %v: %v", entry.PodUID, entry.ContainerName, entry.ResourceName, err)
......
...@@ -229,34 +229,38 @@ func numberOfDevices(node *v1.Node, resourceName string) int64 { ...@@ -229,34 +229,38 @@ func numberOfDevices(node *v1.Node, resourceName string) int64 {
// stubAllocFunc will pass to stub device plugin // stubAllocFunc will pass to stub device plugin
func stubAllocFunc(r *pluginapi.AllocateRequest, devs map[string]pluginapi.Device) (*pluginapi.AllocateResponse, error) { func stubAllocFunc(r *pluginapi.AllocateRequest, devs map[string]pluginapi.Device) (*pluginapi.AllocateResponse, error) {
var response pluginapi.AllocateResponse var responses pluginapi.AllocateResponse
for _, requestID := range r.DevicesIDs { for _, req := range r.ContainerRequests {
dev, ok := devs[requestID] response := &pluginapi.ContainerAllocateResponse{}
if !ok { for _, requestID := range req.DevicesIDs {
return nil, fmt.Errorf("invalid allocation request with non-existing device %s", requestID) dev, ok := devs[requestID]
} if !ok {
return nil, fmt.Errorf("invalid allocation request with non-existing device %s", requestID)
}
if dev.Health != pluginapi.Healthy { if dev.Health != pluginapi.Healthy {
return nil, fmt.Errorf("invalid allocation request with unhealthy device: %s", requestID) return nil, fmt.Errorf("invalid allocation request with unhealthy device: %s", requestID)
} }
// create fake device file // create fake device file
fpath := filepath.Join("/tmp", dev.ID) fpath := filepath.Join("/tmp", dev.ID)
// clean first // clean first
os.RemoveAll(fpath) os.RemoveAll(fpath)
f, err := os.Create(fpath) f, err := os.Create(fpath)
if err != nil && !os.IsExist(err) { if err != nil && !os.IsExist(err) {
return nil, fmt.Errorf("failed to create fake device file: %s", err) return nil, fmt.Errorf("failed to create fake device file: %s", err)
} }
f.Close() f.Close()
response.Mounts = append(response.Mounts, &pluginapi.Mount{ response.Mounts = append(response.Mounts, &pluginapi.Mount{
ContainerPath: fpath, ContainerPath: fpath,
HostPath: fpath, HostPath: fpath,
}) })
}
responses.ContainerResponses = append(responses.ContainerResponses, response)
} }
return &response, nil return &responses, nil
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment