From 7ff08ee8740bed0c289587f4f4114a8897b960fd Mon Sep 17 00:00:00 2001 From: Dmitry Rozhkov Date: Wed, 19 Aug 2020 17:44:29 +0300 Subject: [PATCH] linter: enable staticcheck --- .golangci.yml | 1 + cmd/qat_plugin/kerneldrv/kerneldrv.go | 3 +- cmd/vpu_plugin/vpu_plugin_test.go | 10 +++---- pkg/deviceplugin/api.go | 1 + pkg/deviceplugin/server.go | 24 ++++++++------- pkg/deviceplugin/server_test.go | 43 ++++++++++++++------------- pkg/topology/topology.go | 9 ++++-- pkg/topology/topology_test.go | 6 ++++ 8 files changed, 58 insertions(+), 39 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index d18f3c7e..47f97f4e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -29,6 +29,7 @@ linters: - nakedret - nolintlint - rowserrcheck + - staticcheck - structcheck - stylecheck - typecheck diff --git a/cmd/qat_plugin/kerneldrv/kerneldrv.go b/cmd/qat_plugin/kerneldrv/kerneldrv.go index e54da84d..7e32d285 100644 --- a/cmd/qat_plugin/kerneldrv/kerneldrv.go +++ b/cmd/qat_plugin/kerneldrv/kerneldrv.go @@ -14,6 +14,7 @@ // +build kerneldrv +// Package kerneldrv populates a device tree with QAT devices using the kernel QAT driver. package kerneldrv import ( @@ -199,7 +200,7 @@ func (dp *DevicePlugin) parseConfigs(devices []device) (map[string]section, erro devNum++ for _, section := range config.Sections() { - if section.Name() == "GENERAL" || section.Name() == "KERNEL" || section.Name() == "KERNEL_QAT" || section.Name() == ini.DEFAULT_SECTION { + if section.Name() == "GENERAL" || section.Name() == "KERNEL" || section.Name() == "KERNEL_QAT" || section.Name() == ini.DefaultSection { continue } klog.V(4).Info(section.Name()) diff --git a/cmd/vpu_plugin/vpu_plugin_test.go b/cmd/vpu_plugin/vpu_plugin_test.go index 08b31f53..8acab142 100644 --- a/cmd/vpu_plugin/vpu_plugin_test.go +++ b/cmd/vpu_plugin/vpu_plugin_test.go @@ -25,7 +25,7 @@ import ( ) func init() { - flag.Set("v", "4") + _ = flag.Set("v", "4") } type testCase struct { @@ -78,7 +78,7 @@ func TestScan(t *testing.T) { testPlugin := newDevicePlugin(tc, vendorID, productIDs, 10) if testPlugin == nil { - t.Error("vpu plugin test failed with newDevicePlugin().") + t.Fatal("vpu plugin test failed with newDevicePlugin().") } fN.scanDone = testPlugin.scanDone @@ -92,12 +92,12 @@ func TestScan(t *testing.T) { klog.V(4).Infof("tree len is %d", len(fN.tree[deviceType])) //remove the hddl_service.sock and test with no hddl socket case - f.Close() - os.Remove("/var/tmp/hddl_service.sock") + _ = f.Close() + _ = os.Remove("/var/tmp/hddl_service.sock") testPlugin = newDevicePlugin(tc, vendorID, productIDs, 10) if testPlugin == nil { - t.Error("vpu plugin test failed with newDevicePlugin() in no hddl_service.sock case.") + t.Fatal("vpu plugin test failed with newDevicePlugin() in no hddl_service.sock case.") } fN.scanDone = testPlugin.scanDone diff --git a/pkg/deviceplugin/api.go b/pkg/deviceplugin/api.go index aeb4b683..07915de8 100644 --- a/pkg/deviceplugin/api.go +++ b/pkg/deviceplugin/api.go @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +// Package deviceplugin provides API for reporting available devices to kubelet. package deviceplugin import ( diff --git a/pkg/deviceplugin/server.go b/pkg/deviceplugin/server.go index 39a2190b..19c0a3b3 100644 --- a/pkg/deviceplugin/server.go +++ b/pkg/deviceplugin/server.go @@ -90,7 +90,7 @@ func (srv *server) sendDevices(stream pluginapi.DevicePlugin_ListAndWatchServer) } klog.V(4).Info("Sending to kubelet", resp.Devices) if err := stream.Send(resp); err != nil { - srv.Stop() + _ = srv.Stop() return errors.Wrapf(err, "Cannot update device list") } @@ -204,7 +204,8 @@ func (srv *server) setupAndServe(namespace string, devicePluginPath string, kube if err := waitForServer(pluginSocket, time.Second); err == nil { return errors.Errorf("Socket %s is already in use", pluginSocket) } - os.Remove(pluginSocket) + // We don't care if the plugin's socket file doesn't exist. + _ = os.Remove(pluginSocket) lis, err := net.Listen("unix", pluginSocket) if err != nil { @@ -217,7 +218,9 @@ func (srv *server) setupAndServe(namespace string, devicePluginPath string, kube // Starts device plugin service. go func() { klog.V(1).Infof("Start server for %s at: %s", srv.devType, pluginSocket) - srv.grpcServer.Serve(lis) + if serveErr := srv.grpcServer.Serve(lis); serveErr != nil { + klog.Errorf("unable to start gRPC server: %+v", serveErr) + } }() // Wait for the server to start @@ -278,9 +281,10 @@ func watchFile(file string) error { } func registerWithKubelet(kubeletSocket, pluginEndPoint, resourceName string, options *pluginapi.DevicePluginOptions) error { - conn, err := grpc.Dial(kubeletSocket, grpc.WithInsecure(), - grpc.WithDialer(func(addr string, timeout time.Duration) (net.Conn, error) { - return net.DialTimeout("unix", addr, timeout) + ctx := context.Background() + conn, err := grpc.DialContext(ctx, kubeletSocket, grpc.WithInsecure(), + grpc.WithContextDialer(func(ctx context.Context, addr string) (net.Conn, error) { + return (&net.Dialer{}).DialContext(ctx, "unix", addr) })) if err != nil { return errors.Wrap(err, "Cannot connect to kubelet service") @@ -294,7 +298,7 @@ func registerWithKubelet(kubeletSocket, pluginEndPoint, resourceName string, opt Options: options, } - _, err = client.Register(context.Background(), reqt) + _, err = client.Register(ctx, reqt) if err != nil { return errors.Wrap(err, "Cannot register to kubelet service") } @@ -308,12 +312,12 @@ func waitForServer(socket string, timeout time.Duration) error { ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() conn, err := grpc.DialContext(ctx, socket, grpc.WithInsecure(), grpc.WithBlock(), - grpc.WithDialer(func(addr string, timeout time.Duration) (net.Conn, error) { - return net.DialTimeout("unix", addr, timeout) + grpc.WithContextDialer(func(ctx context.Context, addr string) (net.Conn, error) { + return (&net.Dialer{}).DialContext(ctx, "unix", addr) }), ) if conn != nil { - conn.Close() + _ = conn.Close() } return errors.Wrapf(err, "Failed dial context at %s", socket) } diff --git a/pkg/deviceplugin/server_test.go b/pkg/deviceplugin/server_test.go index e116788f..14a49c6f 100644 --- a/pkg/deviceplugin/server_test.go +++ b/pkg/deviceplugin/server_test.go @@ -51,7 +51,7 @@ type kubeletStub struct { } func init() { - flag.Set("v", "4") //Enable debug output + _ = flag.Set("v", "4") //Enable debug output } // newKubeletStub returns an initialized kubeletStub for testing purpose. @@ -71,7 +71,7 @@ func (k *kubeletStub) Register(ctx context.Context, r *pluginapi.RegisterRequest } func (k *kubeletStub) start() error { - os.Remove(k.socket) + _ = os.Remove(k.socket) s, err := net.Listen("unix", k.socket) if err != nil { return errors.Wrap(err, "Can't listen at the socket") @@ -83,9 +83,7 @@ func (k *kubeletStub) start() error { go k.server.Serve(s) // Wait till the grpcServer is ready to serve services. - waitForServer(k.socket, 10*time.Second) - - return nil + return waitForServer(k.socket, 10*time.Second) } func TestRegisterWithKublet(t *testing.T) { @@ -114,7 +112,9 @@ func TestSetupAndServe(t *testing.T) { var pEndpoint string kubelet := newKubeletStub(kubeletSocket) - kubelet.start() + if err := kubelet.start(); err != nil { + t.Fatalf("unable to start kubelet stub: %+v", err) + } defer kubelet.server.Stop() srv := &server{ @@ -152,16 +152,17 @@ func TestSetupAndServe(t *testing.T) { t.Fatalf("Server was able to start on occupied socket %s: %+v", pluginSocket, err) } - conn, err := grpc.Dial(pluginSocket, grpc.WithInsecure(), - grpc.WithDialer(func(addr string, timeout time.Duration) (net.Conn, error) { - return net.DialTimeout("unix", addr, timeout) + ctx := context.Background() + conn, err := grpc.DialContext(ctx, pluginSocket, grpc.WithInsecure(), + grpc.WithContextDialer(func(ctx context.Context, addr string) (net.Conn, error) { + return (&net.Dialer{}).DialContext(ctx, "unix", addr) })) if err != nil { t.Fatalf("Failed to get connection: %+v", err) } client := pluginapi.NewDevicePluginClient(conn) - _, err = client.Allocate(context.Background(), &pluginapi.AllocateRequest{ + _, err = client.Allocate(ctx, &pluginapi.AllocateRequest{ ContainerRequests: []*pluginapi.ContainerAllocateRequest{ { DevicesIDs: []string{"dev1", "dev2"}, @@ -171,7 +172,7 @@ func TestSetupAndServe(t *testing.T) { if err != nil { t.Errorf("Failed to allocate device dev1: %+v", err) } - conn.Close() + _ = conn.Close() // Check if plugins re-registers after its socket has been removed kubelet.Lock() @@ -180,7 +181,7 @@ func TestSetupAndServe(t *testing.T) { if pEndpoint == "" { t.Fatal("After successful Allocate() pluginEndpoint is empty") } - os.Remove(path.Join(devicePluginPath, pEndpoint)) + _ = os.Remove(path.Join(devicePluginPath, pEndpoint)) for { kubelet.Lock() pEndpoint = kubelet.pluginEndpoint @@ -194,16 +195,16 @@ func TestSetupAndServe(t *testing.T) { klog.V(1).Info("No plugin socket. Waiting...") time.Sleep(1 * time.Second) } - conn, err = grpc.Dial(pluginSocket, grpc.WithInsecure(), - grpc.WithDialer(func(addr string, timeout time.Duration) (net.Conn, error) { - return net.DialTimeout("unix", addr, timeout) + conn, err = grpc.DialContext(ctx, pluginSocket, grpc.WithInsecure(), + grpc.WithContextDialer(func(ctx context.Context, addr string) (net.Conn, error) { + return (&net.Dialer{}).DialContext(ctx, "unix", addr) })) if err != nil { t.Fatalf("Failed to get connection: %+v", err) } client = pluginapi.NewDevicePluginClient(conn) - _, err = client.Allocate(context.Background(), &pluginapi.AllocateRequest{ + _, err = client.Allocate(ctx, &pluginapi.AllocateRequest{ ContainerRequests: []*pluginapi.ContainerAllocateRequest{ { DevicesIDs: []string{"dev1", "dev2"}, @@ -213,7 +214,7 @@ func TestSetupAndServe(t *testing.T) { if err != nil { t.Errorf("Failed to allocate device dev1: %+v", err) } - conn.Close() + _ = conn.Close() } func TestStop(t *testing.T) { @@ -337,7 +338,7 @@ func TestAllocate(t *testing.T) { for _, tt := range tcases { srv.devices = tt.devices srv.postAllocate = tt.postAllocate - resp, err := srv.Allocate(nil, rqt) + resp, err := srv.Allocate(context.Background(), rqt) if tt.expectedErr && err == nil { t.Errorf("Test case '%s': no error returned", tt.name) @@ -481,7 +482,9 @@ func TestListAndWatch(t *testing.T) { func TestGetDevicePluginOptions(t *testing.T) { srv := &server{} - srv.GetDevicePluginOptions(nil, nil) + if _, err := srv.GetDevicePluginOptions(context.Background(), nil); err != nil { + t.Errorf("unexpected error: %+v", err) + } } func TestPreStartContainer(t *testing.T) { @@ -506,7 +509,7 @@ func TestPreStartContainer(t *testing.T) { srv := &server{ preStartContainer: tc.preStartContainer, } - _, err := srv.PreStartContainer(nil, nil) + _, err := srv.PreStartContainer(context.Background(), nil) if !tc.expectedError && err != nil { t.Errorf("unexpected error: %v", err) } else if tc.expectedError && err == nil { diff --git a/pkg/topology/topology.go b/pkg/topology/topology.go index 835709a4..2f6fd0dc 100644 --- a/pkg/topology/topology.go +++ b/pkg/topology/topology.go @@ -52,11 +52,14 @@ type Hint struct { type Hints map[string]Hint func getDevicesFromVirtual(realDevPath string) (devs []string, err error) { - if !filepath.HasPrefix(realDevPath, "/sys/devices/virtual") { - return nil, fmt.Errorf("%s is not a virtual device", realDevPath) + relPath, err := filepath.Rel("/sys/devices/virtual", realDevPath) + if err != nil { + return nil, errors.Wrap(err, "unable to find relative path") } - relPath, _ := filepath.Rel("/sys/devices/virtual", realDevPath) + if strings.HasPrefix(relPath, "..") { + return nil, errors.Errorf("%s is not a virtual device", realDevPath) + } dir, file := filepath.Split(relPath) switch dir { diff --git a/pkg/topology/topology_test.go b/pkg/topology/topology_test.go index 66b2b6d2..4a6e8653 100644 --- a/pkg/topology/topology_test.go +++ b/pkg/topology/topology_test.go @@ -188,6 +188,12 @@ func TestGetDevicesFromVirtual(t *testing.T) { output: nil, expectedErr: true, }, + { + name: "garbage", + input: "./sys/devices/virtual/vfio/42", + output: nil, + expectedErr: true, + }, } for _, tc := range cases {