From d7477d740dfe95d36f97c4101667ed6602eed4e1 Mon Sep 17 00:00:00 2001 From: Victor Morales Date: Fri, 7 Feb 2025 20:14:43 -0800 Subject: [PATCH] Refactor runner methods --- cmd/kar/app/root.go | 13 ++++----- cmd/kar/app/root_test.go | 59 ++++++++++++++++++++++++++++++---------- internal/errors.go | 3 ++ internal/runner.go | 27 ++++++++---------- 4 files changed, 66 insertions(+), 36 deletions(-) diff --git a/cmd/kar/app/root.go b/cmd/kar/app/root.go index 3a6c89a..d65dc14 100644 --- a/cmd/kar/app/root.go +++ b/cmd/kar/app/root.go @@ -42,16 +42,15 @@ func NewRootCommand(ctx context.Context, runner runner.Runner, opts Opts) *cobra } func run(ctx context.Context, runner runner.Runner, opts Opts) error { - err := runner.CreateResources(ctx, opts.VMTemplate, opts.RunnerName, opts.JitConfig) - if err != nil { + if err := runner.CreateResources(ctx, opts.VMTemplate, opts.RunnerName, opts.JitConfig); err != nil { return errors.Wrap(err, "fail to create resources") } - defer runner.DeleteResources(ctx, runner.GetDataVolumeName(), runner.GetDataVolumeName()) - - runner.WaitForVirtualMachineInstance(ctx) + if err := runner.WaitForVirtualMachineInstance(ctx, runner.GetVMIName()); err != nil { + return errors.Wrap(err, "fail to wait for resources") + } - if runner.Failed() { - return errors.New("virtual machine instance has failed") + if err := runner.DeleteResources(ctx, runner.GetVMIName(), runner.GetDataVolumeName()); err != nil { + return errors.Wrap(err, "fail to delete resources") } return nil diff --git a/cmd/kar/app/root_test.go b/cmd/kar/app/root_test.go index 3b49691..8b943a8 100644 --- a/cmd/kar/app/root_test.go +++ b/cmd/kar/app/root_test.go @@ -18,6 +18,7 @@ package app_test import ( "context" + "errors" "slices" @@ -28,7 +29,9 @@ import ( ) type mock struct { - failed bool + createErr error + deleteErr error + waitErr error createCalled bool waitCalled bool deleteCalled bool @@ -37,8 +40,17 @@ type mock struct { jitConfig string } -func (m *mock) Failed() bool { - return m.failed +type Failure uint8 + +const ( + None Failure = 1 << iota + Create + Delete + Wait +) + +func HasOneOf(f, flag Failure) bool { + return f&flag != 0 } func (m *mock) GetVMIName() string { @@ -57,17 +69,19 @@ func (m *mock) CreateResources(_ context.Context, vmTemplate, runnerName, jitCon m.createCalled = true - return nil + return m.createErr } -func (m *mock) WaitForVirtualMachineInstance(_ context.Context) { +func (m *mock) WaitForVirtualMachineInstance(_ context.Context, _ string) error { m.waitCalled = true + + return m.waitErr } func (m *mock) DeleteResources(_ context.Context, _, _ string) error { m.deleteCalled = true - return nil + return m.deleteErr } var _ = Describe("Root Command", func() { @@ -80,9 +94,18 @@ var _ = Describe("Root Command", func() { cmd = app.NewRootCommand(context.TODO(), &runner, opts) }) - DescribeTable("initialization process", func(shouldSucceed, failed bool, args ...string) { + DescribeTable("initialization process", func(shouldSucceed bool, failure Failure, args ...string) { cmd.SetArgs(args) - runner.failed = failed + if HasOneOf(failure, Create) { + runner.createErr = errors.New("create failure") + } + if HasOneOf(failure, Delete) { + runner.deleteErr = errors.New("delete failure") + } + if HasOneOf(failure, Wait) { + runner.waitErr = errors.New("wait failure") + } + err := cmd.Execute() if shouldSucceed { @@ -102,13 +125,21 @@ var _ = Describe("Root Command", func() { } Expect(runner.createCalled).Should(BeTrue()) - Expect(runner.deleteCalled).Should(BeTrue()) + if HasOneOf(failure, Create) { + return + } Expect(runner.waitCalled).Should(BeTrue()) + if HasOneOf(failure, Wait) { + return + } + Expect(runner.deleteCalled).Should(BeTrue()) }, - Entry("when the default options are provided", true, false), - Entry("when config option is provided", true, false, "-c", "test config"), - Entry("when vm template option is provided", true, false, "-t", "vm template"), - Entry("when runner name option is provided", true, false, "-r", "runner name"), - Entry("when the execution failed", false, true), + Entry("when the default options are provided", true, None), + Entry("when config option is provided", true, None, "-c", "test config"), + Entry("when vm template option is provided", true, None, "-t", "vm template"), + Entry("when runner name option is provided", true, None, "-r", "runner name"), + Entry("when the creation failed", false, Create), + Entry("when the delete failed", false, Delete), + Entry("when the wait failed", false, Wait), ) }) diff --git a/internal/errors.go b/internal/errors.go index c4653e4..ab2fb6e 100644 --- a/internal/errors.go +++ b/internal/errors.go @@ -26,3 +26,6 @@ var ErrEmptyRunnerName = errors.New("empty runner name") // ErrEmptyJitConfig indicates that Just-in-Time configuration provided is empty. var ErrEmptyJitConfig = errors.New("empty jit config") + +// ErrRunnerFailed indicates that the runner has a failed during its execution. +var ErrRunnerFailed = errors.New("runner has failed") diff --git a/internal/runner.go b/internal/runner.go index e315ac3..62f3ec2 100644 --- a/internal/runner.go +++ b/internal/runner.go @@ -40,9 +40,8 @@ const ( type Runner interface { CreateResources(ctx context.Context, vmTemplate string, runnerName string, jitConfig string) error - WaitForVirtualMachineInstance(ctx context.Context) + WaitForVirtualMachineInstance(ctx context.Context, vmi string) error DeleteResources(ctx context.Context, vmi string, dv string) error - Failed() bool GetVMIName() string GetDataVolumeName() string } @@ -57,10 +56,6 @@ type KubevirtRunner struct { var _ Runner = (*KubevirtRunner)(nil) -func (rc *KubevirtRunner) Failed() bool { - return rc.currentStatus == v1.Failed -} - func (rc *KubevirtRunner) GetVMIName() string { return rc.virtualMachineInstance } @@ -192,14 +187,14 @@ func (rc *KubevirtRunner) CreateResources(ctx context.Context, return nil } -func (rc *KubevirtRunner) WaitForVirtualMachineInstance(ctx context.Context) { - log.Printf("Watching %s Virtual Machine Instance\n", rc.virtualMachineInstance) +func (rc *KubevirtRunner) WaitForVirtualMachineInstance(ctx context.Context, virtualMachineInstance string) error { + log.Printf("Watching %s Virtual Machine Instance\n", virtualMachineInstance) const reportingElapse = 5.0 watch, err := rc.virtClient.VirtualMachineInstance(rc.namespace).Watch(ctx, k8smetav1.ListOptions{}) if err != nil { - log.Fatalf("Failed to watch Virtual Machine Instance: %v", err) + return errors.Wrap(err, "failed to watch the virtual machine instance") } defer watch.Stop() @@ -214,23 +209,25 @@ func (rc *KubevirtRunner) WaitForVirtualMachineInstance(ctx context.Context) { switch rc.currentStatus { case v1.Succeeded: - log.Printf("%s has successfully completed\n", rc.virtualMachineInstance) + log.Printf("%s has successfully completed\n", virtualMachineInstance) - return + return nil case v1.Failed: - log.Printf("%s has failed\n", rc.virtualMachineInstance) + log.Printf("%s has failed\n", virtualMachineInstance) - return + return ErrRunnerFailed default: - log.Printf("%s has transitioned to %s phase \n", rc.virtualMachineInstance, rc.currentStatus) + log.Printf("%s has transitioned to %s phase \n", virtualMachineInstance, rc.currentStatus) } } else if time.Since(lastTimeChecked).Minutes() > reportingElapse { - log.Printf("%s is in %s phase \n", rc.virtualMachineInstance, rc.currentStatus) + log.Printf("%s is in %s phase \n", virtualMachineInstance, rc.currentStatus) lastTimeChecked = time.Now() } } } + + return nil } func (rc *KubevirtRunner) DeleteResources(ctx context.Context, virtualMachineInstance, dataVolume string) error {