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

chores: use golang standard errors #1518

Merged
merged 5 commits into from
Mar 8, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ on:
env:
# Common versions
GO_VERSION: '1.19'
GOLANGCI_VERSION: 'v1.51'
GOLANGCI_VERSION: 'v1.55.2'
DOCKER_BUILDX_VERSION: 'v0.4.2'

# Common users. We can't run a step 'if secrets.AWS_USR != ""' but we can run
Expand Down Expand Up @@ -53,7 +53,7 @@ jobs:
run: |
make generate
- name: Lint golang code
uses: golangci/golangci-lint-action@v3.5.0
uses: golangci/golangci-lint-action@v4.0.0
with:
version: ${{ env.GOLANGCI_VERSION }}
args: --verbose
Expand Down
7 changes: 7 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ linters-settings:
locale: default
#ignore-words:
# - someword
depguard:
rules:
forbid-pkg-errors:
deny:
- pkg: "github.com/pkg/errors"
dsc: Should be replaced with standard lib errors or fmt.Errorf

linters:
fast: false
Expand All @@ -80,6 +86,7 @@ linters:
- vet
- unconvert
- unused
- depguard
issues:
exclude:
# staticcheck
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ require (
github.com/onsi/gomega v1.19.0
github.com/opencontainers/go-digest v1.0.0
github.com/opencontainers/image-spec v1.1.0-rc3
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.12.1
github.com/robfig/cron/v3 v3.0.1
github.com/spf13/pflag v1.0.5
Expand Down Expand Up @@ -113,6 +112,7 @@ require (
github.com/opencontainers/runc v1.1.6 // indirect
github.com/opencontainers/runtime-spec v1.0.3-0.20210326190908-1c3f411f0417 // indirect
github.com/opencontainers/selinux v1.10.0 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/client_model v0.2.0 // indirect
github.com/prometheus/common v0.32.1 // indirect
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/statefulset/stateful_set_control_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package statefulset

import (
"context"
"errors"
"fmt"
"math/rand"
"reflect"
Expand All @@ -29,7 +30,6 @@ import (
"testing"
"time"

"github.com/pkg/errors"
apps "k8s.io/api/apps/v1"
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down
9 changes: 4 additions & 5 deletions pkg/daemon/criruntime/imageruntime/containerd.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1"
daemonutil "github.com/openkruise/kruise/pkg/daemon/util"
"github.com/openkruise/kruise/pkg/util/secret"
"github.com/pkg/errors"
"golang.org/x/net/http/httpproxy"
"google.golang.org/grpc"
v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -98,7 +97,7 @@
imageRef := fmt.Sprintf("%s:%s", imageName, tag)
namedRef, err := daemonutil.NormalizeImageRef(imageRef)
if err != nil {
return nil, errors.Wrapf(err, "failed to parse image reference %q", imageRef)
return nil, fmt.Errorf("failed to parse image reference %q: %w", imageRef, err)

Check warning on line 100 in pkg/daemon/criruntime/imageruntime/containerd.go

View check run for this annotation

Codecov / codecov/patch

pkg/daemon/criruntime/imageruntime/containerd.go#L100

Added line #L100 was not covered by tests
}

resolver, isSchema1, err := d.getResolver(ctx, namedRef, pullSecrets)
Expand Down Expand Up @@ -330,7 +329,7 @@
rclient := runtimeapi.NewRuntimeServiceClient(conn)
resp, err := rclient.Status(ctx, &runtimeapi.StatusRequest{Verbose: true})
if err != nil {
return "", "", errors.Wrap(err, "failed to fetch cri-containerd status")
return "", "", fmt.Errorf("failed to fetch cri-containerd status: %w", err)

Check warning on line 332 in pkg/daemon/criruntime/imageruntime/containerd.go

View check run for this annotation

Codecov / codecov/patch

pkg/daemon/criruntime/imageruntime/containerd.go#L332

Added line #L332 was not covered by tests
}

var partInfo struct {
Expand All @@ -344,11 +343,11 @@

config, ok := resp.Info["config"]
if !ok {
return "", "", errors.Wrap(err, "failed to get config info from containerd")
return "", "", fmt.Errorf("failed to get config info from containerd: %w", err)

Check warning on line 346 in pkg/daemon/criruntime/imageruntime/containerd.go

View check run for this annotation

Codecov / codecov/patch

pkg/daemon/criruntime/imageruntime/containerd.go#L346

Added line #L346 was not covered by tests
}

if err := json.Unmarshal([]byte(config), &partInfo); err != nil {
return "", "", errors.Wrapf(err, "failed to unmarshal config(%v)", config)
return "", "", fmt.Errorf("failed to unmarshal config(%v): %w", config, err)

Check warning on line 350 in pkg/daemon/criruntime/imageruntime/containerd.go

View check run for this annotation

Codecov / codecov/patch

pkg/daemon/criruntime/imageruntime/containerd.go#L350

Added line #L350 was not covered by tests
}

snapshotter = partInfo.ContainerdConfig.Snapshotter
Expand Down
6 changes: 3 additions & 3 deletions pkg/daemon/criruntime/imageruntime/cri.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@

import (
"context"
"fmt"
"io"
"reflect"
"time"

appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1"
daemonutil "github.com/openkruise/kruise/pkg/daemon/util"
"github.com/openkruise/kruise/pkg/util/secret"
"github.com/pkg/errors"
"google.golang.org/grpc"
v1 "k8s.io/api/core/v1"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
Expand Down Expand Up @@ -189,7 +189,7 @@
// Anonymous pull
_, err = c.criImageClient.PullImage(ctx, pullImageReq)
if err != nil {
return nil, errors.Wrapf(err, "Failed to pull image reference %q", fullImageName)
return nil, fmt.Errorf("Failed to pull image reference %q: %w", fullImageName, err)

Check warning on line 192 in pkg/daemon/criruntime/imageruntime/cri.go

View check run for this annotation

Codecov / codecov/patch

pkg/daemon/criruntime/imageruntime/cri.go#L192

Added line #L192 was not covered by tests
}
pipeW.CloseWithError(io.EOF)
return newImagePullStatusReader(pipeR), nil
Expand Down Expand Up @@ -309,7 +309,7 @@
// Anonymous pull
_, err = c.criImageClientV1alpha2.PullImage(ctx, pullImageReq)
if err != nil {
return nil, errors.Wrapf(err, "Failed to pull image reference %q", fullImageName)
return nil, fmt.Errorf("Failed to pull image reference %q: %w", fullImageName, err)

Check warning on line 312 in pkg/daemon/criruntime/imageruntime/cri.go

View check run for this annotation

Codecov / codecov/patch

pkg/daemon/criruntime/imageruntime/cri.go#L312

Added line #L312 was not covered by tests
}
pipeW.CloseWithError(io.EOF)
return newImagePullStatusReader(pipeR), nil
Expand Down
4 changes: 1 addition & 3 deletions pkg/daemon/util/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ import (
"os"
"strings"
"syscall"

"github.com/pkg/errors"
)

// FilterCloseErr rewrites EOF and EPIPE errors to bool. Use when
Expand All @@ -34,7 +32,7 @@ func FilterCloseErr(err error) bool {
switch {
case err == io.EOF:
return true
case errors.Cause(err) == io.EOF:
case strings.Contains(err.Error(), io.EOF.Error()):
return true
case strings.Contains(err.Error(), "use of closed network connection"):
return true
Expand Down
37 changes: 37 additions & 0 deletions pkg/daemon/util/error_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
Copyright 2024 The Kruise Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package util

import (
"errors"
"fmt"
"io"
"testing"
)

func TestFilterCloseErr(t *testing.T) {
err := errors.New("abc")
ret := FilterCloseErr(errors.New("abc"))
if ret == true {
t.Errorf("FilterCloseErr true")
}

ret = FilterCloseErr(fmt.Errorf("%s: %w", err.Error(), io.EOF))
if ret == false {
t.Errorf("FilterCloseErr false")
}
}
4 changes: 1 addition & 3 deletions test/e2e/framework/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ import (
"os"
"sync"

"github.com/pkg/errors"

v1 "k8s.io/api/core/v1"
clientset "k8s.io/client-go/kubernetes"
)
Expand Down Expand Up @@ -71,7 +69,7 @@ func SetupProviderConfig(providerName string) (ProviderInterface, error) {
defer mutex.Unlock()
factory, ok := providers[providerName]
if !ok {
return nil, errors.Wrapf(os.ErrNotExist, "The provider %s is unknown.", providerName)
return nil, fmt.Errorf("The provider %s is unknown.: %w", providerName, os.ErrNotExist)
}
provider, err := factory()

Expand Down
5 changes: 3 additions & 2 deletions test/e2e/framework/test_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@ limitations under the License.
package framework

import (
"errors"
"flag"
"fmt"
"io/fs"
"os"
"time"

"github.com/onsi/ginkgo/config"
"github.com/pkg/errors"
restclient "k8s.io/client-go/rest"
"k8s.io/client-go/tools/clientcmd"
clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
Expand Down Expand Up @@ -368,7 +369,7 @@ func AfterReadingAllFlags(t *TestContextType) {
if err == nil {
return
}
if !os.IsNotExist(errors.Cause(err)) {
if errors.Is(err, fs.ErrNotExist) {
Failf("Failed to setup provider config: %v", err)
}
// We allow unknown provider parameters for historic reasons. At least log a
Expand Down
Loading