From 287c895daff2e92b883f63404443c52116c63d8e Mon Sep 17 00:00:00 2001 From: Tianxin Dong Date: Tue, 12 Oct 2021 14:36:09 +0800 Subject: [PATCH] Fix: fix unhandled err (#2423) * Fix: fix unhandled err refer to https://lift.sonatype.com/result/bhamail/kubevela/01FFT7CSVNCPF6808ZM856V3HN?tab=results * Test: fix panic err --- cmd/core/main.go | 7 +++++-- e2e/cli.go | 8 +++++++- pkg/builtin/kind/client.go | 7 ++++++- .../v1alpha1/envbinding/envbinding_controller.go | 2 +- .../envbinding/envbinding_controller_test.go | 1 - pkg/oam/testutil/helper.go | 15 ++++++++++----- 6 files changed, 29 insertions(+), 11 deletions(-) diff --git a/cmd/core/main.go b/cmd/core/main.go index 871a0e11a..cf57ca530 100644 --- a/cmd/core/main.go +++ b/cmd/core/main.go @@ -326,8 +326,11 @@ func waitWebhookSecretVolume(certDir string, timeout, interval time.Duration) er if err != nil { return false } - // nolint - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + klog.Error(err, "Failed to close file") + } + }() // check if dir is empty if _, err := f.Readdir(1); errors.Is(err, io.EOF) { return false diff --git a/e2e/cli.go b/e2e/cli.go index 934075c20..474aeaa6b 100644 --- a/e2e/cli.go +++ b/e2e/cli.go @@ -100,8 +100,14 @@ func InteractiveExec(cli string, consoleFn func(*expect.Console)) (string, error command.Stdin = console.Tty() session, err := gexec.Start(command, console.Tty(), console.Tty()) + if err != nil { + return string(output), err + } s := session.Wait(300 * time.Second) - console.Tty().Close() + err = console.Tty().Close() + if err != nil { + return string(output), err + } <-doneC if err != nil { return string(output), err diff --git a/pkg/builtin/kind/client.go b/pkg/builtin/kind/client.go index e000d5ee8..3a7494b9e 100644 --- a/pkg/builtin/kind/client.go +++ b/pkg/builtin/kind/client.go @@ -23,6 +23,7 @@ import ( "os" "path/filepath" + "k8s.io/klog/v2" "sigs.k8s.io/kind/pkg/cluster" "sigs.k8s.io/kind/pkg/cluster/nodes" "sigs.k8s.io/kind/pkg/cluster/nodeutils" @@ -137,7 +138,11 @@ func loadImage(imageTarName string, node nodes.Node) error { if err != nil { return errors.Wrap(err, "failed to open image") } - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + klog.Error(err, "Failed to close file") + } + }() return nodeutils.LoadImageArchive(node, f) } diff --git a/pkg/controller/core.oam.dev/v1alpha1/envbinding/envbinding_controller.go b/pkg/controller/core.oam.dev/v1alpha1/envbinding/envbinding_controller.go index ddf7e52f5..790177785 100644 --- a/pkg/controller/core.oam.dev/v1alpha1/envbinding/envbinding_controller.go +++ b/pkg/controller/core.oam.dev/v1alpha1/envbinding/envbinding_controller.go @@ -284,7 +284,7 @@ func (r *Reconciler) handleFinalizers(ctx context.Context, envBinding *v1alpha1. func (r *Reconciler) endWithNegativeCondition(ctx context.Context, envBinding *v1alpha1.EnvBinding, cond condition.Condition) (ctrl.Result, error) { envBinding.SetConditions(cond) if err := r.Client.Status().Patch(ctx, envBinding, client.Merge); err != nil { - return ctrl.Result{}, errors.WithMessage(err, "cannot update initializer status") + return ctrl.Result{}, errors.WithMessage(err, "cannot update envbinding status") } // if any condition is changed, patching status can trigger requeue the resource and we should return nil to // avoid requeue it again diff --git a/pkg/controller/core.oam.dev/v1alpha1/envbinding/envbinding_controller_test.go b/pkg/controller/core.oam.dev/v1alpha1/envbinding/envbinding_controller_test.go index ceafe0829..c4f3d66cb 100644 --- a/pkg/controller/core.oam.dev/v1alpha1/envbinding/envbinding_controller_test.go +++ b/pkg/controller/core.oam.dev/v1alpha1/envbinding/envbinding_controller_test.go @@ -663,7 +663,6 @@ var _ = Describe("EnvBinding Normal tests", func() { By("Create envBinding") Expect(k8sClient.Create(ctx, envBinding)).Should(BeNil()) - testutil.ReconcileOnce(&r, req) testutil.ReconcileRetry(&r, req) By("Check the Application created by EnvBinding Controller") diff --git a/pkg/oam/testutil/helper.go b/pkg/oam/testutil/helper.go index b6c98d50a..cf87be217 100644 --- a/pkg/oam/testutil/helper.go +++ b/pkg/oam/testutil/helper.go @@ -18,6 +18,7 @@ package testutil import ( "context" + "fmt" "time" "github.com/onsi/gomega" @@ -46,16 +47,20 @@ func ReconcileRetryAndExpectErr(r reconcile.Reconciler, req reconcile.Request) { // ReconcileOnce will just reconcile once func ReconcileOnce(r reconcile.Reconciler, req reconcile.Request) { - //nolint:errcheck - r.Reconcile(context.TODO(), req) + if _, err := r.Reconcile(context.TODO(), req); err != nil { + fmt.Println(err.Error()) + } } // ReconcileOnceAfterFinalizer will reconcile for finalizer -//nolint:errcheck func ReconcileOnceAfterFinalizer(r reconcile.Reconciler, req reconcile.Request) (reconcile.Result, error) { // 1st and 2nd time reconcile to add finalizer - r.Reconcile(context.TODO(), req) - r.Reconcile(context.TODO(), req) + if result, err := r.Reconcile(context.TODO(), req); err != nil { + return result, err + } + if result, err := r.Reconcile(context.TODO(), req); err != nil { + return result, err + } return r.Reconcile(context.TODO(), req) }