Skip to content

Commit

Permalink
Merge pull request #108 from klihub/fixes/native-multierror
Browse files Browse the repository at this point in the history
pkg/multierror: switch to native golang multierror handling.
  • Loading branch information
marquiz authored Aug 31, 2023
2 parents ae6087a + 973fad9 commit 50c0fe6
Show file tree
Hide file tree
Showing 13 changed files with 56 additions and 179 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/project-checks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ on:
- "release/**"

env:
GO_VERSION: "1.19.5"
GO_VERSION: "1.20.7"

jobs:
verify:
Expand Down
5 changes: 2 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ GOLICENSES_VERSION ?= v1.6.0
CONTAINER_RUN_CMD ?= docker run
IMAGE_BUILD_CMD ?= docker build
IMAGE_BUILD_EXTRA_OPTS ?=
BUILDER_IMAGE ?= golang:1.19-bullseye

GO_CMD := go
GO_BUILD := $(GO_CMD) build
Expand All @@ -45,6 +44,7 @@ GO_LINT := golint -set_exit_status
GO_FMT := gofmt
GO_VET := $(GO_CMD) vet
GO_DEPS := $(GO_CMD) list -f '{{ join .Deps "\n" }}'
GO_VERSION ?= 1.20.7

GO_MODULES := $(shell $(GO_CMD) list ./...)
GO_SUBPKGS := $(shell find ./pkg -name go.mod | sed 's:/go.mod::g' | grep -v testdata)
Expand Down Expand Up @@ -322,9 +322,8 @@ image-deployment-%:
image-%:
$(Q)mkdir -p $(IMAGE_PATH); \
bin=$(patsubst image-%,%,$@); tag=nri-resource-policy-$$bin; \
go_version=`$(GO_CMD) list -m -f '{{.GoVersion}}'`; \
$(DOCKER) build . -f "cmd/$$bin/Dockerfile" \
--build-arg GO_VERSION=$${go_version} \
--build-arg GO_VERSION=$(GO_VERSION) \
-t $(IMAGE_REPO)$$tag:$(IMAGE_VERSION)

pkg/sysfs/sst_types%.go: pkg/sysfs/_sst_types%.go pkg/sysfs/gen_sst_types.sh
Expand Down
4 changes: 2 additions & 2 deletions cmd/balloons/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
ARG GO_VERSION=1.19
ARG GO_VERSION=1.20

FROM golang:${GO_VERSION}-buster as builder
FROM golang:${GO_VERSION}-bullseye as builder

WORKDIR /go/builder

Expand Down
4 changes: 2 additions & 2 deletions cmd/template/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
ARG GO_VERSION=1.19
ARG GO_VERSION=1.20

FROM golang:${GO_VERSION}-buster as builder
FROM golang:${GO_VERSION}-bullseye as builder

WORKDIR /go/builder

Expand Down
4 changes: 2 additions & 2 deletions cmd/topology-aware/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
ARG GO_VERSION=1.19
ARG GO_VERSION=1.20

FROM golang:${GO_VERSION}-buster as builder
FROM golang:${GO_VERSION}-bullseye as builder

WORKDIR /go/builder

Expand Down
9 changes: 5 additions & 4 deletions cmd/topology-aware/policy/topology-aware-policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@
package topologyaware

import (
"errors"

"github.com/containers/nri-plugins/pkg/utils/cpuset"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
resapi "k8s.io/apimachinery/pkg/api/resource"

"github.com/containers/nri-plugins/pkg/multierror"
"github.com/prometheus/client_golang/prometheus"

"github.com/containers/nri-plugins/pkg/config"
Expand Down Expand Up @@ -436,7 +437,7 @@ func (p *policy) ExportResourceData(c cache.Container) map[string]string {

// reallocateResources reallocates the given containers using the given pool hints
func (p *policy) reallocateResources(containers []cache.Container, pools map[string]string) error {
var errors error
errs := []error{}

log.Info("reallocating resources...")

Expand All @@ -450,13 +451,13 @@ func (p *policy) reallocateResources(containers []cache.Container, pools map[str

grant, err := p.allocatePool(c, pools[c.GetID()])
if err != nil {
errors = multierror.Append(errors, err)
errs = append(errs, err)
} else {
p.applyGrant(grant)
}
}

if err := multierror.New(errors); err != nil {
if err := errors.Join(errs...); err != nil {
return err
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/containers/nri-plugins

go 1.19
go 1.20

require (
contrib.go.opencensus.io/exporter/prometheus v0.4.2
Expand Down
67 changes: 33 additions & 34 deletions pkg/cgroups/cgroupblkio.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,14 @@
package cgroups

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

"github.com/containers/nri-plugins/pkg/multierror"

logger "github.com/containers/nri-plugins/pkg/log"
)

Expand Down Expand Up @@ -160,9 +159,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 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 @@ -181,8 +180,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 multierror.New(errors)
errs = append(errs, SetBlkioParameters(cgroupsDir, newBlockIO))
return errors.Join(errs...)
}

// resetDevRates adds wanted rate parameters to new and resets unwated rates
Expand All @@ -203,30 +202,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 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, multierror.New(errors)
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 error
errs := []error{}
contents, err := readFromFileInDir(baseDir, filenames)
if err != nil {
return err
Expand All @@ -239,39 +238,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 multierror.New(errors)
return errors.Join(errs...)
}

// readFromFileInDir returns content from the first successfully read file.
func readFromFileInDir(baseDir string, filenames []string) (string, error) {
var errors 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 := multierror.New(errors)
err := errors.Join(errs...)
if err != nil {
return "", fmt.Errorf("could not read any of files %q: %w", filenames, err)
}
Expand All @@ -281,26 +280,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 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 multierror.New(errors)
return errors.Join(errs...)
}

// writeDevValueToFileInDir writes MAJOR:MINOR VALUE to the first existing file under baseDir
Expand All @@ -311,17 +310,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 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 := multierror.New(errors)
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
4 changes: 2 additions & 2 deletions pkg/cgroups/cgroupblkio_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ func TestGetBlkioParameters(t *testing.T) {
cgroupsDir: "/missing/err",
fsContent: map[string]string{},
expectedBlockIO: &OciBlockIOParameters{Weight: -1},
expectedErrorCount: 8,
expectedErrorCount: 6,
expectedErrorSubstrings: []string{
"file not found",
"blkio.bfq.weight",
Expand Down Expand Up @@ -356,7 +356,7 @@ func TestSetBlkioParameters(t *testing.T) {
WeightDevice: OciDeviceWeights{{1, 0, 100}},
},
writesFail: 9999,
expectedErrorCount: 2,
expectedErrorCount: 1,
expectedErrorSubstrings: []string{
"could not write content \"1:0 100\" to any of files",
"\"blkio.bfq.weight_device\"",
Expand Down
82 changes: 0 additions & 82 deletions pkg/multierror/multierror.go

This file was deleted.

Loading

0 comments on commit 50c0fe6

Please sign in to comment.