From fdb691632d4ed319f8d0478beb15a58a5cb0e6f3 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 24 Mar 2025 11:10:04 -0700 Subject: [PATCH 1/6] notify_socket.go: fix staticcheck warning > notify_socket.go:44:24: ST1016: methods on the same type should have the same receiver name (seen 1x "n", 5x "s") (staticcheck) > func (s *notifySocket) Close() error { > ^ As reported by staticcheck from golangci-lint v2.0.0 Signed-off-by: Kir Kolyshkin --- notify_socket.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/notify_socket.go b/notify_socket.go index 34c31cc3f..50920c2f9 100644 --- a/notify_socket.go +++ b/notify_socket.go @@ -103,11 +103,11 @@ func (s *notifySocket) waitForContainer(container *libcontainer.Container) error return s.run(state.InitProcessPid) } -func (n *notifySocket) run(pid1 int) error { - if n.socket == nil { +func (s *notifySocket) run(pid1 int) error { + if s.socket == nil { return nil } - notifySocketHostAddr := net.UnixAddr{Name: n.host, Net: "unixgram"} + notifySocketHostAddr := net.UnixAddr{Name: s.host, Net: "unixgram"} client, err := net.DialUnix("unixgram", nil, ¬ifySocketHostAddr) if err != nil { return err @@ -120,7 +120,7 @@ func (n *notifySocket) run(pid1 int) error { go func() { for { buf := make([]byte, 4096) - r, err := n.socket.Read(buf) + r, err := s.socket.Read(buf) if err != nil { return } From 6405725ca2597a3d99c65a0f03ed251228ee3d4f Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 24 Mar 2025 11:28:23 -0700 Subject: [PATCH 2/6] libct: fix staticcheck QF1006 warning > libcontainer/rootfs_linux.go:1255:13: QF1004: could use strings.ReplaceAll instead (staticcheck) > keyPath := strings.Replace(key, ".", "/", -1) Signed-off-by: Kir Kolyshkin --- libcontainer/rootfs_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index 3df948eeb..c848fa773 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -1252,7 +1252,7 @@ func maskPath(path string, mountLabel string) error { // writeSystemProperty writes the value to a path under /proc/sys as determined from the key. // For e.g. net.ipv4.ip_forward translated to /proc/sys/net/ipv4/ip_forward. func writeSystemProperty(key, value string) error { - keyPath := strings.Replace(key, ".", "/", -1) + keyPath := strings.ReplaceAll(key, ".", "/") return os.WriteFile(path.Join("/proc/sys", keyPath), []byte(value), 0o644) } From 9510ffb658dddbcccde490d49c2b82f5064efd2d Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 24 Mar 2025 11:30:06 -0700 Subject: [PATCH 3/6] Fix a few staticcheck QF1001 warnings Like these: > libcontainer/criu_linux.go:959:3: QF1001: could apply De Morgan's law (staticcheck) > !(req.GetType() == criurpc.CriuReqType_FEATURE_CHECK || > ^ > libcontainer/rootfs_linux.go:360:19: QF1001: could apply De Morgan's law (staticcheck) > if err == nil || !(errors.Is(err, unix.EPERM) || errors.Is(err, unix.EBUSY)) { > ^ Signed-off-by: Kir Kolyshkin --- checkpoint.go | 2 +- libcontainer/criu_linux.go | 4 ++-- libcontainer/rootfs_linux.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/checkpoint.go b/checkpoint.go index afd515abd..50046fb93 100644 --- a/checkpoint.go +++ b/checkpoint.go @@ -70,7 +70,7 @@ checkpointed.`, } err = container.Checkpoint(options) - if err == nil && !(options.LeaveRunning || options.PreDump) { + if err == nil && !options.LeaveRunning && !options.PreDump { // Destroy the container unless we tell CRIU to keep it. if err := container.Destroy(); err != nil { logrus.Warn(err) diff --git a/libcontainer/criu_linux.go b/libcontainer/criu_linux.go index 468c5ba9f..30227289e 100644 --- a/libcontainer/criu_linux.go +++ b/libcontainer/criu_linux.go @@ -956,8 +956,8 @@ func (c *Container) criuSwrk(process *Process, req *criurpc.CriuReq, opts *CriuO // available but empty. criurpc.CriuReqType_VERSION actually // has no req.GetOpts(). if logrus.GetLevel() >= logrus.DebugLevel && - !(req.GetType() == criurpc.CriuReqType_FEATURE_CHECK || - req.GetType() == criurpc.CriuReqType_VERSION) { + (req.GetType() != criurpc.CriuReqType_FEATURE_CHECK && + req.GetType() != criurpc.CriuReqType_VERSION) { val := reflect.ValueOf(req.GetOpts()) v := reflect.Indirect(val) diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index c848fa773..b411b0f13 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -357,7 +357,7 @@ func mountCgroupV2(m *configs.Mount, c *mountConfig) error { err := utils.WithProcfd(c.root, m.Destination, func(dstFd string) error { return mountViaFds(m.Source, nil, m.Destination, dstFd, "cgroup2", uintptr(m.Flags), m.Data) }) - if err == nil || !(errors.Is(err, unix.EPERM) || errors.Is(err, unix.EBUSY)) { + if err == nil || (!errors.Is(err, unix.EPERM) && !errors.Is(err, unix.EBUSY)) { return err } From 30f8acabf6050be2c5e840259cbb5509de0849d9 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 24 Mar 2025 11:32:47 -0700 Subject: [PATCH 4/6] Fix staticcheck ST1020/ST1021 warnings I was pretty sure we have a linter for these but apparently we did not. > libcontainer/capabilities/capabilities.go:108:1: ST1020: comment on exported method ApplyCaps should be of the form "ApplyCaps ..." (staticcheck) > // Apply sets all the capabilities for the current process in the config. > ^ > > > types/events.go:15:1: ST1021: comment on exported type Stats should be of the form "Stats ..." (with optional leading article) (staticcheck) > // stats is the runc specific stats structure for stability when encoding and decoding stats. > ^ Signed-off-by: Kir Kolyshkin --- libcontainer/capabilities/capabilities.go | 2 +- types/events.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libcontainer/capabilities/capabilities.go b/libcontainer/capabilities/capabilities.go index 157d84cd9..b5963a328 100644 --- a/libcontainer/capabilities/capabilities.go +++ b/libcontainer/capabilities/capabilities.go @@ -105,7 +105,7 @@ func (c *Caps) ApplyBoundingSet() error { return c.pid.Apply(capability.BOUNDING) } -// Apply sets all the capabilities for the current process in the config. +// ApplyCaps sets all the capabilities for the current process in the config. func (c *Caps) ApplyCaps() error { if c.pid == nil { return nil diff --git a/types/events.go b/types/events.go index 8af94236b..ed7d2408f 100644 --- a/types/events.go +++ b/types/events.go @@ -12,7 +12,7 @@ type Event struct { Data interface{} `json:"data,omitempty"` } -// stats is the runc specific stats structure for stability when encoding and decoding stats. +// Stats is the runc specific stats structure for stability when encoding and decoding stats. type Stats struct { CPU Cpu `json:"cpu"` CPUSet CPUSet `json:"cpuset"` From 9b3ccc19a688ac2c55e8fc3f8ecdc66700e518eb Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 24 Mar 2025 11:37:35 -0700 Subject: [PATCH 5/6] libct/intelrdt: fix staticcheck ST1020 warnings > libcontainer/intelrdt/cmt.go:5:1: ST1020: comment on exported function IsCMTEnabled should be of the form "IsCMTEnabled ..." (staticcheck) > // Check if Intel RDT/CMT is enabled. > ^ > libcontainer/intelrdt/intelrdt.go:419:1: ST1020: comment on exported function IsCATEnabled should be of the form "IsCATEnabled ..." (staticcheck) > // Check if Intel RDT/CAT is enabled > ^ > libcontainer/intelrdt/intelrdt.go:425:1: ST1020: comment on exported function IsMBAEnabled should be of the form "IsMBAEnabled ..." (staticcheck) > // Check if Intel RDT/MBA is enabled > ^ > libcontainer/intelrdt/intelrdt.go:446:1: ST1020: comment on exported method Apply should be of the form "Apply ..." (staticcheck) > // Applies Intel RDT configuration to the process with the specified pid > ^ > libcontainer/intelrdt/intelrdt.go:481:1: ST1020: comment on exported method Destroy should be of the form "Destroy ..." (staticcheck) > // Destroys the Intel RDT container-specific 'container_id' group > ^ > libcontainer/intelrdt/intelrdt.go:497:1: ST1020: comment on exported method GetPath should be of the form "GetPath ..." (staticcheck) > // Returns Intel RDT path to save in a state file and to be able to > ^ > libcontainer/intelrdt/intelrdt.go:506:1: ST1020: comment on exported method GetStats should be of the form "GetStats ..." (staticcheck) > // Returns statistics for Intel RDT > ^ > libcontainer/intelrdt/mbm.go:6:1: ST1020: comment on exported function IsMBMEnabled should be of the form "IsMBMEnabled ..." (staticcheck) > // Check if Intel RDT/MBM is enabled. > ^ > 8 issues: > * staticcheck: 8 While at it, add missing periods. Signed-off-by: Kir Kolyshkin --- libcontainer/intelrdt/cmt.go | 2 +- libcontainer/intelrdt/intelrdt.go | 14 +++++++------- libcontainer/intelrdt/mbm.go | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/libcontainer/intelrdt/cmt.go b/libcontainer/intelrdt/cmt.go index 6480a1306..56cb956ae 100644 --- a/libcontainer/intelrdt/cmt.go +++ b/libcontainer/intelrdt/cmt.go @@ -2,7 +2,7 @@ package intelrdt var cmtEnabled bool -// Check if Intel RDT/CMT is enabled. +// IsCMTEnabled checks if Intel RDT/CMT is enabled. func IsCMTEnabled() bool { featuresInit() return cmtEnabled diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index 97a9fe4d3..7d4b4d38a 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -416,13 +416,13 @@ func WriteIntelRdtTasks(dir string, pid int) error { return nil } -// Check if Intel RDT/CAT is enabled +// IsCATEnabled checks if Intel RDT/CAT is enabled. func IsCATEnabled() bool { featuresInit() return catEnabled } -// Check if Intel RDT/MBA is enabled +// IsMBAEnabled checks if Intel RDT/MBA is enabled. func IsMBAEnabled() bool { featuresInit() return mbaEnabled @@ -443,7 +443,7 @@ func (m *Manager) getIntelRdtPath() (string, error) { return filepath.Join(rootPath, clos), nil } -// Applies Intel RDT configuration to the process with the specified pid +// Apply applies Intel RDT configuration to the process with the specified pid. func (m *Manager) Apply(pid int) (err error) { // If intelRdt is not specified in config, we do nothing if m.config.IntelRdt == nil { @@ -478,7 +478,7 @@ func (m *Manager) Apply(pid int) (err error) { return nil } -// Destroys the Intel RDT container-specific 'container_id' group +// Destroy destroys the Intel RDT container-specific container_id group. func (m *Manager) Destroy() error { // Don't remove resctrl group if closid has been explicitly specified. The // group is likely externally managed, i.e. by some other entity than us. @@ -494,8 +494,8 @@ func (m *Manager) Destroy() error { return nil } -// Returns Intel RDT path to save in a state file and to be able to -// restore the object later +// GetPath returns Intel RDT path to save in a state file and to be able to +// restore the object later. func (m *Manager) GetPath() string { if m.path == "" { m.path, _ = m.getIntelRdtPath() @@ -503,7 +503,7 @@ func (m *Manager) GetPath() string { return m.path } -// Returns statistics for Intel RDT +// GetStats returns statistics for Intel RDT. func (m *Manager) GetStats() (*Stats, error) { // If intelRdt is not specified in config if m.config.IntelRdt == nil { diff --git a/libcontainer/intelrdt/mbm.go b/libcontainer/intelrdt/mbm.go index 13f31ac7a..669168faf 100644 --- a/libcontainer/intelrdt/mbm.go +++ b/libcontainer/intelrdt/mbm.go @@ -3,7 +3,7 @@ package intelrdt // The flag to indicate if Intel RDT/MBM is enabled var mbmEnabled bool -// Check if Intel RDT/MBM is enabled. +// IsMBMEnabled checks if Intel RDT/MBM is enabled. func IsMBMEnabled() bool { featuresInit() return mbmEnabled From 127e8e68d335aa7b908c7822fa985629d8fbe81d Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 24 Mar 2025 11:45:39 -0700 Subject: [PATCH 6/6] ci: bump to golangci-lint v2.0 The new configuration file was initially generated by golangci-lint migrate, when tweaked to minimize and simplify. golangci-lint v2 switches to a new version of staticcheck which shows much more warnings. Some of them were fixed by a few previous commits, and the rest of them are disabled. In particular, ST1005 had to be disabled (an attempt to fix it was made in https://github.com/opencontainers/runc/pull/3857 but it wasn't merged). Also, golangci-extra was modified to include ALL staticcheck linters. Signed-off-by: Kir Kolyshkin --- .github/workflows/validate.yml | 4 ++-- .golangci-extra.yml | 8 +++++++- .golangci.yml | 26 +++++++++++++++++++------- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index b3ac384d3..8e2e0220c 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -38,9 +38,9 @@ jobs: run: | sudo apt -q update sudo apt -qy install libseccomp-dev - - uses: golangci/golangci-lint-action@v6 + - uses: golangci/golangci-lint-action@v7 with: - version: v1.64 + version: v2.0 # Extra linters, only checking new code from a pull request. - name: lint-extra if: github.event_name == 'pull_request' diff --git a/.golangci-extra.yml b/.golangci-extra.yml index be33f90d7..0b27a1bf0 100644 --- a/.golangci-extra.yml +++ b/.golangci-extra.yml @@ -3,13 +3,19 @@ # # For the default linter config, see .golangci.yml. This config should # only enable additional linters not enabled in the default config. +version: "2" run: build-tags: - seccomp linters: - disable-all: true + default: none enable: - godot - revive + - staticcheck + settings: + staticcheck: + checks: + - all diff --git a/.golangci.yml b/.golangci.yml index 64c285cd0..5d31ed92a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,18 +1,30 @@ -# For documentation, see https://golangci-lint.run/usage/configuration/ +version: "2" run: build-tags: - seccomp -linters: +formatters: enable: - gofumpt + +linters: + enable: - errorlint - nolintlint - unconvert - unparam - -linters-settings: - govet: - enable: - - nilness + settings: + govet: + enable: + - nilness + staticcheck: + checks: + - all + - -ST1000 # https://staticcheck.dev/docs/checks/#ST1000 Incorrect or missing package comment. + - -ST1003 # https://staticcheck.dev/docs/checks/#ST1003 Poorly chosen identifier. + - -ST1005 # https://staticcheck.dev/docs/checks/#ST1005 Incorrectly formatted error string. + - -QF1008 # https://staticcheck.dev/docs/checks/#QF1008 Omit embedded fields from selector expression. + exclusions: + presets: + - std-error-handling