From ab2637e06067a2ef52378d55194503d77921b75d Mon Sep 17 00:00:00 2001 From: Jack Yu Date: Wed, 22 May 2024 18:14:05 +0800 Subject: [PATCH] refactor: fix codefactory and golint Signed-off-by: Jack Yu --- .golangci.json | 3 +- pkg/controller/setup.go | 5 +- .../usbdevice/usbdevice_controller.go | 101 ++++++++++-------- pkg/deviceplugins/usb_device_plugin.go | 10 +- pkg/util/gousb/codegen/load_data.go.tpl | 5 +- pkg/util/gousb/usbid/load_data.go | 7 +- pkg/webhook/usbdeviceclaim.go | 4 +- pkg/webhook/usbdeviceclaim_test.go | 2 +- 8 files changed, 77 insertions(+), 60 deletions(-) diff --git a/.golangci.json b/.golangci.json index a0ecd143..2ebc4c1b 100644 --- a/.golangci.json +++ b/.golangci.json @@ -13,7 +13,8 @@ "issues": { "exclude-files": [ "/zz_generated_", - "_generated" + "_generated", + "load_data.go" ], "exclude-dirs": [ "generated" diff --git a/pkg/controller/setup.go b/pkg/controller/setup.go index 180501ca..71a09dad 100644 --- a/pkg/controller/setup.go +++ b/pkg/controller/setup.go @@ -5,6 +5,7 @@ import ( "fmt" "time" + ctlnetwork "github.com/harvester/harvester-network-controller/pkg/generated/controllers/network.harvesterhci.io" "github.com/rancher/lasso/pkg/cache" "github.com/rancher/lasso/pkg/client" "github.com/rancher/lasso/pkg/controller" @@ -17,14 +18,12 @@ import ( "k8s.io/client-go/util/workqueue" "kubevirt.io/client-go/kubecli" - ctlnetwork "github.com/harvester/harvester-network-controller/pkg/generated/controllers/network.harvesterhci.io" - "github.com/harvester/pcidevices/pkg/controller/usbdevice" - "github.com/harvester/pcidevices/pkg/controller/gpudevice" "github.com/harvester/pcidevices/pkg/controller/nodecleanup" "github.com/harvester/pcidevices/pkg/controller/nodes" "github.com/harvester/pcidevices/pkg/controller/pcideviceclaim" "github.com/harvester/pcidevices/pkg/controller/sriovdevice" + "github.com/harvester/pcidevices/pkg/controller/usbdevice" "github.com/harvester/pcidevices/pkg/crd" ctl "github.com/harvester/pcidevices/pkg/generated/controllers/devices.harvesterhci.io" "github.com/harvester/pcidevices/pkg/webhook" diff --git a/pkg/controller/usbdevice/usbdevice_controller.go b/pkg/controller/usbdevice/usbdevice_controller.go index fe62ebd7..ed7b5177 100644 --- a/pkg/controller/usbdevice/usbdevice_controller.go +++ b/pkg/controller/usbdevice/usbdevice_controller.go @@ -79,7 +79,7 @@ func (h *Handler) OnDeviceChange(_ string, _ string, obj runtime.Object) ([]rela func (h *Handler) ReconcileUSBDevices() error { nodeName := cl.nodeName - err, localUSBDevices := deviceplugins.WalkUSBDevices() + localUSBDevices, err := deviceplugins.WalkUSBDevices() if err != nil { logrus.Errorf("failed to walk USB devices: %v\n", err) return err @@ -96,6 +96,62 @@ func (h *Handler) ReconcileUSBDevices() error { mapStoredUSBDevices[storedUSBDevice.Status.DevicePath] = storedUSBDevice } + createList, updateList := h.getList(localUSBDevices, mapStoredUSBDevices, nodeName) + + err = h.handleList(createList, updateList, mapStoredUSBDevices) + if err != nil { + return err + } + + return nil +} + +func (h *Handler) handleList(createList []*v1beta1.USBDevice, updateList []*v1beta1.USBDevice, mapStoredUSBDevices map[string]v1beta1.USBDevice) error { + for _, usbDevice := range createList { + createdOne := &v1beta1.USBDevice{ + ObjectMeta: metav1.ObjectMeta{ + Name: usbDevice.Name, + Labels: usbDevice.Labels, + }, + } + + newOne, err := h.usbClient.Create(createdOne) + if err != nil { + logrus.Errorf("failed to create USB device: %v\n", err) + return err + } + + newOne.Status = usbDevice.Status + if _, err = h.usbClient.UpdateStatus(newOne); err != nil { + logrus.Errorf("failed to update new created USB device status: %v\n", err) + return err + } + } + + for _, usbDevice := range updateList { + if _, err := h.usbClient.UpdateStatus(usbDevice); err != nil { + logrus.Errorf("failed to update existed USB device status: %v\n", err) + return err + } + } + + // The left devices in mapStoredUSBDevices are not found in localUSBDevices, so we should delete them. + for _, usbDevice := range mapStoredUSBDevices { + if usbDevice.Status.Enabled { + logrus.Warningf("USB device %s is still enabled, but it's not discovered in local usb devices. Please check your node could detect that usb device, skippping delete.\n", usbDevice.Name) + continue + } + + if err := h.usbClient.Delete(usbDevice.Name, &metav1.DeleteOptions{}); err != nil { + logrus.Errorf("failed to delete USB device: %v\n", err) + return err + } + } + + return nil +} + +func (h *Handler) getList(localUSBDevices map[int][]*deviceplugins.USBDevice, mapStoredUSBDevices map[string]v1beta1.USBDevice, nodeName string) ([]*v1beta1.USBDevice, []*v1beta1.USBDevice) { var ( createList []*v1beta1.USBDevice updateList []*v1beta1.USBDevice @@ -142,48 +198,7 @@ func (h *Handler) ReconcileUSBDevices() error { } } - for _, usbDevice := range createList { - createdOne := &v1beta1.USBDevice{ - ObjectMeta: metav1.ObjectMeta{ - Name: usbDevice.Name, - Labels: usbDevice.Labels, - }, - } - - newOne, err := h.usbClient.Create(createdOne) - if err != nil { - logrus.Errorf("failed to create USB device: %v\n", err) - return err - } - - newOne.Status = usbDevice.Status - if _, err = h.usbClient.UpdateStatus(newOne); err != nil { - logrus.Errorf("failed to update new created USB device status: %v\n", err) - return err - } - } - - for _, usbDevice := range updateList { - if _, err := h.usbClient.UpdateStatus(usbDevice); err != nil { - logrus.Errorf("failed to update existed USB device status: %v\n", err) - return err - } - } - - // The left devices in mapStoredUSBDevices are not found in localUSBDevices, so we should delete them. - for _, usbDevice := range mapStoredUSBDevices { - if usbDevice.Status.Enabled { - logrus.Warningf("USB device %s is still enabled, but it's not discovered in local usb devices. Please check your node could detect that usb device, skippping delete.\n", usbDevice.Name) - continue - } - - if err := h.usbClient.Delete(usbDevice.Name, &metav1.DeleteOptions{}); err != nil { - logrus.Errorf("failed to delete USB device: %v\n", err) - return err - } - } - - return nil + return createList, updateList } func usbDeviceName(nodeName string, localUSBDevice *deviceplugins.USBDevice) string { diff --git a/pkg/deviceplugins/usb_device_plugin.go b/pkg/deviceplugins/usb_device_plugin.go index f0c1a37e..1494d4c7 100644 --- a/pkg/deviceplugins/usb_device_plugin.go +++ b/pkg/deviceplugins/usb_device_plugin.go @@ -38,7 +38,6 @@ import ( "k8s.io/apimachinery/pkg/util/rand" v1 "kubevirt.io/api/core/v1" "kubevirt.io/client-go/log" - "kubevirt.io/kubevirt/pkg/safepath" "kubevirt.io/kubevirt/pkg/util" pluginapi "kubevirt.io/kubevirt/pkg/virt-handler/device-manager/deviceplugin/v1beta1" @@ -469,10 +468,11 @@ func (l *LocalDevices) remove(usbdevs []*USBDevice) { // return a list of USBDevices while removing it from the list of local devices func (l *LocalDevices) fetch(selectors []v1.USBSelector) ([]*USBDevice, bool) { - usbdevs := []*USBDevice{} + var usbdevs []*USBDevice // we have to find all devices under this resource name for _, selector := range selectors { + selector := selector vendor, product, err := parseSelector(&selector) if err != nil { log.Log.Reason(err).Warningf("Failed to convert selector: %+v", selector) @@ -651,7 +651,7 @@ func parseSysUeventFile(path string) *USBDevice { return &u } -func WalkUSBDevices() (error, map[int][]*USBDevice) { +func WalkUSBDevices() (map[int][]*USBDevice, error) { usbDevices := make(map[int][]*USBDevice, 0) err := filepath.Walk("/sys/bus/usb/devices", func(path string, info os.FileInfo, err error) error { if err != nil { @@ -679,10 +679,10 @@ func WalkUSBDevices() (error, map[int][]*USBDevice) { }) if err != nil { - return err, nil + return nil, err } - return nil, usbDevices + return usbDevices, nil } func parseUSBSymLinkToPCIAddress(link string) string { diff --git a/pkg/util/gousb/codegen/load_data.go.tpl b/pkg/util/gousb/codegen/load_data.go.tpl index 81b7f00f..19411cd2 100644 --- a/pkg/util/gousb/codegen/load_data.go.tpl +++ b/pkg/util/gousb/codegen/load_data.go.tpl @@ -24,7 +24,8 @@ import "time" // LastUpdate stores the latest time that the library was updated. // // The baked-in data was last generated: -// {{.Generated}} +// +// {{.Generated}} var LastUpdate = time.Unix(0, {{.Generated.UnixNano}}) -const usbIDListData = `{{printf "%s" .Data}}` \ No newline at end of file +const usbIDListData = `{{printf "%s" .Data}}` diff --git a/pkg/util/gousb/usbid/load_data.go b/pkg/util/gousb/usbid/load_data.go index 8584d9e1..e6227235 100644 --- a/pkg/util/gousb/usbid/load_data.go +++ b/pkg/util/gousb/usbid/load_data.go @@ -24,8 +24,9 @@ import "time" // LastUpdate stores the latest time that the library was updated. // // The baked-in data was last generated: -// 2024-05-22 16:38:38.612281 +0800 CST m=+1.944060751 -var LastUpdate = time.Unix(0, 1716367118612281000) +// +// 2024-05-22 17:59:32.168051 +0800 CST m=+1.146710792 +var LastUpdate = time.Unix(0, 1716371972168051000) const usbIDListData = `# # List of USB ID's @@ -25654,4 +25655,4 @@ VT 0400 External Vendor Specific VT 0401 Composite Video VT 0402 S-Video VT 0403 Component Video -` \ No newline at end of file +` diff --git a/pkg/webhook/usbdeviceclaim.go b/pkg/webhook/usbdeviceclaim.go index 682e37cf..61ccb362 100644 --- a/pkg/webhook/usbdeviceclaim.go +++ b/pkg/webhook/usbdeviceclaim.go @@ -3,12 +3,12 @@ package webhook import ( "fmt" + kubevirtctl "github.com/harvester/harvester/pkg/generated/controllers/kubevirt.io/v1" + "github.com/harvester/harvester/pkg/webhook/types" "github.com/sirupsen/logrus" admissionregv1 "k8s.io/api/admissionregistration/v1" "k8s.io/apimachinery/pkg/runtime" - kubevirtctl "github.com/harvester/harvester/pkg/generated/controllers/kubevirt.io/v1" - "github.com/harvester/harvester/pkg/webhook/types" devicesv1beta1 "github.com/harvester/pcidevices/pkg/apis/devices.harvesterhci.io/v1beta1" ) diff --git a/pkg/webhook/usbdeviceclaim_test.go b/pkg/webhook/usbdeviceclaim_test.go index dbadbfa8..6bc06f59 100644 --- a/pkg/webhook/usbdeviceclaim_test.go +++ b/pkg/webhook/usbdeviceclaim_test.go @@ -3,11 +3,11 @@ package webhook import ( "testing" + harvesterfake "github.com/harvester/harvester/pkg/generated/clientset/versioned/fake" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kubevirtv1 "kubevirt.io/api/core/v1" - harvesterfake "github.com/harvester/harvester/pkg/generated/clientset/versioned/fake" devicesv1beta1 "github.com/harvester/pcidevices/pkg/apis/devices.harvesterhci.io/v1beta1" "github.com/harvester/pcidevices/pkg/util/fakeclients" )