From c4f3d18e28d20d5d90007acfe1bb002237b3ce0e Mon Sep 17 00:00:00 2001 From: jo Date: Tue, 16 Jan 2024 17:13:56 +0100 Subject: [PATCH 1/5] ci: add lint workflow --- .github/workflows/lint.yml | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 .github/workflows/lint.yml diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 00000000..ffdcb2a9 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,24 @@ +name: Lint + +on: + push: + branches: [main] + pull_request: + +jobs: + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + cache: false + + - uses: golangci/golangci-lint-action@v3 + with: + version: v1.55.2 # renovate: datasource=github-releases depName=golangci/golangci-lint + + # In general linting is quite fast with warm caches, but a fresh run might + # take some time. + args: --timeout 5m From eb57ac2642ff4a3bdd82cd477d66f41e1bc934fe Mon Sep 17 00:00:00 2001 From: jo Date: Tue, 16 Jan 2024 17:23:34 +0100 Subject: [PATCH 2/5] ci: remove goimports workflow in favor of lint workflow --- .github/workflows/goimports.yml | 18 ------------------ 1 file changed, 18 deletions(-) delete mode 100644 .github/workflows/goimports.yml diff --git a/.github/workflows/goimports.yml b/.github/workflows/goimports.yml deleted file mode 100644 index 4a13b51e..00000000 --- a/.github/workflows/goimports.yml +++ /dev/null @@ -1,18 +0,0 @@ -name: Check imports -on: [push] -jobs: - test: - runs-on: ubuntu-latest - steps: - - uses: actions/setup-go@v5 - with: - go-version: "1.21" - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - name: Check imports - shell: bash - run: | - export PATH=$(go env GOPATH)/bin:$PATH - go get golang.org/x/tools/cmd/goimports - diff -u <(echo -n) <(goimports -d .) From 2a3360febc664f7f35777253fb25d15943b45acd Mon Sep 17 00:00:00 2001 From: jo Date: Tue, 23 Jan 2024 14:27:23 +0100 Subject: [PATCH 3/5] refactor: fix linting issues --- .golangci.yaml | 43 +++++++++++++++++++++++ api/volume.go | 2 +- app/app.go | 5 +-- cmd/aio/main.go | 1 + cmd/controller/main.go | 1 + cmd/node/main.go | 1 + driver/controller.go | 8 ++--- driver/controller_test.go | 52 ++++++++++++++-------------- driver/node.go | 16 ++++----- driver/node_test.go | 12 +++---- driver/sanity_test.go | 32 ++++++++--------- metrics/metrics.go | 3 +- mock/volume.go | 3 +- test/integration/cryptsetup_test.go | 5 +-- test/integration/docker.go | 10 +++++- test/integration/integration_test.go | 34 +++++++++--------- test/integration/volumes_test.go | 36 +++++++++---------- volumes/mount.go | 22 +++++------- 18 files changed, 167 insertions(+), 119 deletions(-) create mode 100644 .golangci.yaml diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 00000000..48e54b7c --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,43 @@ +--- +linters-settings: + errcheck: + exclude-functions: + - (github.com/go-kit/log.Logger).Log + misspell: + locale: US + gci: + sections: + - standard + - default + - prefix(github.com/hetznercloud) + +linters: + disable-all: true + enable: + - bodyclose + - errcheck + - exportloopref + - gci + - gocritic + - gofmt + - goimports + - gosimple + - govet + - ineffassign + - misspell + - prealloc + - revive + - rowserrcheck + - staticcheck + - typecheck + - unparam + - unused + - whitespace + +issues: + exclude-rules: + - path: _test\.go + linters: + - errcheck + - path: _test\.go + text: unused-parameter diff --git a/api/volume.go b/api/volume.go index 9c7f3289..c26a09d6 100644 --- a/api/volume.go +++ b/api/volume.go @@ -6,10 +6,10 @@ import ( "github.com/go-kit/log" "github.com/go-kit/log/level" - "github.com/hetznercloud/hcloud-go/v2/hcloud" "github.com/hetznercloud/csi-driver/csi" "github.com/hetznercloud/csi-driver/volumes" + "github.com/hetznercloud/hcloud-go/v2/hcloud" ) type VolumeService struct { diff --git a/app/app.go b/app/app.go index c750305a..09b31348 100644 --- a/app/app.go +++ b/app/app.go @@ -13,12 +13,13 @@ import ( "github.com/go-kit/log" "github.com/go-kit/log/level" grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" + "github.com/prometheus/client_golang/prometheus" + "google.golang.org/grpc" + "github.com/hetznercloud/csi-driver/driver" "github.com/hetznercloud/csi-driver/metrics" "github.com/hetznercloud/hcloud-go/v2/hcloud" "github.com/hetznercloud/hcloud-go/v2/hcloud/metadata" - "github.com/prometheus/client_golang/prometheus" - "google.golang.org/grpc" ) func parseLogLevel(lvl string) level.Option { diff --git a/cmd/aio/main.go b/cmd/aio/main.go index 82d5bf9e..461d218e 100644 --- a/cmd/aio/main.go +++ b/cmd/aio/main.go @@ -9,6 +9,7 @@ import ( proto "github.com/container-storage-interface/spec/lib/go/csi" "github.com/go-kit/log" "github.com/go-kit/log/level" + "github.com/hetznercloud/csi-driver/api" "github.com/hetznercloud/csi-driver/app" "github.com/hetznercloud/csi-driver/driver" diff --git a/cmd/controller/main.go b/cmd/controller/main.go index 55a613ef..2121f47f 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -6,6 +6,7 @@ import ( proto "github.com/container-storage-interface/spec/lib/go/csi" "github.com/go-kit/log" "github.com/go-kit/log/level" + "github.com/hetznercloud/csi-driver/api" "github.com/hetznercloud/csi-driver/app" "github.com/hetznercloud/csi-driver/driver" diff --git a/cmd/node/main.go b/cmd/node/main.go index 3c39da9f..dec6af3b 100644 --- a/cmd/node/main.go +++ b/cmd/node/main.go @@ -9,6 +9,7 @@ import ( proto "github.com/container-storage-interface/spec/lib/go/csi" "github.com/go-kit/log" "github.com/go-kit/log/level" + "github.com/hetznercloud/csi-driver/app" "github.com/hetznercloud/csi-driver/driver" "github.com/hetznercloud/csi-driver/volumes" diff --git a/driver/controller.go b/driver/controller.go index 2628f11c..db49f4cb 100644 --- a/driver/controller.go +++ b/driver/controller.go @@ -75,7 +75,7 @@ func (s *ControllerService) CreateVolume(ctx context.Context, req *proto.CreateV "err", err, ) code := codes.Internal - switch err { + switch err { //nolint:gocritic case volumes.ErrVolumeAlreadyExists: code = codes.AlreadyExists } @@ -368,7 +368,7 @@ func (s *ControllerService) ControllerExpandVolume(ctx context.Context, req *pro if err := s.volumeService.Resize(ctx, volume, minSize); err != nil { code := codes.Internal - switch err { + switch err { //nolint:gocritic case volumes.ErrVolumeNotFound: code = codes.NotFound } @@ -377,7 +377,7 @@ func (s *ControllerService) ControllerExpandVolume(ctx context.Context, req *pro if volume, err = s.volumeService.GetByID(ctx, volumeID); err != nil { code := codes.Internal - switch err { + switch err { //nolint:gocritic case volumes.ErrVolumeNotFound: code = codes.NotFound } @@ -391,6 +391,6 @@ func (s *ControllerService) ControllerExpandVolume(ctx context.Context, req *pro return resp, nil } -func (s *ControllerService) ControllerGetVolume(ctx context.Context, req *proto.ControllerGetVolumeRequest) (*proto.ControllerGetVolumeResponse, error) { +func (s *ControllerService) ControllerGetVolume(_ context.Context, _ *proto.ControllerGetVolumeRequest) (*proto.ControllerGetVolumeResponse, error) { return nil, status.Errorf(codes.Unimplemented, "method ControllerGetVolume not implemented") } diff --git a/driver/controller_test.go b/driver/controller_test.go index c8a0e5c0..e442c504 100644 --- a/driver/controller_test.go +++ b/driver/controller_test.go @@ -7,8 +7,8 @@ import ( proto "github.com/container-storage-interface/spec/lib/go/csi" "github.com/go-kit/log" - "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/hetznercloud/csi-driver/csi" "github.com/hetznercloud/csi-driver/mock" @@ -69,7 +69,7 @@ func TestControllerServiceCreateVolume(t *testing.T) { LimitBytes: 2 * MinVolumeSize * GB, }, VolumeCapabilities: []*proto.VolumeCapability{ - &proto.VolumeCapability{ + { AccessType: &proto.VolumeCapability_Mount{ Mount: &proto.VolumeCapability_MountVolume{}, }, @@ -121,7 +121,7 @@ func TestControllerServiceCreateVolumeWithLocation(t *testing.T) { LimitBytes: 10 * GB, }, VolumeCapabilities: []*proto.VolumeCapability{ - &proto.VolumeCapability{ + { AccessType: &proto.VolumeCapability_Mount{ Mount: &proto.VolumeCapability_MountVolume{}, }, @@ -132,7 +132,7 @@ func TestControllerServiceCreateVolumeWithLocation(t *testing.T) { }, AccessibilityRequirements: &proto.TopologyRequirement{ Preferred: []*proto.Topology{ - &proto.Topology{ + { Segments: map[string]string{ TopologySegmentLocation: "explicit", }, @@ -170,7 +170,7 @@ func TestControllerServiceCreateVolumeInputErrors(t *testing.T) { LimitBytes: 10 * GB, }, VolumeCapabilities: []*proto.VolumeCapability{ - &proto.VolumeCapability{ + { AccessType: &proto.VolumeCapability_Mount{ Mount: &proto.VolumeCapability_MountVolume{}, }, @@ -202,7 +202,7 @@ func TestControllerServiceCreateVolumeInputErrors(t *testing.T) { LimitBytes: 5 * GB, }, VolumeCapabilities: []*proto.VolumeCapability{ - &proto.VolumeCapability{ + { AccessType: &proto.VolumeCapability_Mount{ Mount: &proto.VolumeCapability_MountVolume{}, }, @@ -223,7 +223,7 @@ func TestControllerServiceCreateVolumeInputErrors(t *testing.T) { LimitBytes: 10 * GB, }, VolumeCapabilities: []*proto.VolumeCapability{ - &proto.VolumeCapability{ + { AccessType: &proto.VolumeCapability_Mount{ Mount: &proto.VolumeCapability_MountVolume{}, }, @@ -240,7 +240,7 @@ func TestControllerServiceCreateVolumeInputErrors(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { _, err := env.service.CreateVolume(env.ctx, testCase.Req) - if grpc.Code(err) != testCase.Code { + if status.Code(err) != testCase.Code { t.Fatalf("unexpected error: %v", err) } }) @@ -280,7 +280,7 @@ func TestControllerServiceCreateVolumeCreateErrors(t *testing.T) { LimitBytes: 10 * GB, }, VolumeCapabilities: []*proto.VolumeCapability{ - &proto.VolumeCapability{ + { AccessType: &proto.VolumeCapability_Mount{ Mount: &proto.VolumeCapability_MountVolume{}, }, @@ -290,7 +290,7 @@ func TestControllerServiceCreateVolumeCreateErrors(t *testing.T) { }, }, }) - if grpc.Code(err) != testCase.Code { + if status.Code(err) != testCase.Code { t.Fatalf("unexpected error: %v", err) } }) @@ -336,7 +336,7 @@ func TestControllerServiceDeleteVolumeInputErrors(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { _, err := env.service.DeleteVolume(env.ctx, testCase.Req) - if grpc.Code(err) != testCase.Code { + if status.Code(err) != testCase.Code { t.Fatalf("unexpected error: %v", err) } }) @@ -353,7 +353,7 @@ func TestControllerServiceDeleteVolumeAttached(t *testing.T) { _, err := env.service.DeleteVolume(env.ctx, &proto.DeleteVolumeRequest{ VolumeId: "1", }) - if grpc.Code(err) != codes.FailedPrecondition { + if status.Code(err) != codes.FailedPrecondition { t.Fatalf("unexpected error: %v", err) } } @@ -383,7 +383,7 @@ func TestControllerServiceDeleteVolumeInternalError(t *testing.T) { _, err := env.service.DeleteVolume(env.ctx, &proto.DeleteVolumeRequest{ VolumeId: "1", }) - if grpc.Code(err) != codes.Internal { + if status.Code(err) != codes.Internal { t.Fatalf("unexpected error: %v", err) } } @@ -531,7 +531,7 @@ func TestControllerServicePublishVolumeInputErrors(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { _, err := env.service.ControllerPublishVolume(env.ctx, testCase.Req) - if grpc.Code(err) != testCase.Code { + if status.Code(err) != testCase.Code { t.Fatalf("unexpected error: %v", err) } }) @@ -587,7 +587,7 @@ func TestControllerServicePublishVolumeAttachErrors(t *testing.T) { }, }, }) - if grpc.Code(err) != testCase.Code { + if status.Code(err) != testCase.Code { t.Fatalf("unexpected error: %v", err) } }) @@ -678,7 +678,7 @@ func TestControllerServiceUnpublishVolumeInputErrors(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { _, err := env.service.ControllerUnpublishVolume(env.ctx, testCase.Req) - if grpc.Code(err) != testCase.Code { + if status.Code(err) != testCase.Code { t.Fatalf("unexpected error: %v", err) } }) @@ -714,7 +714,7 @@ func TestControllerServiceUnpublishVolumeDetachErrors(t *testing.T) { VolumeId: "1", NodeId: "2", }) - if grpc.Code(err) != testCase.Code { + if status.Code(err) != testCase.Code { t.Fatalf("unexpected error: %v", err) } }) @@ -744,7 +744,7 @@ func TestControllerServiceValidateVolumeCapabilities(t *testing.T) { req := &proto.ValidateVolumeCapabilitiesRequest{ VolumeId: "1", VolumeCapabilities: []*proto.VolumeCapability{ - &proto.VolumeCapability{ + { AccessMode: &proto.VolumeCapability_AccessMode{ Mode: proto.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, }, @@ -776,7 +776,7 @@ func TestControllerServiceValidateVolumeCapabilitiesInputErrors(t *testing.T) { Req: &proto.ValidateVolumeCapabilitiesRequest{ VolumeId: "", VolumeCapabilities: []*proto.VolumeCapability{ - &proto.VolumeCapability{ + { AccessMode: &proto.VolumeCapability_AccessMode{ Mode: proto.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, }, @@ -797,7 +797,7 @@ func TestControllerServiceValidateVolumeCapabilitiesInputErrors(t *testing.T) { Req: &proto.ValidateVolumeCapabilitiesRequest{ VolumeId: "xxx", VolumeCapabilities: []*proto.VolumeCapability{ - &proto.VolumeCapability{ + { AccessMode: &proto.VolumeCapability_AccessMode{ Mode: proto.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, }, @@ -811,7 +811,7 @@ func TestControllerServiceValidateVolumeCapabilitiesInputErrors(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { _, err := env.service.ValidateVolumeCapabilities(env.ctx, testCase.Req) - if grpc.Code(err) != testCase.Code { + if status.Code(err) != testCase.Code { t.Fatalf("unexpected error: %v", err) } }) @@ -828,7 +828,7 @@ func TestControllerServiceValidateVolumeCapabilitiesVolumeNotFound(t *testing.T) req := &proto.ValidateVolumeCapabilitiesRequest{ VolumeId: "1", VolumeCapabilities: []*proto.VolumeCapability{ - &proto.VolumeCapability{ + { AccessMode: &proto.VolumeCapability_AccessMode{ Mode: proto.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, }, @@ -836,7 +836,7 @@ func TestControllerServiceValidateVolumeCapabilitiesVolumeNotFound(t *testing.T) }, } _, err := env.service.ValidateVolumeCapabilities(env.ctx, req) - if grpc.Code(err) != codes.NotFound { + if status.Code(err) != codes.NotFound { t.Fatalf("unexpected error: %v", err) } } @@ -851,7 +851,7 @@ func TestControllerServiceValidateVolumeCapabilitiesInternalError(t *testing.T) req := &proto.ValidateVolumeCapabilitiesRequest{ VolumeId: "1", VolumeCapabilities: []*proto.VolumeCapability{ - &proto.VolumeCapability{ + { AccessMode: &proto.VolumeCapability_AccessMode{ Mode: proto.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, }, @@ -859,7 +859,7 @@ func TestControllerServiceValidateVolumeCapabilitiesInternalError(t *testing.T) }, } _, err := env.service.ValidateVolumeCapabilities(env.ctx, req) - if grpc.Code(err) != codes.Internal { + if status.Code(err) != codes.Internal { t.Fatalf("unexpected error: %v", err) } } @@ -874,7 +874,7 @@ func TestControllerServiceValidateVolumeCapabilitiesUnsupportedCapability(t *tes req := &proto.ValidateVolumeCapabilitiesRequest{ VolumeId: "1", VolumeCapabilities: []*proto.VolumeCapability{ - &proto.VolumeCapability{ + { AccessMode: &proto.VolumeCapability_AccessMode{ Mode: proto.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER, }, diff --git a/driver/node.go b/driver/node.go index 31ee0088..481292f8 100644 --- a/driver/node.go +++ b/driver/node.go @@ -48,17 +48,17 @@ func NewNodeService( const encryptionPassphraseKey = "encryption-passphrase" -func (s *NodeService) NodeStageVolume(ctx context.Context, req *proto.NodeStageVolumeRequest) (*proto.NodeStageVolumeResponse, error) { +func (s *NodeService) NodeStageVolume(_ context.Context, _ *proto.NodeStageVolumeRequest) (*proto.NodeStageVolumeResponse, error) { // while we dont do anything here, Swarm 23.0.1 might require this return &proto.NodeStageVolumeResponse{}, nil } -func (s *NodeService) NodeUnstageVolume(ctx context.Context, req *proto.NodeUnstageVolumeRequest) (*proto.NodeUnstageVolumeResponse, error) { +func (s *NodeService) NodeUnstageVolume(_ context.Context, _ *proto.NodeUnstageVolumeRequest) (*proto.NodeUnstageVolumeResponse, error) { // while we dont do anything here, Swarm 23.0.1 might require this return &proto.NodeUnstageVolumeResponse{}, nil } -func (s *NodeService) NodePublishVolume(ctx context.Context, req *proto.NodePublishVolumeRequest) (*proto.NodePublishVolumeResponse, error) { +func (s *NodeService) NodePublishVolume(_ context.Context, req *proto.NodePublishVolumeRequest) (*proto.NodePublishVolumeResponse, error) { if req.VolumeId == "" { return nil, status.Error(codes.InvalidArgument, "missing volume id") } @@ -96,7 +96,7 @@ func (s *NodeService) NodePublishVolume(ctx context.Context, req *proto.NodePubl return &proto.NodePublishVolumeResponse{}, nil } -func (s *NodeService) NodeUnpublishVolume(ctx context.Context, req *proto.NodeUnpublishVolumeRequest) (*proto.NodeUnpublishVolumeResponse, error) { +func (s *NodeService) NodeUnpublishVolume(_ context.Context, req *proto.NodeUnpublishVolumeRequest) (*proto.NodeUnpublishVolumeResponse, error) { if req.VolumeId == "" { return nil, status.Error(codes.InvalidArgument, "missing volume id") } @@ -112,7 +112,7 @@ func (s *NodeService) NodeUnpublishVolume(ctx context.Context, req *proto.NodeUn return resp, nil } -func (s *NodeService) NodeGetVolumeStats(ctx context.Context, req *proto.NodeGetVolumeStatsRequest) (*proto.NodeGetVolumeStatsResponse, error) { +func (s *NodeService) NodeGetVolumeStats(_ context.Context, req *proto.NodeGetVolumeStatsRequest) (*proto.NodeGetVolumeStatsResponse, error) { if req.VolumeId == "" { return nil, status.Error(codes.InvalidArgument, "missing volume id") } @@ -156,7 +156,7 @@ func (s *NodeService) NodeGetVolumeStats(ctx context.Context, req *proto.NodeGet }, nil } -func (s *NodeService) NodeGetCapabilities(ctx context.Context, req *proto.NodeGetCapabilitiesRequest) (*proto.NodeGetCapabilitiesResponse, error) { +func (s *NodeService) NodeGetCapabilities(_ context.Context, _ *proto.NodeGetCapabilitiesRequest) (*proto.NodeGetCapabilitiesResponse, error) { capabilities := []*proto.NodeServiceCapability{ { Type: &proto.NodeServiceCapability_Rpc{ @@ -191,7 +191,7 @@ func (s *NodeService) NodeGetCapabilities(ctx context.Context, req *proto.NodeGe return resp, nil } -func (s *NodeService) NodeGetInfo(context.Context, *proto.NodeGetInfoRequest) (*proto.NodeGetInfoResponse, error) { +func (s *NodeService) NodeGetInfo(_ context.Context, _ *proto.NodeGetInfoRequest) (*proto.NodeGetInfoResponse, error) { resp := &proto.NodeGetInfoResponse{ NodeId: s.serverID, MaxVolumesPerNode: MaxVolumesPerNode, @@ -204,7 +204,7 @@ func (s *NodeService) NodeGetInfo(context.Context, *proto.NodeGetInfoRequest) (* return resp, nil } -func (s *NodeService) NodeExpandVolume(ctx context.Context, req *proto.NodeExpandVolumeRequest) (*proto.NodeExpandVolumeResponse, error) { +func (s *NodeService) NodeExpandVolume(_ context.Context, req *proto.NodeExpandVolumeRequest) (*proto.NodeExpandVolumeResponse, error) { if req.VolumeId == "" { return nil, status.Error(codes.InvalidArgument, "missing volume id") } diff --git a/driver/node_test.go b/driver/node_test.go index b7d1fbdd..f83fac83 100644 --- a/driver/node_test.go +++ b/driver/node_test.go @@ -7,8 +7,8 @@ import ( proto "github.com/container-storage-interface/spec/lib/go/csi" "github.com/go-kit/log" - "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/hetznercloud/csi-driver/mock" "github.com/hetznercloud/csi-driver/volumes" @@ -139,7 +139,7 @@ func TestNodeServiceNodePublishPublishError(t *testing.T) { "devicePath": "devpath", }, }) - if grpc.Code(err) != codes.Internal { + if status.Code(err) != codes.Internal { t.Fatalf("unexpected error: %v", err) } } @@ -206,7 +206,7 @@ func TestNodeServiceNodePublishVolumeInputErrors(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { _, err := env.service.NodePublishVolume(env.ctx, testCase.Req) - if grpc.Code(err) != testCase.Code { + if status.Code(err) != testCase.Code { t.Fatalf("unexpected error: %s", err) } }) @@ -243,7 +243,7 @@ func TestNodeServiceNodeUnpublishUnpublishError(t *testing.T) { VolumeId: "1", TargetPath: "target", }) - if grpc.Code(err) != codes.Internal { + if status.Code(err) != codes.Internal { t.Fatalf("unexpected error: %v", err) } } @@ -275,7 +275,7 @@ func TestNodeServiceNodeUnpublishVolumeInputErrors(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { _, err := env.service.NodeUnpublishVolume(env.ctx, testCase.Req) - if grpc.Code(err) != testCase.Code { + if status.Code(err) != testCase.Code { t.Fatalf("unexpected error: %s", err) } }) @@ -401,7 +401,7 @@ func TestNodeServiceNodeExpandVolumeInputErrors(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { _, err := env.service.NodeExpandVolume(env.ctx, testCase.Req) - if grpc.Code(err) != testCase.Code { + if status.Code(err) != testCase.Code { t.Fatalf("unexpected error: %s", err) } }) diff --git a/driver/sanity_test.go b/driver/sanity_test.go index e0b25938..d10705e8 100644 --- a/driver/sanity_test.go +++ b/driver/sanity_test.go @@ -5,7 +5,6 @@ import ( "context" "errors" "fmt" - "io/ioutil" "net" "os" "sync" @@ -70,7 +69,7 @@ func TestSanity(t *testing.T) { } }() - tempDir, err := ioutil.TempDir("", "csi") + tempDir, err := os.MkdirTemp("", "csi") if err != nil { t.Fatal(err) } @@ -89,7 +88,7 @@ type sanityVolumeService struct { volumes list.List } -func (s *sanityVolumeService) All(ctx context.Context) ([]*csi.Volume, error) { +func (s *sanityVolumeService) All(_ context.Context) ([]*csi.Volume, error) { s.mu.Lock() defer s.mu.Unlock() @@ -97,12 +96,11 @@ func (s *sanityVolumeService) All(ctx context.Context) ([]*csi.Volume, error) { for e := s.volumes.Front(); e != nil; e = e.Next() { v := e.Value.(*csi.Volume) vols = append(vols, v) - } return vols, nil } -func (s *sanityVolumeService) Create(ctx context.Context, opts volumes.CreateOpts) (*csi.Volume, error) { +func (s *sanityVolumeService) Create(_ context.Context, opts volumes.CreateOpts) (*csi.Volume, error) { s.mu.Lock() defer s.mu.Unlock() @@ -125,7 +123,7 @@ func (s *sanityVolumeService) Create(ctx context.Context, opts volumes.CreateOpt return volume, nil } -func (s *sanityVolumeService) GetByID(ctx context.Context, id int64) (*csi.Volume, error) { +func (s *sanityVolumeService) GetByID(_ context.Context, id int64) (*csi.Volume, error) { s.mu.Lock() defer s.mu.Unlock() @@ -139,7 +137,7 @@ func (s *sanityVolumeService) GetByID(ctx context.Context, id int64) (*csi.Volum return nil, volumes.ErrVolumeNotFound } -func (s *sanityVolumeService) GetByName(ctx context.Context, name string) (*csi.Volume, error) { +func (s *sanityVolumeService) GetByName(_ context.Context, name string) (*csi.Volume, error) { s.mu.Lock() defer s.mu.Unlock() @@ -153,7 +151,7 @@ func (s *sanityVolumeService) GetByName(ctx context.Context, name string) (*csi. return nil, volumes.ErrVolumeNotFound } -func (s *sanityVolumeService) Delete(ctx context.Context, volume *csi.Volume) error { +func (s *sanityVolumeService) Delete(_ context.Context, volume *csi.Volume) error { s.mu.Lock() defer s.mu.Unlock() @@ -168,7 +166,7 @@ func (s *sanityVolumeService) Delete(ctx context.Context, volume *csi.Volume) er return volumes.ErrVolumeNotFound } -func (s *sanityVolumeService) Resize(ctx context.Context, volume *csi.Volume, size int) error { +func (s *sanityVolumeService) Resize(_ context.Context, volume *csi.Volume, size int) error { s.mu.Lock() defer s.mu.Unlock() @@ -183,21 +181,21 @@ func (s *sanityVolumeService) Resize(ctx context.Context, volume *csi.Volume, si return volumes.ErrVolumeNotFound } -func (s *sanityVolumeService) Attach(ctx context.Context, volume *csi.Volume, server *csi.Server) error { +func (s *sanityVolumeService) Attach(_ context.Context, _ *csi.Volume, _ *csi.Server) error { return nil } -func (s *sanityVolumeService) Detach(ctx context.Context, volume *csi.Volume, server *csi.Server) error { +func (s *sanityVolumeService) Detach(_ context.Context, _ *csi.Volume, _ *csi.Server) error { return nil } type sanityMountService struct{} -func (s *sanityMountService) Publish(targetPath string, devicePath string, opts volumes.MountOpts) error { +func (s *sanityMountService) Publish(_ string, _ string, _ volumes.MountOpts) error { return nil } -func (s *sanityMountService) Unpublish(targetPath string) error { +func (s *sanityMountService) Unpublish(_ string) error { return nil } @@ -208,11 +206,11 @@ func (s *sanityMountService) PathExists(path string) (bool, error) { return true, nil } -func (s *sanityMountService) FormatDisk(disk string, fstype string) error { +func (s *sanityMountService) FormatDisk(_ string, _ string) error { return nil } -func (s *sanityMountService) DetectDiskFormat(disk string) (string, error) { +func (s *sanityMountService) DetectDiskFormat(_ string) (string, error) { return "ext4", nil } @@ -227,9 +225,9 @@ func (s *sanityResizeService) Resize(volumePath string) error { type sanityStatsService struct{} -func (s *sanityStatsService) ByteFilesystemStats(volumePath string) (totalBytes int64, availableBytes int64, usedBytes int64, err error) { +func (s *sanityStatsService) ByteFilesystemStats(_ string) (totalBytes int64, availableBytes int64, usedBytes int64, err error) { return 1, 1, 1, nil } -func (s *sanityStatsService) INodeFilesystemStats(volumePath string) (total int64, used int64, free int64, err error) { +func (s *sanityStatsService) INodeFilesystemStats(_ string) (total int64, used int64, free int64, err error) { return 1, 1, 1, nil } diff --git a/metrics/metrics.go b/metrics/metrics.go index 3b5ffe75..fb26a805 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -7,6 +7,7 @@ import ( "github.com/go-kit/log/level" grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus" "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/collectors" "github.com/prometheus/client_golang/prometheus/promhttp" "google.golang.org/grpc" ) @@ -28,7 +29,7 @@ func New(logger log.Logger, addr string) *Metrics { addr: addr, reg: prometheus.NewRegistry(), grpcMetrics: grpc_prometheus.NewServerMetrics(), - goMetrics: prometheus.NewGoCollector(), + goMetrics: collectors.NewGoCollector(), } level.Debug(metrics.logger).Log( diff --git a/mock/volume.go b/mock/volume.go index 6370ff95..3d44a363 100644 --- a/mock/volume.go +++ b/mock/volume.go @@ -3,10 +3,9 @@ package mock import ( "context" - "github.com/hetznercloud/hcloud-go/v2/hcloud" - "github.com/hetznercloud/csi-driver/csi" "github.com/hetznercloud/csi-driver/volumes" + "github.com/hetznercloud/hcloud-go/v2/hcloud" ) type VolumeService struct { diff --git a/test/integration/cryptsetup_test.go b/test/integration/cryptsetup_test.go index 7e108a3a..1ed18441 100644 --- a/test/integration/cryptsetup_test.go +++ b/test/integration/cryptsetup_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/go-kit/log" + "github.com/hetznercloud/csi-driver/volumes" ) @@ -31,7 +32,7 @@ func TestCryptSetup(t *testing.T) { t.Fatal(err) } decryptedDevice := "/dev/mapper/" + decryptedName - defer runCmd("cryptsetup", "luksClose", decryptedName) + defer mustRunCmd(t, "cryptsetup", "luksClose", decryptedName) if _, err := runCmd("mkfs.ext4", decryptedDevice); err != nil { t.Fatal(err) @@ -43,7 +44,7 @@ func TestCryptSetup(t *testing.T) { if _, err := runCmd("mount", "-t", "ext4", decryptedDevice, decryptedMount); err != nil { t.Fatal(err) } - defer runCmd("umount", decryptedMount) + defer mustRunCmd(t, "umount", decryptedMount) if _, err := runCmd("umount", decryptedMount); err != nil { t.Fatal(err) diff --git a/test/integration/docker.go b/test/integration/docker.go index 7909d05c..050302e8 100644 --- a/test/integration/docker.go +++ b/test/integration/docker.go @@ -5,6 +5,7 @@ import ( "os/exec" "runtime" "strings" + "testing" ) const dockerExecutable = "docker" @@ -48,7 +49,14 @@ func runCmdWithStdin(stdin string, name string, args ...string) (string, error) outputBytes, err := cmd.CombinedOutput() output := string(outputBytes) if err != nil { - return output, fmt.Errorf("run command %s failed: %w\n", strings.Join(append([]string{name}, args...), " "), err) + return output, fmt.Errorf("run command %s failed: %w", strings.Join(append([]string{name}, args...), " "), err) } return output, nil } + +func mustRunCmd(t *testing.T, name string, args ...string) { + _, err := runCmd(name, args...) + if err != nil { + t.Fatal(err) + } +} diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index 64d05f80..ddcbba70 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -2,6 +2,7 @@ package integration import ( "fmt" + "log" "os" "regexp" "strconv" @@ -13,7 +14,9 @@ const testImageEnvironmentVariable = "HCLOUD_CSI_DRIVER_INTEGRATIONTESTS" func TestMain(t *testing.M) { if os.Getenv(testImageEnvironmentVariable) != "true" { - prepareDockerImage() + if err := prepareDockerImage(); err != nil { + log.Fatal(err) + } } os.Exit(t.Run()) @@ -27,25 +30,23 @@ func prepareDockerImage() error { os.Setenv("CGO_ENABLED", "0") defer os.Unsetenv("CGO_ENABLED") if output, err := runCmd("go", "test", "-c", "-o", "integration.tests"); err != nil { - fmt.Printf("Error compiling test binary: %v\n%s\n", err, output) - os.Exit(1) + return fmt.Errorf("error compiling test binary: %w\n%s", err, output) } if output, err := DockerBuild(testImageName, "."); err != nil { - fmt.Printf("Error building docker image: %v\n%s\n", err, output) - os.Exit(1) + return fmt.Errorf("error building docker image: %w\n%s", err, output) } return nil } -func runTestInDockerImage(t *testing.T, privileged bool) bool { +func runTestInDockerImage(t *testing.T, privileged bool) bool { //nolint:unparam if os.Getenv(testImageEnvironmentVariable) == "true" { return true } if output, err := DockerRun(testImageName, []string{testImageEnvironmentVariable + "=true"}, []string{"-test.v", "-test.run", t.Name()}, privileged); err != nil { - err := fmt.Errorf("Error running test in docker image: %w\n%s\n", err, output) + err := fmt.Errorf("error running test in docker image: %w\n%s", err, output) t.Fatal(err) } else { t.Log(output) @@ -74,17 +75,18 @@ func increaseFakeDeviceSize(name string, megabytesToAdd int) error { } func getFakeDeviceSizeKilobytes(mountPoint string) (int, error) { - if output, err := runCmd("df", "--output=size", "-k", mountPoint); err != nil { + output, err := runCmd("df", "--output=size", "-k", mountPoint) + if err != nil { return -1, err - } else { - regex := regexp.MustCompile(`(?ms)^\s*1K-blocks\s*(\d+)\s*$`) - match := regex.FindStringSubmatch(output) - if match == nil { - return -1, fmt.Errorf("unexpected df command output") - } - size, _ := strconv.Atoi(match[1]) - return size, nil } + + regex := regexp.MustCompile(`(?ms)^\s*1K-blocks\s*(\d+)\s*$`) + match := regex.FindStringSubmatch(output) + if match == nil { + return -1, fmt.Errorf("unexpected df command output") + } + size, _ := strconv.Atoi(match[1]) + return size, nil } type TestingWriter struct { diff --git a/test/integration/volumes_test.go b/test/integration/volumes_test.go index 49418b0b..5a039706 100644 --- a/test/integration/volumes_test.go +++ b/test/integration/volumes_test.go @@ -2,7 +2,6 @@ package integration import ( "fmt" - "io/ioutil" "math" "os" "path" @@ -10,6 +9,7 @@ import ( "testing" "github.com/go-kit/log" + "github.com/hetznercloud/csi-driver/volumes" ) @@ -63,10 +63,7 @@ func TestVolumePublishUnpublish(t *testing.T) { "encrypted-correct-formatted-1", volumes.MountOpts{EncryptionPassphrase: "passphrase"}, func(svc volumes.MountService, cs *volumes.CryptSetup, device string) error { - if err := cs.Format(device, "passphrase"); err != nil { - return err - } - return nil + return cs.Format(device, "passphrase") }, nil, }, @@ -77,16 +74,16 @@ func TestVolumePublishUnpublish(t *testing.T) { if err := cs.Format(device, "passphrase"); err != nil { return err } + luksDeviceName := volumes.GenerateLUKSDeviceName(device) if err := cs.Open(device, luksDeviceName, "passphrase"); err != nil { return err } defer cs.Close(luksDeviceName) + luksDevicePath := volumes.GenerateLUKSDevicePath(luksDeviceName) - if err := svc.FormatDisk(luksDevicePath, "ext4"); err != nil { - return err - } - return nil + + return svc.FormatDisk(luksDevicePath, "ext4") }, nil, }, @@ -102,7 +99,6 @@ func TestVolumePublishUnpublish(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - logger := log.NewLogfmtLogger(NewTestingWriter(t)) mountService := volumes.NewLinuxMountService(logger) cryptSetup := volumes.NewCryptSetup(logger) @@ -117,11 +113,11 @@ func TestVolumePublishUnpublish(t *testing.T) { } } - targetPath, err := ioutil.TempDir(os.TempDir(), "csi-driver") + targetPath, err := os.MkdirTemp(os.TempDir(), "csi-driver") if err != nil { t.Fatal() } - // Make sure that target path is non-existant + // Make sure that target path is non-existent // Required as FS volumes require target dir, but block volumes require // target file targetPath = path.Join(targetPath, "target-path") @@ -138,12 +134,16 @@ func TestVolumePublishUnpublish(t *testing.T) { // Makes no sense to continue verification if we got the error that we expected _ = mountService.Unpublish(targetPath) return - } else { + } + if err != nil { + t.Fatal(publishErr) + } + defer func() { + err := mountService.Unpublish(targetPath) if err != nil { - t.Fatal(publishErr) + t.Fatal(err) } - } - defer mountService.Unpublish(targetPath) + }() // Verify target exists and is of expected type fileInfo, err := os.Stat(targetPath) @@ -180,7 +180,6 @@ func TestVolumePublishUnpublish(t *testing.T) { t.Fatal(err) } } - }) } } @@ -213,7 +212,7 @@ func TestVolumeResize(t *testing.T) { if err != nil { t.Fatal(err) } - targetPath, err := ioutil.TempDir(os.TempDir(), "") + targetPath, err := os.MkdirTemp(os.TempDir(), "") if err != nil { t.Fatal() } @@ -339,5 +338,4 @@ func TestDetectDiskFormat(t *testing.T) { } }) } - } diff --git a/volumes/mount.go b/volumes/mount.go index 71ec9a15..7b8cb8bc 100644 --- a/volumes/mount.go +++ b/volumes/mount.go @@ -52,20 +52,21 @@ func NewLinuxMountService(logger log.Logger) *LinuxMountService { } func (s *LinuxMountService) Publish(targetPath string, devicePath string, opts MountOpts) error { - isNotMountPoint, err := mount.IsNotMountPoint(s.mounter, targetPath) + isMountPoint, err := s.mounter.IsMountPoint(targetPath) if err != nil { if os.IsNotExist(err) { - isNotMountPoint = true + isMountPoint = false } else { return err } } - var mountOptions []string - - if !isNotMountPoint { + if isMountPoint { return nil } + + var mountOptions []string + targetPathPermissions := os.FileMode(0750) if opts.BlockVolume { mountOptions = append(mountOptions, "bind") @@ -152,11 +153,7 @@ func (s *LinuxMountService) Publish(targetPath string, devicePath string, opts M "encrypted", opts.EncryptionPassphrase != "", ) - if err := s.mounter.Mount(devicePath, targetPath, opts.FSType, mountOptions); err != nil { - return err - } - - return nil + return s.mounter.Mount(devicePath, targetPath, opts.FSType, mountOptions) } func (s *LinuxMountService) Unpublish(targetPath string) error { @@ -176,11 +173,8 @@ func (s *LinuxMountService) Unpublish(targetPath string) error { } luksDeviceName := GenerateLUKSDeviceName(devicePath) - if err := s.cryptSetup.Close(luksDeviceName); err != nil { - return err - } - return nil + return s.cryptSetup.Close(luksDeviceName) } func (s *LinuxMountService) PathExists(path string) (bool, error) { From 26a7b9691dcef6f632a93550b97f40375e39bdfe Mon Sep 17 00:00:00 2001 From: jo Date: Tue, 23 Jan 2024 14:40:57 +0100 Subject: [PATCH 4/5] test: fix crypt setup test --- test/integration/cryptsetup_test.go | 4 ++-- test/integration/docker.go | 8 -------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/test/integration/cryptsetup_test.go b/test/integration/cryptsetup_test.go index 1ed18441..bcf09025 100644 --- a/test/integration/cryptsetup_test.go +++ b/test/integration/cryptsetup_test.go @@ -32,7 +32,7 @@ func TestCryptSetup(t *testing.T) { t.Fatal(err) } decryptedDevice := "/dev/mapper/" + decryptedName - defer mustRunCmd(t, "cryptsetup", "luksClose", decryptedName) + defer runCmd("cryptsetup", "luksClose", decryptedName) if _, err := runCmd("mkfs.ext4", decryptedDevice); err != nil { t.Fatal(err) @@ -44,7 +44,7 @@ func TestCryptSetup(t *testing.T) { if _, err := runCmd("mount", "-t", "ext4", decryptedDevice, decryptedMount); err != nil { t.Fatal(err) } - defer mustRunCmd(t, "umount", decryptedMount) + defer runCmd("umount", decryptedMount) if _, err := runCmd("umount", decryptedMount); err != nil { t.Fatal(err) diff --git a/test/integration/docker.go b/test/integration/docker.go index 050302e8..a2979f29 100644 --- a/test/integration/docker.go +++ b/test/integration/docker.go @@ -5,7 +5,6 @@ import ( "os/exec" "runtime" "strings" - "testing" ) const dockerExecutable = "docker" @@ -53,10 +52,3 @@ func runCmdWithStdin(stdin string, name string, args ...string) (string, error) } return output, nil } - -func mustRunCmd(t *testing.T, name string, args ...string) { - _, err := runCmd(name, args...) - if err != nil { - t.Fatal(err) - } -} From 263c511922413b1bc4ce9fc8d6c16e17ae5ce049 Mon Sep 17 00:00:00 2001 From: jo Date: Tue, 23 Jan 2024 14:43:07 +0100 Subject: [PATCH 5/5] chore: remove lint script --- script/checkall.bash | 4 ---- 1 file changed, 4 deletions(-) delete mode 100755 script/checkall.bash diff --git a/script/checkall.bash b/script/checkall.bash deleted file mode 100755 index 051b8578..00000000 --- a/script/checkall.bash +++ /dev/null @@ -1,4 +0,0 @@ -#!/bin/bash -e -diff -u <(echo -n) <(goimports -d .) -go vet ./... -go test ./...