Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Environment variable case issues when device name is not found. #70

Open
greg-bock opened this issue May 31, 2023 · 2 comments
Open

Environment variable case issues when device name is not found. #70

greg-bock opened this issue May 31, 2023 · 2 comments

Comments

@greg-bock
Copy link

If a device id is not present in the pci id file it defaults to using the device id as the device name. However device name normalization and capitalization for use in the environment variable only occurs in getDeviceName during pci id lookup. This results in the resource name staying lowercase as well as the resource name portion of the environment variable. Kubevirt expects an all uppercase environment variable.

Suggested fixes include:

+++ b/pkg/device_plugin/device_plugin.go
@@ -101,7 +101,7 @@ func createDevicePlugins() {
                deviceName := getDeviceName(k)
                if deviceName == "" {
                        log.Printf("Error: Could not find device name for device id: %s", k)
-                       deviceName = k
+                       deviceName = strings.ToUpper(k)
                }
                log.Printf("DP Name %s", deviceName)
                dp := NewGenericDevicePlugin(deviceName, "/sys/kernel/iommu_groups/", devs)
@@ -123,7 +123,7 @@ func createDevicePlugins() {
                }
                deviceName := getDeviceName(k)
                if deviceName == "" {
-                       deviceName = k
+                       deviceName = strings.ToUpper(k)
                }
                log.Printf("DP Name %s", deviceName)
                dp := NewGenericVGpuDevicePlugin(deviceName, vGpuBasePath, devs)

or possibly

+++ b/pkg/device_plugin/device_plugin.go
@@ -99,10 +99,6 @@ func createDevicePlugins() {
                        })
                }
                deviceName := getDeviceName(k)
-               if deviceName == "" {
-                       log.Printf("Error: Could not find device name for device id: %s", k)
-                       deviceName = k
-               }
                log.Printf("DP Name %s", deviceName)
                dp := NewGenericDevicePlugin(deviceName, "/sys/kernel/iommu_groups/", devs)
                err := startDevicePlugin(dp)
@@ -122,9 +118,6 @@ func createDevicePlugins() {
                        })
                }
                deviceName := getDeviceName(k)
-               if deviceName == "" {
-                       deviceName = k
-               }
                log.Printf("DP Name %s", deviceName)
                dp := NewGenericVGpuDevicePlugin(deviceName, vGpuBasePath, devs)
                err := startVgpuDevicePlugin(dp)
@@ -319,7 +312,6 @@ func getDeviceName(deviceID string) string {
                                continue
                        }
                        deviceName = strings.TrimSpace(splits[1])
-                       deviceName = strings.ToUpper(deviceName)
                        deviceName = strings.Replace(deviceName, "/", "_", -1)
                        deviceName = strings.Replace(deviceName, ".", "_", -1)
                        reg, _ := regexp.Compile("\\s+")
@@ -333,5 +325,13 @@ func getDeviceName(deviceID string) string {
        if err := scanner.Err(); err != nil {
                log.Printf("Error reading pci ids file %s", err)
        }
+
+       if deviceName == "" {
+               log.Printf("Error: Could not find device name for device id: %s", deviceID)
+               deviceName = deviceID
+       }
+
+       deviceName = strings.ToUpper(deviceName)
+
        return deviceName
 }
@rthallisey
Copy link
Collaborator

@greg-bock can you open a PR? I'll review and get it merged, thanks.

@greg-bock
Copy link
Author

We've decided to use the internal kubevirt device plugin but I thought I'd drop a note since I ran across this during some testing. I'm unsure which method would be preferred and I have to focus my efforts elsewhere (depending on the fix some test cases will also need to be adjusted and/or added). I should also note that the capitalization test Returns the device name from pci.ids in capital letters test in pkg/device_plugin/device_plugin_test.go is using a device name that is already in all caps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants