diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 860c465f..123b6250 100644 --- 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 index b91ad579..3e21117c 100644 --- 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 index 2f86f02b..88672205 100644 --- 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 index 3be8ad93..0370ad91 100644 --- 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 index d327effc..335a255b 100644 --- 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 index 3a73ab10..0cd5f7f3 100644 --- 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 index d243d579..b9b5e278 100644 --- 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