Skip to content

Commit

Permalink
Merge pull request containerd#174 from thaJeztah/various_linting_issu…
Browse files Browse the repository at this point in the history
…es_and_cleanup

Various linting issues and cleanup
  • Loading branch information
estesp authored Jul 10, 2020
2 parents 4ef69bc + 8c0b12b commit 318312a
Show file tree
Hide file tree
Showing 19 changed files with 40 additions and 79 deletions.
16 changes: 4 additions & 12 deletions blkio.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (b *blkioController) Create(path string, resources *specs.LinuxResources) e
for _, t := range createBlkioSettings(resources.BlockIO) {
if t.value != nil {
if err := retryingWriteFile(
filepath.Join(b.Path(path), fmt.Sprintf("blkio.%s", t.name)),
filepath.Join(b.Path(path), "blkio."+t.name),
t.format(t.value),
defaultFilePerm,
); err != nil {
Expand All @@ -93,7 +93,7 @@ func (b *blkioController) Stat(path string, stats *v1.Metrics) error {
var settings []blkioStatSettings

// Try to read CFQ stats available on all CFQ enabled kernels first
if _, err := os.Lstat(filepath.Join(b.Path(path), fmt.Sprintf("blkio.io_serviced_recursive"))); err == nil {
if _, err := os.Lstat(filepath.Join(b.Path(path), "blkio.io_serviced_recursive")); err == nil {
settings = []blkioStatSettings{
{
name: "sectors_recursive",
Expand Down Expand Up @@ -173,7 +173,7 @@ func (b *blkioController) Stat(path string, stats *v1.Metrics) error {
}

func (b *blkioController) readEntry(devices map[deviceKey]string, path, name string, entry *[]*v1.BlkIOEntry) error {
f, err := os.Open(filepath.Join(b.Path(path), fmt.Sprintf("blkio.%s", name)))
f, err := os.Open(filepath.Join(b.Path(path), "blkio."+name))
if err != nil {
return err
}
Expand All @@ -187,7 +187,7 @@ func (b *blkioController) readEntry(devices map[deviceKey]string, path, name str
// skip total line
continue
} else {
return fmt.Errorf("Invalid line found while parsing %s: %s", path, sc.Text())
return fmt.Errorf("invalid line found while parsing %s: %s", path, sc.Text())
}
}
major, err := strconv.ParseUint(fields[0], 10, 64)
Expand Down Expand Up @@ -356,11 +356,3 @@ func getDevices(r io.Reader) (map[deviceKey]string, error) {
}
return devices, s.Err()
}

func major(devNumber uint64) uint64 {
return (devNumber >> 8) & 0xfff
}

func minor(devNumber uint64) uint64 {
return (devNumber & 0xff) | ((devNumber >> 12) & 0xfff00)
}
10 changes: 5 additions & 5 deletions cgroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,15 +216,15 @@ func (c *cgroup) Delete() error {
if c.err != nil {
return c.err
}
var errors []string
var errs []string
for _, s := range c.subsystems {
if d, ok := s.(deleter); ok {
sp, err := c.path(s.Name())
if err != nil {
return err
}
if err := d.Delete(sp); err != nil {
errors = append(errors, string(s.Name()))
errs = append(errs, string(s.Name()))
}
continue
}
Expand All @@ -235,12 +235,12 @@ func (c *cgroup) Delete() error {
}
path := p.Path(sp)
if err := remove(path); err != nil {
errors = append(errors, path)
errs = append(errs, path)
}
}
}
if len(errors) > 0 {
return fmt.Errorf("cgroups: unable to remove paths %s", strings.Join(errors, ", "))
if len(errs) > 0 {
return fmt.Errorf("cgroups: unable to remove paths %s", strings.Join(errs, ", "))
}
c.err = ErrCgroupDeleted
return nil
Expand Down
4 changes: 2 additions & 2 deletions cgroup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func checkPid(mock *mockCgroup, path string, expected int) error {
if err != nil {
return err
}
v, err := strconv.Atoi(string(data))
v, err := strconv.Atoi(data)
if err != nil {
return err
}
Expand All @@ -222,7 +222,7 @@ func checkTaskid(mock *mockCgroup, path string, expected int) error {
if err != nil {
return err
}
v, err := strconv.Atoi(string(data))
v, err := strconv.Atoi(data)
if err != nil {
return err
}
Expand Down
3 changes: 1 addition & 2 deletions cpu.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package cgroups

import (
"bufio"
"fmt"
"os"
"path/filepath"
"strconv"
Expand Down Expand Up @@ -84,7 +83,7 @@ func (c *cpuController) Create(path string, resources *specs.LinuxResources) err
}
if value != nil {
if err := retryingWriteFile(
filepath.Join(c.Path(path), fmt.Sprintf("cpu.%s", t.name)),
filepath.Join(c.Path(path), "cpu."+t.name),
value,
defaultFilePerm,
); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion cpuset.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (c *cpusetController) Create(path string, resources *specs.LinuxResources)
} {
if t.value != "" {
if err := retryingWriteFile(
filepath.Join(c.Path(path), fmt.Sprintf("cpuset.%s", t.name)),
filepath.Join(c.Path(path), "cpuset."+t.name),
[]byte(t.value),
defaultFilePerm,
); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ func (m *memoryController) set(path string, settings []memorySettings) error {
for _, t := range settings {
if t.value != nil {
if err := retryingWriteFile(
filepath.Join(m.Path(path), fmt.Sprintf("memory.%s", t.name)),
filepath.Join(m.Path(path), "memory."+t.name),
[]byte(strconv.FormatInt(*t.value, 10)),
defaultFilePerm,
); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@ func newInitConfig() *InitConfig {
type InitCheck func(Subsystem, Path, error) error

// AllowAny allows any subsystem errors to be skipped
func AllowAny(s Subsystem, p Path, err error) error {
func AllowAny(_ Subsystem, _ Path, _ error) error {
return ErrIgnoreSubsystem
}

// RequireDevices requires the device subsystem but no others
func RequireDevices(s Subsystem, p Path, err error) error {
func RequireDevices(s Subsystem, _ Path, _ error) error {
if s.Name() == Devices {
return ErrDevicesRequired
}
Expand Down
6 changes: 3 additions & 3 deletions paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (

type Path func(subsystem Name) (string, error)

func RootPath(subsysem Name) (string, error) {
func RootPath(subsystem Name) (string, error) {
return "/", nil
}

Expand Down Expand Up @@ -63,7 +63,7 @@ var ErrControllerNotActive = errors.New("controller is not supported")
func existingPath(paths map[string]string, suffix string) Path {
// localize the paths based on the root mount dest for nested cgroups
for n, p := range paths {
dest, err := getCgroupDestination(string(n))
dest, err := getCgroupDestination(n)
if err != nil {
return errorPath(err)
}
Expand All @@ -79,7 +79,7 @@ func existingPath(paths map[string]string, suffix string) Path {
return func(name Name) (string, error) {
root, ok := paths[string(name)]
if !ok {
if root, ok = paths[fmt.Sprintf("name=%s", name)]; !ok {
if root, ok = paths["name="+string(name)]; !ok {
return "", ErrControllerNotActive
}
}
Expand Down
9 changes: 2 additions & 7 deletions systemd.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package cgroups

import (
"fmt"
"path/filepath"
"strings"
"sync"
Expand Down Expand Up @@ -78,7 +77,7 @@ func (s *SystemdController) Name() Name {
return SystemdDbus
}

func (s *SystemdController) Create(path string, resources *specs.LinuxResources) error {
func (s *SystemdController) Create(path string, _ *specs.LinuxResources) error {
conn, err := systemdDbus.New()
if err != nil {
return err
Expand All @@ -105,7 +104,7 @@ func (s *SystemdController) Create(path string, resources *specs.LinuxResources)
}
once.Do(checkDelegate)
properties := []systemdDbus.Property{
systemdDbus.PropDescription(fmt.Sprintf("cgroup %s", name)),
systemdDbus.PropDescription("cgroup " + name),
systemdDbus.PropWants(slice),
newProperty("DefaultDependencies", false),
newProperty("MemoryAccounting", true),
Expand Down Expand Up @@ -150,10 +149,6 @@ func newProperty(name string, units interface{}) systemdDbus.Property {
}
}

func unitName(name string) string {
return fmt.Sprintf("%s.slice", name)
}

func splitName(path string) (slice string, unit string) {
slice, unit = filepath.Split(path)
return strings.TrimSuffix(slice, "/"), unit
Expand Down
4 changes: 2 additions & 2 deletions utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -382,14 +382,14 @@ func cleanPath(path string) string {
if !filepath.IsAbs(path) {
path, _ = filepath.Rel(string(os.PathSeparator), filepath.Clean(string(os.PathSeparator)+path))
}
return filepath.Clean(path)
return path
}

func retryingWriteFile(path string, data []byte, mode os.FileMode) error {
// Retry writes on EINTR; see:
// https://github.com/golang/go/issues/38033
for {
err := ioutil.WriteFile(path, []byte(data), mode)
err := ioutil.WriteFile(path, data, mode)
if err == nil {
return nil
} else if !errors.Is(err, syscall.EINTR) {
Expand Down
2 changes: 1 addition & 1 deletion v2/cpuv2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func TestExtractQuotaAndPeriod(t *testing.T) {
assert.Equal(t, quota, tquota)
assert.Equal(t, period, tPeriod)

//case with nil quota which makes it "max" - max int val
// case with nil quota which makes it "max" - max int val
cpuMax2 := NewCPUMax(nil, &period)
tquota2, tPeriod2 := cpuMax2.extractQuotaAndPeriod()

Expand Down
4 changes: 0 additions & 4 deletions v2/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,3 @@ func IgnoreNotExist(err error) error {
}
return err
}

func errPassthrough(err error) error {
return err
}
2 changes: 1 addition & 1 deletion v2/iov2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestCgroupv2IOController(t *testing.T) {
group := "/io-test-cg"
groupPath := fmt.Sprintf("%s-%d", group, os.Getpid())
var (
//weight uint16 = 100
// weight uint16 = 100
maj int64 = 8
min int64 = 0
rate uint64 = 120
Expand Down
23 changes: 6 additions & 17 deletions v2/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,22 @@ package v2

import (
"bufio"
stderrors "errors"
"fmt"
"io/ioutil"
"math"
"os"
"path/filepath"
"strconv"
"strings"
"sync"
"syscall"
"time"

"golang.org/x/sys/unix"

"github.com/containerd/cgroups/v2/stats"
systemdDbus "github.com/coreos/go-systemd/v22/dbus"
"github.com/godbus/dbus/v5"
"github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"

systemdDbus "github.com/coreos/go-systemd/v22/dbus"
"golang.org/x/sys/unix"
)

const (
Expand All @@ -50,13 +45,8 @@ const (

var (
canDelegate bool
once sync.Once
)

type cgValuer interface {
Values() []Value
}

type Event struct {
Low uint64
High uint64
Expand Down Expand Up @@ -161,7 +151,7 @@ func (c *Value) write(path string, perm os.FileMode) error {
)
if err == nil {
return nil
} else if !stderrors.Is(err, syscall.EINTR) {
} else if !errors.Is(err, syscall.EINTR) {
return err
}
}
Expand Down Expand Up @@ -270,7 +260,7 @@ func (c *Manager) ToggleControllers(controllers []string, t ControllerToggle) er
// Note that /sys/fs/cgroup/foo/bar/baz/cgroup.subtree_control does not need to be written.
split := strings.Split(c.path, "/")
var lastErr error
for i, _ := range split {
for i := range split {
f := strings.Join(split[:i], "/")
if !strings.HasPrefix(f, c.unifiedMountpoint) || f == c.path {
continue
Expand Down Expand Up @@ -373,8 +363,7 @@ func (c *Manager) Stat() (*stats.Metrics, error) {
for _, controller := range controllers {
switch controller {
case "cpu", "memory":
filename := fmt.Sprintf("%s.stat", controller)
if err := readKVStatsFile(c.path, filename, out); err != nil {
if err := readKVStatsFile(c.path, controller+".stat", out); err != nil {
if os.IsNotExist(err) {
continue
}
Expand Down Expand Up @@ -681,7 +670,7 @@ func NewSystemd(slice, group string, pid int, resources *Resources) (*Manager, e
defer conn.Close()

properties := []systemdDbus.Property{
systemdDbus.PropDescription(fmt.Sprintf("cgroup %s", group)),
systemdDbus.PropDescription("cgroup " + group),
newSystemdProperty("DefaultDependencies", false),
newSystemdProperty("MemoryAccounting", true),
newSystemdProperty("CPUAccounting", true),
Expand Down
2 changes: 1 addition & 1 deletion v2/paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func NestedGroupPath(suffix string) (string, error) {
if err != nil {
return "", err
}
return filepath.Join(string(path), suffix), nil
return filepath.Join(path, suffix), nil
}

// PidGroupPath will return the correct cgroup paths for an existing process running inside a cgroup
Expand Down
2 changes: 1 addition & 1 deletion v2/pidsv2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,5 @@ func TestSystemdCgroupPidsController(t *testing.T) {
if err != nil {
t.Fatal("failed to init new cgroup systemd manager: ", err)
}
checkFileContent(t, c.path, "cgroup.procs", strconv.Itoa(int(pid)))
checkFileContent(t, c.path, "cgroup.procs", strconv.Itoa(pid))
}
1 change: 0 additions & 1 deletion v2/testutils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"testing"

"github.com/stretchr/testify/assert"

"golang.org/x/sys/unix"
)

Expand Down
Loading

0 comments on commit 318312a

Please sign in to comment.