From eb54beaf61b4292759a13f26b135b9cc38b543e4 Mon Sep 17 00:00:00 2001 From: Eugene Sumin <95425330+e-sumin@users.noreply.github.com> Date: Fri, 29 Nov 2024 11:21:23 +0100 Subject: [PATCH] chore: Errkit migration 8.3 - migrate remaining piece of `pkg/kube` to errkit (#3256) Signed-off-by: Eugen Sumin Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- pkg/kube/exec_test.go | 6 +++--- pkg/kube/log_reader_test.go | 4 ++-- pkg/kube/pod.go | 5 ++--- pkg/kube/pod_command_executor_test.go | 6 +++--- pkg/kube/pod_controller_test.go | 29 +++++++++++++-------------- pkg/kube/pod_file_writer_test.go | 5 ++--- pkg/kube/pod_runner_test.go | 6 +++--- pkg/kube/pod_test.go | 25 +++++++++++++++++++---- 8 files changed, 50 insertions(+), 36 deletions(-) diff --git a/pkg/kube/exec_test.go b/pkg/kube/exec_test.go index 9242abb4ec..2475a54fad 100644 --- a/pkg/kube/exec_test.go +++ b/pkg/kube/exec_test.go @@ -20,10 +20,10 @@ package kube import ( "bytes" "context" - "errors" "strings" "time" + "github.com/kanisterio/errkit" "gopkg.in/check.v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -191,7 +191,7 @@ func (s *ExecSuite) TestErrorInExecWithOptions(c *check.C) { c.Assert(err1, check.Not(check.IsNil)) var ee1 *ExecError - ok := errors.As(err1, &ee1) + ok := errkit.As(err1, &ee1) c.Assert(ok, check.Equals, true) c.Assert(ee1.Stdout(), check.Not(check.Equals), testCase.expectedOut) c.Assert(ee1.Stderr(), check.Not(check.Equals), testCase.expectedErr) @@ -208,7 +208,7 @@ func (s *ExecSuite) TestErrorInExecWithOptions(c *check.C) { c.Assert(err2, check.Not(check.IsNil)) var ee2 *ExecError - ok = errors.As(err2, &ee2) + ok = errkit.As(err2, &ee2) c.Assert(ok, check.Equals, true) // When error happens, stdout/stderr buffers should contain all lines produced by an app diff --git a/pkg/kube/log_reader_test.go b/pkg/kube/log_reader_test.go index b83a89ea29..55fed677c7 100644 --- a/pkg/kube/log_reader_test.go +++ b/pkg/kube/log_reader_test.go @@ -3,9 +3,9 @@ package kube import ( "bytes" "context" - "errors" "io" + "github.com/kanisterio/errkit" "gopkg.in/check.v1" "k8s.io/client-go/rest" ) @@ -39,7 +39,7 @@ func (frw *fakeResponseWrapper) Stream(context.Context) (io.ReadCloser, error) { } func (s *LogReaderSuite) TestLogReader(c *check.C) { - err := errors.New("TEST") + err := errkit.New("TEST") for _, tc := range []struct { rw *fakeResponseWrapper err error diff --git a/pkg/kube/pod.go b/pkg/kube/pod.go index 8baf644565..09be10c94c 100644 --- a/pkg/kube/pod.go +++ b/pkg/kube/pod.go @@ -27,7 +27,6 @@ import ( "github.com/gofrs/uuid" json "github.com/json-iterator/go" "github.com/kanisterio/errkit" - "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -451,7 +450,7 @@ func checkPVCAndPVStatus(ctx context.Context, vol corev1.Volume, p *corev1.Pod, pvcName := vol.VolumeSource.PersistentVolumeClaim.ClaimName pvc, err := cli.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, pvcName, metav1.GetOptions{}) if err != nil { - if apierrors.IsNotFound(errors.Cause(err)) { + if apierrors.IsNotFound(err) { // Do not return err, wait for timeout, since sometimes in case of statefulsets, they trigger creation of a volume return nil } @@ -470,7 +469,7 @@ func checkPVCAndPVStatus(ctx context.Context, vol corev1.Volume, p *corev1.Pod, } pv, err := cli.CoreV1().PersistentVolumes().Get(ctx, pvName, metav1.GetOptions{}) if err != nil { - if apierrors.IsNotFound(errors.Cause(err)) { + if apierrors.IsNotFound(err) { // wait for timeout return nil } diff --git a/pkg/kube/pod_command_executor_test.go b/pkg/kube/pod_command_executor_test.go index bc037dad47..064e6b35c0 100644 --- a/pkg/kube/pod_command_executor_test.go +++ b/pkg/kube/pod_command_executor_test.go @@ -17,11 +17,11 @@ package kube import ( "bytes" "context" - "errors" "os" "sync" "time" + "github.com/kanisterio/errkit" "gopkg.in/check.v1" "k8s.io/client-go/kubernetes/fake" ) @@ -129,7 +129,7 @@ func (s *PodCommandExecutorTestSuite) TestPodRunnerExec(c *check.C) { prp.execWithOptionsSyncEnd.Sync() c.Assert(err, check.Not(check.IsNil)) - c.Assert(errors.Is(err, context.DeadlineExceeded), check.Equals, true) + c.Assert(errkit.Is(err, context.DeadlineExceeded), check.Equals, true) }, "Cancelled": func(ctx context.Context, pr PodCommandExecutor, prp *fakePodCommandExecutorProcessor) { var err error @@ -151,7 +151,7 @@ func (s *PodCommandExecutorTestSuite) TestPodRunnerExec(c *check.C) { prp.execWithOptionsSyncEnd.Sync() // Release ExecWithOptions c.Assert(err, check.Not(check.IsNil)) - c.Assert(errors.Is(err, context.Canceled), check.Equals, true) + c.Assert(errkit.Is(err, context.Canceled), check.Equals, true) }, "Successful execution": func(ctx context.Context, pr PodCommandExecutor, prp *fakePodCommandExecutorProcessor) { var err error diff --git a/pkg/kube/pod_controller_test.go b/pkg/kube/pod_controller_test.go index b2172ca21c..1555070569 100644 --- a/pkg/kube/pod_controller_test.go +++ b/pkg/kube/pod_controller_test.go @@ -16,7 +16,6 @@ package kube import ( "context" - "errors" "fmt" "os" "time" @@ -46,14 +45,14 @@ func (s *PodControllerTestSuite) TestPodControllerStartPod(c *check.C) { ctx := context.Background() cli := fake.NewSimpleClientset() - simulatedError := errors.New("SimulatedError") + simulatedError := errkit.New("SimulatedError") cases := map[string]func(prp *FakePodControllerProcessor, pr PodController){ "Pod creation failure": func(pcp *FakePodControllerProcessor, pc PodController) { pcp.CreatePodErr = simulatedError err := pc.StartPod(ctx) c.Assert(err, check.Not(check.IsNil)) - c.Assert(errors.Is(err, simulatedError), check.Equals, true) + c.Assert(errkit.Is(err, simulatedError), check.Equals, true) c.Assert(pcp.InCreatePodOptions, check.DeepEquals, &PodOptions{ Namespace: podControllerNS, Name: podControllerPodName, @@ -81,11 +80,11 @@ func (s *PodControllerTestSuite) TestPodControllerStartPod(c *check.C) { prp.InCreatePodOptions = nil prp.CreatePodRet = nil - prp.CreatePodErr = errors.New("CreatePod should not be invoked") + prp.CreatePodErr = errkit.New("CreatePod should not be invoked") err = pr.StartPod(ctx) c.Assert(err, check.Not(check.IsNil)) - c.Assert(errors.Is(err, ErrPodControllerPodAlreadyStarted), check.Equals, true) + c.Assert(errkit.Is(err, ErrPodControllerPodAlreadyStarted), check.Equals, true) c.Assert(prp.InCreatePodOptions, check.IsNil) }, } @@ -108,13 +107,13 @@ func (s *PodControllerTestSuite) TestPodControllerWaitPod(c *check.C) { ctx := context.Background() cli := fake.NewSimpleClientset() - simulatedError := errkit.Wrap(errors.New("SimulatedError"), "Wrapped") + simulatedError := errkit.Wrap(errkit.New("SimulatedError"), "Wrapped") cases := map[string]func(pcp *FakePodControllerProcessor, pc PodController){ "Waiting failed because pod not started yet": func(pcp *FakePodControllerProcessor, pc PodController) { err := pc.WaitForPodReady(ctx) c.Assert(err, check.Not(check.IsNil)) - c.Assert(errors.Is(err, ErrPodControllerPodNotStarted), check.Equals, true) + c.Assert(errkit.Is(err, ErrPodControllerPodNotStarted), check.Equals, true) c.Assert(pcp.InCreatePodOptions, check.IsNil) }, "Waiting failed due to timeout": func(pcp *FakePodControllerProcessor, pc PodController) { @@ -132,7 +131,7 @@ func (s *PodControllerTestSuite) TestPodControllerWaitPod(c *check.C) { c.Assert(err, check.Not(check.IsNil)) c.Assert(pcp.InWaitForPodReadyPodName, check.Equals, podControllerPodName) c.Assert(pcp.InWaitForPodReadyNamespace, check.Equals, podControllerNS) - c.Assert(errors.Is(err, pcp.WaitForPodReadyErr), check.Equals, true) + c.Assert(errkit.Is(err, pcp.WaitForPodReadyErr), check.Equals, true) c.Assert(err.Error(), check.Equals, fmt.Sprintf("Pod failed to become ready in time: %s", simulatedError.Error())) // Check that POD deletion was also invoked with expected arguments @@ -169,13 +168,13 @@ func (s *PodControllerTestSuite) TestPodControllerStopPod(c *check.C) { cli := fake.NewSimpleClientset() untouchedStr := "DEADBEEF" - simulatedError := errors.New("SimulatedError") + simulatedError := errkit.New("SimulatedError") cases := map[string]func(pcp *FakePodControllerProcessor, pc PodController){ "Pod not started yet": func(pcp *FakePodControllerProcessor, pc PodController) { err := pc.StopPod(ctx, 30*time.Second, int64(0)) c.Assert(err, check.Not(check.IsNil)) - c.Assert(errors.Is(err, ErrPodControllerPodNotStarted), check.Equals, true) + c.Assert(errkit.Is(err, ErrPodControllerPodNotStarted), check.Equals, true) c.Assert(pcp.InDeletePodPodName, check.Equals, untouchedStr) c.Assert(pcp.InDeletePodNamespace, check.Equals, untouchedStr) }, @@ -192,7 +191,7 @@ func (s *PodControllerTestSuite) TestPodControllerStopPod(c *check.C) { pcp.DeletePodErr = simulatedError err = pc.StopPod(ctx, 30*time.Second, int64(0)) c.Assert(err, check.Not(check.IsNil)) - c.Assert(errors.Is(err, simulatedError), check.Equals, true) + c.Assert(errkit.Is(err, simulatedError), check.Equals, true) }, "Pod successfully deleted": func(pcp *FakePodControllerProcessor, pc PodController) { pcp.CreatePodRet = &corev1.Pod{ @@ -239,12 +238,12 @@ func (s *PodControllerTestSuite) TestPodControllerGetCommandExecutorAndFileWrite pce, err := pc.GetCommandExecutor() c.Assert(pce, check.IsNil) c.Assert(err, check.Not(check.IsNil)) - c.Assert(errors.Is(err, ErrPodControllerPodNotStarted), check.Equals, true) + c.Assert(errkit.Is(err, ErrPodControllerPodNotStarted), check.Equals, true) pfw, err := pc.GetFileWriter() c.Assert(pfw, check.IsNil) c.Assert(err, check.Not(check.IsNil)) - c.Assert(errors.Is(err, ErrPodControllerPodNotStarted), check.Equals, true) + c.Assert(errkit.Is(err, ErrPodControllerPodNotStarted), check.Equals, true) }, "Pod not ready yet": func(pcp *FakePodControllerProcessor, pc PodController) { pcp.CreatePodRet = &corev1.Pod{ @@ -258,12 +257,12 @@ func (s *PodControllerTestSuite) TestPodControllerGetCommandExecutorAndFileWrite pce, err := pc.GetCommandExecutor() c.Assert(pce, check.IsNil) c.Assert(err, check.Not(check.IsNil)) - c.Assert(errors.Is(err, ErrPodControllerPodNotReady), check.Equals, true) + c.Assert(errkit.Is(err, ErrPodControllerPodNotReady), check.Equals, true) pfw, err := pc.GetFileWriter() c.Assert(pfw, check.IsNil) c.Assert(err, check.Not(check.IsNil)) - c.Assert(errors.Is(err, ErrPodControllerPodNotReady), check.Equals, true) + c.Assert(errkit.Is(err, ErrPodControllerPodNotReady), check.Equals, true) }, "CommandExecutor successfully returned": func(pcp *FakePodControllerProcessor, pc PodController) { pcp.CreatePodRet = &corev1.Pod{ diff --git a/pkg/kube/pod_file_writer_test.go b/pkg/kube/pod_file_writer_test.go index c9c45636b1..85e6a95e4b 100644 --- a/pkg/kube/pod_file_writer_test.go +++ b/pkg/kube/pod_file_writer_test.go @@ -17,7 +17,6 @@ package kube import ( "bytes" "context" - "errors" "io" "os" @@ -90,7 +89,7 @@ func (s *PodFileWriterTestSuite) TestPodRunnerWriteFile(c *check.C) { buf := bytes.NewBuffer([]byte("some file content")) remover, err := pfw.Write(ctx, "/path/to/file", buf) c.Assert(err, check.Not(check.IsNil)) - c.Assert(errors.Is(err, simulatedError), check.Equals, true) + c.Assert(errkit.Is(err, simulatedError), check.Equals, true) c.Assert(remover, check.IsNil) c.Assert(pfwp.podWriter.inWriteNamespace, check.Equals, podFileWriterNS) @@ -130,7 +129,7 @@ func (s *PodFileWriterTestSuite) TestPodRunnerWriteFile(c *check.C) { err = remover.Remove(ctx) c.Assert(err, check.Not(check.IsNil)) - c.Assert(errors.Is(err, simulatedError), check.Equals, true) + c.Assert(errkit.Is(err, simulatedError), check.Equals, true) c.Assert(pfwp.podWriter.inRemoveNamespace, check.Equals, podFileWriterNS) c.Assert(pfwp.podWriter.inRemovePodName, check.Equals, podFileWriterPodName) c.Assert(pfwp.podWriter.inRemoveContainerName, check.Equals, podFileWriterContainerName) diff --git a/pkg/kube/pod_runner_test.go b/pkg/kube/pod_runner_test.go index a3b8c21d61..20a312b1c8 100644 --- a/pkg/kube/pod_runner_test.go +++ b/pkg/kube/pod_runner_test.go @@ -19,7 +19,7 @@ import ( "os" "path" - "github.com/pkg/errors" + "github.com/kanisterio/errkit" "gopkg.in/check.v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" @@ -191,10 +191,10 @@ func afterPodRunTestKeyPresentFunc(labelKey, expectedLabelValue string, isLabelE <-ch labelValue, found := pc.Pod().Labels[labelKey] if found != isLabelExpected { - return nil, errors.New("Got different label than expected") + return nil, errkit.New("Got different label than expected") } if isLabelExpected && labelValue != expectedLabelValue { - return nil, errors.New("Found label doesn't match with expected label") + return nil, errkit.New("Found label doesn't match with expected label") } return nil, nil } diff --git a/pkg/kube/pod_test.go b/pkg/kube/pod_test.go index 852dd29f10..e14878821b 100644 --- a/pkg/kube/pod_test.go +++ b/pkg/kube/pod_test.go @@ -19,14 +19,15 @@ package kube import ( "context" - "errors" "fmt" "os" "strings" "time" + "github.com/kanisterio/errkit" "gopkg.in/check.v1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -315,7 +316,7 @@ func (s *PodSuite) TestPodWithFilesystemModeVolumes(c *check.C) { ca := action.(testing.CreateAction) p = ca.GetObject().(*corev1.Pod) if len(p.Spec.Volumes[0].Name) > 63 { - return true, nil, errors.New("spec.volumes[0].name must be no more than 63 characters") + return true, nil, errkit.New("spec.volumes[0].name must be no more than 63 characters") } return false, nil, nil }) @@ -364,7 +365,7 @@ func (s *PodSuite) TestPodWithFilesystemModeReadOnlyVolumes(c *check.C) { ca := action.(testing.CreateAction) p = ca.GetObject().(*corev1.Pod) if len(p.Spec.Volumes[0].Name) > 63 { - return true, nil, errors.New("spec.volumes[0].name must be no more than 63 characters") + return true, nil, errkit.New("spec.volumes[0].name must be no more than 63 characters") } return false, nil, nil }) @@ -415,7 +416,7 @@ func (s *PodSuite) TestPodWithBlockModeVolumes(c *check.C) { ca := action.(testing.CreateAction) p = ca.GetObject().(*corev1.Pod) if len(p.Spec.Volumes[0].Name) > 63 { - return true, nil, errors.New("spec.volumes[0].name must be no more than 63 characters") + return true, nil, errkit.New("spec.volumes[0].name must be no more than 63 characters") } return false, nil, nil }) @@ -1244,3 +1245,19 @@ func (s *PodSuite) TestAddAnnotations(c *check.C) { c.Assert(tc.podOptions, check.DeepEquals, tc.expectedPodOptions) } } + +// TestErrkitApiErrorsWrapping verifies that apierrors wrapped with errkit.Wrap are still matchable using apierrors matchers +func (s *PodSuite) TestErrkitApiErrorsWrapping(c *check.C) { + // Create the fake client + fakeClient := fake.NewSimpleClientset() + + // Add a reactor to simulate an error when trying to get a PVC + fakeClient.PrependReactor("get", "persistentvolumeclaims", func(action testing.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errkit.Wrap(apierrors.NewNotFound(action.GetResource().GroupResource(), action.GetSubresource()), "Some context") + }) + + _, err := fakeClient.CoreV1().PersistentVolumeClaims("abc").Get(context.TODO(), "def", metav1.GetOptions{}) + if err != nil { + c.Assert(apierrors.IsNotFound(err), check.Equals, true) + } +}