Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use golang builtin multierror #1023

Merged
merged 3 commits into from Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions go.mod
Expand Up @@ -8,7 +8,6 @@ require (
github.com/cilium/ebpf v0.7.0
github.com/evanphx/json-patch v4.12.0+incompatible
github.com/google/go-cmp v0.5.9
github.com/hashicorp/go-multierror v1.1.1
github.com/intel/cri-resource-manager/pkg/topology v0.0.0
github.com/intel/goresctrl v0.3.0
github.com/pkg/errors v0.9.1
Expand Down Expand Up @@ -67,7 +66,6 @@ require (
github.com/google/gnostic v0.5.7-v3refs // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/google/uuid v1.3.0 // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/imdario/mergo v0.3.12 // indirect
github.com/inconshreveable/mousetrap v1.0.0 // indirect
github.com/jmespath/go-jmespath v0.4.0 // indirect
Expand Down
3 changes: 0 additions & 3 deletions go.sum
Expand Up @@ -225,11 +225,8 @@ github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+
github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg=
github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk=
github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I=
github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo=
github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM=
github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
Expand Down
39 changes: 19 additions & 20 deletions pkg/blockio/blockio.go
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package blockio

import (
"errors"
"fmt"
"os"
"path/filepath"
Expand All @@ -28,8 +29,6 @@ import (

"k8s.io/apimachinery/pkg/api/resource"

"github.com/hashicorp/go-multierror"

"github.com/intel/cri-resource-manager/pkg/cgroups"
"github.com/intel/cri-resource-manager/pkg/cri/resource-manager/cache"
logger "github.com/intel/cri-resource-manager/pkg/log"
Expand Down Expand Up @@ -180,27 +179,27 @@ func getCurrentIOSchedulers() (map[string]string, error) {

// deviceParametersToOci converts single blockio class parameters into OCI BlockIO structure.
func devicesParametersToOci(dps []DevicesParameters, currentIOSchedulers map[string]string) (cgroups.OciBlockIOParameters, error) {
var errors *multierror.Error
errs := []error{}
oci := cgroups.NewOciBlockIOParameters()
for _, dp := range dps {
var err error
var weight, throttleReadBps, throttleWriteBps, throttleReadIOPS, throttleWriteIOPS int64
weight, err = parseAndValidateInt64("Weight", dp.Weight, -1, 10, 1000)
errors = multierror.Append(errors, err)
errs = append(errs, err)
throttleReadBps, err = parseAndValidateInt64("ThrottleReadBps", dp.ThrottleReadBps, -1, 0, -1)
errors = multierror.Append(errors, err)
errs = append(errs, err)
throttleWriteBps, err = parseAndValidateInt64("ThrottleWriteBps", dp.ThrottleWriteBps, -1, 0, -1)
errors = multierror.Append(errors, err)
errs = append(errs, err)
throttleReadIOPS, err = parseAndValidateInt64("ThrottleReadIOPS", dp.ThrottleReadIOPS, -1, 0, -1)
errors = multierror.Append(errors, err)
errs = append(errs, err)
throttleWriteIOPS, err = parseAndValidateInt64("ThrottleWriteIOPS", dp.ThrottleWriteIOPS, -1, 0, -1)
errors = multierror.Append(errors, err)
errs = append(errs, err)
if dp.Devices == nil {
if weight > -1 {
oci.Weight = weight
}
if throttleReadBps > -1 || throttleWriteBps > -1 || throttleReadIOPS > -1 || throttleWriteIOPS > -1 {
errors = multierror.Append(errors, fmt.Errorf("ignoring throttling (rbps=%#v wbps=%#v riops=%#v wiops=%#v): Devices not listed",
errs = append(errs, fmt.Errorf("ignoring throttling (rbps=%#v wbps=%#v riops=%#v wiops=%#v): Devices not listed",
dp.ThrottleReadBps, dp.ThrottleWriteBps, dp.ThrottleReadIOPS, dp.ThrottleWriteIOPS))
}
} else {
Expand Down Expand Up @@ -238,7 +237,7 @@ func devicesParametersToOci(dps []DevicesParameters, currentIOSchedulers map[str
}
}
}
return oci, errors.ErrorOrNil()
return oci, errors.Join(errs...)
}

// parseAndValidateInt64 parses quantities, like "64 M", and validates that they are in given range.
Expand Down Expand Up @@ -277,7 +276,7 @@ var currentPlatform platformInterface = defaultPlatform{}
func (dpm defaultPlatform) configurableBlockDevices(devWildcards []string) ([]BlockDeviceInfo, error) {
// Return map {devNode: BlockDeviceInfo}
// Example: {"/dev/sda": {Major:8, Minor:0, Origin:"from symlink /dev/disk/by-id/ata-VendorXSSD from wildcard /dev/disk/by-id/*SSD*"}}
var errors *multierror.Error
errs := []error{}
blockDevices := []BlockDeviceInfo{}
var origin string

Expand All @@ -287,11 +286,11 @@ func (dpm defaultPlatform) configurableBlockDevices(devWildcards []string) ([]Bl
for _, devWildcard := range devWildcards {
devWildcardMatches, err := filepath.Glob(devWildcard)
if err != nil {
errors = multierror.Append(errors, fmt.Errorf("bad device wildcard %#v: %w", devWildcard, err))
errs = append(errs, fmt.Errorf("bad device wildcard %#v: %w", devWildcard, err))
continue
}
if len(devWildcardMatches) == 0 {
errors = multierror.Append(errors, fmt.Errorf("device wildcard %#v does not match any device nodes", devWildcard))
errs = append(errs, fmt.Errorf("device wildcard %#v does not match any device nodes", devWildcard))
continue
}
for _, devMatch := range devWildcardMatches {
Expand All @@ -310,7 +309,7 @@ func (dpm defaultPlatform) configurableBlockDevices(devWildcards []string) ([]Bl
for devMatch, devOrigin := range devMatches {
realDevNode, err := filepath.EvalSymlinks(devMatch)
if err != nil {
errors = multierror.Append(errors, fmt.Errorf("cannot filepath.EvalSymlinks(%#v): %w", devMatch, err))
errs = append(errs, fmt.Errorf("cannot filepath.EvalSymlinks(%#v): %w", devMatch, err))
continue
}
if realDevNode != devMatch {
Expand All @@ -330,27 +329,27 @@ func (dpm defaultPlatform) configurableBlockDevices(devWildcards []string) ([]Bl
}
fileInfo, err := os.Stat(devRealpath)
if err != nil {
errors = multierror.Append(errors, fmt.Errorf("cannot os.Stat(%#v): %w%s", devRealpath, err, origin))
errs = append(errs, fmt.Errorf("cannot os.Stat(%#v): %w%s", devRealpath, err, origin))
continue
}
fileMode := fileInfo.Mode()
if fileMode&os.ModeDevice == 0 {
errors = multierror.Append(errors, fmt.Errorf("file %#v is not a device%s", devRealpath, origin))
errs = append(errs, fmt.Errorf("file %#v is not a device%s", devRealpath, origin))
continue
}
if fileMode&os.ModeCharDevice != 0 {
errors = multierror.Append(errors, fmt.Errorf("file %#v is a character device%s", devRealpath, origin))
errs = append(errs, fmt.Errorf("file %#v is a character device%s", devRealpath, origin))
continue
}
sys, ok := fileInfo.Sys().(*syscall.Stat_t)
major := unix.Major(sys.Rdev)
minor := unix.Minor(sys.Rdev)
if !ok {
errors = multierror.Append(errors, fmt.Errorf("cannot get syscall stat_t from %#v: %w%s", devRealpath, err, origin))
errs = append(errs, fmt.Errorf("cannot get syscall stat_t from %#v: %w%s", devRealpath, err, origin))
continue
}
if minor&0xf != 0 {
errors = multierror.Append(errors, fmt.Errorf("skipping %#v: cannot weight/throttle partitions%s", devRealpath, origin))
errs = append(errs, fmt.Errorf("skipping %#v: cannot weight/throttle partitions%s", devRealpath, origin))
continue
}
blockDevices = append(blockDevices, BlockDeviceInfo{
Expand All @@ -360,7 +359,7 @@ func (dpm defaultPlatform) configurableBlockDevices(devWildcards []string) ([]Bl
Origin: devOrigin,
})
}
return blockDevices, errors.ErrorOrNil()
return blockDevices, errors.Join(errs...)
}

// blockioError creates a formatted error message.
Expand Down
67 changes: 33 additions & 34 deletions pkg/cgroups/cgroupblkio.go
Expand Up @@ -15,14 +15,13 @@
package cgroups

import (
"errors"
"fmt"
"os"
"path/filepath"
"strconv"
"strings"

"github.com/hashicorp/go-multierror"

logger "github.com/intel/cri-resource-manager/pkg/log"
)

Expand Down Expand Up @@ -159,9 +158,9 @@ type devMajMin struct {

// ResetBlkioParameters adds new, changes existing and removes missing blockIO parameters in cgroupsDir
func ResetBlkioParameters(cgroupsDir string, blockIO OciBlockIOParameters) error {
var errors *multierror.Error
errs := []error{}
oldBlockIO, getErr := GetBlkioParameters(cgroupsDir)
errors = multierror.Append(errors, getErr)
errs = append(errs, getErr)
newBlockIO := NewOciBlockIOParameters()
newBlockIO.Weight = blockIO.Weight
// Set new device weights
Expand All @@ -180,8 +179,8 @@ func ResetBlkioParameters(cgroupsDir string, blockIO OciBlockIOParameters) error
newBlockIO.ThrottleWriteBpsDevice = resetDevRates(oldBlockIO.ThrottleWriteBpsDevice, blockIO.ThrottleWriteBpsDevice)
newBlockIO.ThrottleReadIOPSDevice = resetDevRates(oldBlockIO.ThrottleReadIOPSDevice, blockIO.ThrottleReadIOPSDevice)
newBlockIO.ThrottleWriteIOPSDevice = resetDevRates(oldBlockIO.ThrottleWriteIOPSDevice, blockIO.ThrottleWriteIOPSDevice)
errors = multierror.Append(errors, SetBlkioParameters(cgroupsDir, newBlockIO))
return errors.ErrorOrNil()
errs = append(errs, SetBlkioParameters(cgroupsDir, newBlockIO))
return errors.Join(errs...)
}

// resetDevRates adds wanted rate parameters to new and resets unwated rates
Expand All @@ -202,30 +201,30 @@ func resetDevRates(old, wanted []OciDeviceRate) []OciDeviceRate {

// GetBlkioParameters returns OCI BlockIO parameters from files in cgroups blkio controller directory.
func GetBlkioParameters(cgroupsDir string) (OciBlockIOParameters, error) {
var errors *multierror.Error
errs := []error{}
blockIO := NewOciBlockIOParameters()
content, err := readFromFileInDir(cgroupsDir, blkioWeightFiles)
if err == nil {
weight, err := strconv.ParseInt(strings.TrimSuffix(content, "\n"), 10, 64)
if err == nil {
blockIO.Weight = weight
} else {
errors = multierror.Append(errors, fmt.Errorf("parsing weight from %#v failed: %w", content, err))
errs = append(errs, fmt.Errorf("parsing weight from %#v failed: %w", content, err))
}
} else {
errors = multierror.Append(errors, err)
errs = append(errs, err)
}
errors = multierror.Append(errors, readOciDeviceParameters(cgroupsDir, blkioWeightDeviceFiles, &blockIO.WeightDevice))
errors = multierror.Append(errors, readOciDeviceParameters(cgroupsDir, blkioThrottleReadBpsFiles, &blockIO.ThrottleReadBpsDevice))
errors = multierror.Append(errors, readOciDeviceParameters(cgroupsDir, blkioThrottleWriteBpsFiles, &blockIO.ThrottleWriteBpsDevice))
errors = multierror.Append(errors, readOciDeviceParameters(cgroupsDir, blkioThrottleReadIOPSFiles, &blockIO.ThrottleReadIOPSDevice))
errors = multierror.Append(errors, readOciDeviceParameters(cgroupsDir, blkioThrottleWriteIOPSFiles, &blockIO.ThrottleWriteIOPSDevice))
return blockIO, errors.ErrorOrNil()
errs = append(errs, readOciDeviceParameters(cgroupsDir, blkioWeightDeviceFiles, &blockIO.WeightDevice))
errs = append(errs, readOciDeviceParameters(cgroupsDir, blkioThrottleReadBpsFiles, &blockIO.ThrottleReadBpsDevice))
errs = append(errs, readOciDeviceParameters(cgroupsDir, blkioThrottleWriteBpsFiles, &blockIO.ThrottleWriteBpsDevice))
errs = append(errs, readOciDeviceParameters(cgroupsDir, blkioThrottleReadIOPSFiles, &blockIO.ThrottleReadIOPSDevice))
errs = append(errs, readOciDeviceParameters(cgroupsDir, blkioThrottleWriteIOPSFiles, &blockIO.ThrottleWriteIOPSDevice))
return blockIO, errors.Join(errs...)
}

// readOciDeviceParameters parses device lines used for weights and throttling rates
func readOciDeviceParameters(baseDir string, filenames []string, params OciDeviceParameters) error {
var errors *multierror.Error
errs := []error{}
contents, err := readFromFileInDir(baseDir, filenames)
if err != nil {
return err
Expand All @@ -238,39 +237,39 @@ func readOciDeviceParameters(baseDir string, filenames []string, params OciDevic
// Expect syntax MAJOR:MINOR VALUE
devVal := strings.Split(line, " ")
if len(devVal) != 2 {
errors = multierror.Append(errors, fmt.Errorf("invalid line %q, single space expected", line))
errs = append(errs, fmt.Errorf("invalid line %q, single space expected", line))
continue
}
majMin := strings.Split(devVal[0], ":")
if len(majMin) != 2 {
errors = multierror.Append(errors, fmt.Errorf("invalid line %q, single colon expected before space", line))
errs = append(errs, fmt.Errorf("invalid line %q, single colon expected before space", line))
continue
}
major, majErr := strconv.ParseInt(majMin[0], 10, 64)
minor, minErr := strconv.ParseInt(majMin[1], 10, 64)
value, valErr := strconv.ParseInt(devVal[1], 10, 64)
if majErr != nil || minErr != nil || valErr != nil {
errors = multierror.Append(errors, fmt.Errorf("invalid number when parsing \"major:minor value\" from \"%s:%s %s\"", majMin[0], majMin[1], devVal[1]))
errs = append(errs, fmt.Errorf("invalid number when parsing \"major:minor value\" from \"%s:%s %s\"", majMin[0], majMin[1], devVal[1]))
continue
}
params.Append(major, minor, value)
}
return errors.ErrorOrNil()
return errors.Join(errs...)
}

// readFromFileInDir returns content from the first successfully read file.
func readFromFileInDir(baseDir string, filenames []string) (string, error) {
var errors *multierror.Error
errs := []error{}
// If reading all the files fails, return list of read errors.
for _, filename := range filenames {
filepath := filepath.Join(baseDir, filename)
content, err := currentPlatform.readFromFile(filepath)
if err == nil {
return content, nil
}
errors = multierror.Append(errors, err)
errs = append(errs, err)
}
err := errors.ErrorOrNil()
err := errors.Join(errs...)
if err != nil {
return "", fmt.Errorf("could not read any of files %q: %w", filenames, err)
}
Expand All @@ -280,26 +279,26 @@ func readFromFileInDir(baseDir string, filenames []string) (string, error) {
// SetBlkioParameters writes OCI BlockIO parameters to files in cgroups blkio contoller directory.
func SetBlkioParameters(cgroupsDir string, blockIO OciBlockIOParameters) error {
log.Debug("configuring cgroups blkio controller in directory %#v with parameters %+v", cgroupsDir, blockIO)
var errors *multierror.Error
errs := []error{}
if blockIO.Weight >= 0 {
errors = multierror.Append(errors, writeToFileInDir(cgroupsDir, blkioWeightFiles, strconv.FormatInt(blockIO.Weight, 10)))
errs = append(errs, writeToFileInDir(cgroupsDir, blkioWeightFiles, strconv.FormatInt(blockIO.Weight, 10)))
}
for _, weightDevice := range blockIO.WeightDevice {
errors = multierror.Append(errors, writeDevValueToFileInDir(cgroupsDir, blkioWeightDeviceFiles, weightDevice.Major, weightDevice.Minor, weightDevice.Weight))
errs = append(errs, writeDevValueToFileInDir(cgroupsDir, blkioWeightDeviceFiles, weightDevice.Major, weightDevice.Minor, weightDevice.Weight))
}
for _, rateDevice := range blockIO.ThrottleReadBpsDevice {
errors = multierror.Append(errors, writeDevValueToFileInDir(cgroupsDir, blkioThrottleReadBpsFiles, rateDevice.Major, rateDevice.Minor, rateDevice.Rate))
errs = append(errs, writeDevValueToFileInDir(cgroupsDir, blkioThrottleReadBpsFiles, rateDevice.Major, rateDevice.Minor, rateDevice.Rate))
}
for _, rateDevice := range blockIO.ThrottleWriteBpsDevice {
errors = multierror.Append(errors, writeDevValueToFileInDir(cgroupsDir, blkioThrottleWriteBpsFiles, rateDevice.Major, rateDevice.Minor, rateDevice.Rate))
errs = append(errs, writeDevValueToFileInDir(cgroupsDir, blkioThrottleWriteBpsFiles, rateDevice.Major, rateDevice.Minor, rateDevice.Rate))
}
for _, rateDevice := range blockIO.ThrottleReadIOPSDevice {
errors = multierror.Append(errors, writeDevValueToFileInDir(cgroupsDir, blkioThrottleReadIOPSFiles, rateDevice.Major, rateDevice.Minor, rateDevice.Rate))
errs = append(errs, writeDevValueToFileInDir(cgroupsDir, blkioThrottleReadIOPSFiles, rateDevice.Major, rateDevice.Minor, rateDevice.Rate))
}
for _, rateDevice := range blockIO.ThrottleWriteIOPSDevice {
errors = multierror.Append(errors, writeDevValueToFileInDir(cgroupsDir, blkioThrottleWriteIOPSFiles, rateDevice.Major, rateDevice.Minor, rateDevice.Rate))
errs = append(errs, writeDevValueToFileInDir(cgroupsDir, blkioThrottleWriteIOPSFiles, rateDevice.Major, rateDevice.Minor, rateDevice.Rate))
}
return errors.ErrorOrNil()
return errors.Join(errs...)
}

// writeDevValueToFileInDir writes MAJOR:MINOR VALUE to the first existing file under baseDir
Expand All @@ -310,17 +309,17 @@ func writeDevValueToFileInDir(baseDir string, filenames []string, major, minor,

// writeToFileInDir writes content to the first existing file in the list under baseDir.
func writeToFileInDir(baseDir string, filenames []string, content string) error {
var errors *multierror.Error
errs := []error{}
// Returns list of errors from writes, list of single error due to all filenames missing or nil on success.
for _, filename := range filenames {
filepath := filepath.Join(baseDir, filename)
err := currentPlatform.writeToFile(filepath, content)
if err == nil {
return nil
}
errors = multierror.Append(errors, err)
errs = append(errs, err)
}
err := errors.ErrorOrNil()
err := errors.Join(errs...)
if err != nil {
return fmt.Errorf("could not write content %#v to any of files %q: %w", content, filenames, err)
}
Expand Down
9 changes: 4 additions & 5 deletions pkg/cri/resource-manager/control/blockio/blockio.go
Expand Up @@ -15,10 +15,9 @@
package blockio

import (
"errors"
"fmt"

"github.com/hashicorp/go-multierror"

"github.com/intel/cri-resource-manager/pkg/blockio"
"github.com/intel/cri-resource-manager/pkg/config"
"github.com/intel/cri-resource-manager/pkg/cri/client"
Expand Down Expand Up @@ -162,7 +161,7 @@ func (ctl *blockioctl) configNotify(event config.Event, source config.Source) er

// reconfigureRunningContainers force setting current blockio configuration to all containers running on the node
func (ctl *blockioctl) reconfigureRunningContainers() error {
var errors *multierror.Error
errs := []error{}
if ctl.cache == nil {
return nil
}
Expand All @@ -171,10 +170,10 @@ func (ctl *blockioctl) reconfigureRunningContainers() error {
log.Debug("%q: configure blockio class %q", c.PrettyName(), class)
err := blockio.SetContainerClass(c, class)
if err != nil {
errors = multierror.Append(errors, err)
errs = append(errs, err)
}
}
return errors.ErrorOrNil()
return errors.Join(errs...)
}

// blockioError creates a block I/O-controller-specific formatted error message.
Expand Down