Skip to content

Commit

Permalink
chores: use golang standard errors (#1518)
Browse files Browse the repository at this point in the history
* use golang standard errors

Signed-off-by: dongjiang1989 <[email protected]>

* update test_context.go

Signed-off-by: dongjiang1989 <[email protected]>

* add unittest casae

Signed-off-by: dongjiang1989 <[email protected]>

* fix unittest

Signed-off-by: dongjiang1989 <[email protected]>

* add golangci lint

Signed-off-by: dongjiang1989 <[email protected]>

---------

Signed-off-by: dongjiang1989 <[email protected]>
  • Loading branch information
dongjiang1989 committed Mar 8, 2024
1 parent 04254fb commit c7e1daa
Show file tree
Hide file tree
Showing 10 changed files with 60 additions and 20 deletions.
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 @@ 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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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
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 @@ package imageruntime

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 @@ 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
Expand Down Expand Up @@ -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
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

0 comments on commit c7e1daa

Please sign in to comment.