diff --git a/cmd/fpga_plugin/fpga_plugin.go b/cmd/fpga_plugin/fpga_plugin.go index eb58ec5d..a5c6140d 100644 --- a/cmd/fpga_plugin/fpga_plugin.go +++ b/cmd/fpga_plugin/fpga_plugin.go @@ -50,8 +50,8 @@ const ( unhealthyAfuID = "ffffffffffffffffffffffffffffffff" unhealthyInterfaceID = "ffffffffffffffffffffffffffffffff" - // Frequency of device scans - scanFrequency = 5 * time.Second + // Period of device scans + scanPeriod = 5 * time.Second ) type getDevTreeFunc func(devices []device) dpapi.DeviceTree @@ -206,7 +206,7 @@ func newDevicePlugin(mode string, rootPath string) (*devicePlugin, error) { return nil, err } - dp.scanTicker = time.NewTicker(scanFrequency) + dp.scanTicker = time.NewTicker(scanPeriod) dp.scanDone = make(chan bool, 1) // buffered as we may send to it before Scan starts receiving from it return dp, nil diff --git a/cmd/gpu_plugin/gpu_plugin.go b/cmd/gpu_plugin/gpu_plugin.go index 7d6cf5ca..59bb6aa5 100644 --- a/cmd/gpu_plugin/gpu_plugin.go +++ b/cmd/gpu_plugin/gpu_plugin.go @@ -42,6 +42,9 @@ const ( // Device plugin settings. namespace = "gpu.intel.com" deviceType = "i915" + + // Period of device scans + scanPeriod = 5 * time.Second ) type devicePlugin struct { @@ -52,6 +55,9 @@ type devicePlugin struct { gpuDeviceReg *regexp.Regexp controlDeviceReg *regexp.Regexp + + scanTicker *time.Ticker + scanDone chan bool } func newDevicePlugin(sysfsDir, devfsDir string, sharedDevNum int) *devicePlugin { @@ -61,10 +67,13 @@ func newDevicePlugin(sysfsDir, devfsDir string, sharedDevNum int) *devicePlugin sharedDevNum: sharedDevNum, gpuDeviceReg: regexp.MustCompile(gpuDeviceRE), controlDeviceReg: regexp.MustCompile(controlDeviceRE), + scanTicker: time.NewTicker(scanPeriod), + scanDone: make(chan bool, 1), // buffered as we may send to it before Scan starts receiving from it } } func (dp *devicePlugin) Scan(notifier dpapi.Notifier) error { + defer dp.scanTicker.Stop() var previouslyFound int = -1 for { @@ -81,7 +90,11 @@ func (dp *devicePlugin) Scan(notifier dpapi.Notifier) error { notifier.Notify(devTree) - time.Sleep(5 * time.Second) + select { + case <-dp.scanDone: + return nil + case <-dp.scanTicker.C: + } } } diff --git a/cmd/gpu_plugin/gpu_plugin_test.go b/cmd/gpu_plugin/gpu_plugin_test.go index 5f0695c6..9f10ce77 100644 --- a/cmd/gpu_plugin/gpu_plugin_test.go +++ b/cmd/gpu_plugin/gpu_plugin_test.go @@ -16,56 +16,73 @@ package main import ( "flag" - "fmt" "io/ioutil" "os" "path" "testing" - "time" + + dpapi "github.com/intel/intel-device-plugins-for-kubernetes/pkg/deviceplugin" ) func init() { flag.Set("v", "4") //Enable debug output } +// mockNotifier implements Notifier interface. +type mockNotifier struct { + scanDone chan bool + devCount int +} + +// Notify stops plugin Scan +func (n *mockNotifier) Notify(newDeviceTree dpapi.DeviceTree) { + n.devCount = len(newDeviceTree[deviceType]) + n.scanDone <- true +} + func TestScan(t *testing.T) { - tmpdir := fmt.Sprintf("/tmp/gpuplugin-test-%d", time.Now().Unix()) - sysfs := path.Join(tmpdir, "sysfs") - devfs := path.Join(tmpdir, "devfs") + root, err := ioutil.TempDir("", "test_new_device_plugin") + if err != nil { + t.Fatalf("can't create temporary directory: %+v", err) + } + + defer os.RemoveAll(root) + + sysfs := path.Join(root, "sys") + devfs := path.Join(root, "dev") + tcases := []struct { + name string devfsdirs []string sysfsdirs []string sysfsfiles map[string][]byte expectedDevs int - expectedErr bool }{ { - expectedErr: true, - expectedDevs: 0, + name: "no sysfs mounted", }, { - sysfsdirs: []string{"card0"}, - expectedDevs: 0, - expectedErr: false, + name: "no device installed", + sysfsdirs: []string{"card0"}, }, { + name: "missing dev node", sysfsdirs: []string{"card0/device"}, sysfsfiles: map[string][]byte{ "card0/device/vendor": []byte("0x8086"), }, - expectedDevs: 0, - expectedErr: true, }, { - sysfsdirs: []string{"card0/device/drm/card0"}, + name: "all is correct", + sysfsdirs: []string{"card0/device/drm/card0", "card0/device/drm/controlD64"}, sysfsfiles: map[string][]byte{ "card0/device/vendor": []byte("0x8086"), }, devfsdirs: []string{"card0"}, expectedDevs: 1, - expectedErr: false, }, { + name: "two sysfs records but one dev node", sysfsdirs: []string{ "card0/device/drm/card0", "card1/device/drm/card1", @@ -76,64 +93,53 @@ func TestScan(t *testing.T) { }, devfsdirs: []string{"card0"}, expectedDevs: 1, - expectedErr: false, }, { + name: "wrong vendor", sysfsdirs: []string{"card0/device/drm/card0"}, sysfsfiles: map[string][]byte{ "card0/device/vendor": []byte("0xbeef"), }, - devfsdirs: []string{"card0"}, - expectedDevs: 0, - expectedErr: false, + devfsdirs: []string{"card0"}, }, { - sysfsdirs: []string{"non_gpu_card"}, - expectedDevs: 0, - expectedErr: false, + name: "no sysfs records", + sysfsdirs: []string{"non_gpu_card"}, }, } - testPlugin := newDevicePlugin(sysfs, devfs, 1) - - if testPlugin == nil { - t.Fatal("Failed to create a deviceManager") - } - - for _, tcase := range tcases { - for _, devfsdir := range tcase.devfsdirs { - err := os.MkdirAll(path.Join(devfs, devfsdir), 0755) - if err != nil { - t.Fatalf("Failed to create fake device directory: %+v", err) + for _, tc := range tcases { + t.Run(tc.name, func(t *testing.T) { + for _, devfsdir := range tc.devfsdirs { + if err := os.MkdirAll(path.Join(devfs, devfsdir), 0755); err != nil { + t.Fatalf("Failed to create fake device directory: %+v", err) + } } - } - for _, sysfsdir := range tcase.sysfsdirs { - err := os.MkdirAll(path.Join(sysfs, sysfsdir), 0755) - if err != nil { - t.Fatalf("Failed to create fake device directory: %+v", err) + for _, sysfsdir := range tc.sysfsdirs { + if err := os.MkdirAll(path.Join(sysfs, sysfsdir), 0755); err != nil { + t.Fatalf("Failed to create fake device directory: %+v", err) + } } - } - for filename, body := range tcase.sysfsfiles { - err := ioutil.WriteFile(path.Join(sysfs, filename), body, 0644) - if err != nil { - t.Fatalf("Failed to create fake vendor file: %+v", err) + for filename, body := range tc.sysfsfiles { + if err := ioutil.WriteFile(path.Join(sysfs, filename), body, 0644); err != nil { + t.Fatalf("Failed to create fake vendor file: %+v", err) + } } - } - tree, err := testPlugin.scan() - if tcase.expectedErr && err == nil { - t.Error("Expected error hasn't been triggered") - } - if !tcase.expectedErr && err != nil { - t.Errorf("Unexpcted error: %+v", err) - } - if tcase.expectedDevs != len(tree[deviceType]) { - t.Errorf("Wrong number of discovered devices") - } + plugin := newDevicePlugin(sysfs, devfs, 1) - err = os.RemoveAll(tmpdir) - if err != nil { - t.Fatalf("Failed to remove fake device directory: %+v", err) - } + notifier := &mockNotifier{ + scanDone: plugin.scanDone, + } + + err := plugin.Scan(notifier) + // Scans in GPU plugin never fail + if err != nil { + t.Errorf("unexpected error: %+v", err) + } + if tc.expectedDevs != notifier.devCount { + t.Errorf("Wrong number of discovered devices") + } + }) } }