Merge pull request topolvm#999 from topolvm/fix-test

Fix tests

.github/workflows/main.yaml

@@ -17,7 +17,8 @@ jobs:
         uses: docker/setup-buildx-action@v3
       - run: make setup
       - run: make check-uncommitted
-      - run: make test
+      # use sudo to test root required operations
+      - run: sudo make test
       - run: make build-topolvm GOARCH=s390x
         name: "Build TopoLVM for s390x architecture"
       - run: make groupname-test

.golangci.yml

@@ -17,6 +17,11 @@ issues:
         - dupl
         - lll
 
+    # exclude staticcheck SA1019 because it can not be disabled via comments
+    - linters:
+        - staticcheck
+      text: "SA1019: (.*)\\.(Get)?SizeGb is deprecated: Marked as deprecated in pkg/lvmd/proto/lvmd.proto."
+
     # FIXME: remove this once we have fixed the issues
     - path: "internal/lvmd/*"
       linters:
@@ -25,11 +30,6 @@ issues:
       linters:
         - gocyclo
 linters:
-  disable:
-    # The staticcheck built into golangci-lint does not allow to exclude single lines and specific rules,
-    # so we disable it and run it independently.
-    # https://github.com/golangci/golangci-lint/issues/741
-    - staticcheck
   enable:
     - dupl
     - errcheck
@@ -45,6 +45,7 @@ linters:
     - misspell
     - nakedret
     - prealloc
+    - staticcheck
     - typecheck
     - unconvert
     - unparam

Makefile

@@ -4,7 +4,6 @@ SUDO := sudo
 CURL := curl -sSLf
 BINDIR := $(shell pwd)/bin
 CONTROLLER_GEN := $(BINDIR)/controller-gen
-STATICCHECK := $(BINDIR)/staticcheck
 CONTAINER_STRUCTURE_TEST := $(BINDIR)/container-structure-test
 GOLANGCI_LINT = $(BINDIR)/golangci-lint
 PROTOC := PATH=$(BINDIR):$(PATH) $(BINDIR)/protoc -I=$(shell pwd)/include:.
@@ -122,7 +121,6 @@ check-uncommitted: generate ## Check if latest generated artifacts are committed
 lint: ## Run lint
 	test -z "$$(gofmt -s -l . | grep -vE '^vendor|^api/v1/zz_generated.deepcopy.go' | tee /dev/stderr)"
 	$(GOLANGCI_LINT) run
-	$(STATICCHECK) ./...
 	go vet ./...
 	test -z "$$(go vet ./... | grep -v '^vendor' | tee /dev/stderr)"
 
@@ -273,7 +271,6 @@ install-helm-docs: | $(BINDIR)
 
 .PHONY: tools
 tools: install-kind install-container-structure-test install-helm install-helm-docs | $(BINDIR) ## Install development tools.
-	GOBIN=$(BINDIR) go install honnef.co/go/tools/cmd/staticcheck@latest
 	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(shell dirname $(GOLANGCI_LINT)) $(GOLANGCI_LINT_VERSION)
 	GOBIN=$(BINDIR) go install sigs.k8s.io/controller-runtime/tools/setup-envtest@$(ENVTEST_BRANCH)
 	GOBIN=$(BINDIR) go install sigs.k8s.io/controller-tools/cmd/controller-gen@v$(CONTROLLER_TOOLS_VERSION)

internal/controller/logicalvolume_controller_test.go

@@ -55,8 +55,7 @@ type MockLVServiceClient struct {
 // CreateLV implements proto.LVServiceClient.
 func (c MockLVServiceClient) CreateLV(ctx context.Context, in *proto.CreateLVRequest, opts ...grpc.CallOption) (*proto.CreateLVResponse, error) {
 	lv := proto.LogicalVolume{
-		Name: in.Name,
-		//lint:ignore SA1019 gRPC API has two fields for Gb and Bytes, both are valid
+		Name:      in.Name,
 		SizeGb:    in.SizeGb,
 		SizeBytes: in.SizeBytes,
 	}

internal/lvmd/command/lvm_command_test.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -91,7 +92,7 @@ func Test_lvm_command(t *testing.T) {
 			t.Fatal("data should be empty as the command should fail")
 		}
 		err = dataStream.Close()
-		if err != nil {
+		if err == nil {
 			t.Fatal(err, "data stream should fail during close")
 		}
 
@@ -111,9 +112,6 @@ func Test_lvm_command(t *testing.T) {
 		if !strings.Contains(lvmErr.Error(), fmt.Sprintf("No device found for %s", fakeDeviceName)) {
 			t.Fatal("No device found message not contained in error")
 		}
-		if dataStream != nil {
-			t.Fatal("data stream should be nil")
-		}
 	})
 
 	t.Run("callLVM should succeed for non-json based calls", func(t *testing.T) {
@@ -134,7 +132,8 @@ func Test_lvm_command(t *testing.T) {
 			t.Fatal("no messages logged")
 		}
 
-		if !strings.Contains(messages[0], `"args"=["/sbin/lvm","version"]`) {
+		match, _ := regexp.MatchString(`"args"=\[.* "/sbin/lvm" "version"\]`, messages[0])
+		if !match {
 			t.Fatal("command log was not found")
 		}
 

internal/lvmd/command/lvm_state_json_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"io"
 	"os"
+	"slices"
 	"strings"
 	"testing"
 
@@ -313,12 +314,16 @@ func TestLvmRetrieval(t *testing.T) {
 		t.Fatal("Unexpected err returned: ", err)
 	}
 
-	if lvs != nil {
-		t.Fatal("Expected lvs to be nil")
+	if slices.ContainsFunc(lvs, func(lv lv) bool {
+		return lv.name == lvName
+	}) {
+		t.Fatalf("Expected lvs not to contains %s", lvName)
 	}
 
-	if len(vgs) != 1 {
-		t.Fatal("Expected 1 vg: ", len(vgs))
+	if !slices.ContainsFunc(vgs, func(vg vg) bool {
+		return vg.name == vgName
+	}) {
+		t.Fatalf("Expected vgs to contains %s", vgName)
 	}
 
 	err = testutils.MakeLoopbackLV(ctx, lvName, vgName)
@@ -331,21 +336,15 @@ func TestLvmRetrieval(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if lvs == nil {
-		t.Fatal("Expected LVs to exist")
+	if !slices.ContainsFunc(lvs, func(lv lv) bool {
+		return lv.name == lvName
+	}) {
+		t.Fatalf("Expected lvs to contains %s", lvName)
 	}
 
-	if len(lvs) != 1 {
-		t.Fatal("Expected 1 LV to exist")
-	}
-
-	vg := vgs[0]
-	if vg.name != vgName {
-		t.Fatal("Incorrect vg name: ", vg.name)
-	}
-
-	lv := lvs[0]
-	if lv.name != lvName {
-		t.Fatal("Incorrect lv name: ", lv.name)
+	if !slices.ContainsFunc(vgs, func(vg vg) bool {
+		return vg.name == vgName
+	}) {
+		t.Fatalf("Expected vgs to contains %s", vgName)
 	}
 }

internal/lvmd/lvservice.go

@@ -54,7 +54,6 @@ func (s *lvService) CreateLV(ctx context.Context, req *proto.CreateLVRequest) (*
 		requested = uint64(req.GetSizeBytes())
 	} else {
 		// legacy conversion from SizeGb to SizeBytes
-		//lint:ignore SA1019 gRPC API has two fields for Gb and Bytes, both are valid until next minor
 		requested = req.GetSizeGb() << 30
 	}
 
@@ -229,7 +228,6 @@ func (s *lvService) CreateLVSnapshot(ctx context.Context, req *proto.CreateLVSna
 		desiredSize = uint64(req.GetSizeBytes())
 	} else {
 		// legacy conversion from SizeGb to SizeBytes
-		//lint:ignore SA1019 gRPC API has two fields for Gb and Bytes, both are valid until next minor
 		desiredSize = req.GetSizeGb() << 30
 	}
 
@@ -343,7 +341,6 @@ func (s *lvService) ResizeLV(ctx context.Context, req *proto.ResizeLVRequest) (*
 		requested = uint64(req.GetSizeBytes())
 	} else {
 		// legacy conversion from SizeGb to SizeBytes
-		//lint:ignore SA1019 gRPC API has two fields for Gb and Bytes, both are valid until next minor
 		requested = req.GetSizeGb() << 30
 	}
 	current := lv.Size()

internal/lvmd/lvservice_test.go

@@ -99,7 +99,6 @@ func TestLVService(t *testing.T) {
 	if res.GetVolume().GetName() != "test1" {
 		t.Errorf(`res.Volume.Name != "test1": %s`, res.GetVolume().GetName())
 	}
-	//lint:ignore SA1019 gRPC API has two fields for Gb and Bytes, both are valid
 	if sizeGB := res.GetVolume().GetSizeGb(); sizeGB != 1 {
 		t.Errorf(`res.Volume.SizeGb != 1: %d`, sizeGB)
 	}
@@ -212,7 +211,6 @@ func TestLVService(t *testing.T) {
 	if res.GetVolume().GetName() != "testp1" {
 		t.Errorf(`res.Volume.Name != "testp1": %s`, res.GetVolume().GetName())
 	}
-	//lint:ignore SA1019 gRPC API has two fields for Gb and Bytes, both are valid
 	if sizeGB := res.GetVolume().GetSizeGb(); sizeGB != 1 {
 		t.Errorf(`res.Volume.SizeGb != 1: %d`, sizeGB)
 	}
@@ -315,7 +313,6 @@ func TestLVService(t *testing.T) {
 	if res.GetVolume().GetName() != "sourceVol" {
 		t.Errorf(`res.Volume.Name != "sourceVol": %s`, res.GetVolume().GetName())
 	}
-	//lint:ignore SA1019 gRPC API has two fields for Gb and Bytes, both are valid
 	if sizeGB := res.GetVolume().GetSizeGb(); sizeGB != originalSizeGb {
 		t.Errorf(`res.Volume.SizeGb != %d: %d`, originalSizeGb, sizeGB)
 	}
@@ -364,14 +361,12 @@ func TestLVService(t *testing.T) {
 	if snapRes.GetSnapshot().GetName() != "snap1" {
 		t.Errorf(`res.Volume.Name != "snap1": %s`, res.GetVolume().GetName())
 	}
-	//lint:ignore SA1019 gRPC API has two fields for Gb and Bytes, both are valid
 	if sizeGB := res.GetVolume().GetSizeGb(); sizeGB != originalSizeGb {
 		t.Errorf(`res.Volume.SizeGb != %d: %d`, originalSizeGb, sizeGB)
 	}
 	if res.GetVolume().GetSizeBytes() != int64(originalSizeGb<<30) {
 		t.Errorf(`res.Volume.SizeBytes != %d: %d`, int64(originalSizeGb<<30), res.GetVolume().GetSizeBytes())
 	}
-	//lint:ignore SA1019 gRPC API has two fields for Gb and Bytes, both are valid
 	if sizeGB := snapRes.GetSnapshot().GetSizeGb(); sizeGB != snapshotDesiredSizeGb {
 		t.Errorf(`res.Volume.SizeGb != %d: %d`, snapshotDesiredSizeGb, sizeGB)
 	}
@@ -416,14 +411,12 @@ func TestLVService(t *testing.T) {
 	if snapRes.GetSnapshot().GetName() != "restoredsnap1" {
 		t.Errorf(`res.Volume.Name != "restoredsnap1": %s`, res.GetVolume().GetName())
 	}
-	//lint:ignore SA1019 gRPC API has two fields for Gb and Bytes, both are valid
 	if sizeGB := res.GetVolume().GetSizeGb(); sizeGB != originalSizeGb {
 		t.Errorf(`res.Volume.SizeGb != %d: %d`, originalSizeGb, sizeGB)
 	}
 	if res.GetVolume().GetSizeBytes() != int64(originalSizeGb<<30) {
 		t.Errorf(`res.Volume.SizeBytes != %d: %d`, int64(originalSizeGb<<30), res.GetVolume().GetSizeBytes())
 	}
-	//lint:ignore SA1019 gRPC API has two fields for Gb and Bytes, both are valid
 	if sizeGB := snapRes.GetSnapshot().GetSizeGb(); sizeGB != snapshotDesiredSizeGb {
 		t.Errorf(`res.Volume.SizeGb != %d: %d`, snapshotDesiredSizeGb, sizeGB)
 	}

internal/lvmd/vgservice_test.go

@@ -258,7 +258,6 @@ func testVGService(t *testing.T, vg *command.VolumeGroup) {
 	if vol.GetName() != "testp1" {
 		t.Errorf(`Volume.Name != "test1": %s`, vol.GetName())
 	}
-	//lint:ignore SA1019 gRPC API has two fields for Gb and Bytes, both are valid
 	if sizeGB := vol.GetSizeGb(); sizeGB != 1 {
 		t.Errorf(`Volume.SizeGb != 1: %d`, sizeGB)
 	}