From c26996e1c17a2813df822b07c9d4b95f91bc9821 Mon Sep 17 00:00:00 2001 From: dongjiang1989 Date: Thu, 7 Mar 2024 14:02:32 +0800 Subject: [PATCH 1/5] use golang standard errors Signed-off-by: dongjiang1989 --- .golangci.yml | 7 +++++++ go.mod | 2 +- pkg/controller/statefulset/stateful_set_control_test.go | 2 +- pkg/daemon/criruntime/imageruntime/containerd.go | 9 ++++----- pkg/daemon/criruntime/imageruntime/cri.go | 6 +++--- pkg/daemon/util/error.go | 4 +--- test/e2e/framework/provider.go | 4 +--- test/e2e/framework/test_context.go | 1 - 8 files changed, 18 insertions(+), 17 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 2f6572b976..abecc23583 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -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 @@ -80,6 +86,7 @@ linters: - vet - unconvert - unused + - depguard issues: exclude: # staticcheck diff --git a/go.mod b/go.mod index 6f7a336993..ae72a19c37 100644 --- a/go.mod +++ b/go.mod @@ -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 @@ -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 diff --git a/pkg/controller/statefulset/stateful_set_control_test.go b/pkg/controller/statefulset/stateful_set_control_test.go index 32735fa41d..cb2b26bfe9 100644 --- a/pkg/controller/statefulset/stateful_set_control_test.go +++ b/pkg/controller/statefulset/stateful_set_control_test.go @@ -19,6 +19,7 @@ package statefulset import ( "context" + "errors" "fmt" "math/rand" "reflect" @@ -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" diff --git a/pkg/daemon/criruntime/imageruntime/containerd.go b/pkg/daemon/criruntime/imageruntime/containerd.go index 67ff645047..baf4402ab3 100644 --- a/pkg/daemon/criruntime/imageruntime/containerd.go +++ b/pkg/daemon/criruntime/imageruntime/containerd.go @@ -40,7 +40,6 @@ import ( 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" @@ -98,7 +97,7 @@ func (d *containerdImageClient) PullImage(ctx context.Context, imageName, tag st 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) } resolver, isSchema1, err := d.getResolver(ctx, namedRef, pullSecrets) @@ -330,7 +329,7 @@ func getDefaultValuesFromCRIStatus(conn *grpc.ClientConn) (snapshotter string, h 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) } var partInfo struct { @@ -344,11 +343,11 @@ func getDefaultValuesFromCRIStatus(conn *grpc.ClientConn) (snapshotter string, h 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) } 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) } snapshotter = partInfo.ContainerdConfig.Snapshotter diff --git a/pkg/daemon/criruntime/imageruntime/cri.go b/pkg/daemon/criruntime/imageruntime/cri.go index 7e7a3cfdf9..515bc84b79 100644 --- a/pkg/daemon/criruntime/imageruntime/cri.go +++ b/pkg/daemon/criruntime/imageruntime/cri.go @@ -15,6 +15,7 @@ package imageruntime import ( "context" + "fmt" "io" "reflect" "time" @@ -22,7 +23,6 @@ import ( 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" @@ -189,7 +189,7 @@ func (c *commonCRIImageService) pullImageV1(ctx context.Context, imageName, tag // 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) } pipeW.CloseWithError(io.EOF) return newImagePullStatusReader(pipeR), nil @@ -309,7 +309,7 @@ func (c *commonCRIImageService) pullImageV1alpha2(ctx context.Context, imageName // 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) } pipeW.CloseWithError(io.EOF) return newImagePullStatusReader(pipeR), nil diff --git a/pkg/daemon/util/error.go b/pkg/daemon/util/error.go index ec359db293..f3059ee256 100644 --- a/pkg/daemon/util/error.go +++ b/pkg/daemon/util/error.go @@ -22,8 +22,6 @@ import ( "os" "strings" "syscall" - - "github.com/pkg/errors" ) // FilterCloseErr rewrites EOF and EPIPE errors to bool. Use when @@ -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 diff --git a/test/e2e/framework/provider.go b/test/e2e/framework/provider.go index c4ec86a9c7..d7e814e9fd 100644 --- a/test/e2e/framework/provider.go +++ b/test/e2e/framework/provider.go @@ -22,8 +22,6 @@ import ( "os" "sync" - "github.com/pkg/errors" - v1 "k8s.io/api/core/v1" clientset "k8s.io/client-go/kubernetes" ) @@ -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() diff --git a/test/e2e/framework/test_context.go b/test/e2e/framework/test_context.go index 6fa6b143a0..f020e157a9 100644 --- a/test/e2e/framework/test_context.go +++ b/test/e2e/framework/test_context.go @@ -24,7 +24,6 @@ import ( "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" From 9d35f50bb4c5cc5cbd88b2fb8107917434555ac0 Mon Sep 17 00:00:00 2001 From: dongjiang1989 Date: Thu, 7 Mar 2024 14:37:02 +0800 Subject: [PATCH 2/5] update test_context.go Signed-off-by: dongjiang1989 --- test/e2e/framework/test_context.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/e2e/framework/test_context.go b/test/e2e/framework/test_context.go index f020e157a9..e69aeadba9 100644 --- a/test/e2e/framework/test_context.go +++ b/test/e2e/framework/test_context.go @@ -18,8 +18,10 @@ limitations under the License. package framework import ( + "errors" "flag" "fmt" + "io/fs" "os" "time" @@ -367,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 From 47d5ae9844b73c44596e88e29164b496ad3fe0cf Mon Sep 17 00:00:00 2001 From: dongjiang1989 Date: Thu, 7 Mar 2024 15:24:12 +0800 Subject: [PATCH 3/5] add unittest casae Signed-off-by: dongjiang1989 --- pkg/daemon/util/error_test.go | 37 +++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 pkg/daemon/util/error_test.go diff --git a/pkg/daemon/util/error_test.go b/pkg/daemon/util/error_test.go new file mode 100644 index 0000000000..ffb235a338 --- /dev/null +++ b/pkg/daemon/util/error_test.go @@ -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" + "io" + "net/http" + "testing" +) + +func TestFilterCloseErr(t *testing.T) { + err := errors.New("abc") + ret := FilterCloseErr(errors.New("abc")) + if ret == true { + t.Errorf("FilterCloseErr true") + } + + ret = FilterCloseErr(errors.Join(err, io.EOF)) + if ret == false { + t.Errorf("FilterCloseErr false") + } +} From f49db34b2d3f1fddf070b994edae8f2440689b74 Mon Sep 17 00:00:00 2001 From: dongjiang1989 Date: Thu, 7 Mar 2024 15:43:48 +0800 Subject: [PATCH 4/5] fix unittest Signed-off-by: dongjiang1989 --- pkg/daemon/util/error_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/daemon/util/error_test.go b/pkg/daemon/util/error_test.go index ffb235a338..2faa7f9055 100644 --- a/pkg/daemon/util/error_test.go +++ b/pkg/daemon/util/error_test.go @@ -19,7 +19,6 @@ package util import ( "errors" "io" - "net/http" "testing" ) From 6c5f1504561db8519ca1dc53ed0e9bdae3dc06c1 Mon Sep 17 00:00:00 2001 From: dongjiang1989 Date: Thu, 7 Mar 2024 16:00:08 +0800 Subject: [PATCH 5/5] add golangci lint Signed-off-by: dongjiang1989 --- .github/workflows/ci.yaml | 4 ++-- pkg/daemon/util/error_test.go | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 8b237aba8e..2f91c1f3da 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -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 @@ -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 diff --git a/pkg/daemon/util/error_test.go b/pkg/daemon/util/error_test.go index 2faa7f9055..dd5f303fb2 100644 --- a/pkg/daemon/util/error_test.go +++ b/pkg/daemon/util/error_test.go @@ -18,6 +18,7 @@ package util import ( "errors" + "fmt" "io" "testing" ) @@ -29,7 +30,7 @@ func TestFilterCloseErr(t *testing.T) { t.Errorf("FilterCloseErr true") } - ret = FilterCloseErr(errors.Join(err, io.EOF)) + ret = FilterCloseErr(fmt.Errorf("%s: %w", err.Error(), io.EOF)) if ret == false { t.Errorf("FilterCloseErr false") }