Add Build/Test/Lint/go mod/vendor consistency workflow

.golangci.yml

@@ -34,8 +34,6 @@ linters-settings:
     min-complexity: 15
   goimports:
     local-prefixes: github.com/k8snetworkplumbingwg/sriov-network-device-plugin
-  golint:
-    min-confidence: 0
   gomnd:
     settings:
       mnd:
@@ -72,7 +70,6 @@ linters:
     - gocyclo
     - gofmt
     - goimports
-    - golint
     - gomnd
     - goprintffuncname
     - gosec
@@ -103,6 +100,9 @@ issues:
         - gomnd
         - gosec
         - dupl
+        - lll
+        - stylecheck
+        - goconst
 
 run:
   skip-dirs:

cmd/sriovdp/main.go

@@ -81,10 +81,9 @@ func main() {
 	signal.Notify(sigCh, syscall.SIGHUP, syscall.SIGINT, syscall.SIGTERM, syscall.SIGQUIT)
 
 	// Catch termination signals
-	select {
-	case sig := <-sigCh:
-		glog.Infof("Received signal \"%v\", shutting down.", sig)
-		rm.stopAllServers()
-		return
+	sig := <-sigCh
+	glog.Infof("Received signal \"%v\", shutting down.", sig)
+	if err := rm.stopAllServers(); err != nil {
+		glog.Errorf("stopping servers produced error: %s", err.Error())
 	}
 }

cmd/sriovdp/manager.go

@@ -68,15 +68,13 @@ func newResourceManager(cp *cliParams) *resourceManager {
 	}
 }
 
-//readConfig reads and validate configurations from Config file
+// readConfig reads and validate configurations from Config file
 func (rm *resourceManager) readConfig() error {
-
 	resources := &types.ResourceConfList{}
 	rawBytes, err := ioutil.ReadFile(rm.configFile)
 
 	if err != nil {
 		return fmt.Errorf("error reading file %s, %v", rm.configFile, err)
-
 	}
 
 	glog.Infof("raw ResourceList: %s", rawBytes)
@@ -89,19 +87,15 @@ func (rm *resourceManager) readConfig() error {
 		// Validate deviceType
 		if conf.DeviceType == "" {
 			conf.DeviceType = types.NetDeviceType // Default to NetDeviceType
-		} else {
-			// Check if the DeviceType is supported
-			if _, ok := types.SupportedDevices[conf.DeviceType]; !ok {
-				return fmt.Errorf("unsupported deviceType:  \"%s\"", conf.DeviceType)
-			}
+		} else if _, ok := types.SupportedDevices[conf.DeviceType]; !ok {
+			return fmt.Errorf("unsupported deviceType:  \"%s\"", conf.DeviceType)
 		}
 		if conf.SelectorObj, err = rm.rFactory.GetDeviceFilter(conf); err == nil {
 			rm.configList = append(rm.configList, &resources.ResourceList[i])
 		} else {
 			glog.Warningf("unable to get SelectorObj from selectors list:'%s' for deviceType: %s error: %s",
 				*conf.Selectors, conf.DeviceType, err)
 		}
-
 	}
 	glog.Infof("unmarshalled ResourceList: %+v", resources.ResourceList)
 	return nil
@@ -182,7 +176,7 @@ func (rm *resourceManager) validConfigs() bool {
 			return false
 		}
 
-		// resourcePrefix might be overriden for a given resource pool
+		// resourcePrefix might be overridden for a given resource pool
 		resourcePrefix := rm.cliParams.resourcePrefix
 		if conf.ResourcePrefix != "" {
 			resourcePrefix = conf.ResourcePrefix
@@ -212,7 +206,6 @@ func (rm *resourceManager) validConfigs() bool {
 }
 
 func (rm *resourceManager) discoverHostDevices() error {
-
 	pci, err := ghw.PCI()
 	if err != nil {
 		return fmt.Errorf("discoverDevices(): error getting PCI info: %v", err)
@@ -225,7 +218,9 @@ func (rm *resourceManager) discoverHostDevices() error {
 
 	for k, v := range types.SupportedDevices {
 		if dp, ok := rm.deviceProviders[k]; ok {
-			dp.AddTargetDevices(devices, v)
+			if err := dp.AddTargetDevices(devices, v); err != nil {
+				glog.Errorf("adding supported device identifier '%d' to device provider failed: %s", v, err.Error())
+			}
 		}
 	}
 	return nil

cmd/sriovdp/manager_test.go

@@ -37,14 +37,6 @@ func TestSriovdp(t *testing.T) {
 	RunSpecs(t, "Sriovdp Suite")
 }
 
-func assertShouldFail(err error, shouldFail bool) {
-	if shouldFail {
-		Expect(err).To(HaveOccurred())
-	} else {
-		Expect(err).NotTo(HaveOccurred())
-	}
-}
-
 var _ = Describe("Resource manager", func() {
 	var (
 		cp *cliParams
@@ -60,7 +52,7 @@ var _ = Describe("Resource manager", func() {
 		})
 		Context("when there's an error reading file", func() {
 			BeforeEach(func() {
-				os.RemoveAll("/tmp/sriovdp")
+				_ = os.RemoveAll("/tmp/sriovdp")
 			})
 			It("should fail", func() {
 				err := rm.readConfig()
@@ -73,7 +65,7 @@ var _ = Describe("Resource manager", func() {
 				if err != nil {
 					panic(err)
 				}
-				ioutil.WriteFile("/tmp/sriovdp/test_config", []byte("junk"), 0644)
+				_ = ioutil.WriteFile("/tmp/sriovdp/test_config", []byte("junk"), 0644)
 			})
 			AfterEach(func() {
 				err := os.RemoveAll("/tmp/sriovdp")
@@ -181,7 +173,7 @@ var _ = Describe("Resource manager", func() {
 				if err != nil {
 					panic(err)
 				}
-				rm.readConfig()
+				_ = rm.readConfig()
 			})
 			It("should return false", func() {
 				defer fs.Use()()
@@ -215,7 +207,7 @@ var _ = Describe("Resource manager", func() {
 				if err != nil {
 					panic(err)
 				}
-				rm.readConfig()
+				_ = rm.readConfig()
 			})
 			It("should return false", func() {
 				defer fs.Use()()
@@ -292,15 +284,17 @@ var _ = Describe("Resource manager", func() {
 	DescribeTable("discovering devices",
 		func(fs *utils.FakeFilesystem) {
 			defer fs.Use()()
-			os.Setenv("GHW_CHROOT", fs.RootDir)
-			defer os.Unsetenv("GHW_CHROOT")
+			_ = os.Setenv("GHW_CHROOT", fs.RootDir)
+			defer func() {
+				_ = os.Unsetenv("GHW_CHROOT")
+			}()
 
 			rf := factory.NewResourceFactory("fake", "fake", true)
 
 			rm := &resourceManager{
 				rFactory: rf,
 				configList: []*types.ResourceConfig{
-					&types.ResourceConfig{
+					{
 						ResourceName: "fake",
 						DeviceType:   types.NetDeviceType,
 					},

go.mod

@@ -13,8 +13,8 @@ require (
 	github.com/pkg/errors v0.9.1
 	github.com/stretchr/testify v1.5.1
 	github.com/vishvananda/netlink v1.1.0
-	golang.org/x/net v0.0.0-20201021035429-f5854403a974
-	golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 // indirect
+	golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4
+	golang.org/x/sys v0.0.0-20210510120138-977fb7262007 // indirect
 	google.golang.org/grpc v1.28.1
 	k8s.io/kubelet v0.18.1
 )

go.sum

@@ -191,8 +191,9 @@ golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLL
 golang.org/x/net v0.0.0-20190827160401-ba9fcec4b297/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20191004110552-13f9640d40b9/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
-golang.org/x/net v0.0.0-20201021035429-f5854403a974 h1:IX6qOQeG5uLjB/hjjwjedwfjND0hgjPMMyO1RoIXQNI=
 golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
+golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4 h1:4nGaVu0QrbjT/AK2PRLuQfQuh6DJve+pELhqTdAj3x0=
+golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
 golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 h1:SVwTIAaPC2U/AvvLNZ2a7OVsmBpC8L5BlwK1whH3hm0=
@@ -215,8 +216,11 @@ golang.org/x/sys v0.0.0-20190616124812-15dcb6c0061f/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20191022100944-742c48ecaeb7/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20191120155948-bd437916bb0e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k=
-golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20210510120138-977fb7262007 h1:gG67DSER+11cZvqIMb8S8bt0vZtiN6xWYARwirrOSfE=
+golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/text v0.0.0-20160726164857-2910a502d2bf/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=

pkg/accelerator/accelDevice.go

@@ -28,7 +28,6 @@ type accelDevice struct {
 
 // NewAccelDevice returns an instance of AccelDevice interface
 func NewAccelDevice(dev *ghw.PCIDevice, rFactory types.ResourceFactory) (types.AccelDevice, error) {
-
 	pciDev, err := resources.NewPciDevice(dev, rFactory, nil)
 	if err != nil {
 		return nil, err

pkg/accelerator/accelDeviceProvider.go

@@ -24,6 +24,13 @@ import (
 	"github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/types"
 )
 
+const (
+	classIDBaseInt = 16
+	classIDBitSize = 64
+	maxVendorName  = 20
+	maxProductName = 40
+)
+
 type accelDeviceProvider struct {
 	deviceList []*ghw.PCIDevice
 	rFactory   types.ResourceFactory
@@ -54,9 +61,8 @@ func (ap *accelDeviceProvider) GetDevices(rc *types.ResourceConfig) []types.PciD
 }
 
 func (ap *accelDeviceProvider) AddTargetDevices(devices []*ghw.PCIDevice, deviceCode int) error {
-
 	for _, device := range devices {
-		devClass, err := strconv.ParseInt(device.Class.ID, 16, 64)
+		devClass, err := strconv.ParseInt(device.Class.ID, classIDBaseInt, classIDBitSize)
 		if err != nil {
 			glog.Warningf("accelerator AddTargetDevices(): unable to parse device class for device %+v %q", device, err)
 			continue
@@ -65,15 +71,16 @@ func (ap *accelDeviceProvider) AddTargetDevices(devices []*ghw.PCIDevice, device
 		if devClass == int64(deviceCode) {
 			vendor := device.Vendor
 			vendorName := vendor.Name
-			if len(vendor.Name) > 20 {
+			if len(vendor.Name) > maxVendorName {
 				vendorName = string([]byte(vendorName)[0:17]) + "..."
 			}
 			product := device.Product
 			productName := product.Name
-			if len(product.Name) > 40 {
+			if len(product.Name) > maxProductName {
 				productName = string([]byte(productName)[0:37]) + "..."
 			}
-			glog.Infof("accelerator AddTargetDevices(): device found: %-12s\t%-12s\t%-20s\t%-40s", device.Address, device.Class.ID, vendorName, productName)
+			glog.Infof("accelerator AddTargetDevices(): device found: %-12s\t%-12s\t%-20s\t%-40s", device.Address,
+				device.Class.ID, vendorName, productName)
 
 			ap.deviceList = append(ap.deviceList, device)
 		}
@@ -82,7 +89,6 @@ func (ap *accelDeviceProvider) AddTargetDevices(devices []*ghw.PCIDevice, device
 }
 
 func (ap *accelDeviceProvider) GetFilteredDevices(devices []types.PciDevice, rc *types.ResourceConfig) ([]types.PciDevice, error) {
-
 	filteredDevice := devices
 	af, ok := rc.SelectorObj.(*types.AccelDeviceSelectors)
 	if !ok {
@@ -120,9 +126,7 @@ func (ap *accelDeviceProvider) GetFilteredDevices(devices []types.PciDevice, rc
 
 	// convert to []AccelDevice to []PciDevice
 	newDeviceList := make([]types.PciDevice, len(filteredDevice))
-	for i, d := range filteredDevice {
-		newDeviceList[i] = d
-	}
+	copy(newDeviceList, filteredDevice)
 
 	return newDeviceList, nil
 }

pkg/accelerator/accelDeviceProvider_test.go

@@ -136,7 +136,7 @@ var _ = Describe("AcceleratorProvider", func() {
 				md := []string{"igb_uio", "igb_uio", "igb_uio", "iavf", "vfio-pci"}
 				pa := []string{"0000:03:02.0", "0000:03:02.1", "0000:03:02.2", "0000:03:02.3", "0000:03:02.4"}
 
-				for i, _ := range mocked {
+				for i := range mocked {
 					mocked[i].
 						On("GetVendor").Return(ve[i]).
 						On("GetDeviceCode").Return(de[i]).
@@ -154,7 +154,8 @@ var _ = Describe("AcceleratorProvider", func() {
 					{"vendors", &types.AccelDeviceSelectors{DeviceSelectors: types.DeviceSelectors{Vendors: []string{"8086"}}}, []types.PciDevice{all[0], all[1]}},
 					{"devices", &types.AccelDeviceSelectors{DeviceSelectors: types.DeviceSelectors{Devices: []string{"abcd"}}}, []types.PciDevice{all[0], all[2]}},
 					{"drivers", &types.AccelDeviceSelectors{DeviceSelectors: types.DeviceSelectors{Drivers: []string{"igb_uio"}}}, []types.PciDevice{all[0], all[1], all[2]}},
-					{"pciAddresses", &types.AccelDeviceSelectors{DeviceSelectors: types.DeviceSelectors{PciAddresses: []string{"0000:03:02.0", "0000:03:02.3"}}}, []types.PciDevice{all[0], all[3]}},
+					{"pciAddresses", &types.AccelDeviceSelectors{DeviceSelectors: types.DeviceSelectors{PciAddresses: []string{"0000:03:02.0", "0000:03:02.3"}}},
+						[]types.PciDevice{all[0], all[3]}},
 				}
 
 				for _, tc := range testCases {

pkg/accelerator/accelDevice_test.go

@@ -27,7 +27,7 @@ import (
 
 var _ = Describe("Accelerator", func() {
 	Describe("creating new accelerator device", func() {
-		Context("succesfully", func() {
+		Context("successfully", func() {
 			It("should return AccelDevice object instance", func() {
 				fs := &utils.FakeFilesystem{
 					Dirs: []string{

pkg/accelerator/accelResourcePool.go

@@ -31,7 +31,8 @@ type accelResourcePool struct {
 var _ types.ResourcePool = &accelResourcePool{}
 
 // NewAccelResourcePool returns an instance of resourcePool
-func NewAccelResourcePool(rc *types.ResourceConfig, apiDevices map[string]*pluginapi.Device, devicePool map[string]types.PciDevice) types.ResourcePool {
+func NewAccelResourcePool(rc *types.ResourceConfig, apiDevices map[string]*pluginapi.Device,
+	devicePool map[string]types.PciDevice) types.ResourcePool {
 	rp := resources.NewResourcePool(rc, apiDevices, devicePool)
 	s, _ := rc.SelectorObj.(*types.AccelDeviceSelectors)
 	return &accelResourcePool{
@@ -50,15 +51,12 @@ func (rp *accelResourcePool) GetDeviceSpecs(deviceIDs []string) []*pluginapi.Dev
 	// Add device driver specific devices
 	for _, id := range deviceIDs {
 		if dev, ok := devicePool[id]; ok {
-			accelDev := dev.(types.AccelDevice) // convert generic PciDevice to AccelDevice
-			newSpecs := accelDev.GetDeviceSpecs()
+			newSpecs := dev.GetDeviceSpecs()
 			for _, ds := range newSpecs {
 				if !rp.DeviceSpecExist(devSpecs, ds) {
 					devSpecs = append(devSpecs, ds)
 				}
-
 			}
-
 		}
 	}
 	return devSpecs

pkg/accelerator/accelResourcePool_test.go

@@ -15,14 +15,14 @@
 package accelerator_test
 
 import (
-	"github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/accelerator"
-	"github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/types"
-	"github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/types/mocks"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
 	"k8s.io/kubelet/pkg/apis/deviceplugin/v1beta1"
 	pluginapi "k8s.io/kubelet/pkg/apis/deviceplugin/v1beta1"
 
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/gomega"
+	"github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/accelerator"
+	"github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/types"
+	"github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pkg/types/mocks"
 )
 
 var _ = Describe("AccelResourcePool", func() {
@@ -48,15 +48,15 @@ var _ = Describe("AccelResourcePool", func() {
 			// fake1 will have 2 device specs
 			fake1 := &mocks.AccelDevice{}
 			fake1ds := []*pluginapi.DeviceSpec{
-				&pluginapi.DeviceSpec{ContainerPath: "/fake/path", HostPath: "/dev/fake1a"},
-				&pluginapi.DeviceSpec{ContainerPath: "/fake/path", HostPath: "/dev/fake1b"},
+				{ContainerPath: "/fake/path", HostPath: "/dev/fake1a"},
+				{ContainerPath: "/fake/path", HostPath: "/dev/fake1b"},
 			}
 			fake1.On("GetDeviceSpecs").Return(fake1ds)
 
 			// fake2 will have 1 device spec
 			fake2 := &mocks.AccelDevice{}
 			fake2ds := []*pluginapi.DeviceSpec{
-				&pluginapi.DeviceSpec{ContainerPath: "/fake/path", HostPath: "/dev/fake2"},
+				{ContainerPath: "/fake/path", HostPath: "/dev/fake2"},
 			}
 			fake2.On("GetDeviceSpecs").Return(fake2ds)