diff --git a/pkg/kube/fake_pod_controller_processor.go b/pkg/kube/fake_pod_controller_processor.go index a5a924830c..d94f237063 100644 --- a/pkg/kube/fake_pod_controller_processor.go +++ b/pkg/kube/fake_pod_controller_processor.go @@ -19,10 +19,9 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" ) -// FakePodControllerProcessor implements podControllerProcessor +// FakePodControllerProcessor implements PodControllerProcessor type FakePodControllerProcessor struct { InWaitForPodReadyNamespace string InWaitForPodReadyPodName string @@ -37,25 +36,23 @@ type FakePodControllerProcessor struct { InDeletePodOptions metav1.DeleteOptions DeletePodErr error - InCreatePodCli kubernetes.Interface InCreatePodOptions *PodOptions CreatePodRet *corev1.Pod CreatePodErr error } -func (f *FakePodControllerProcessor) CreatePod(_ context.Context, cli kubernetes.Interface, options *PodOptions) (*corev1.Pod, error) { - f.InCreatePodCli = cli +func (f *FakePodControllerProcessor) CreatePod(_ context.Context, options *PodOptions) (*corev1.Pod, error) { f.InCreatePodOptions = options return f.CreatePodRet, f.CreatePodErr } -func (f *FakePodControllerProcessor) WaitForPodCompletionPCP(ctx context.Context, namespace, podName string) error { +func (f *FakePodControllerProcessor) WaitForPodCompletion(_ context.Context, namespace, podName string) error { f.InWaitForPodCompletionNamespace = namespace f.InWaitForPodCompletionPodName = podName return f.WaitForPodCompletionErr } -func (f *FakePodControllerProcessor) WaitForPodReadyPCP(ctx context.Context, namespace, podName string) error { +func (f *FakePodControllerProcessor) WaitForPodReady(_ context.Context, namespace, podName string) error { f.InWaitForPodReadyPodName = podName f.InWaitForPodReadyNamespace = namespace return f.WaitForPodReadyErr diff --git a/pkg/kube/pod_command_executor.go b/pkg/kube/pod_command_executor.go index e6d7ee9351..2de7d8487e 100644 --- a/pkg/kube/pod_command_executor.go +++ b/pkg/kube/pod_command_executor.go @@ -53,11 +53,6 @@ type PodCommandExecutor interface { Exec(ctx context.Context, command []string, stdin io.Reader, stdout, stderr io.Writer) error } -// podCommandExecutorProcessor aids in unit testing -type podCommandExecutorProcessor interface { - execWithOptions(cli kubernetes.Interface, opts ExecOptions) (string, string, error) -} - // podCommandExecutor keeps everything required to execute command within a pod type podCommandExecutor struct { cli kubernetes.Interface @@ -65,7 +60,7 @@ type podCommandExecutor struct { podName string containerName string - pcep podCommandExecutorProcessor + pcep PodCommandExecutorProcessor } // Exec runs the command and logs stdout and stderr. @@ -95,7 +90,7 @@ func (p *podCommandExecutor) Exec(ctx context.Context, command []string, stdin i } go func() { - _, _, err = p.pcep.execWithOptions(p.cli, opts) + _, _, err = p.pcep.ExecWithOptions(opts) close(cmdDone) }() @@ -114,7 +109,3 @@ func (p *podCommandExecutor) Exec(ctx context.Context, command []string, stdin i return err } - -func (p *podCommandExecutor) execWithOptions(cli kubernetes.Interface, opts ExecOptions) (string, string, error) { - return ExecWithOptions(p.cli, opts) -} diff --git a/pkg/kube/pod_command_executor_processor.go b/pkg/kube/pod_command_executor_processor.go new file mode 100644 index 0000000000..94555bc597 --- /dev/null +++ b/pkg/kube/pod_command_executor_processor.go @@ -0,0 +1,35 @@ +// Copyright 2023 The Kanister 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 kube + +import ( + "k8s.io/client-go/kubernetes" +) + +// PodCommandExecutorProcessor is an interface wrapping kubernetes API invocation +// it is purposed to be replaced by fake implementation in tests +type PodCommandExecutorProcessor interface { + ExecWithOptions(opts ExecOptions) (string, string, error) +} + +type podCommandExecutorProcessor struct { + cli kubernetes.Interface +} + +// ExecWithOptions executes a command in the specified pod and container, +// returning stdout, stderr and error. +func (p *podCommandExecutorProcessor) ExecWithOptions(opts ExecOptions) (string, string, error) { + return ExecWithOptions(p.cli, opts) +} diff --git a/pkg/kube/pod_command_executor_test.go b/pkg/kube/pod_command_executor_test.go index d8d94d5d8d..98290815ed 100644 --- a/pkg/kube/pod_command_executor_test.go +++ b/pkg/kube/pod_command_executor_test.go @@ -25,7 +25,6 @@ import ( "github.com/pkg/errors" . "gopkg.in/check.v1" - "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" ) @@ -69,7 +68,6 @@ func (s *testBarrier) SyncWithController() { // background method } type fakePodCommandExecutorProcessor struct { - inExecWithOptionsCli kubernetes.Interface inExecWithOptionsOpts *ExecOptions execWithOptionsStdout string execWithOptionsStderr string @@ -77,14 +75,13 @@ type fakePodCommandExecutorProcessor struct { execWithOptionsRet2 string execWithOptionsErr error - // Signal to `execWithOptions` to start "executing" command. + // Signal to `ExecWithOptions` to start "executing" command. // Command will remain "executing" until `execWithOptionsSyncEnd.Sync()` execWithOptionsSyncStart testBarrier execWithOptionsSyncEnd testBarrier } -func (fprp *fakePodCommandExecutorProcessor) execWithOptions(cli kubernetes.Interface, opts ExecOptions) (string, string, error) { - fprp.inExecWithOptionsCli = cli +func (fprp *fakePodCommandExecutorProcessor) ExecWithOptions(opts ExecOptions) (string, string, error) { fprp.inExecWithOptionsOpts = &opts fprp.execWithOptionsSyncStart.SyncWithController() if opts.Stdout != nil && len(fprp.execWithOptionsStdout) > 0 { @@ -150,7 +147,7 @@ func (s *PodCommandExecutorTestSuite) TestPodRunnerExec(c *C) { }() prp.execWithOptionsSyncStart.Sync() // Ensure ExecWithOptions is called wg.Wait() - prp.execWithOptionsSyncEnd.Sync() // Release execWithOptions + prp.execWithOptionsSyncEnd.Sync() // Release ExecWithOptions c.Assert(err, Not(IsNil)) c.Assert(errors.Is(err, context.Canceled), Equals, true) @@ -171,10 +168,9 @@ func (s *PodCommandExecutorTestSuite) TestPodRunnerExec(c *C) { }() prp.execWithOptionsSyncStart.Sync() // Ensure ExecWithOptions is called wg.Wait() - prp.execWithOptionsSyncEnd.Sync() // Release execWithOptions + prp.execWithOptionsSyncEnd.Sync() // Release ExecWithOptions c.Assert(err, IsNil) - c.Assert(prp.inExecWithOptionsCli, Equals, cli) c.Assert(prp.inExecWithOptionsOpts.Command, DeepEquals, command) c.Assert(prp.inExecWithOptionsOpts.Namespace, Equals, podCommandExecutorNS) c.Assert(prp.inExecWithOptionsOpts.PodName, Equals, podCommandExecutorPodName) @@ -214,7 +210,7 @@ func (s *PodCommandExecutorTestSuite) TestPodRunnerExec(c *C) { }() prp.execWithOptionsSyncStart.Sync() // Ensure ExecWithOptions is called wg.Wait() - prp.execWithOptionsSyncEnd.Sync() // Release execWithOptions + prp.execWithOptionsSyncEnd.Sync() // Release ExecWithOptions c.Assert(err, Not(IsNil)) c.Assert(prp.inExecWithOptionsOpts.Stdout, Not(IsNil)) diff --git a/pkg/kube/pod_controller.go b/pkg/kube/pod_controller.go index 176a794cd6..e73aa061d7 100644 --- a/pkg/kube/pod_controller.go +++ b/pkg/kube/pod_controller.go @@ -52,14 +52,6 @@ type PodController interface { GetFileWriter() (PodFileWriter, error) } -// podControllerProcessor aids in unit testing -type podControllerProcessor interface { - CreatePod(ctx context.Context, cli kubernetes.Interface, options *PodOptions) (*corev1.Pod, error) - WaitForPodReadyPCP(ctx context.Context, namespace, podName string) error - WaitForPodCompletionPCP(ctx context.Context, namespace, podName string) error - DeletePod(ctx context.Context, namespace string, podName string, opts metav1.DeleteOptions) error -} - // podController specifies Kubernetes Client and PodOptions needed for creating // a Pod. It implements the podControllerProcessor interface. type podController struct { @@ -70,13 +62,13 @@ type podController struct { podReady bool podName string - pcp podControllerProcessor + pcp PodControllerProcessor } type PodControllerOption func(p *podController) // WithPodControllerProcessor provides mechanism for passing fake podControllerProcessor for testing purposes. -func WithPodControllerProcessor(processor podControllerProcessor) PodControllerOption { +func WithPodControllerProcessor(processor PodControllerProcessor) PodControllerOption { return func(p *podController) { p.pcp = processor } @@ -89,12 +81,17 @@ func NewPodController(cli kubernetes.Interface, options *PodOptions, opts ...Pod podOptions: options, } - r.pcp = r - for _, opt := range opts { opt(r) } + // If pod controller processor has not been set by PodControllerOption, we create default implementation here + if r.pcp == nil { + r.pcp = &podControllerProcessor{ + cli: cli, + } + } + return r } @@ -102,7 +99,10 @@ func NewPodController(cli kubernetes.Interface, options *PodOptions, opts ...Pod // Client and existing pod details. func NewPodControllerForExistingPod(cli kubernetes.Interface, pod *corev1.Pod) PodController { r := &podController{ - cli: cli, + cli: cli, + pcp: &podControllerProcessor{ + cli: cli, + }, pod: pod, podName: pod.Name, } @@ -114,8 +114,6 @@ func NewPodControllerForExistingPod(cli kubernetes.Interface, pod *corev1.Pod) P } r.podOptions = options - r.pcp = r - return r } @@ -137,7 +135,7 @@ func (p *podController) StartPod(ctx context.Context) error { return errors.Wrap(ErrPodControllerNotInitialized, "Failed to create pod") } - pod, err := p.pcp.CreatePod(ctx, p.cli, p.podOptions) + pod, err := p.pcp.CreatePod(ctx, p.podOptions) if err != nil { log.WithError(err).Print("Failed to create pod", field.M{"PodName": p.podOptions.Name, "Namespace": p.podOptions.Namespace}) return errors.Wrap(err, "Failed to create pod") @@ -155,7 +153,7 @@ func (p *podController) WaitForPodReady(ctx context.Context) error { return ErrPodControllerPodNotStarted } - if err := p.pcp.WaitForPodReadyPCP(ctx, p.pod.Namespace, p.pod.Name); err != nil { + if err := p.pcp.WaitForPodReady(ctx, p.pod.Namespace, p.pod.Name); err != nil { log.WithError(err).Print("Pod failed to become ready in time", field.M{"PodName": p.podName, "Namespace": p.podOptions.Namespace}) return errors.Wrap(err, "Pod failed to become ready in time") } @@ -175,7 +173,7 @@ func (p *podController) WaitForPodCompletion(ctx context.Context) error { return ErrPodControllerPodNotReady } - if err := p.pcp.WaitForPodCompletionPCP(ctx, p.pod.Namespace, p.pod.Name); err != nil { + if err := p.pcp.WaitForPodCompletion(ctx, p.pod.Namespace, p.pod.Name); err != nil { log.WithError(err).Print("Pod failed to complete in time", field.M{"PodName": p.podName, "Namespace": p.podOptions.Namespace}) return errors.Wrap(err, "Pod failed to complete in time") } @@ -249,7 +247,9 @@ func (p *podController) GetCommandExecutor() (PodCommandExecutor, error) { containerName: p.getContainerName(), } - pce.pcep = pce + pce.pcep = &podCommandExecutorProcessor{ + cli: p.cli, + } return pce, nil } @@ -270,27 +270,9 @@ func (p *podController) GetFileWriter() (PodFileWriter, error) { containerName: p.getContainerName(), } - pfw.fileWriterProcessor = pfw + pfw.fileWriterProcessor = &podFileWriterProcessor{ + cli: p.cli, + } return pfw, nil } - -// This is wrapped for unit testing. -func (p *podController) CreatePod(ctx context.Context, cli kubernetes.Interface, options *PodOptions) (*corev1.Pod, error) { - return CreatePod(ctx, cli, options) -} - -// This is wrapped for unit testing. -func (p *podController) WaitForPodReadyPCP(ctx context.Context, namespace, podName string) error { - return WaitForPodReady(ctx, p.cli, namespace, podName) -} - -// This is wrapped for unit testing -func (p *podController) WaitForPodCompletionPCP(ctx context.Context, namespace, podName string) error { - return WaitForPodCompletion(ctx, p.cli, namespace, podName) -} - -// This is wrapped for unit testing. -func (p *podController) DeletePod(ctx context.Context, namespace string, podName string, opts metav1.DeleteOptions) error { - return p.cli.CoreV1().Pods(namespace).Delete(ctx, podName, opts) -} diff --git a/pkg/kube/pod_controller_processor.go b/pkg/kube/pod_controller_processor.go new file mode 100644 index 0000000000..054364fd32 --- /dev/null +++ b/pkg/kube/pod_controller_processor.go @@ -0,0 +1,51 @@ +// Copyright 2023 The Kanister 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 kube + +import ( + "context" + + "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" +) + +// PodControllerProcessor is an interface wrapping kubernetes API invocation +// it is purposed to be replaced by fake implementation in tests +type PodControllerProcessor interface { + CreatePod(ctx context.Context, options *PodOptions) (*v1.Pod, error) + WaitForPodReady(ctx context.Context, namespace, podName string) error + WaitForPodCompletion(ctx context.Context, namespace, podName string) error + DeletePod(ctx context.Context, namespace string, podName string, opts metav1.DeleteOptions) error +} + +type podControllerProcessor struct { + cli kubernetes.Interface +} + +func (p *podControllerProcessor) CreatePod(ctx context.Context, options *PodOptions) (*v1.Pod, error) { + return CreatePod(ctx, p.cli, options) +} + +func (p *podControllerProcessor) WaitForPodReady(ctx context.Context, namespace, podName string) error { + return WaitForPodReady(ctx, p.cli, namespace, podName) +} + +func (p *podControllerProcessor) WaitForPodCompletion(ctx context.Context, namespace, podName string) error { + return WaitForPodCompletion(ctx, p.cli, namespace, podName) +} + +func (p *podControllerProcessor) DeletePod(ctx context.Context, namespace string, podName string, opts metav1.DeleteOptions) error { + return p.cli.CoreV1().Pods(namespace).Delete(ctx, podName, opts) +} diff --git a/pkg/kube/pod_controller_test.go b/pkg/kube/pod_controller_test.go index 21ffe06475..a755ad9e0c 100644 --- a/pkg/kube/pod_controller_test.go +++ b/pkg/kube/pod_controller_test.go @@ -52,7 +52,6 @@ func (s *PodControllerTestSuite) TestPodControllerStartPod(c *C) { err := pc.StartPod(ctx) c.Assert(err, Not(IsNil)) c.Assert(errors.Is(err, simulatedError), Equals, true) - c.Assert(pcp.InCreatePodCli, Equals, cli) c.Assert(pcp.InCreatePodOptions, DeepEquals, &PodOptions{ Namespace: podControllerNS, Name: podControllerPodName, @@ -77,9 +76,7 @@ func (s *PodControllerTestSuite) TestPodControllerStartPod(c *C) { err := pr.StartPod(ctx) c.Assert(err, IsNil) - c.Assert(prp.InCreatePodCli, Equals, cli) - prp.InCreatePodCli = nil prp.InCreatePodOptions = nil prp.CreatePodRet = nil prp.CreatePodErr = errors.New("CreatePod should not be invoked") @@ -87,7 +84,6 @@ func (s *PodControllerTestSuite) TestPodControllerStartPod(c *C) { err = pr.StartPod(ctx) c.Assert(err, Not(IsNil)) c.Assert(errors.Is(err, ErrPodControllerPodAlreadyStarted), Equals, true) - c.Assert(prp.InCreatePodCli, IsNil) c.Assert(prp.InCreatePodOptions, IsNil) }, } @@ -118,7 +114,6 @@ func (s *PodControllerTestSuite) TestPodControllerWaitPod(c *C) { c.Assert(err, Not(IsNil)) c.Assert(errors.Is(err, ErrPodControllerPodNotStarted), Equals, true) c.Assert(pcp.InCreatePodOptions, IsNil) - c.Assert(pcp.InCreatePodCli, IsNil) }, "Waiting failed due to timeout": func(pcp *FakePodControllerProcessor, pc PodController) { pcp.CreatePodRet = &corev1.Pod{ diff --git a/pkg/kube/pod_file_writer.go b/pkg/kube/pod_file_writer.go index 96d8967349..b310504656 100644 --- a/pkg/kube/pod_file_writer.go +++ b/pkg/kube/pod_file_writer.go @@ -49,11 +49,6 @@ type PodFileWriter interface { Write(ctx context.Context, filePath string, content io.Reader) (PodFileRemover, error) } -// podFileWriterProcessor aids in unit testing. -type podFileWriterProcessor interface { - newPodWriter(cli kubernetes.Interface, filePath string, content io.Reader) PodWriter -} - // podFileWriter keeps everything required to write a file to POD. type podFileWriter struct { cli kubernetes.Interface @@ -61,13 +56,13 @@ type podFileWriter struct { namespace string containerName string - fileWriterProcessor podFileWriterProcessor + fileWriterProcessor PodFileWriterProcessor } // WriteFileToPod writes specified file content to a file in the pod and returns an interface // with which the file can be removed. func (p *podFileWriter) Write(ctx context.Context, filePath string, content io.Reader) (PodFileRemover, error) { - pw := p.fileWriterProcessor.newPodWriter(p.cli, filePath, content) + pw := p.fileWriterProcessor.NewPodWriter(filePath, content) if err := pw.Write(ctx, p.namespace, p.podName, p.containerName); err != nil { return nil, errors.Wrap(err, "Write file to pod failed") } @@ -80,7 +75,3 @@ func (p *podFileWriter) Write(ctx context.Context, filePath string, content io.R path: filePath, }, nil } - -func (p *podFileWriter) newPodWriter(cli kubernetes.Interface, filepath string, content io.Reader) PodWriter { - return NewPodWriter(cli, filepath, content) -} diff --git a/pkg/kube/pod_file_writer_processor.go b/pkg/kube/pod_file_writer_processor.go new file mode 100644 index 0000000000..5de3eefe14 --- /dev/null +++ b/pkg/kube/pod_file_writer_processor.go @@ -0,0 +1,36 @@ +// Copyright 2023 The Kanister 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 kube + +import ( + "io" + + "k8s.io/client-go/kubernetes" +) + +// PodFileWriterProcessor is an interface wrapping kubernetes API invocation +// it is purposed to be replaced by fake implementation in tests +type PodFileWriterProcessor interface { + NewPodWriter(filePath string, content io.Reader) PodWriter +} + +type podFileWriterProcessor struct { + cli kubernetes.Interface +} + +// NewPodWriter returns a new PodWriter given path of file and content +func (p *podFileWriterProcessor) NewPodWriter(filepath string, content io.Reader) PodWriter { + return NewPodWriter(p.cli, filepath, content) +} diff --git a/pkg/kube/pod_file_writer_test.go b/pkg/kube/pod_file_writer_test.go index 4b3dc533f5..a93656912a 100644 --- a/pkg/kube/pod_file_writer_test.go +++ b/pkg/kube/pod_file_writer_test.go @@ -22,7 +22,6 @@ import ( "github.com/pkg/errors" . "gopkg.in/check.v1" - "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" ) @@ -44,7 +43,7 @@ type fakePodFileWriterProcessor struct { podWriter *fakePodWriter } -func (p *fakePodFileWriterProcessor) newPodWriter(cli kubernetes.Interface, filepath string, content io.Reader) PodWriter { +func (p *fakePodFileWriterProcessor) NewPodWriter(filepath string, content io.Reader) PodWriter { return p.podWriter } diff --git a/pkg/kube/pod_runner.go b/pkg/kube/pod_runner.go index be24f3aaca..64b864b0f7 100644 --- a/pkg/kube/pod_runner.go +++ b/pkg/kube/pod_runner.go @@ -50,7 +50,7 @@ func NewPodRunner(cli kubernetes.Interface, options *PodOptions) PodRunner { } // NewPodRunnerWithPodController returns a new PodRunner given PodController object -// This provides mechanism for passing fake podControllerProcessor through PodController for testing purposes. +// This provides mechanism for passing fake PodControllerProcessor through PodController for testing purposes. func NewPodRunnerWithPodController(pc PodController) PodRunner { r := &podRunner{ pc: pc,