Skip to content

Commit

Permalink
Merge pull request #1023 from marquiz/devel/multierror
Browse files Browse the repository at this point in the history 
Use golang builtin multierror
  • Loading branch information
@klihub
klihub committed Aug 14, 2023
2 parents ddcc74b + f70c9d4 commit 53b476d
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 77 deletions.
2 changes: 0 additions & 2 deletions go.mod
View file
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
View file
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
View file
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
View file
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
View file
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

0 comments on commit 53b476d

Please sign in to comment.