From ac591f1913541710374ad171be9e44649858e5e3 Mon Sep 17 00:00:00 2001 From: Krisztian Litkey Date: Mon, 9 Dec 2024 19:32:32 +0200 Subject: [PATCH] resmgr/cache: golangci-lint fixes. Signed-off-by: Krisztian Litkey --- pkg/resmgr/cache/cache.go | 68 +++++++++++++++++------------- pkg/resmgr/cache/container.go | 22 ++-------- pkg/resmgr/cache/container_test.go | 40 +++++++++--------- pkg/resmgr/cache/pod.go | 2 +- pkg/resmgr/cache/utils.go | 28 +----------- 5 files changed, 63 insertions(+), 97 deletions(-) diff --git a/pkg/resmgr/cache/cache.go b/pkg/resmgr/cache/cache.go index dc0982617..cee39af62 100644 --- a/pkg/resmgr/cache/cache.go +++ b/pkg/resmgr/cache/cache.go @@ -539,7 +539,9 @@ func (cch *cache) ResetActivePolicy() error { func (cch *cache) InsertPod(nriPod *nri.PodSandbox, ch <-chan *podresapi.PodResources) Pod { p := cch.createPod(nriPod, ch) cch.Pods[nriPod.GetId()] = p - cch.Save() + if err := cch.Save(); err != nil { + log.Warnf("failed to save cache: %v", err) + } return p } @@ -554,7 +556,9 @@ func (cch *cache) DeletePod(id string) Pod { log.Debug("removing pod %s (%s)", p.PrettyName(), p.GetID()) delete(cch.Pods, id) - cch.Save() + if err := cch.Save(); err != nil { + log.Warnf("failed to save cache: %v", err) + } return p } @@ -569,18 +573,18 @@ func (cch *cache) LookupPod(id string) (Pod, bool) { func (cch *cache) InsertContainer(ctr *nri.Container) (Container, error) { var err error - c := &container{ - cache: cch, - } - - c, err = cch.createContainer(ctr) + c, err := cch.createContainer(ctr) if err != nil { return nil, cacheError("failed to insert container %s: %v", c.GetID(), err) } cch.Containers[c.GetID()] = c - cch.createContainerDirectory(c.GetID()) - cch.Save() + if err := cch.createContainerDirectory(c.GetID()); err != nil { + log.Warnf("failed to create container directory for %s: %v", c.GetID(), err) + } + if err := cch.Save(); err != nil { + log.Warnf("failed to save cache: %v", err) + } return c, nil } @@ -593,10 +597,14 @@ func (cch *cache) DeleteContainer(id string) Container { } log.Debug("removing container %s", c.PrettyName()) - cch.removeContainerDirectory(c.GetID()) + if err := cch.removeContainerDirectory(c.GetID()); err != nil { + log.Warnf("failed to remove container directory for %s: %v", c.GetID(), err) + } delete(cch.Containers, c.GetID()) - cch.Save() + if err := cch.Save(); err != nil { + log.Warnf("failed to save cache: %v", err) + } return c } @@ -624,7 +632,7 @@ func (cch *cache) LookupContainerByCgroup(path string) (Container, bool) { continue } - if strings.Index(path, c.GetID()) != -1 { + if strings.Contains(path, c.GetID()) { return c, true } } @@ -827,12 +835,12 @@ func (cch *cache) GetPolicyEntry(key string, ptr interface{}) bool { // Marshal an opaque policy entry, special-casing cpusets and maps of cpusets. func marshalEntry(obj interface{}) ([]byte, error) { - switch obj.(type) { + switch obj := obj.(type) { case cpuset.CPUSet: - return []byte("\"" + obj.(cpuset.CPUSet).String() + "\""), nil + return []byte("\"" + obj.String() + "\""), nil case map[string]cpuset.CPUSet: dst := make(map[string]string) - for key, cset := range obj.(map[string]cpuset.CPUSet) { + for key, cset := range obj { dst[key] = cset.String() } return json.Marshal(dst) @@ -844,13 +852,13 @@ func marshalEntry(obj interface{}) ([]byte, error) { // Unmarshal an opaque policy entry, special-casing cpusets and maps of cpusets. func unmarshalEntry(data []byte, ptr interface{}) error { - switch ptr.(type) { + switch ptr := ptr.(type) { case *cpuset.CPUSet: cset, err := cpuset.Parse(string(data[1 : len(data)-1])) if err != nil { return err } - *ptr.(*cpuset.CPUSet) = cset + *ptr = cset return nil case *map[string]cpuset.CPUSet: @@ -868,7 +876,7 @@ func unmarshalEntry(data []byte, ptr interface{}) error { dst[key] = cset } - *ptr.(*map[string]cpuset.CPUSet) = dst + *ptr = dst return nil default: @@ -884,32 +892,32 @@ func (cch *cache) cacheEntry(key string, ptr interface{}) error { return nil } - switch ptr.(type) { + switch ptr := ptr.(type) { case *cpuset.CPUSet: - cch.policyData[key] = *ptr.(*cpuset.CPUSet) + cch.policyData[key] = *ptr case *map[string]cpuset.CPUSet: - cch.policyData[key] = *ptr.(*map[string]cpuset.CPUSet) + cch.policyData[key] = *ptr case *map[string]string: - cch.policyData[key] = *ptr.(*map[string]string) + cch.policyData[key] = *ptr case *string: - cch.policyData[key] = *ptr.(*string) + cch.policyData[key] = *ptr case *bool: - cch.policyData[key] = *ptr.(*bool) + cch.policyData[key] = *ptr case *int32: - cch.policyData[key] = *ptr.(*int32) + cch.policyData[key] = *ptr case *uint32: - cch.policyData[key] = *ptr.(*uint32) + cch.policyData[key] = *ptr case *int64: - cch.policyData[key] = *ptr.(*int64) + cch.policyData[key] = *ptr case *uint64: - cch.policyData[key] = *ptr.(*uint64) + cch.policyData[key] = *ptr case *int: - cch.policyData[key] = *ptr.(*int) + cch.policyData[key] = *ptr case *uint: - cch.policyData[key] = *ptr.(*uint) + cch.policyData[key] = *ptr default: return cacheError("can't handle policy data of type %T", ptr) diff --git a/pkg/resmgr/cache/container.go b/pkg/resmgr/cache/container.go index 3031e1f5b..26bccc360 100644 --- a/pkg/resmgr/cache/container.go +++ b/pkg/resmgr/cache/container.go @@ -19,6 +19,7 @@ import ( "fmt" "path/filepath" "regexp" + "slices" "sort" "strconv" "strings" @@ -272,11 +273,11 @@ func isReadOnlyDevice(rules []*nri.LinuxDeviceCgroup, d *nri.LinuxDevice) bool { rType, rMajor, rMinor := r.Type, r.GetMajor().GetValue(), r.GetMinor().GetValue() switch { case rType == "" && rMajor == 0 && rMinor == 0: - if strings.IndexAny(r.Access, "w") > -1 { + if strings.Contains(r.Access, "w") { readOnly = false } case d.Type == rType && d.Major == rMajor && d.Minor == rMinor: - if strings.IndexAny(r.Access, "w") > -1 { + if strings.Contains(r.Access, "w") { readOnly = false } return readOnly @@ -416,15 +417,11 @@ func (c *container) GetMounts() []*Mount { var mounts []*Mount for _, m := range c.Ctr.GetMounts() { - var options []string - for _, o := range m.Options { - options = append(options, o) - } mounts = append(mounts, &Mount{ Destination: m.Destination, Source: m.Source, Type: m.Type, - Options: options, + Options: slices.Clone(m.Options), }) } @@ -890,17 +887,6 @@ func getTopologyHintsForDevice(devType string, major, minor int64, allowPathList return topology.Hints{} } -func getKubeletHint(cpus, mems string) (ret topology.Hints) { - if cpus != "" || mems != "" { - ret = topology.Hints{ - topology.ProviderKubelet: topology.Hint{ - Provider: topology.ProviderKubelet, - CPUs: cpus, - NUMAs: mems}} - } - return -} - func (c *container) GetAffinity() ([]*Affinity, error) { pod, ok := c.GetPod() if !ok { diff --git a/pkg/resmgr/cache/container_test.go b/pkg/resmgr/cache/container_test.go index f13ec97c1..5d0d3a831 100644 --- a/pkg/resmgr/cache/container_test.go +++ b/pkg/resmgr/cache/container_test.go @@ -19,6 +19,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "syscall" "github.com/containers/nri-plugins/pkg/log" @@ -799,24 +800,24 @@ func checkAllowedHints(annotations map[string]string, expectedHints int) bool { ann := "allow" + "." + cache.TopologyHintsKey allow, ok := ctr.GetEffectiveAnnotation(ann) if !ok { - fmt.Errorf("unable to get annotation %s (%s)", ann, allow) + log.Get("cache").Errorf("unable to get annotation %s (%s)", ann, allow) return false } if err := yaml.Unmarshal([]byte(allow), &pathList); err != nil { - fmt.Errorf("Error (%v) when trying to parse \"%s\"", err, allow) + log.Get("cache").Errorf("Error (%v) when trying to parse \"%s\"", err, allow) return false } ann = "deny" + "." + cache.TopologyHintsKey deny, ok := ctr.GetEffectiveAnnotation(ann) if !ok { - fmt.Errorf("unable to get annotation %s (%s)", ann, deny) + log.Get("cache").Errorf("unable to get annotation %s (%s)", ann, deny) return false } if err := yaml.Unmarshal([]byte(deny), &pathList); err != nil { - fmt.Errorf("Error (%v) when trying to parse \"%s\"", err, deny) + log.Get("cache").Errorf("Error (%v) when trying to parse \"%s\"", err, deny) return false } @@ -840,6 +841,9 @@ func createSysFsDevice(devType string, major, minor int64) error { } realDevPath, err := filepath.EvalSymlinks(devPath) + if err != nil { + return err + } if err := os.MkdirAll(testdataDir+"/"+realDevPath, 0770); err != nil { return err } @@ -852,7 +856,9 @@ func createSysFsDevice(devType string, major, minor int64) error { return err } - f.Write([]byte(cpulist)) + if _, err := f.Write([]byte(cpulist)); err != nil { + log.Get("cache").Errorf("unable to write to %s: %v", realDevPath+"/local_cpulist", err) + } f.Close() f, err = os.Create(realDevPath + "/numa_node") @@ -860,7 +866,9 @@ func createSysFsDevice(devType string, major, minor int64) error { return err } - f.Write([]byte(numanode)) + if _, err := f.Write([]byte(numanode)); err != nil { + log.Get("cache").Errorf("unable to write to %s: %v", realDevPath+"/numa_node", err) + } f.Close() return nil @@ -927,10 +935,7 @@ func WithCtrArgs(args []string) CtrOption { nriCtr.Args = nil return nil } - nriCtr.Args = make([]string, len(args), len(args)) - for i, a := range args { - nriCtr.Args[i] = a - } + nriCtr.Args = slices.Clone(args) return nil } } @@ -941,10 +946,7 @@ func WithCtrEnv(env []string) CtrOption { nriCtr.Env = nil return nil } - nriCtr.Env = make([]string, len(env), len(env)) - for i, e := range env { - nriCtr.Env[i] = e - } + nriCtr.Env = slices.Clone(env) return nil } } @@ -955,17 +957,13 @@ func WithCtrMounts(mounts []*nri.Mount) CtrOption { nriCtr.Mounts = nil return nil } - nriCtr.Mounts = make([]*nri.Mount, len(mounts), len(mounts)) + nriCtr.Mounts = make([]*nri.Mount, len(mounts)) for i, m := range mounts { - var options []string - for _, o := range m.Options { - options = append(options, o) - } nriCtr.Mounts[i] = &nri.Mount{ Destination: m.Destination, Source: m.Source, Type: m.Type, - Options: options, + Options: slices.Clone(m.Options), } } return nil @@ -983,7 +981,7 @@ func WithCtrDevices(devices []*nri.LinuxDevice) CtrOption { if nriCtr.Linux == nil { nriCtr.Linux = &nri.LinuxContainer{} } - nriCtr.Linux.Devices = make([]*nri.LinuxDevice, len(devices), len(devices)) + nriCtr.Linux.Devices = make([]*nri.LinuxDevice, len(devices)) for i, d := range devices { nriCtr.Linux.Devices[i] = &nri.LinuxDevice{ Path: d.Path, diff --git a/pkg/resmgr/cache/pod.go b/pkg/resmgr/cache/pod.go index 2c82083b5..9fbed7274 100644 --- a/pkg/resmgr/cache/pod.go +++ b/pkg/resmgr/cache/pod.go @@ -157,7 +157,7 @@ func (p *pod) setPodResources(podRes *podresapi.PodResources) { func (p *pod) GetPodResources() *podresapi.PodResources { if p.waitResCh != nil { log.Debug("waiting for pod resources fetch to complete...") - _ = <-p.waitResCh + <-p.waitResCh } return p.PodResources } diff --git a/pkg/resmgr/cache/utils.go b/pkg/resmgr/cache/utils.go index 93513fcde..940dd5fe3 100644 --- a/pkg/resmgr/cache/utils.go +++ b/pkg/resmgr/cache/utils.go @@ -15,7 +15,6 @@ package cache import ( - "io/ioutil" "os" "path" "strconv" @@ -97,7 +96,7 @@ func getMemoryCapacity() int64 { return memoryCapacity } - if data, err = ioutil.ReadFile("/proc/meminfo"); err != nil { + if data, err = os.ReadFile("/proc/meminfo"); err != nil { return -1 } @@ -124,27 +123,6 @@ func getMemoryCapacity() int64 { return memoryCapacity } -// cgroupParentToQOS tries to map Pod cgroup parent to QOS class. -func cgroupParentToQOS(dir string) corev1.PodQOSClass { - var qos corev1.PodQOSClass - - // The parent directory naming scheme depends on the cgroup driver in use. - // Thus, rely on substring matching - split := strings.Split(strings.TrimPrefix(dir, "/"), "/") - switch { - case len(split) < 2: - qos = corev1.PodQOSClass("") - case strings.Index(split[1], strings.ToLower(string(corev1.PodQOSBurstable))) != -1: - qos = corev1.PodQOSBurstable - case strings.Index(split[1], strings.ToLower(string(corev1.PodQOSBestEffort))) != -1: - qos = corev1.PodQOSBestEffort - default: - qos = corev1.PodQOSGuaranteed - } - - return qos -} - // findContainerDir brute-force searches for a container cgroup dir. func findContainerDir(podCgroupDir, podID, ID string) string { var dirs []string @@ -178,10 +156,6 @@ func findContainerDir(podCgroupDir, podID, ID string) string { return "" } -func isSupportedQoSComputeResource(name corev1.ResourceName) bool { - return name == corev1.ResourceCPU || name == corev1.ResourceMemory -} - func init() { // TODO: get rid of this eventually, use pkg/sysfs instead... getMemoryCapacity()