Add Golangci-lint to CI

Resolve warnings found by linter. Signed-off-by: Austin Vazquez <[email protected]>
containerd · Feb 9, 2023 · ef1dea3 · ef1dea3
1 parent 157c4da
commit ef1dea3
Show file tree

Hide file tree

Showing 7 changed files with 147 additions and 26 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -47,6 +47,34 @@ jobs:
         with:
           working-directory: src/github.com/containerd/cgroups
 
+  lint:
+    name: Lint
+    timeout-minutes: 10
+    needs: [project]
+    runs-on: ubuntu-22.04
+
+    strategy:
+      matrix:
+        go-version: [1.19.x, 1.20.x]
+
+    steps:
+      - name: Install Go
+        uses: actions/setup-go@v3
+        with:
+          go-version: ${{ matrix.go }}
+
+      - name: Checkout cgroups
+        uses: actions/checkout@v3
+        with:
+          path: src/github.com/containerd/cgroups
+
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v3
+        with:
+          version: v1.51.1
+          args: --verbose
+          working-directory: src/github.com/containerd/cgroups
+
   test:
     name: Test cgroups
     timeout-minutes: 15

diff --git a/cgroup1/cgroup_test.go b/cgroup1/cgroup_test.go
@@ -33,7 +33,11 @@ func TestCreate(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	control, err := New(StaticPath("test"), &specs.LinuxResources{}, WithHiearchy(mock.hierarchy))
 	if err != nil {
 		t.Error(err)
@@ -60,7 +64,11 @@ func TestStat(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	control, err := New(StaticPath("test"), &specs.LinuxResources{}, WithHiearchy(mock.hierarchy))
 	if err != nil {
 		t.Error(err)
@@ -82,7 +90,11 @@ func TestAdd(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	control, err := New(StaticPath("test"), &specs.LinuxResources{}, WithHiearchy(mock.hierarchy))
 	if err != nil {
 		t.Error(err)
@@ -105,7 +117,11 @@ func TestAddFilteredSubsystems(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	control, err := New(StaticPath("test"), &specs.LinuxResources{}, WithHiearchy(mock.hierarchy))
 	if err != nil {
 		t.Error(err)
@@ -162,7 +178,11 @@ func TestAddTask(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	control, err := New(StaticPath("test"), &specs.LinuxResources{}, WithHiearchy(mock.hierarchy))
 	if err != nil {
 		t.Error(err)
@@ -185,7 +205,11 @@ func TestAddTaskFilteredSubsystems(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	control, err := New(StaticPath("test"), &specs.LinuxResources{}, WithHiearchy(mock.hierarchy))
 	if err != nil {
 		t.Error(err)
@@ -227,7 +251,11 @@ func TestListPids(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	control, err := New(StaticPath("test"), &specs.LinuxResources{}, WithHiearchy(mock.hierarchy))
 	if err != nil {
 		t.Error(err)
@@ -262,7 +290,11 @@ func TestListTasksPids(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	control, err := New(StaticPath("test"), &specs.LinuxResources{}, WithHiearchy(mock.hierarchy))
 	if err != nil {
 		t.Error(err)
@@ -349,12 +381,21 @@ func TestLoad(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	control, err := New(StaticPath("test"), &specs.LinuxResources{}, WithHiearchy(mock.hierarchy))
 	if err != nil {
 		t.Error(err)
 		return
 	}
+	defer func() {
+		if err := control.Delete(); err != nil {
+			t.Errorf("failed to delete cgroup control: %v", err)
+		}
+	}()
 	if control, err = Load(StaticPath("test"), WithHiearchy(mock.hierarchy)); err != nil {
 		t.Error(err)
 		return
@@ -370,7 +411,11 @@ func TestLoadWithMissingSubsystems(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	subsystems, err := mock.hierarchy()
 	if err != nil {
 		t.Error(err)
@@ -404,7 +449,11 @@ func TestDelete(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	control, err := New(StaticPath("test"), &specs.LinuxResources{}, WithHiearchy(mock.hierarchy))
 	if err != nil {
 		t.Error(err)
@@ -420,7 +469,11 @@ func TestCreateSubCgroup(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	control, err := New(StaticPath("test"), &specs.LinuxResources{}, WithHiearchy(mock.hierarchy))
 	if err != nil {
 		t.Error(err)
@@ -458,7 +511,11 @@ func TestFreezeThaw(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	control, err := New(StaticPath("test"), &specs.LinuxResources{}, WithHiearchy(mock.hierarchy))
 	if err != nil {
 		t.Error(err)
@@ -487,7 +544,11 @@ func TestSubsystems(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	control, err := New(StaticPath("test"), &specs.LinuxResources{}, WithHiearchy(mock.hierarchy))
 	if err != nil {
 		t.Error(err)
@@ -510,13 +571,21 @@ func TestCpusetParent(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	control, err := New(StaticPath("/parent/child"), &specs.LinuxResources{}, WithHiearchy(mock.hierarchy))
 	if err != nil {
 		t.Error(err)
 		return
 	}
-	defer control.Delete()
+	defer func() {
+		if err := control.Delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	for _, file := range []string{
 		"parent/cpuset.cpus",
 		"parent/cpuset.mems",

diff --git a/cgroup1/cpuacct_test.go b/cgroup1/cpuacct_test.go
@@ -32,7 +32,11 @@ func TestGetUsage(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	cpuacct := NewCpuacct(mock.root)
 	if cpuacct == nil {
 		t.Fatal("cpuacct is nil")

diff --git a/cgroup1/pids_test.go b/cgroup1/pids_test.go
@@ -32,7 +32,11 @@ func TestPids(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	pids := NewPids(mock.root)
 	if pids == nil {
 		t.Fatal("pids is nil")
@@ -89,7 +93,11 @@ func TestPidsMissingCurrent(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	pids := NewPids(mock.root)
 	if pids == nil {
 		t.Fatal("pids is nil")
@@ -106,7 +114,11 @@ func TestPidsMissingMax(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	pids := NewPids(mock.root)
 	if pids == nil {
 		t.Fatal("pids is nil")
@@ -135,7 +147,11 @@ func TestPidsOverflowMax(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer mock.delete()
+	defer func() {
+		if err := mock.delete(); err != nil {
+			t.Errorf("failed delete: %v", err)
+		}
+	}()
 	pids := NewPids(mock.root)
 	if pids == nil {
 		t.Fatal("pids is nil")

diff --git a/cgroup1/systemd.go b/cgroup1/systemd.go
@@ -29,7 +29,7 @@ import (
 
 const (
 	SystemdDbus  Name = "systemd"
-	defaultSlice      = "system.slice"
+	defaultSlice Name = "system.slice"
 )
 
 var (
@@ -56,7 +56,7 @@ func Systemd() ([]Subsystem, error) {
 
 func Slice(slice, name string) Path {
 	if slice == "" {
-		slice = defaultSlice
+		slice = string(defaultSlice)
 	}
 	return func(subsystem Name) (string, error) {
 		return filepath.Join(slice, name), nil
@@ -70,7 +70,6 @@ func NewSystemd(root string) (*SystemdController, error) {
 }
 
 type SystemdController struct {
-	mu   sync.Mutex
 	root string
 }
 

diff --git a/cgroup2/devicefilter.go b/cgroup2/devicefilter.go
@@ -167,7 +167,7 @@ func (p *program) appendDevice(dev specs.LinuxDeviceCgroup) error {
 	}
 	p.insts = append(p.insts, acceptBlock(dev.Allow)...)
 	// set blockSym to the first instruction we added in this iteration
-	p.insts[prevBlockLastIdx+1] = p.insts[prevBlockLastIdx+1].Sym(blockSym)
+	p.insts[prevBlockLastIdx+1] = p.insts[prevBlockLastIdx+1].WithSymbol(blockSym)
 	p.blockID++
 	return nil
 }
@@ -180,7 +180,7 @@ func (p *program) finalize() (asm.Instructions, error) {
 	blockSym := fmt.Sprintf("block-%d", p.blockID)
 	p.insts = append(p.insts,
 		// R0 <- 0
-		asm.Mov.Imm32(asm.R0, 0).Sym(blockSym),
+		asm.Mov.Imm32(asm.R0, 0).WithSymbol(blockSym),
 		asm.Return(),
 	)
 	p.blockID = -1

diff --git a/cgroup2/manager_test.go b/cgroup2/manager_test.go
@@ -42,6 +42,7 @@ func TestEventChanCleanupOnCgroupRemoval(t *testing.T) {
 	proc := cmd.Process
 	if proc == nil {
 		t.Fatal("Process is nil")
+		return
 	}
 
 	group := fmt.Sprintf("testing-watcher-%d.scope", proc.Pid)
@@ -212,6 +213,10 @@ func TestMoveTo(t *testing.T) {
 		return
 	}
 	desProcs, err := destination.Procs(true)
+	if err != nil {
+		t.Error(err)
+		return
+	}
 	desMap := make(map[int]bool)
 	for _, p := range desProcs {
 		desMap[int(p)] = true