From f5daa01f928d75a2bbf69aba6e0fc9606be49db5 Mon Sep 17 00:00:00 2001 From: michaeljguarino Date: Mon, 27 May 2024 11:54:17 -0400 Subject: [PATCH] feat: Properly handle all controller errors (#196) * Properly handle all controller errors It looks like there might not be error handling for issues that happen pre execution, this will solve for that a bit better in lieu of a more robust way of reporting run completion. * add workdir support * switch to chainguard wolfi base image for the package manager * simplify ordered execution logic * refactor code and update apply args modifier * cleanup, add workdir arg handling to job controller, ensure postStart hook on ctrl start early return * fix lint * fix import cycle * fix lint * set step status to failed if stack run fails or is cancelled * mark stack run as pending approval when approval is required and plan has been uploaded * properly handle stack run statuses before and after approval * properly handle stack run statuses before and after approval * simplify ordered execution logic * fix lint * use custom exec work dir from stack and restore controller cleanup via finish fn * create additional files in the stack workdir * revert job.go change * run post start on controller finish --------- Co-authored-by: Sebastian Florek --- cmd/harness/main.go | 4 +- dockerfiles/harness/base.Dockerfile | 8 +- go.mod | 2 +- go.sum | 2 + internal/controller/stackrunjob_controller.go | 2 +- pkg/client/console.go | 4 +- pkg/client/stack.go | 4 +- pkg/controller/stacks/job.go | 1 + pkg/harness/controller/controller.go | 73 +++++++++++-------- pkg/harness/controller/controller_hooks.go | 49 ++++++++----- pkg/harness/controller/controller_types.go | 9 ++- pkg/harness/controller/executor.go | 68 +++-------------- pkg/harness/controller/executor_options.go | 10 --- pkg/harness/controller/executor_types.go | 9 --- pkg/harness/environment/environment.go | 6 +- .../environment/environment_options.go | 11 ++- pkg/harness/environment/environment_types.go | 10 ++- pkg/harness/exec/exec.go | 10 +-- pkg/harness/exec/exec_options.go | 4 +- pkg/harness/exec/exec_types.go | 4 +- pkg/harness/stackrun/helpers.go | 65 +++++++++++++++++ pkg/harness/stackrun/{ => v1}/types.go | 4 +- pkg/harness/tool/terraform/modifier.go | 13 +++- pkg/harness/tool/terraform/modifier_types.go | 3 + pkg/harness/tool/terraform/terraform.go | 2 +- pkg/test/mocks/Client_mock.go | 6 +- 26 files changed, 216 insertions(+), 167 deletions(-) create mode 100644 pkg/harness/stackrun/helpers.go rename pkg/harness/stackrun/{ => v1}/types.go (95%) diff --git a/cmd/harness/main.go b/cmd/harness/main.go index 75ef0e09..3434dacf 100644 --- a/cmd/harness/main.go +++ b/cmd/harness/main.go @@ -23,7 +23,6 @@ func main() { ) ctx := signals.NewCancelableContext( signals.SetupSignalHandler(signals.ExitCodeTerminated), - //signals.NewTimeoutSignal(args.Timeout()), signals.NewConsoleSignal(consoleClient, args.StackRunID()), ) @@ -43,13 +42,12 @@ func main() { } if err = ctrl.Start(ctx); err != nil { + _ = ctrl.Finish(err) handleFatalError(err) } } func handleFatalError(err error) { - // TODO: initiate a graceful shutdown procedure - switch { case errors.Is(err, internalerrors.ErrTimeout): klog.ErrorS(err, "timed out waiting for stack run to complete", "timeout", args.Timeout()) diff --git a/dockerfiles/harness/base.Dockerfile b/dockerfiles/harness/base.Dockerfile index b900f989..0e87dcd8 100644 --- a/dockerfiles/harness/base.Dockerfile +++ b/dockerfiles/harness/base.Dockerfile @@ -31,10 +31,12 @@ FROM busybox:1.35.0-uclibc as environment RUN mkdir /plural RUN mkdir /tmp/plural -FROM gcr.io/distroless/base-debian12:nonroot as final +FROM cgr.dev/chainguard/wolfi-base:latest as final -# Switch to the nonroot user -USER nonroot:nonroot +RUN apk update --no-cache && apk add git + +# # Switch to the nonroot user +USER nonroot # Set up the environment # 1. copy plural and tmp directories with proper permissions for the nonroot user diff --git a/go.mod b/go.mod index d3901482..17faa76b 100644 --- a/go.mod +++ b/go.mod @@ -19,7 +19,7 @@ require ( github.com/open-policy-agent/gatekeeper/v3 v3.15.1 github.com/orcaman/concurrent-map/v2 v2.0.1 github.com/pkg/errors v0.9.1 - github.com/pluralsh/console-client-go v0.5.8 + github.com/pluralsh/console-client-go v0.5.9 github.com/pluralsh/controller-reconcile-helper v0.0.4 github.com/pluralsh/gophoenix v0.1.3-0.20231201014135-dff1b4309e34 github.com/pluralsh/polly v0.1.10 diff --git a/go.sum b/go.sum index 33ddc0e7..423b6859 100644 --- a/go.sum +++ b/go.sum @@ -528,6 +528,8 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pkg/sftp v1.13.1/go.mod h1:3HaPG6Dq1ILlpPZRO0HVMrsydcdLt6HRDccSgb87qRg= github.com/pluralsh/console-client-go v0.5.8 h1:Qm7vS+gCbmWqy5i4saLPc5/SUZaW6RCzxWF+uxyPA+Y= github.com/pluralsh/console-client-go v0.5.8/go.mod h1:eyCiLA44YbXiYyJh8303jk5JdPkt9McgCo5kBjk4lKo= +github.com/pluralsh/console-client-go v0.5.9 h1:r5YMD4dU2zWiDApWtqu45l/02X4RnsNeVEFzuuyehEA= +github.com/pluralsh/console-client-go v0.5.9/go.mod h1:eyCiLA44YbXiYyJh8303jk5JdPkt9McgCo5kBjk4lKo= github.com/pluralsh/controller-reconcile-helper v0.0.4 h1:1o+7qYSyoeqKFjx+WgQTxDz4Q2VMpzprJIIKShxqG0E= github.com/pluralsh/controller-reconcile-helper v0.0.4/go.mod h1:AfY0gtteD6veBjmB6jiRx/aR4yevEf6K0M13/pGan/s= github.com/pluralsh/gophoenix v0.1.3-0.20231201014135-dff1b4309e34 h1:ab2PN+6if/Aq3/sJM0AVdy1SYuMAnq4g20VaKhTm/Bw= diff --git a/internal/controller/stackrunjob_controller.go b/internal/controller/stackrunjob_controller.go index 1ec6a672..1cf15b57 100644 --- a/internal/controller/stackrunjob_controller.go +++ b/internal/controller/stackrunjob_controller.go @@ -118,7 +118,7 @@ func (r *StackRunJobReconciler) getStepStatusUpdate(stackStatus console.StackSta } if stackStatus == console.StackStatusFailed || stackStatus == console.StackStatusCancelled { - return lo.ToPtr(console.StepStatusSuccessful) + return lo.ToPtr(console.StepStatusFailed) } return nil diff --git a/pkg/client/console.go b/pkg/client/console.go index 042761bb..cccf3860 100644 --- a/pkg/client/console.go +++ b/pkg/client/console.go @@ -9,7 +9,7 @@ import ( "github.com/pluralsh/deployment-operator/api/v1alpha1" "github.com/pluralsh/deployment-operator/internal/helpers" - "github.com/pluralsh/deployment-operator/pkg/harness/stackrun" + v1 "github.com/pluralsh/deployment-operator/pkg/harness/stackrun/v1" ) var lock = &sync.Mutex{} @@ -69,7 +69,7 @@ type Client interface { UpsertConstraints(constraints []*console.PolicyConstraintAttributes) (*console.UpsertPolicyConstraints, error) GetNamespace(id string) (*console.ManagedNamespaceFragment, error) ListNamespaces(after *string, first *int64) (*console.ListClusterNamespaces_ClusterManagedNamespaces, error) - GetStackRunBase(id string) (*stackrun.StackRun, error) + GetStackRunBase(id string) (*v1.StackRun, error) GetStackRun(id string) (*console.StackRunFragment, error) AddStackRunLogs(id, logs string) error CompleteStackRun(id string, attributes console.StackRunAttributes) error diff --git a/pkg/client/stack.go b/pkg/client/stack.go index 422bba76..79e95826 100644 --- a/pkg/client/stack.go +++ b/pkg/client/stack.go @@ -8,11 +8,11 @@ import ( internalerrors "github.com/pluralsh/deployment-operator/internal/errors" "github.com/pluralsh/deployment-operator/pkg/harness/errors" - "github.com/pluralsh/deployment-operator/pkg/harness/stackrun" + v1 "github.com/pluralsh/deployment-operator/pkg/harness/stackrun/v1" "github.com/pluralsh/deployment-operator/pkg/log" ) -func (c *client) GetStackRunBase(id string) (result *stackrun.StackRun, err error) { +func (c *client) GetStackRunBase(id string) (result *v1.StackRun, err error) { stackRun, err := c.consoleClient.GetStackRunBase(c.ctx, id) if err != nil && !internalerrors.IsNotFound(err) { return nil, err diff --git a/pkg/controller/stacks/job.go b/pkg/controller/stacks/job.go index 018292a0..91b06257 100644 --- a/pkg/controller/stacks/job.go +++ b/pkg/controller/stacks/job.go @@ -102,6 +102,7 @@ func (r *StackReconciler) GenerateRunJob(run *console.StackRunFragment, name str jobSpec.Template.Spec.RestartPolicy = corev1.RestartPolicyNever jobSpec.BackoffLimit = lo.ToPtr(int32(0)) + jobSpec.TTLSecondsAfterFinished = lo.ToPtr(int32(60 * 60)) jobSpec.Template.Spec.Containers = r.ensureDefaultContainer(jobSpec.Template.Spec.Containers, run) diff --git a/pkg/harness/controller/controller.go b/pkg/harness/controller/controller.go index 5229a287..12bd85a2 100644 --- a/pkg/harness/controller/controller.go +++ b/pkg/harness/controller/controller.go @@ -4,6 +4,7 @@ import ( "cmp" "context" "fmt" + "path" "slices" "sync" @@ -14,7 +15,7 @@ import ( "github.com/pluralsh/deployment-operator/pkg/harness/environment" "github.com/pluralsh/deployment-operator/pkg/harness/exec" "github.com/pluralsh/deployment-operator/pkg/harness/sink" - "github.com/pluralsh/deployment-operator/pkg/harness/stackrun" + v1 "github.com/pluralsh/deployment-operator/pkg/harness/stackrun/v1" "github.com/pluralsh/deployment-operator/pkg/harness/tool" "github.com/pluralsh/deployment-operator/pkg/log" ) @@ -78,6 +79,14 @@ func (in *stackRunController) Start(ctx context.Context) (retErr error) { return in.postStart(retErr) } +func (in *stackRunController) Finish(stackRunErr error) error { + if stackRunErr == nil { + return nil + } + + return in.postStart(stackRunErr) +} + func (in *stackRunController) executables(ctx context.Context) []exec.Executable { // Ensure that steps are sorted in the correct order slices.SortFunc(in.stackRun.Steps, func(s1, s2 *gqlclient.RunStepFragment) int { @@ -118,42 +127,36 @@ func (in *stackRunController) toExecutable(ctx context.Context, step *gqlclient. return exec.NewExecutable( step.Cmd, - exec.WithDir(in.dir), + exec.WithDir(in.execWorkDir()), exec.WithEnv(in.stackRun.Env()), exec.WithArgs(args), exec.WithID(step.ID), exec.WithLogSink(consoleWriter), - exec.WithHook(stackrun.LifecyclePreStart, in.preExecHook(step.Stage, step.ID)), - exec.WithHook(stackrun.LifecyclePostStart, in.postExecHook(step.Stage, step.ID)), + exec.WithHook(v1.LifecyclePreStart, in.preExecHook(step.Stage, step.ID)), + exec.WithHook(v1.LifecyclePostStart, in.postExecHook(step.Stage)), ) } -func (in *stackRunController) markStackRun(status gqlclient.StackStatus) error { - return in.consoleClient.UpdateStackRun(in.stackRunID, gqlclient.StackRunAttributes{ - Status: status, - }) -} - -func (in *stackRunController) markStackRunStep(id string, status gqlclient.StepStatus) error { - return in.consoleClient.UpdateStackRunStep(id, gqlclient.RunStepAttributes{ - Status: status, - }) -} - func (in *stackRunController) completeStackRun(status gqlclient.StackStatus, stackRunErr error) error { - state, err := in.tool.State() - if err != nil { - klog.ErrorS(err, "could not prepare state attributes") - } + var state *gqlclient.StackStateAttributes + var output []*gqlclient.StackOutputAttributes + var err error + + if in.tool != nil { + state, err = in.tool.State() + if err != nil { + klog.ErrorS(err, "could not prepare state attributes") + } - klog.V(log.LogLevelTrace).InfoS("generated console state", "state", state) + klog.V(log.LogLevelTrace).InfoS("generated console state", "state", state) - output, err := in.tool.Output() - if err != nil { - klog.ErrorS(err, "could not prepare output attributes") - } + output, err = in.tool.Output() + if err != nil { + klog.ErrorS(err, "could not prepare output attributes") + } - klog.V(log.LogLevelTrace).InfoS("generated console output", "output", output) + klog.V(log.LogLevelTrace).InfoS("generated console output", "output", output) + } serviceErrorAttributes := make([]*gqlclient.ServiceErrorAttributes, 0) if stackRunErr != nil { @@ -170,16 +173,29 @@ func (in *stackRunController) completeStackRun(status gqlclient.StackStatus, sta }) } +func (in *stackRunController) execWorkDir() string { + if in.stackRun.ExecWorkDir != nil && len(*in.stackRun.ExecWorkDir) > 0 { + return path.Join(in.dir, *in.stackRun.ExecWorkDir) + } + + return in.dir +} + func (in *stackRunController) prepare() error { env := environment.New( environment.WithStackRun(in.stackRun), environment.WithWorkingDir(in.dir), + environment.WithFilesDir(in.execWorkDir()), environment.WithFetchClient(in.fetchClient), ) - in.tool = tool.New(in.stackRun.Type, in.dir) + if err := env.Setup(); err != nil { + return err + } + + in.tool = tool.New(in.stackRun.Type, in.execWorkDir()) - return env.Setup() + return nil } func (in *stackRunController) init() (Controller, error) { @@ -215,7 +231,6 @@ func NewStackRunController(options ...Option) (Controller, error) { ctrl.executor = newExecutor( errChan, finishedChan, - //WithPreRunFunc(ctrl.preStepRun), WithPostRunFunc(ctrl.postStepRun), ) diff --git a/pkg/harness/controller/controller_hooks.go b/pkg/harness/controller/controller_hooks.go index 982e81e2..63a7044e 100644 --- a/pkg/harness/controller/controller_hooks.go +++ b/pkg/harness/controller/controller_hooks.go @@ -12,6 +12,7 @@ import ( "github.com/pluralsh/deployment-operator/pkg/harness/environment" internalerrors "github.com/pluralsh/deployment-operator/pkg/harness/errors" "github.com/pluralsh/deployment-operator/pkg/harness/stackrun" + v1 "github.com/pluralsh/deployment-operator/pkg/harness/stackrun/v1" "github.com/pluralsh/deployment-operator/pkg/log" ) @@ -19,16 +20,18 @@ var ( runApproved = false ) +// preStart function is executed before stack run steps. func (in *stackRunController) preStart() { if in.stackRun.Status != gqlclient.StackStatusPending && !environment.IsDev() { klog.Fatalf("could not start stack run: invalid status: %s", in.stackRun.Status) } - if err := in.markStackRun(gqlclient.StackStatusRunning); err != nil { + if err := stackrun.StartStackRun(in.consoleClient, in.stackRunID); err != nil { klog.ErrorS(err, "could not update stack run status") } } +// postStart function is executed after all stack run steps. func (in *stackRunController) postStart(err error) error { var status gqlclient.StackStatus @@ -49,6 +52,9 @@ func (in *stackRunController) postStart(err error) error { return err } +// postStepRun is a callback function started by the executor after executable finishes. +// It provides the information about run step that was executed and if it exited with error +// or not. func (in *stackRunController) postStepRun(id string, err error) { var status gqlclient.StepStatus @@ -59,18 +65,14 @@ func (in *stackRunController) postStepRun(id string, err error) { status = gqlclient.StepStatusFailed } - if err := in.markStackRunStep(id, status); err != nil { + if err := stackrun.MarkStackRunStep(in.consoleClient, id, status); err != nil { klog.ErrorS(err, "could not update stack run step status") } } -func (in *stackRunController) preStepRun(id string) { - if err := in.markStackRunStep(id, gqlclient.StepStatusRunning); err != nil { - klog.ErrorS(err, "could not update stack run status") - } -} - -func (in *stackRunController) postExecHook(stage gqlclient.StepStage, id string) stackrun.HookFunction { +// postExecHook is a callback function started by the exec.Executable after it finishes. +// It differs from the +func (in *stackRunController) postExecHook(stage gqlclient.StepStage) v1.HookFunction { return func() error { if stage != gqlclient.StepStagePlan { return nil @@ -80,27 +82,31 @@ func (in *stackRunController) postExecHook(stage gqlclient.StepStage, id string) } } -func (in *stackRunController) preExecHook(stage gqlclient.StepStage, id string) stackrun.HookFunction { +func (in *stackRunController) preExecHook(stage gqlclient.StepStage, id string) v1.HookFunction { return func() error { - if stage == gqlclient.StepStageApply { - if err := in.approvalCheck(); err != nil { - return err - } + if stage == gqlclient.StepStageApply && in.requiresApproval() { + in.waitForApproval() } - if err := in.markStackRunStep(id, gqlclient.StepStatusRunning); err != nil { + if err := stackrun.StartStackRunStep(in.consoleClient, id); err != nil { klog.ErrorS(err, "could not update stack run status") } + return nil } } -func (in *stackRunController) approvalCheck() error { - if !in.stackRun.Approval || runApproved { - return nil - } +func (in *stackRunController) requiresApproval() bool { + return in.stackRun.Approval && !runApproved +} - return wait.PollUntilContextCancel(context.Background(), 5*time.Second, true, func(_ context.Context) (done bool, err error) { +func (in *stackRunController) waitForApproval() { + // Retry here to make sure that the pending approval status will be set before we start waiting. + stackrun.MarkStackRunWithRetry(in.consoleClient, in.stackRunID, gqlclient.StackStatusPendingApproval, 5*time.Second) + + klog.V(log.LogLevelInfo).InfoS("waiting for approval to proceed") + // Condition function never returns error. We can ignore it. + _ = wait.PollUntilContextCancel(context.Background(), 5*time.Second, true, func(_ context.Context) (done bool, err error) { if runApproved { return true, nil } @@ -114,6 +120,9 @@ func (in *stackRunController) approvalCheck() error { runApproved = stack.ApprovedAt != nil return runApproved, nil }) + + // Retry here to make sure that we resume the stack run status to running after it has been approved. + stackrun.MarkStackRunWithRetry(in.consoleClient, in.stackRunID, gqlclient.StackStatusRunning, 5*time.Second) } func (in *stackRunController) uploadPlan() error { diff --git a/pkg/harness/controller/controller_types.go b/pkg/harness/controller/controller_types.go index 2e58d58d..e571221c 100644 --- a/pkg/harness/controller/controller_types.go +++ b/pkg/harness/controller/controller_types.go @@ -8,12 +8,13 @@ import ( "github.com/pluralsh/deployment-operator/internal/helpers" console "github.com/pluralsh/deployment-operator/pkg/client" "github.com/pluralsh/deployment-operator/pkg/harness/sink" - "github.com/pluralsh/deployment-operator/pkg/harness/stackrun" - v1 "github.com/pluralsh/deployment-operator/pkg/harness/tool/v1" + stackrunv1 "github.com/pluralsh/deployment-operator/pkg/harness/stackrun/v1" + toolv1 "github.com/pluralsh/deployment-operator/pkg/harness/tool/v1" ) type Controller interface { Start(ctx context.Context) error + Finish(stackRunErr error) error } type stackRunController struct { @@ -35,7 +36,7 @@ type stackRunController struct { stackRunStepTimeout time.Duration // stackRun - stackRun *stackrun.StackRun + stackRun *stackrunv1.StackRun // consoleClient consoleClient console.Client @@ -55,7 +56,7 @@ type stackRunController struct { // List of supported tools is based on the gqlclient.StackType. // It is mainly responsible for: // - gathering state - tool v1.Tool + tool toolv1.Tool // wg wg sync.WaitGroup diff --git a/pkg/harness/controller/executor.go b/pkg/harness/controller/executor.go index c5c9cf36..b653dcb3 100644 --- a/pkg/harness/controller/executor.go +++ b/pkg/harness/controller/executor.go @@ -8,7 +8,6 @@ import ( "k8s.io/klog/v2" "github.com/pluralsh/deployment-operator/pkg/harness/exec" - "github.com/pluralsh/deployment-operator/pkg/harness/stackrun" "github.com/pluralsh/deployment-operator/pkg/log" ) @@ -51,32 +50,15 @@ func (in *executor) ordered(ctx context.Context) { klog.V(log.LogLevelDebug).InfoS("starting executables in order", "queue", len(in.startQueue)) - go func() { - // Queue up all executables for execution - for _, executable := range in.startQueue { - in.ch <- executable - } - }() - // Read executables and run them in order go func() { - for { - // Get executable from the queue - executable := <-in.ch - - // Run the executable and wait for it to finish + for _, executable := range in.startQueue { if err := in.run(ctx, executable); err != nil { in.errChan <- err - break - } - - if empty := in.dequeue(executable); empty { - // We are finished when execution queue is empty. - // Send finish signal and return. - close(in.finishedChan) return } } + close(in.finishedChan) }() } @@ -110,56 +92,26 @@ func (in *executor) parallel(ctx context.Context) { } func (in *executor) run(ctx context.Context, executable exec.Executable) (retErr error) { - in.preRun(executable.ID()) + if in.preRunFunc != nil { + in.preRunFunc(executable.ID()) + } if err := executable.Run(ctx); err != nil { retErr = fmt.Errorf("command execution failed: %s: err: %w", executable.Command(), err) } - return in.postRun(executable.ID(), retErr) -} - -func (in *executor) preRun(id string) { - if in.preRunFunc != nil { - in.preRunFunc(id) - } -} - -func (in *executor) postRun(id string, err error) error { if in.postRunFunc != nil { - in.postRunFunc(id, err) - } - - return err -} - -func (in *executor) dequeue(executable exec.Executable) (empty bool) { - for i, existing := range in.startQueue { - if existing == executable { - // Remove the item from the start queue. - in.startQueue = append(in.startQueue[:i], in.startQueue[i+1:]...) - break - } + in.postRunFunc(executable.ID(), retErr) } - return len(in.startQueue) == 0 -} - -func (in *executor) runLifecycleFunction(lifecycle stackrun.Lifecycle) error { - if fn, exists := in.hookFunctions[lifecycle]; exists { - return fn() - } - - return nil + return retErr } func newExecutor(errChan chan error, finishedChan chan struct{}, options ...ExecutorOption) *executor { result := &executor{ - errChan: errChan, - finishedChan: finishedChan, - strategy: ExecutionStrategyOrdered, - ch: make(chan exec.Executable), - hookFunctions: make(map[stackrun.Lifecycle]stackrun.HookFunction), + errChan: errChan, + finishedChan: finishedChan, + strategy: ExecutionStrategyOrdered, } for _, option := range options { diff --git a/pkg/harness/controller/executor_options.go b/pkg/harness/controller/executor_options.go index 2e52d429..668350ca 100644 --- a/pkg/harness/controller/executor_options.go +++ b/pkg/harness/controller/executor_options.go @@ -1,9 +1,5 @@ package controller -import ( - "github.com/pluralsh/deployment-operator/pkg/harness/stackrun" -) - func WithExecutionStrategy(strategy ExecutionStrategy) ExecutorOption { return func(e *executor) { e.strategy = strategy @@ -21,9 +17,3 @@ func WithPreRunFunc(fn func(string)) ExecutorOption { e.preRunFunc = fn } } - -func WithHook(lifecycle stackrun.Lifecycle, fn stackrun.HookFunction) ExecutorOption { - return func(e *executor) { - e.hookFunctions[lifecycle] = fn - } -} diff --git a/pkg/harness/controller/executor_types.go b/pkg/harness/controller/executor_types.go index 8ff983d4..8b952eb6 100644 --- a/pkg/harness/controller/executor_types.go +++ b/pkg/harness/controller/executor_types.go @@ -4,7 +4,6 @@ import ( "sync" "github.com/pluralsh/deployment-operator/pkg/harness/exec" - "github.com/pluralsh/deployment-operator/pkg/harness/stackrun" ) type executor struct { @@ -37,14 +36,6 @@ type executor struct { // ExecutionStrategyOrdered - will run executables one by one and wait // for the previous one to finish first. strategy ExecutionStrategy - - // ch is the internal channel where the executables are read off from. - // It is used only by the ExecutionStrategyOrdered to ensure ordered - // run of the executables. - ch chan exec.Executable - - // hookFunctions ... - hookFunctions map[stackrun.Lifecycle]stackrun.HookFunction } type ExecutorOption func(*executor) diff --git a/pkg/harness/environment/environment.go b/pkg/harness/environment/environment.go index 800391b6..86b7a9c8 100644 --- a/pkg/harness/environment/environment.go +++ b/pkg/harness/environment/environment.go @@ -38,7 +38,7 @@ func (in *environment) prepareFiles() error { } for _, fragment := range in.stackRun.Files { - destination := path.Join(in.dir, fragment.Path) + destination := path.Join(in.filesDir, fragment.Path) if err := helpers.File().Create(destination, fragment.Content); err != nil { klog.ErrorS(err, "failed preparing files", "path", destination) return err @@ -60,6 +60,10 @@ func (in *environment) init() Environment { helpers.EnsureDirOrDie(in.dir) } + if len(in.filesDir) == 0 { + in.filesDir = in.dir + } + return in } diff --git a/pkg/harness/environment/environment_options.go b/pkg/harness/environment/environment_options.go index 4c1329fa..e73c2a9f 100644 --- a/pkg/harness/environment/environment_options.go +++ b/pkg/harness/environment/environment_options.go @@ -2,7 +2,7 @@ package environment import ( "github.com/pluralsh/deployment-operator/internal/helpers" - "github.com/pluralsh/deployment-operator/pkg/harness/stackrun" + v1 "github.com/pluralsh/deployment-operator/pkg/harness/stackrun/v1" ) // WithWorkingDir allows changing the default working directory of the Environment. @@ -12,6 +12,13 @@ func WithWorkingDir(dir string) Option { } } +// WithFilesDir allow changing the default path where all additional files are being created. +func WithFilesDir(dir string) Option { + return func(e *environment) { + e.filesDir = dir + } +} + // WithFetchClient allows configuring helpers.FetchClient used by the Environment // to download files. func WithFetchClient(client helpers.FetchClient) Option { @@ -22,7 +29,7 @@ func WithFetchClient(client helpers.FetchClient) Option { // WithStackRun provides information about stack run used to initialize // the Environment. -func WithStackRun(stackRun *stackrun.StackRun) Option { +func WithStackRun(stackRun *v1.StackRun) Option { return func(e *environment) { e.stackRun = stackRun } diff --git a/pkg/harness/environment/environment_types.go b/pkg/harness/environment/environment_types.go index a5edfa00..8599eb3d 100644 --- a/pkg/harness/environment/environment_types.go +++ b/pkg/harness/environment/environment_types.go @@ -2,7 +2,7 @@ package environment import ( "github.com/pluralsh/deployment-operator/internal/helpers" - "github.com/pluralsh/deployment-operator/pkg/harness/stackrun" + v1 "github.com/pluralsh/deployment-operator/pkg/harness/stackrun/v1" ) // Environment is responsible for handling harness working directory. @@ -23,10 +23,12 @@ type environment struct { // execution of the stack run. For example, it provides // URL of the tarball with mandatory files needed to run // stack run step commands. - stackRun *stackrun.StackRun - // dir is a working directory where all files/directories - // are being created. + stackRun *v1.StackRun + // dir is a working directory where tarball files/dirs are unpacked. dir string + // filesDir is a working directory where all additional files should be + // unpacked/created. It is equal to dir if empty. + filesDir string // fetchClient is a helper client used to download and unpack the tarball. fetchClient helpers.FetchClient } diff --git a/pkg/harness/exec/exec.go b/pkg/harness/exec/exec.go index 9155ee17..32dd5b4d 100644 --- a/pkg/harness/exec/exec.go +++ b/pkg/harness/exec/exec.go @@ -11,7 +11,7 @@ import ( "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/klog/v2" - "github.com/pluralsh/deployment-operator/pkg/harness/stackrun" + v1 "github.com/pluralsh/deployment-operator/pkg/harness/stackrun/v1" "github.com/pluralsh/deployment-operator/pkg/log" ) @@ -38,7 +38,7 @@ func (in *executable) Run(ctx context.Context) error { cmd.Dir = in.workingDirectory } - if err := in.runLifecycleFunction(stackrun.LifecyclePreStart); err != nil { + if err := in.runLifecycleFunction(v1.LifecyclePreStart); err != nil { return err } @@ -47,7 +47,7 @@ func (in *executable) Run(ctx context.Context) error { return err } - return in.runLifecycleFunction(stackrun.LifecyclePostStart) + return in.runLifecycleFunction(v1.LifecyclePostStart) } func (in *executable) RunWithOutput(ctx context.Context) ([]byte, error) { @@ -96,7 +96,7 @@ func (in *executable) close(w io.WriteCloser) { } } -func (in *executable) runLifecycleFunction(lifecycle stackrun.Lifecycle) error { +func (in *executable) runLifecycleFunction(lifecycle v1.Lifecycle) error { if fn, exists := in.hookFunctions[lifecycle]; exists { return fn() } @@ -109,7 +109,7 @@ func NewExecutable(command string, options ...Option) Executable { command: command, args: make([]string, 0), env: make([]string, 0), - hookFunctions: make(map[stackrun.Lifecycle]stackrun.HookFunction), + hookFunctions: make(map[v1.Lifecycle]v1.HookFunction), } for _, o := range options { diff --git a/pkg/harness/exec/exec_options.go b/pkg/harness/exec/exec_options.go index 0296e6b6..245792a3 100644 --- a/pkg/harness/exec/exec_options.go +++ b/pkg/harness/exec/exec_options.go @@ -3,7 +3,7 @@ package exec import ( "io" - "github.com/pluralsh/deployment-operator/pkg/harness/stackrun" + v1 "github.com/pluralsh/deployment-operator/pkg/harness/stackrun/v1" ) func WithDir(workingDirectory string) Option { @@ -36,7 +36,7 @@ func WithID(id string) Option { } } -func WithHook(lifecycle stackrun.Lifecycle, fn stackrun.HookFunction) Option { +func WithHook(lifecycle v1.Lifecycle, fn v1.HookFunction) Option { return func(e *executable) { e.hookFunctions[lifecycle] = fn } diff --git a/pkg/harness/exec/exec_types.go b/pkg/harness/exec/exec_types.go index e4da286c..f7df57bf 100644 --- a/pkg/harness/exec/exec_types.go +++ b/pkg/harness/exec/exec_types.go @@ -4,7 +4,7 @@ import ( "context" "io" - "github.com/pluralsh/deployment-operator/pkg/harness/stackrun" + v1 "github.com/pluralsh/deployment-operator/pkg/harness/stackrun/v1" ) type Executable interface { @@ -41,7 +41,7 @@ type executable struct { logSink io.WriteCloser // hookFunctions ... - hookFunctions map[stackrun.Lifecycle]stackrun.HookFunction + hookFunctions map[v1.Lifecycle]v1.HookFunction } type Option func(*executable) diff --git a/pkg/harness/stackrun/helpers.go b/pkg/harness/stackrun/helpers.go new file mode 100644 index 00000000..8a840a6e --- /dev/null +++ b/pkg/harness/stackrun/helpers.go @@ -0,0 +1,65 @@ +package stackrun + +import ( + "context" + "time" + + gqlclient "github.com/pluralsh/console-client-go" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/klog/v2" + + console "github.com/pluralsh/deployment-operator/pkg/client" +) + +func MarkStackRun(client console.Client, id string, status gqlclient.StackStatus) error { + return client.UpdateStackRun(id, gqlclient.StackRunAttributes{ + Status: status, + }) +} + +func MarkStackRunWithRetry(client console.Client, id string, status gqlclient.StackStatus, interval time.Duration) { + // Ignore error since we never return it from the condition function. + _ = wait.PollUntilContextCancel(context.Background(), interval, true, func(ctx context.Context) (done bool, err error) { + err = MarkStackRun(client, id, status) + if err != nil { + klog.Errorf("stack run update failed: %v", err) + return false, nil + } + + return true, nil + }) +} + +func StartStackRun(client console.Client, id string) error { + return MarkStackRun(client, id, gqlclient.StackStatusRunning) +} + +func CompleteStackRun(client console.Client, id string) error { + return MarkStackRun(client, id, gqlclient.StackStatusSuccessful) +} + +func CancelStackRun(client console.Client, id string) error { + return MarkStackRun(client, id, gqlclient.StackStatusCancelled) +} + +func FailStackRun(client console.Client, id string) error { + return MarkStackRun(client, id, gqlclient.StackStatusFailed) +} + +func MarkStackRunStep(client console.Client, id string, status gqlclient.StepStatus) error { + return client.UpdateStackRunStep(id, gqlclient.RunStepAttributes{ + Status: status, + }) +} + +func StartStackRunStep(client console.Client, id string) error { + return MarkStackRunStep(client, id, gqlclient.StepStatusRunning) +} + +func CompleteStackRunStep(client console.Client, id string) error { + return MarkStackRunStep(client, id, gqlclient.StepStatusSuccessful) +} + +func FailStackRunStep(client console.Client, id string) error { + return MarkStackRunStep(client, id, gqlclient.StepStatusFailed) +} diff --git a/pkg/harness/stackrun/types.go b/pkg/harness/stackrun/v1/types.go similarity index 95% rename from pkg/harness/stackrun/types.go rename to pkg/harness/stackrun/v1/types.go index 372f1cab..a546dd5e 100644 --- a/pkg/harness/stackrun/types.go +++ b/pkg/harness/stackrun/v1/types.go @@ -1,4 +1,4 @@ -package stackrun +package v1 import ( "fmt" @@ -14,6 +14,7 @@ type StackRun struct { Steps []*gqlclient.RunStepFragment Files []*gqlclient.StackFileFragment Environment []*gqlclient.StackEnvironmentFragment + ExecWorkDir *string Approval bool ApprovedAt *string } @@ -29,6 +30,7 @@ func (in *StackRun) FromStackRunBaseFragment(fragment *gqlclient.StackRunBaseFra Environment: fragment.Environment, Approval: fragment.Approval != nil && *fragment.Approval, ApprovedAt: fragment.ApprovedAt, + ExecWorkDir: fragment.Workdir, } } diff --git a/pkg/harness/tool/terraform/modifier.go b/pkg/harness/tool/terraform/modifier.go index d98ce2f3..ce46a1a5 100644 --- a/pkg/harness/tool/terraform/modifier.go +++ b/pkg/harness/tool/terraform/modifier.go @@ -2,6 +2,7 @@ package terraform import ( "fmt" + "path" "github.com/pluralsh/polly/algorithms" @@ -33,13 +34,17 @@ func NewPlanModifier(planFileName string) *PlanModifier { } func (in *ApplyModifier) Args(args []string) []string { - if !helpers.IsExists(in.planFileName) { + if !helpers.IsExists(path.Join(in.dir, in.planFileName)) || + // This is to avoid doubling plan file arg if API already adds it + algorithms.Index(args, func(a string) bool { + return a == in.planFileName + }) >= 0 { return args } - return append(args, fmt.Sprintf(in.planFileName)) + return append(args, in.planFileName) } -func NewApplyModifier(planFileName string) *ApplyModifier { - return &ApplyModifier{planFileName} +func NewApplyModifier(dir, planFileName string) *ApplyModifier { + return &ApplyModifier{planFileName, dir} } diff --git a/pkg/harness/tool/terraform/modifier_types.go b/pkg/harness/tool/terraform/modifier_types.go index 5adfa2f1..68960579 100644 --- a/pkg/harness/tool/terraform/modifier_types.go +++ b/pkg/harness/tool/terraform/modifier_types.go @@ -13,4 +13,7 @@ type PlanModifier struct { type ApplyModifier struct { // planFileName planFileName string + + // dir + dir string } diff --git a/pkg/harness/tool/terraform/terraform.go b/pkg/harness/tool/terraform/terraform.go index ef997765..9f40ca6f 100644 --- a/pkg/harness/tool/terraform/terraform.go +++ b/pkg/harness/tool/terraform/terraform.go @@ -73,7 +73,7 @@ func (in *Terraform) Modifier(stage console.StepStage) v1.Modifier { case console.StepStagePlan: return NewPlanModifier(in.planFileName) case console.StepStageApply: - return NewApplyModifier(in.planFileName) + return NewApplyModifier(in.dir, in.planFileName) } return v1.NewProxyModifier() diff --git a/pkg/test/mocks/Client_mock.go b/pkg/test/mocks/Client_mock.go index 35a96ca2..eb2be1d1 100644 --- a/pkg/test/mocks/Client_mock.go +++ b/pkg/test/mocks/Client_mock.go @@ -4,11 +4,11 @@ package mocks import ( gqlclient "github.com/pluralsh/console-client-go" - mock "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/mock" - stackrun "github.com/pluralsh/deployment-operator/pkg/harness/stackrun" + stackrun "github.com/pluralsh/deployment-operator/pkg/harness/stackrun/v1" - v1alpha1 "github.com/pluralsh/deployment-operator/api/v1alpha1" + "github.com/pluralsh/deployment-operator/api/v1alpha1" ) // ClientMock is an autogenerated mock type for the Client type