Fix the device spec file creation in rdma exclusive mode

when using exclusive mode in RDMA if we restart the device plugin it will find the right rdma device if the device is allocated. this change makes the file creation when we get a new allocation and not in the device plugin initialization Signed-off-by: Sebastian Sch <[email protected]>
SchSeba · Dec 9, 2024 · d7bd803 · d7bd803
1 parent a380ca5
commit d7bd803
Show file tree

Hide file tree

Showing 29 changed files with 98 additions and 84 deletions.
diff --git a/pkg/cdi/mocks/CDI.go b/pkg/cdi/mocks/CDI.go
diff --git a/pkg/infoprovider/rdmaInfoProvider.go b/pkg/infoprovider/rdmaInfoProvider.go
@@ -54,6 +54,7 @@ func (ip *rdmaInfoProvider) GetDeviceSpecs() []*pluginapi.DeviceSpec {
 	}
 
 	devsSpec := ip.rdmaSpec.GetRdmaDeviceSpec()
+	glog.Infof("GetDeviceSpecs(): GetRdmaDeviceSpec returned %v", devsSpec)
 	return devsSpec
 }
 

diff --git a/pkg/netdevice/netResourcePool.go b/pkg/netdevice/netResourcePool.go
@@ -68,9 +68,13 @@ func (rp *netResourcePool) GetDeviceSpecs(deviceIDs []string) []*pluginapi.Devic
 
 // StoreDeviceInfoFile stores the Device Info files according to the
 // k8snetworkplumbingwg/device-info-spec
-func (rp *netResourcePool) StoreDeviceInfoFile(resourceNamePrefix string) error {
+// for the requested deviceIDs
+func (rp *netResourcePool) StoreDeviceInfoFile(resourceNamePrefix string, deviceIDs []string) error {
 	var devInfo nettypes.DeviceInfo
-	for id, dev := range rp.GetDevicePool() {
+	devicePool := rp.GetDevicePool()
+
+	for _, id := range deviceIDs {
+		dev := devicePool[id]
 		netDev, ok := dev.(types.PciNetDevice)
 		if !ok {
 			return fmt.Errorf("storeDeviceInfoFile: Only pciNetDevices are supported")

diff --git a/pkg/netdevice/netResourcePool_test.go b/pkg/netdevice/netResourcePool_test.go
@@ -150,7 +150,7 @@ var _ = Describe("NetResourcePool", func() {
 						return nil
 					})
 				rp := netdevice.NewNetResourcePool(nadutils, rc, pcis)
-				err := rp.StoreDeviceInfoFile("fakeOrg.io")
+				err := rp.StoreDeviceInfoFile("fakeOrg.io", []string{"fake1", "fake2"})
 				nadutils.AssertExpectations(t)
 				Expect(err).ToNot(HaveOccurred())
 			})
@@ -217,7 +217,7 @@ var _ = Describe("NetResourcePool", func() {
 						return nil
 					})
 				rp := netdevice.NewNetResourcePool(nadutils, rc, pcis)
-				err := rp.StoreDeviceInfoFile("fakeOrg.io")
+				err := rp.StoreDeviceInfoFile("fakeOrg.io", []string{"fake1", "fake2"})
 				Expect(err).ToNot(HaveOccurred())
 				nadutils.AssertExpectations(t)
 			})

diff --git a/pkg/resources/pool_stub.go b/pkg/resources/pool_stub.go
@@ -167,7 +167,7 @@ func (rp *ResourcePoolImpl) GetDevicePool() map[string]types.HostDevice {
 
 // StoreDeviceInfoFile does nothing. DeviceType-specific ResourcePools might
 // store information according to the k8snetworkplumbingwg/device-info-spec
-func (rp *ResourcePoolImpl) StoreDeviceInfoFile(resourceNamePrefix string) error {
+func (rp *ResourcePoolImpl) StoreDeviceInfoFile(resourceNamePrefix string, deviceIDs []string) error {
 	return nil
 }
 

diff --git a/pkg/resources/server.go b/pkg/resources/server.go
@@ -145,6 +145,12 @@ func (rs *resourceServer) Allocate(ctx context.Context, rqt *pluginapi.AllocateR
 			containerResp.Mounts = rs.resourcePool.GetMounts(container.DevicesIDs)
 		}
 
+		err = rs.resourcePool.StoreDeviceInfoFile(rs.resourceNamePrefix, container.DevicesIDs)
+		if err != nil {
+			glog.Errorf("failed to store device info file for device IDs %v: %v", container.DevicesIDs, err)
+			return nil, err
+		}
+
 		containerResp.Envs = envs
 		resp.ContainerResponses = append(resp.ContainerResponses, containerResp)
 	}
@@ -247,10 +253,6 @@ func (rs *resourceServer) Start() error {
 	resourceName := rs.resourcePool.GetResourceName()
 	_ = rs.cleanUp() // try tp clean up and continue
 
-	if err := rs.resourcePool.StoreDeviceInfoFile(rs.resourceNamePrefix); err != nil {
-		glog.Errorf("%s: error creating DeviceInfo File: %s", rs.resourcePool.GetResourceName(), err.Error())
-	}
-
 	glog.Infof("starting %s device plugin endpoint at: %s\n", resourceName, rs.endPoint)
 	lis, err := net.Listen(unix, rs.sockPath)
 	if err != nil {

diff --git a/pkg/resources/server_test.go b/pkg/resources/server_test.go
@@ -61,7 +61,6 @@ var _ = Describe("Server", func() {
 			rp := mocks.ResourcePool{}
 			rp.On("Probe").Return(false)
 			rp.On("GetResourceName").Return("fakename")
-			rp.On("StoreDeviceInfoFile", "fakeprefix").Return(nil)
 			rp.On("CleanDeviceInfoFile", "fakeprefix").Return(nil)
 
 			// Use faked dir as socket dir
@@ -78,7 +77,6 @@ var _ = Describe("Server", func() {
 				if shouldEnablePluginWatch {
 					_ = rs.Start()
 					rp.AssertCalled(t, "CleanDeviceInfoFile", "fakeprefix")
-					rp.AssertCalled(t, "StoreDeviceInfoFile", "fakeprefix")
 				} else {
 					_ = os.MkdirAll(pluginapi.DevicePluginPath, 0755)
 					registrationServer.start()
@@ -153,7 +151,6 @@ var _ = Describe("Server", func() {
 					On("DiscoverDevices").Return(nil).
 					On("GetDevices").Return(map[string]*pluginapi.Device{}).
 					On("Probe").Return(true).
-					On("StoreDeviceInfoFile", "fake").Return(nil).
 					On("CleanDeviceInfoFile", "fake").Return(nil)
 
 				// Create ResourceServer with plugin watch mode disabled
@@ -195,7 +192,6 @@ var _ = Describe("Server", func() {
 					On("DiscoverDevices").Return(nil).
 					On("GetDevices").Return(map[string]*pluginapi.Device{}).
 					On("Probe").Return(true).
-					On("StoreDeviceInfoFile", "fake").Return(nil).
 					On("CleanDeviceInfoFile", "fake").Return(nil)
 				// Create ResourceServer with plugin watch mode enabled
 				rs := NewResourceServer("fake", "fake", true, false, &rp).(*resourceServer)
@@ -230,7 +226,6 @@ var _ = Describe("Server", func() {
 					On("DiscoverDevices").Return(nil).
 					On("GetDevices").Return(map[string]*pluginapi.Device{}).
 					On("Probe").Return(true).
-					On("StoreDeviceInfoFile", "fake").Return(nil).
 					On("CleanDeviceInfoFile", "fake").Return(nil)
 
 				// Create ResourceServer with plugin watch mode disabled
@@ -274,7 +269,9 @@ var _ = Describe("Server", func() {
 				On("GetEnvs", "fake.com", []string{"00:00.01"}).
 				Return(map[string]string{"PCIDEVICE_FAKE_COM_FAKE_INFO": "{\"00:00.01\":{\"netdevice\":{\"pci\":\"00:00.01\"}}}"}, nil).
 				On("GetMounts", []string{"00:00.01"}).
-				Return([]*pluginapi.Mount{{ContainerPath: "/dev/fake", HostPath: "/dev/fake", ReadOnly: false}})
+				Return([]*pluginapi.Mount{{ContainerPath: "/dev/fake", HostPath: "/dev/fake", ReadOnly: false}}).
+				On("StoreDeviceInfoFile", "fake.com", []string{"00:00.01"}).
+				Return(nil)
 
 			rs := NewResourceServer("fake.com", "fake", true, false, &rp).(*resourceServer)
 
@@ -318,7 +315,9 @@ var _ = Describe("Server", func() {
 				On("GetMounts", []string{"00:00.01"}).
 				Return([]*pluginapi.Mount{{ContainerPath: "/dev/fake", HostPath: "/dev/fake", ReadOnly: false}}).
 				On("GetCDIName").
-				Return("fake.com")
+				Return("fake.com").
+				On("StoreDeviceInfoFile", "fake.com", []string{"00:00.01"}).
+				Return(nil)
 
 			rs := NewResourceServer("fake.com", "fake", true, true, &rp).(*resourceServer)
 

diff --git a/pkg/types/mocks/APIDevice.go b/pkg/types/mocks/APIDevice.go
diff --git a/pkg/types/mocks/AccelDevice.go b/pkg/types/mocks/AccelDevice.go
diff --git a/pkg/types/mocks/AuxNetDevice.go b/pkg/types/mocks/AuxNetDevice.go
diff --git a/pkg/types/mocks/DeviceInfoProvider.go b/pkg/types/mocks/DeviceInfoProvider.go
diff --git a/pkg/types/mocks/DeviceProvider.go b/pkg/types/mocks/DeviceProvider.go
diff --git a/pkg/types/mocks/DeviceSelector.go b/pkg/types/mocks/DeviceSelector.go
diff --git a/pkg/types/mocks/HostDevice.go b/pkg/types/mocks/HostDevice.go
diff --git a/pkg/types/mocks/LinkWatcher.go b/pkg/types/mocks/LinkWatcher.go
diff --git a/pkg/types/mocks/NadUtils.go b/pkg/types/mocks/NadUtils.go
diff --git a/pkg/types/mocks/NetDevice.go b/pkg/types/mocks/NetDevice.go
diff --git a/pkg/types/mocks/PciDevice.go b/pkg/types/mocks/PciDevice.go
diff --git a/pkg/types/mocks/PciNetDevice.go b/pkg/types/mocks/PciNetDevice.go
diff --git a/pkg/types/mocks/RdmaSpec.go b/pkg/types/mocks/RdmaSpec.go
diff --git a/pkg/types/mocks/ResourceFactory.go b/pkg/types/mocks/ResourceFactory.go
diff --git a/pkg/types/mocks/ResourcePool.go b/pkg/types/mocks/ResourcePool.go
diff --git a/pkg/types/mocks/ResourceServer.go b/pkg/types/mocks/ResourceServer.go
diff --git a/pkg/types/mocks/VdpaDevice.go b/pkg/types/mocks/VdpaDevice.go
diff --git a/pkg/types/types.go b/pkg/types/types.go
@@ -194,7 +194,7 @@ type ResourcePool interface {
 	GetDeviceSpecs(deviceIDs []string) []*pluginapi.DeviceSpec
 	GetEnvs(prefix string, deviceIDs []string) (map[string]string, error)
 	GetMounts(deviceIDs []string) []*pluginapi.Mount
-	StoreDeviceInfoFile(resourceNamePrefix string) error
+	StoreDeviceInfoFile(resourceNamePrefix string, deviceIDs []string) error
 	CleanDeviceInfoFile(resourceNamePrefix string) error
 	GetCDIName() string
 }

diff --git a/pkg/utils/mocks/NetlinkProvider.go b/pkg/utils/mocks/NetlinkProvider.go
diff --git a/pkg/utils/mocks/RdmaProvider.go b/pkg/utils/mocks/RdmaProvider.go
-Original file line number
+Diff line change
@@ Expand Up @@
     	}
     	devsSpec := ip.rdmaSpec.GetRdmaDeviceSpec()
+    	glog.Infof("GetDeviceSpecs(): GetRdmaDeviceSpec returned %v", devsSpec)
     	return devsSpec
     }
@@ Expand Down @@