diff --git a/pkg/kubelet/cm/devicemanager/manager.go b/pkg/kubelet/cm/devicemanager/manager.go index 8f35d87a761..5d1925f9458 100644 --- a/pkg/kubelet/cm/devicemanager/manager.go +++ b/pkg/kubelet/cm/devicemanager/manager.go @@ -709,7 +709,7 @@ func (m *ManagerImpl) devicesToAllocate(podUID, contName, resource string, requi if err != nil { return nil, err } - if allocateRemainingFrom(preferred.Intersection(aligned.Union(allocated))) { + if allocateRemainingFrom(preferred.Intersection(aligned)) { return allocated, nil } // Then fallback to allocate from the aligned set if no preferred list @@ -730,11 +730,11 @@ func (m *ManagerImpl) devicesToAllocate(podUID, contName, resource string, requi // Then give the plugin the chance to influence the decision on any // remaining devices to allocate. - preferred, err := m.callGetPreferredAllocationIfAvailable(podUID, contName, resource, available.Union(devices), devices, required) + preferred, err := m.callGetPreferredAllocationIfAvailable(podUID, contName, resource, available.Union(allocated), allocated, required) if err != nil { return nil, err } - if allocateRemainingFrom(preferred.Intersection(available.Union(allocated))) { + if allocateRemainingFrom(preferred.Intersection(available)) { return allocated, nil } @@ -997,8 +997,10 @@ func (m *ManagerImpl) callGetPreferredAllocationIfAvailable(podUID, contName, re if err != nil { return nil, fmt.Errorf("device plugin GetPreferredAllocation rpc failed with err: %v", err) } - // TODO: Add metrics support for init RPC - return sets.NewString(resp.ContainerResponses[0].DeviceIDs...), nil + if resp != nil && len(resp.ContainerResponses) > 0 { + return sets.NewString(resp.ContainerResponses[0].DeviceIDs...), nil + } + return sets.NewString(), nil } // sanitizeNodeAllocatable scans through allocatedDevices in the device manager diff --git a/pkg/kubelet/cm/devicemanager/topology_hints_test.go b/pkg/kubelet/cm/devicemanager/topology_hints_test.go index 3cbd271802b..3cd072183e8 100644 --- a/pkg/kubelet/cm/devicemanager/topology_hints_test.go +++ b/pkg/kubelet/cm/devicemanager/topology_hints_test.go @@ -791,3 +791,192 @@ func TestTopologyAlignedAllocation(t *testing.T) { } } } + +func TestGetPreferredAllocationParameters(t *testing.T) { + tcases := []struct { + description string + resource string + request int + allDevices []pluginapi.Device + allocatedDevices []string + reusableDevices []string + hint topologymanager.TopologyHint + expectedAvailable []string + expectedMustInclude []string + expectedSize int + }{ + { + description: "Request for 1, socket 0, 0 already allocated, 0 reusable", + resource: "resource", + request: 1, + allDevices: []pluginapi.Device{ + makeNUMADevice("Dev0", 0), + makeNUMADevice("Dev1", 0), + makeNUMADevice("Dev2", 0), + makeNUMADevice("Dev3", 0), + }, + allocatedDevices: []string{}, + reusableDevices: []string{}, + hint: topologymanager.TopologyHint{ + NUMANodeAffinity: makeSocketMask(0), + Preferred: true, + }, + expectedAvailable: []string{"Dev0", "Dev1", "Dev2", "Dev3"}, + expectedMustInclude: []string{}, + expectedSize: 1, + }, + { + description: "Request for 4, socket 0, 2 already allocated, 2 reusable", + resource: "resource", + request: 4, + allDevices: []pluginapi.Device{ + makeNUMADevice("Dev0", 0), + makeNUMADevice("Dev1", 0), + makeNUMADevice("Dev2", 0), + makeNUMADevice("Dev3", 0), + makeNUMADevice("Dev4", 0), + makeNUMADevice("Dev5", 0), + makeNUMADevice("Dev6", 0), + makeNUMADevice("Dev7", 0), + }, + allocatedDevices: []string{"Dev0", "Dev5"}, + reusableDevices: []string{"Dev0", "Dev5"}, + hint: topologymanager.TopologyHint{ + NUMANodeAffinity: makeSocketMask(0), + Preferred: true, + }, + expectedAvailable: []string{"Dev0", "Dev1", "Dev2", "Dev3", "Dev4", "Dev5", "Dev6", "Dev7"}, + expectedMustInclude: []string{"Dev0", "Dev5"}, + expectedSize: 4, + }, + { + description: "Request for 4, socket 0, 4 already allocated, 2 reusable", + resource: "resource", + request: 4, + allDevices: []pluginapi.Device{ + makeNUMADevice("Dev0", 0), + makeNUMADevice("Dev1", 0), + makeNUMADevice("Dev2", 0), + makeNUMADevice("Dev3", 0), + makeNUMADevice("Dev4", 0), + makeNUMADevice("Dev5", 0), + makeNUMADevice("Dev6", 0), + makeNUMADevice("Dev7", 0), + }, + allocatedDevices: []string{"Dev0", "Dev5", "Dev4", "Dev1"}, + reusableDevices: []string{"Dev0", "Dev5"}, + hint: topologymanager.TopologyHint{ + NUMANodeAffinity: makeSocketMask(0), + Preferred: true, + }, + expectedAvailable: []string{"Dev0", "Dev2", "Dev3", "Dev5", "Dev6", "Dev7"}, + expectedMustInclude: []string{"Dev0", "Dev5"}, + expectedSize: 4, + }, + { + description: "Request for 6, multisocket, 2 already allocated, 2 reusable", + resource: "resource", + request: 6, + allDevices: []pluginapi.Device{ + makeNUMADevice("Dev0", 0), + makeNUMADevice("Dev1", 0), + makeNUMADevice("Dev2", 0), + makeNUMADevice("Dev3", 0), + makeNUMADevice("Dev4", 1), + makeNUMADevice("Dev5", 1), + makeNUMADevice("Dev6", 1), + makeNUMADevice("Dev7", 1), + }, + allocatedDevices: []string{"Dev1", "Dev6"}, + reusableDevices: []string{"Dev1", "Dev6"}, + hint: topologymanager.TopologyHint{ + NUMANodeAffinity: makeSocketMask(0), + Preferred: true, + }, + expectedAvailable: []string{"Dev0", "Dev1", "Dev2", "Dev3", "Dev4", "Dev5", "Dev6", "Dev7"}, + expectedMustInclude: []string{"Dev0", "Dev1", "Dev2", "Dev3", "Dev6"}, + expectedSize: 6, + }, + { + description: "Request for 6, multisocket, 4 already allocated, 2 reusable", + resource: "resource", + request: 6, + allDevices: []pluginapi.Device{ + makeNUMADevice("Dev0", 0), + makeNUMADevice("Dev1", 0), + makeNUMADevice("Dev2", 0), + makeNUMADevice("Dev3", 0), + makeNUMADevice("Dev4", 1), + makeNUMADevice("Dev5", 1), + makeNUMADevice("Dev6", 1), + makeNUMADevice("Dev7", 1), + }, + allocatedDevices: []string{"Dev0", "Dev1", "Dev6", "Dev7"}, + reusableDevices: []string{"Dev1", "Dev6"}, + hint: topologymanager.TopologyHint{ + NUMANodeAffinity: makeSocketMask(0), + Preferred: true, + }, + expectedAvailable: []string{"Dev1", "Dev2", "Dev3", "Dev4", "Dev5", "Dev6"}, + expectedMustInclude: []string{"Dev1", "Dev2", "Dev3", "Dev6"}, + expectedSize: 6, + }, + } + for _, tc := range tcases { + m := ManagerImpl{ + allDevices: make(map[string]map[string]pluginapi.Device), + healthyDevices: make(map[string]sets.String), + allocatedDevices: make(map[string]sets.String), + endpoints: make(map[string]endpointInfo), + podDevices: make(podDevices), + sourcesReady: &sourcesReadyStub{}, + activePods: func() []*v1.Pod { return []*v1.Pod{} }, + topologyAffinityStore: &mockAffinityStore{tc.hint}, + } + + m.allDevices[tc.resource] = make(map[string]pluginapi.Device) + m.healthyDevices[tc.resource] = sets.NewString() + for _, d := range tc.allDevices { + m.allDevices[tc.resource][d.ID] = d + m.healthyDevices[tc.resource].Insert(d.ID) + } + + m.allocatedDevices[tc.resource] = sets.NewString() + for _, d := range tc.allocatedDevices { + m.allocatedDevices[tc.resource].Insert(d) + } + + actualAvailable := []string{} + actualMustInclude := []string{} + actualSize := 0 + m.endpoints[tc.resource] = endpointInfo{ + e: &MockEndpoint{ + getPreferredAllocationFunc: func(available, mustInclude []string, size int) (*pluginapi.PreferredAllocationResponse, error) { + actualAvailable = append(actualAvailable, available...) + actualMustInclude = append(actualMustInclude, mustInclude...) + actualSize = size + return nil, nil + }, + }, + opts: &pluginapi.DevicePluginOptions{GetPreferredAllocationAvailable: true}, + } + + _, err := m.devicesToAllocate("podUID", "containerName", tc.resource, tc.request, sets.NewString(tc.reusableDevices...)) + if err != nil { + t.Errorf("Unexpected error: %v", err) + continue + } + + if !sets.NewString(actualAvailable...).Equal(sets.NewString(tc.expectedAvailable...)) { + t.Errorf("%v. expected available: %v but got: %v", tc.description, tc.expectedAvailable, actualAvailable) + } + + if !sets.NewString(actualAvailable...).Equal(sets.NewString(tc.expectedAvailable...)) { + t.Errorf("%v. expected mustInclude: %v but got: %v", tc.description, tc.expectedMustInclude, actualMustInclude) + } + + if actualSize != tc.expectedSize { + t.Errorf("%v. expected size: %v but got: %v", tc.description, tc.expectedSize, actualSize) + } + } +}