From fd47d05975a0a30a16c37afc106c64f7f5690a3d Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 16 Aug 2022 17:36:22 +0300 Subject: [PATCH 01/14] fix: Allow terraform provisions to be gracefully cancelled This change allows terraform commands to be gracefully cancelled on Unix-like platforms by signaling interrupt on provision cancellation. One implementation detail to note is that we do not necessarily kill a running terraform command immediately even if the stream is closed. The reason for this is to allow for graceful cancellation even in such an event. Currently the timeout is set to one minute, which was chosen arbitrarily. Also note that the `force` flag was added to `provisioner.proto` and is handled in the `terraform` package, however, it is not used by the `provisionerd/runner`. The reason is that the `runner` would need to be refactored. Currently the "force stop" context is used as a base for streams, and this would mean that in a force stop event, the stream would close and we can't send out our force stop cancellation message. Related: #2683 The above issue may be partially of fully fixed by this change. --- provisioner/terraform/executor.go | 95 +++++++++++------ provisioner/terraform/executor_unix.go | 17 ++++ provisioner/terraform/executor_windows.go | 18 ++++ provisioner/terraform/provision.go | 66 ++++++++---- provisionersdk/proto/provisioner.pb.go | 107 +++++++++++--------- provisionersdk/proto/provisioner.proto | 4 +- provisionersdk/proto/provisioner_drpc.pb.go | 2 +- 7 files changed, 213 insertions(+), 96 deletions(-) create mode 100644 provisioner/terraform/executor_unix.go create mode 100644 provisioner/terraform/executor_windows.go diff --git a/provisioner/terraform/executor.go b/provisioner/terraform/executor.go index 4eb432252a849..abd5bce94326b 100644 --- a/provisioner/terraform/executor.go +++ b/provisioner/terraform/executor.go @@ -41,7 +41,7 @@ func (e executor) basicEnv() []string { return env } -func (e executor) execWriteOutput(ctx context.Context, args, env []string, stdOutWriter, stdErrWriter io.WriteCloser) (err error) { +func (e executor) execWriteOutput(ctx, killCtx context.Context, args, env []string, stdOutWriter, stdErrWriter io.WriteCloser) (err error) { defer func() { closeErr := stdOutWriter.Close() if err == nil && closeErr != nil { @@ -52,8 +52,12 @@ func (e executor) execWriteOutput(ctx context.Context, args, env []string, stdOu err = closeErr } }() + if ctx.Err() != nil { + return ctx.Err() + } + // #nosec - cmd := exec.CommandContext(ctx, e.binaryPath, args...) + cmd := exec.CommandContext(killCtx, e.binaryPath, args...) cmd.Dir = e.workdir cmd.Env = env @@ -63,19 +67,36 @@ func (e executor) execWriteOutput(ctx context.Context, args, env []string, stdOu cmd.Stdout = syncWriter{mut, stdOutWriter} cmd.Stderr = syncWriter{mut, stdErrWriter} - return cmd.Run() + err = cmd.Start() + if err != nil { + return err + } + interruptCommandOnCancel(ctx, killCtx, cmd) + + return cmd.Wait() } -func (e executor) execParseJSON(ctx context.Context, args, env []string, v interface{}) error { +func (e executor) execParseJSON(ctx, killCtx context.Context, args, env []string, v interface{}) error { + if ctx.Err() != nil { + return ctx.Err() + } + // #nosec - cmd := exec.CommandContext(ctx, e.binaryPath, args...) + cmd := exec.CommandContext(killCtx, e.binaryPath, args...) cmd.Dir = e.workdir cmd.Env = env out := &bytes.Buffer{} stdErr := &bytes.Buffer{} cmd.Stdout = out cmd.Stderr = stdErr - err := cmd.Run() + + err := cmd.Start() + if err != nil { + return err + } + interruptCommandOnCancel(ctx, killCtx, cmd) + + err = cmd.Wait() if err != nil { errString, _ := io.ReadAll(stdErr) return xerrors.Errorf("%s: %w", errString, err) @@ -109,6 +130,10 @@ func (e executor) version(ctx context.Context) (*version.Version, error) { } func versionFromBinaryPath(ctx context.Context, binaryPath string) (*version.Version, error) { + if ctx.Err() != nil { + return nil, ctx.Err() + } + // #nosec cmd := exec.CommandContext(ctx, binaryPath, "version", "-json") out, err := cmd.Output() @@ -130,7 +155,7 @@ func versionFromBinaryPath(ctx context.Context, binaryPath string) (*version.Ver return version.NewVersion(vj.Version) } -func (e executor) init(ctx context.Context, logr logger) error { +func (e executor) init(ctx, killCtx context.Context, logr logger) error { outWriter, doneOut := logWriter(logr, proto.LogLevel_DEBUG) errWriter, doneErr := logWriter(logr, proto.LogLevel_ERROR) defer func() { @@ -156,11 +181,11 @@ func (e executor) init(ctx context.Context, logr logger) error { defer e.initMu.Unlock() } - return e.execWriteOutput(ctx, args, e.basicEnv(), outWriter, errWriter) + return e.execWriteOutput(ctx, killCtx, args, e.basicEnv(), outWriter, errWriter) } // revive:disable-next-line:flag-parameter -func (e executor) plan(ctx context.Context, env, vars []string, logr logger, destroy bool) (*proto.Provision_Response, error) { +func (e executor) plan(ctx, killCtx context.Context, env, vars []string, logr logger, destroy bool) (*proto.Provision_Response, error) { planfilePath := filepath.Join(e.workdir, "terraform.tfplan") args := []string{ "plan", @@ -184,11 +209,11 @@ func (e executor) plan(ctx context.Context, env, vars []string, logr logger, des <-doneErr }() - err := e.execWriteOutput(ctx, args, env, outWriter, errWriter) + err := e.execWriteOutput(ctx, killCtx, args, env, outWriter, errWriter) if err != nil { return nil, xerrors.Errorf("terraform plan: %w", err) } - resources, err := e.planResources(ctx, planfilePath) + resources, err := e.planResources(ctx, killCtx, planfilePath) if err != nil { return nil, err } @@ -201,40 +226,52 @@ func (e executor) plan(ctx context.Context, env, vars []string, logr logger, des }, nil } -func (e executor) planResources(ctx context.Context, planfilePath string) ([]*proto.Resource, error) { - plan, err := e.showPlan(ctx, planfilePath) +func (e executor) planResources(ctx, killCtx context.Context, planfilePath string) ([]*proto.Resource, error) { + plan, err := e.showPlan(ctx, killCtx, planfilePath) if err != nil { return nil, xerrors.Errorf("show terraform plan file: %w", err) } - rawGraph, err := e.graph(ctx) + rawGraph, err := e.graph(ctx, killCtx) if err != nil { return nil, xerrors.Errorf("graph: %w", err) } return ConvertResources(plan.PlannedValues.RootModule, rawGraph) } -func (e executor) showPlan(ctx context.Context, planfilePath string) (*tfjson.Plan, error) { +func (e executor) showPlan(ctx, killCtx context.Context, planfilePath string) (*tfjson.Plan, error) { args := []string{"show", "-json", "-no-color", planfilePath} p := new(tfjson.Plan) - err := e.execParseJSON(ctx, args, e.basicEnv(), p) + err := e.execParseJSON(ctx, killCtx, args, e.basicEnv(), p) return p, err } -func (e executor) graph(ctx context.Context) (string, error) { - // #nosec - cmd := exec.CommandContext(ctx, e.binaryPath, "graph") +func (e executor) graph(ctx, killCtx context.Context) (string, error) { + if ctx.Err() != nil { + return "", ctx.Err() + } + + var out bytes.Buffer + cmd := exec.CommandContext(killCtx, e.binaryPath, "graph") // #nosec + cmd.Stdout = &out cmd.Dir = e.workdir cmd.Env = e.basicEnv() - out, err := cmd.Output() + + err := cmd.Start() + if err != nil { + return "", err + } + interruptCommandOnCancel(ctx, killCtx, cmd) + + err = cmd.Wait() if err != nil { return "", xerrors.Errorf("graph: %w", err) } - return string(out), nil + return out.String(), nil } // revive:disable-next-line:flag-parameter -func (e executor) apply(ctx context.Context, env, vars []string, logr logger, destroy bool, +func (e executor) apply(ctx, killCtx context.Context, env, vars []string, logr logger, destroy bool, ) (*proto.Provision_Response, error) { args := []string{ "apply", @@ -258,11 +295,11 @@ func (e executor) apply(ctx context.Context, env, vars []string, logr logger, de <-doneErr }() - err := e.execWriteOutput(ctx, args, env, outWriter, errWriter) + err := e.execWriteOutput(ctx, killCtx, args, env, outWriter, errWriter) if err != nil { return nil, xerrors.Errorf("terraform apply: %w", err) } - resources, err := e.stateResources(ctx) + resources, err := e.stateResources(ctx, killCtx) if err != nil { return nil, err } @@ -281,12 +318,12 @@ func (e executor) apply(ctx context.Context, env, vars []string, logr logger, de }, nil } -func (e executor) stateResources(ctx context.Context) ([]*proto.Resource, error) { - state, err := e.state(ctx) +func (e executor) stateResources(ctx, killCtx context.Context) ([]*proto.Resource, error) { + state, err := e.state(ctx, killCtx) if err != nil { return nil, err } - rawGraph, err := e.graph(ctx) + rawGraph, err := e.graph(ctx, killCtx) if err != nil { return nil, xerrors.Errorf("get terraform graph: %w", err) } @@ -300,10 +337,10 @@ func (e executor) stateResources(ctx context.Context) ([]*proto.Resource, error) return resources, nil } -func (e executor) state(ctx context.Context) (*tfjson.State, error) { +func (e executor) state(ctx, killCtx context.Context) (*tfjson.State, error) { args := []string{"show", "-json"} state := &tfjson.State{} - err := e.execParseJSON(ctx, args, e.basicEnv(), state) + err := e.execParseJSON(ctx, killCtx, args, e.basicEnv(), state) if err != nil { return nil, xerrors.Errorf("terraform show state: %w", err) } diff --git a/provisioner/terraform/executor_unix.go b/provisioner/terraform/executor_unix.go new file mode 100644 index 0000000000000..829bedcb2940e --- /dev/null +++ b/provisioner/terraform/executor_unix.go @@ -0,0 +1,17 @@ +package terraform + +import ( + "context" + "os" + "os/exec" +) + +func interruptCommandOnCancel(ctx, killCtx context.Context, cmd *exec.Cmd) { + go func() { + select { + case <-ctx.Done(): + _ = cmd.Process.Signal(os.Interrupt) + case <-killCtx.Done(): + } + }() +} diff --git a/provisioner/terraform/executor_windows.go b/provisioner/terraform/executor_windows.go new file mode 100644 index 0000000000000..c97285a0f5a2a --- /dev/null +++ b/provisioner/terraform/executor_windows.go @@ -0,0 +1,18 @@ +package terraform + +import ( + "context" + "os" + "os/exec" +) + +func interruptCommandOnCancel(ctx, killCtx context.Context, cmd *exec.Cmd) { + go func() { + select { + case <-ctx.Done(): + // On Windows we can't sent an interrupt, so we just kill the process. + _ = cmd.Process.Kill() + case <-killCtx.Done(): + } + }() +} diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index 65e972ddf27b3..6f1047fbb60c9 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "strings" + "time" "golang.org/x/xerrors" @@ -14,11 +15,7 @@ import ( ) // Provision executes `terraform apply` or `terraform plan` for dry runs. -func (t *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { - logr := streamLogger{stream: stream} - shutdown, shutdownFunc := context.WithCancel(stream.Context()) - defer shutdownFunc() - +func (s *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { request, err := stream.Recv() if err != nil { return err @@ -30,26 +27,61 @@ func (t *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { if request.GetStart() == nil { return nil } + + // Create a context for graceful cancellation bound to the stream + // context. This ensures that we will perform graceful cancellation + // even on connection loss. + ctx, cancel := context.WithCancel(stream.Context()) + defer cancel() + + // Create a separate context for forcefull cancellation not tied to + // the stream so that we can control when to terminate the process. + killCtx, kill := context.WithCancel(context.Background()) + defer kill() + + // Ensure processes are eventually cleaned up on graceful + // cancellation or disconnect. + go func() { + <-stream.Context().Done() + + // TODO(mafredri): We should track this provision request as + // part of graceful server shutdown procedure. Waiting on a + // process here should delay provisioner/coder shutdown. + select { + case <-time.After(time.Minute): + kill() + case <-killCtx.Done(): + } + }() + go func() { for { request, err := stream.Recv() if err != nil { return } - if request.GetCancel() == nil { - // This is only to process cancels! - continue + + rc := request.GetCancel() + switch { + case rc == nil: + case rc.GetForce(): + kill() + return + default: + cancel() + // We will continue waiting for forceful cancellation + // (or until the stream is closed). } - shutdownFunc() - return } }() + + logr := streamLogger{stream: stream} start := request.GetStart() if err != nil { return xerrors.Errorf("create new terraform executor: %w", err) } - e := t.executor(start.Directory) + e := s.executor(start.Directory) if err := e.checkMinVersion(stream.Context()); err != nil { return err } @@ -87,12 +119,12 @@ func (t *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { }) } - t.logger.Debug(shutdown, "running initialization") - err = e.init(stream.Context(), logr) + s.logger.Debug(ctx, "running initialization") + err = e.init(ctx, killCtx, logr) if err != nil { return xerrors.Errorf("initialize terraform: %w", err) } - t.logger.Debug(shutdown, "ran initialization") + s.logger.Debug(ctx, "ran initialization") env, err := provisionEnv(start) if err != nil { @@ -104,15 +136,15 @@ func (t *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { } var resp *proto.Provision_Response if start.DryRun { - resp, err = e.plan(shutdown, env, vars, logr, + resp, err = e.plan(ctx, killCtx, env, vars, logr, start.Metadata.WorkspaceTransition == proto.WorkspaceTransition_DESTROY) } else { - resp, err = e.apply(shutdown, env, vars, logr, + resp, err = e.apply(ctx, killCtx, env, vars, logr, start.Metadata.WorkspaceTransition == proto.WorkspaceTransition_DESTROY) } if err != nil { if start.DryRun { - if shutdown.Err() != nil { + if ctx.Err() != nil { return stream.Send(&proto.Provision_Response{ Type: &proto.Provision_Response_Complete{ Complete: &proto.Provision_Complete{ diff --git a/provisionersdk/proto/provisioner.pb.go b/provisionersdk/proto/provisioner.pb.go index ce1ba8403733f..801da271a909c 100644 --- a/provisionersdk/proto/provisioner.pb.go +++ b/provisionersdk/proto/provisioner.pb.go @@ -1,7 +1,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.26.0 -// protoc v3.20.0 +// protoc-gen-go v1.28.0 +// protoc v3.12.4 // source: provisionersdk/proto/provisioner.proto package proto @@ -1496,6 +1496,8 @@ type Provision_Cancel struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache unknownFields protoimpl.UnknownFields + + Force bool `protobuf:"varint,1,opt,name=force,proto3" json:"force,omitempty"` } func (x *Provision_Cancel) Reset() { @@ -1530,6 +1532,13 @@ func (*Provision_Cancel) Descriptor() ([]byte, []int) { return file_provisionersdk_proto_provisioner_proto_rawDescGZIP(), []int{11, 2} } +func (x *Provision_Cancel) GetForce() bool { + if x != nil { + return x.Force + } + return false +} + type Provision_Request struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -1901,7 +1910,7 @@ var file_provisionersdk_proto_provisioner_proto_rawDesc = []byte{ 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1b, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x61, 0x72, 0x73, 0x65, 0x2e, 0x43, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, 0x65, 0x48, 0x00, 0x52, 0x08, 0x63, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, 0x65, 0x42, 0x06, 0x0a, - 0x04, 0x74, 0x79, 0x70, 0x65, 0x22, 0xae, 0x07, 0x0a, 0x09, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, + 0x04, 0x74, 0x79, 0x70, 0x65, 0x22, 0xc4, 0x07, 0x0a, 0x09, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x1a, 0xd1, 0x02, 0x0a, 0x08, 0x4d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x12, 0x1b, 0x0a, 0x09, 0x63, 0x6f, 0x64, 0x65, 0x72, 0x5f, 0x75, 0x72, 0x6c, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08, 0x63, 0x6f, 0x64, 0x65, 0x72, 0x55, 0x72, 0x6c, 0x12, 0x53, 0x0a, @@ -1937,51 +1946,53 @@ var file_provisionersdk_proto_provisioner_proto_rawDesc = []byte{ 0x64, 0x61, 0x74, 0x61, 0x12, 0x14, 0x0a, 0x05, 0x73, 0x74, 0x61, 0x74, 0x65, 0x18, 0x04, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x05, 0x73, 0x74, 0x61, 0x74, 0x65, 0x12, 0x17, 0x0a, 0x07, 0x64, 0x72, 0x79, 0x5f, 0x72, 0x75, 0x6e, 0x18, 0x05, 0x20, 0x01, 0x28, 0x08, 0x52, 0x06, 0x64, 0x72, 0x79, - 0x52, 0x75, 0x6e, 0x1a, 0x08, 0x0a, 0x06, 0x43, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x1a, 0x80, 0x01, - 0x0a, 0x07, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x34, 0x0a, 0x05, 0x73, 0x74, 0x61, - 0x72, 0x74, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1c, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, - 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, - 0x2e, 0x53, 0x74, 0x61, 0x72, 0x74, 0x48, 0x00, 0x52, 0x05, 0x73, 0x74, 0x61, 0x72, 0x74, 0x12, - 0x37, 0x0a, 0x06, 0x63, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, - 0x1d, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x72, - 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, 0x43, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x48, 0x00, - 0x52, 0x06, 0x63, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x42, 0x06, 0x0a, 0x04, 0x74, 0x79, 0x70, 0x65, - 0x1a, 0x6b, 0x0a, 0x08, 0x43, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, 0x65, 0x12, 0x14, 0x0a, 0x05, - 0x73, 0x74, 0x61, 0x74, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x05, 0x73, 0x74, 0x61, - 0x74, 0x65, 0x12, 0x14, 0x0a, 0x05, 0x65, 0x72, 0x72, 0x6f, 0x72, 0x18, 0x02, 0x20, 0x01, 0x28, - 0x09, 0x52, 0x05, 0x65, 0x72, 0x72, 0x6f, 0x72, 0x12, 0x33, 0x0a, 0x09, 0x72, 0x65, 0x73, 0x6f, - 0x75, 0x72, 0x63, 0x65, 0x73, 0x18, 0x03, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x15, 0x2e, 0x70, 0x72, - 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, - 0x63, 0x65, 0x52, 0x09, 0x72, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x73, 0x1a, 0x77, 0x0a, - 0x08, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x24, 0x0a, 0x03, 0x6c, 0x6f, 0x67, - 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x10, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, - 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x4c, 0x6f, 0x67, 0x48, 0x00, 0x52, 0x03, 0x6c, 0x6f, 0x67, 0x12, - 0x3d, 0x0a, 0x08, 0x63, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, - 0x0b, 0x32, 0x1f, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, - 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, 0x43, 0x6f, 0x6d, 0x70, 0x6c, 0x65, - 0x74, 0x65, 0x48, 0x00, 0x52, 0x08, 0x63, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, 0x65, 0x42, 0x06, - 0x0a, 0x04, 0x74, 0x79, 0x70, 0x65, 0x2a, 0x3f, 0x0a, 0x08, 0x4c, 0x6f, 0x67, 0x4c, 0x65, 0x76, - 0x65, 0x6c, 0x12, 0x09, 0x0a, 0x05, 0x54, 0x52, 0x41, 0x43, 0x45, 0x10, 0x00, 0x12, 0x09, 0x0a, - 0x05, 0x44, 0x45, 0x42, 0x55, 0x47, 0x10, 0x01, 0x12, 0x08, 0x0a, 0x04, 0x49, 0x4e, 0x46, 0x4f, - 0x10, 0x02, 0x12, 0x08, 0x0a, 0x04, 0x57, 0x41, 0x52, 0x4e, 0x10, 0x03, 0x12, 0x09, 0x0a, 0x05, - 0x45, 0x52, 0x52, 0x4f, 0x52, 0x10, 0x04, 0x2a, 0x37, 0x0a, 0x13, 0x57, 0x6f, 0x72, 0x6b, 0x73, - 0x70, 0x61, 0x63, 0x65, 0x54, 0x72, 0x61, 0x6e, 0x73, 0x69, 0x74, 0x69, 0x6f, 0x6e, 0x12, 0x09, - 0x0a, 0x05, 0x53, 0x54, 0x41, 0x52, 0x54, 0x10, 0x00, 0x12, 0x08, 0x0a, 0x04, 0x53, 0x54, 0x4f, - 0x50, 0x10, 0x01, 0x12, 0x0b, 0x0a, 0x07, 0x44, 0x45, 0x53, 0x54, 0x52, 0x4f, 0x59, 0x10, 0x02, - 0x32, 0xa3, 0x01, 0x0a, 0x0b, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, - 0x12, 0x42, 0x0a, 0x05, 0x50, 0x61, 0x72, 0x73, 0x65, 0x12, 0x1a, 0x2e, 0x70, 0x72, 0x6f, 0x76, - 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x61, 0x72, 0x73, 0x65, 0x2e, 0x52, 0x65, - 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x1b, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, - 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x61, 0x72, 0x73, 0x65, 0x2e, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, - 0x73, 0x65, 0x30, 0x01, 0x12, 0x50, 0x0a, 0x09, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, - 0x6e, 0x12, 0x1e, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, - 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, - 0x74, 0x1a, 0x1f, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, - 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, - 0x73, 0x65, 0x28, 0x01, 0x30, 0x01, 0x42, 0x2d, 0x5a, 0x2b, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, - 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x63, 0x6f, 0x64, 0x65, 0x72, 0x2f, 0x63, 0x6f, 0x64, 0x65, 0x72, - 0x2f, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x73, 0x64, 0x6b, 0x2f, - 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x52, 0x75, 0x6e, 0x1a, 0x1e, 0x0a, 0x06, 0x43, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x12, 0x14, 0x0a, + 0x05, 0x66, 0x6f, 0x72, 0x63, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x08, 0x52, 0x05, 0x66, 0x6f, + 0x72, 0x63, 0x65, 0x1a, 0x80, 0x01, 0x0a, 0x07, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, + 0x34, 0x0a, 0x05, 0x73, 0x74, 0x61, 0x72, 0x74, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1c, + 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x72, 0x6f, + 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, 0x53, 0x74, 0x61, 0x72, 0x74, 0x48, 0x00, 0x52, 0x05, + 0x73, 0x74, 0x61, 0x72, 0x74, 0x12, 0x37, 0x0a, 0x06, 0x63, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x18, + 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1d, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, + 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, 0x43, 0x61, + 0x6e, 0x63, 0x65, 0x6c, 0x48, 0x00, 0x52, 0x06, 0x63, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x42, 0x06, + 0x0a, 0x04, 0x74, 0x79, 0x70, 0x65, 0x1a, 0x6b, 0x0a, 0x08, 0x43, 0x6f, 0x6d, 0x70, 0x6c, 0x65, + 0x74, 0x65, 0x12, 0x14, 0x0a, 0x05, 0x73, 0x74, 0x61, 0x74, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, + 0x0c, 0x52, 0x05, 0x73, 0x74, 0x61, 0x74, 0x65, 0x12, 0x14, 0x0a, 0x05, 0x65, 0x72, 0x72, 0x6f, + 0x72, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x65, 0x72, 0x72, 0x6f, 0x72, 0x12, 0x33, + 0x0a, 0x09, 0x72, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x73, 0x18, 0x03, 0x20, 0x03, 0x28, + 0x0b, 0x32, 0x15, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, + 0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x52, 0x09, 0x72, 0x65, 0x73, 0x6f, 0x75, 0x72, + 0x63, 0x65, 0x73, 0x1a, 0x77, 0x0a, 0x08, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, + 0x24, 0x0a, 0x03, 0x6c, 0x6f, 0x67, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x10, 0x2e, 0x70, + 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x4c, 0x6f, 0x67, 0x48, 0x00, + 0x52, 0x03, 0x6c, 0x6f, 0x67, 0x12, 0x3d, 0x0a, 0x08, 0x63, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, + 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1f, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, + 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, + 0x43, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, 0x65, 0x48, 0x00, 0x52, 0x08, 0x63, 0x6f, 0x6d, 0x70, + 0x6c, 0x65, 0x74, 0x65, 0x42, 0x06, 0x0a, 0x04, 0x74, 0x79, 0x70, 0x65, 0x2a, 0x3f, 0x0a, 0x08, + 0x4c, 0x6f, 0x67, 0x4c, 0x65, 0x76, 0x65, 0x6c, 0x12, 0x09, 0x0a, 0x05, 0x54, 0x52, 0x41, 0x43, + 0x45, 0x10, 0x00, 0x12, 0x09, 0x0a, 0x05, 0x44, 0x45, 0x42, 0x55, 0x47, 0x10, 0x01, 0x12, 0x08, + 0x0a, 0x04, 0x49, 0x4e, 0x46, 0x4f, 0x10, 0x02, 0x12, 0x08, 0x0a, 0x04, 0x57, 0x41, 0x52, 0x4e, + 0x10, 0x03, 0x12, 0x09, 0x0a, 0x05, 0x45, 0x52, 0x52, 0x4f, 0x52, 0x10, 0x04, 0x2a, 0x37, 0x0a, + 0x13, 0x57, 0x6f, 0x72, 0x6b, 0x73, 0x70, 0x61, 0x63, 0x65, 0x54, 0x72, 0x61, 0x6e, 0x73, 0x69, + 0x74, 0x69, 0x6f, 0x6e, 0x12, 0x09, 0x0a, 0x05, 0x53, 0x54, 0x41, 0x52, 0x54, 0x10, 0x00, 0x12, + 0x08, 0x0a, 0x04, 0x53, 0x54, 0x4f, 0x50, 0x10, 0x01, 0x12, 0x0b, 0x0a, 0x07, 0x44, 0x45, 0x53, + 0x54, 0x52, 0x4f, 0x59, 0x10, 0x02, 0x32, 0xa3, 0x01, 0x0a, 0x0b, 0x50, 0x72, 0x6f, 0x76, 0x69, + 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x12, 0x42, 0x0a, 0x05, 0x50, 0x61, 0x72, 0x73, 0x65, 0x12, + 0x1a, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x61, + 0x72, 0x73, 0x65, 0x2e, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x1b, 0x2e, 0x70, 0x72, + 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x61, 0x72, 0x73, 0x65, 0x2e, + 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x30, 0x01, 0x12, 0x50, 0x0a, 0x09, 0x50, 0x72, + 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x12, 0x1e, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, + 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, + 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x1f, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, + 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, + 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x28, 0x01, 0x30, 0x01, 0x42, 0x2d, 0x5a, 0x2b, + 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x63, 0x6f, 0x64, 0x65, 0x72, + 0x2f, 0x63, 0x6f, 0x64, 0x65, 0x72, 0x2f, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, + 0x65, 0x72, 0x73, 0x64, 0x6b, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x06, 0x70, 0x72, 0x6f, + 0x74, 0x6f, 0x33, } var ( diff --git a/provisionersdk/proto/provisioner.proto b/provisionersdk/proto/provisioner.proto index ab521256853c0..c1fe5ce13ee57 100644 --- a/provisionersdk/proto/provisioner.proto +++ b/provisionersdk/proto/provisioner.proto @@ -151,7 +151,9 @@ message Provision { bytes state = 4; bool dry_run = 5; } - message Cancel {} + message Cancel { + bool force = 1; + } message Request { oneof type { Start start = 1; diff --git a/provisionersdk/proto/provisioner_drpc.pb.go b/provisionersdk/proto/provisioner_drpc.pb.go index c990f6f645b7f..de7957fcf2d68 100644 --- a/provisionersdk/proto/provisioner_drpc.pb.go +++ b/provisionersdk/proto/provisioner_drpc.pb.go @@ -1,5 +1,5 @@ // Code generated by protoc-gen-go-drpc. DO NOT EDIT. -// protoc-gen-go-drpc version: v0.0.26 +// protoc-gen-go-drpc version: v0.0.32 // source: provisionersdk/proto/provisioner.proto package proto From 72f723e7a21ef02f3413e55750edda15e479d51d Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 16 Aug 2022 17:58:07 +0300 Subject: [PATCH 02/14] Cleanup --- provisioner/terraform/provision.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index 6f1047fbb60c9..efcb68b201939 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -65,6 +65,9 @@ func (s *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { switch { case rc == nil: case rc.GetForce(): + // Likely not needed, but this ensures + // cancel happens before kill. + cancel() kill() return default: @@ -78,20 +81,17 @@ func (s *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { logr := streamLogger{stream: stream} start := request.GetStart() - if err != nil { - return xerrors.Errorf("create new terraform executor: %w", err) - } e := s.executor(start.Directory) - if err := e.checkMinVersion(stream.Context()); err != nil { + if err = e.checkMinVersion(stream.Context()); err != nil { return err } - if err := logTerraformEnvVars(logr); err != nil { + if err = logTerraformEnvVars(logr); err != nil { return err } statefilePath := filepath.Join(start.Directory, "terraform.tfstate") if len(start.State) > 0 { - err := os.WriteFile(statefilePath, start.State, 0600) + err = os.WriteFile(statefilePath, start.State, 0o600) if err != nil { return xerrors.Errorf("write statefile %q: %w", statefilePath, err) } From 00c774630105e2b956a3614bb7ca7a97f938b92e Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 16 Aug 2022 17:58:37 +0300 Subject: [PATCH 03/14] Revert protoc version bumps --- provisionersdk/proto/provisioner.pb.go | 4 ++-- provisionersdk/proto/provisioner_drpc.pb.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/provisionersdk/proto/provisioner.pb.go b/provisionersdk/proto/provisioner.pb.go index 801da271a909c..de5cd470fa6ce 100644 --- a/provisionersdk/proto/provisioner.pb.go +++ b/provisionersdk/proto/provisioner.pb.go @@ -1,7 +1,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.28.0 -// protoc v3.12.4 +// protoc-gen-go v1.26.0 +// protoc v3.20.0 // source: provisionersdk/proto/provisioner.proto package proto diff --git a/provisionersdk/proto/provisioner_drpc.pb.go b/provisionersdk/proto/provisioner_drpc.pb.go index de7957fcf2d68..c990f6f645b7f 100644 --- a/provisionersdk/proto/provisioner_drpc.pb.go +++ b/provisionersdk/proto/provisioner_drpc.pb.go @@ -1,5 +1,5 @@ // Code generated by protoc-gen-go-drpc. DO NOT EDIT. -// protoc-gen-go-drpc version: v0.0.32 +// protoc-gen-go-drpc version: v0.0.26 // source: provisionersdk/proto/provisioner.proto package proto From 7df45e12e5ca819c0385a5c79e457bfc057411d0 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 16 Aug 2022 18:12:16 +0300 Subject: [PATCH 04/14] fix: Remove unused import from Windows --- provisioner/terraform/executor_windows.go | 1 - 1 file changed, 1 deletion(-) diff --git a/provisioner/terraform/executor_windows.go b/provisioner/terraform/executor_windows.go index c97285a0f5a2a..7b85a6779b087 100644 --- a/provisioner/terraform/executor_windows.go +++ b/provisioner/terraform/executor_windows.go @@ -2,7 +2,6 @@ package terraform import ( "context" - "os" "os/exec" ) From 211b7f9f179671878611a2d0c15300b632135c16 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 16 Aug 2022 18:17:05 +0300 Subject: [PATCH 05/14] fix: Add build tags --- provisioner/terraform/executor_unix.go | 2 ++ provisioner/terraform/executor_windows.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/provisioner/terraform/executor_unix.go b/provisioner/terraform/executor_unix.go index 829bedcb2940e..bbd6d78af1338 100644 --- a/provisioner/terraform/executor_unix.go +++ b/provisioner/terraform/executor_unix.go @@ -1,3 +1,5 @@ +//go:build !windows + package terraform import ( diff --git a/provisioner/terraform/executor_windows.go b/provisioner/terraform/executor_windows.go index 7b85a6779b087..fe3788ef73dfa 100644 --- a/provisioner/terraform/executor_windows.go +++ b/provisioner/terraform/executor_windows.go @@ -1,3 +1,5 @@ +//go:build windows + package terraform import ( From 3121f6ac57360f6712967c5f5920bc1b72789f2a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 16 Aug 2022 19:58:13 +0300 Subject: [PATCH 06/14] fix: Amend PR comments --- provisioner/terraform/executor.go | 17 ++++++++++++++ provisioner/terraform/executor_unix.go | 19 ---------------- provisioner/terraform/executor_windows.go | 19 ---------------- provisioner/terraform/provision.go | 27 ++++++++++------------- 4 files changed, 29 insertions(+), 53 deletions(-) delete mode 100644 provisioner/terraform/executor_unix.go delete mode 100644 provisioner/terraform/executor_windows.go diff --git a/provisioner/terraform/executor.go b/provisioner/terraform/executor.go index abd5bce94326b..46ba26c63bc67 100644 --- a/provisioner/terraform/executor.go +++ b/provisioner/terraform/executor.go @@ -347,6 +347,23 @@ func (e executor) state(ctx, killCtx context.Context) (*tfjson.State, error) { return state, nil } +func interruptCommandOnCancel(ctx, killCtx context.Context, cmd *exec.Cmd) { + go func() { + select { + case <-ctx.Done(): + switch runtime.GOOS { + case "windows": + // Interrupts aren't supported by Windows. + _ = cmd.Process.Kill() + default: + _ = cmd.Process.Signal(os.Interrupt) + } + + case <-killCtx.Done(): + } + }() +} + type logger interface { Log(*proto.Log) error } diff --git a/provisioner/terraform/executor_unix.go b/provisioner/terraform/executor_unix.go deleted file mode 100644 index bbd6d78af1338..0000000000000 --- a/provisioner/terraform/executor_unix.go +++ /dev/null @@ -1,19 +0,0 @@ -//go:build !windows - -package terraform - -import ( - "context" - "os" - "os/exec" -) - -func interruptCommandOnCancel(ctx, killCtx context.Context, cmd *exec.Cmd) { - go func() { - select { - case <-ctx.Done(): - _ = cmd.Process.Signal(os.Interrupt) - case <-killCtx.Done(): - } - }() -} diff --git a/provisioner/terraform/executor_windows.go b/provisioner/terraform/executor_windows.go deleted file mode 100644 index fe3788ef73dfa..0000000000000 --- a/provisioner/terraform/executor_windows.go +++ /dev/null @@ -1,19 +0,0 @@ -//go:build windows - -package terraform - -import ( - "context" - "os/exec" -) - -func interruptCommandOnCancel(ctx, killCtx context.Context, cmd *exec.Cmd) { - go func() { - select { - case <-ctx.Done(): - // On Windows we can't sent an interrupt, so we just kill the process. - _ = cmd.Process.Kill() - case <-killCtx.Done(): - } - }() -} diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index efcb68b201939..82c8bfc6a7475 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -14,6 +14,12 @@ import ( "github.com/coder/coder/provisionersdk/proto" ) +const ( + // Define how long we will wait for Terraform to exit cleanly + // after receiving an interrupt (if the provision was stopped). + terraformCancelTimeout = 5 * time.Minute +) + // Provision executes `terraform apply` or `terraform plan` for dry runs. func (s *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { request, err := stream.Recv() @@ -48,7 +54,7 @@ func (s *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { // part of graceful server shutdown procedure. Waiting on a // process here should delay provisioner/coder shutdown. select { - case <-time.After(time.Minute): + case <-time.After(terraformCancelTimeout): kill() case <-killCtx.Done(): } @@ -60,21 +66,12 @@ func (s *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { if err != nil { return } - - rc := request.GetCancel() - switch { - case rc == nil: - case rc.GetForce(): - // Likely not needed, but this ensures - // cancel happens before kill. - cancel() - kill() - return - default: - cancel() - // We will continue waiting for forceful cancellation - // (or until the stream is closed). + if request.GetCancel() == nil { + // We only process cancellation requests here. + continue } + cancel() + return } }() From 27fe6f0f84cf551237128c3ae0a919154c288712 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 16 Aug 2022 19:58:26 +0300 Subject: [PATCH 07/14] chore: Revert proto changes --- provisionersdk/proto/provisioner.pb.go | 103 +++++++++++-------------- provisionersdk/proto/provisioner.proto | 1 - 2 files changed, 46 insertions(+), 58 deletions(-) diff --git a/provisionersdk/proto/provisioner.pb.go b/provisionersdk/proto/provisioner.pb.go index de5cd470fa6ce..ce1ba8403733f 100644 --- a/provisionersdk/proto/provisioner.pb.go +++ b/provisionersdk/proto/provisioner.pb.go @@ -1496,8 +1496,6 @@ type Provision_Cancel struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache unknownFields protoimpl.UnknownFields - - Force bool `protobuf:"varint,1,opt,name=force,proto3" json:"force,omitempty"` } func (x *Provision_Cancel) Reset() { @@ -1532,13 +1530,6 @@ func (*Provision_Cancel) Descriptor() ([]byte, []int) { return file_provisionersdk_proto_provisioner_proto_rawDescGZIP(), []int{11, 2} } -func (x *Provision_Cancel) GetForce() bool { - if x != nil { - return x.Force - } - return false -} - type Provision_Request struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -1910,7 +1901,7 @@ var file_provisionersdk_proto_provisioner_proto_rawDesc = []byte{ 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1b, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x61, 0x72, 0x73, 0x65, 0x2e, 0x43, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, 0x65, 0x48, 0x00, 0x52, 0x08, 0x63, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, 0x65, 0x42, 0x06, 0x0a, - 0x04, 0x74, 0x79, 0x70, 0x65, 0x22, 0xc4, 0x07, 0x0a, 0x09, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, + 0x04, 0x74, 0x79, 0x70, 0x65, 0x22, 0xae, 0x07, 0x0a, 0x09, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x1a, 0xd1, 0x02, 0x0a, 0x08, 0x4d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x12, 0x1b, 0x0a, 0x09, 0x63, 0x6f, 0x64, 0x65, 0x72, 0x5f, 0x75, 0x72, 0x6c, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08, 0x63, 0x6f, 0x64, 0x65, 0x72, 0x55, 0x72, 0x6c, 0x12, 0x53, 0x0a, @@ -1946,53 +1937,51 @@ var file_provisionersdk_proto_provisioner_proto_rawDesc = []byte{ 0x64, 0x61, 0x74, 0x61, 0x12, 0x14, 0x0a, 0x05, 0x73, 0x74, 0x61, 0x74, 0x65, 0x18, 0x04, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x05, 0x73, 0x74, 0x61, 0x74, 0x65, 0x12, 0x17, 0x0a, 0x07, 0x64, 0x72, 0x79, 0x5f, 0x72, 0x75, 0x6e, 0x18, 0x05, 0x20, 0x01, 0x28, 0x08, 0x52, 0x06, 0x64, 0x72, 0x79, - 0x52, 0x75, 0x6e, 0x1a, 0x1e, 0x0a, 0x06, 0x43, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x12, 0x14, 0x0a, - 0x05, 0x66, 0x6f, 0x72, 0x63, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x08, 0x52, 0x05, 0x66, 0x6f, - 0x72, 0x63, 0x65, 0x1a, 0x80, 0x01, 0x0a, 0x07, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, - 0x34, 0x0a, 0x05, 0x73, 0x74, 0x61, 0x72, 0x74, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1c, - 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x72, 0x6f, - 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, 0x53, 0x74, 0x61, 0x72, 0x74, 0x48, 0x00, 0x52, 0x05, - 0x73, 0x74, 0x61, 0x72, 0x74, 0x12, 0x37, 0x0a, 0x06, 0x63, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x18, - 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1d, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, - 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, 0x43, 0x61, - 0x6e, 0x63, 0x65, 0x6c, 0x48, 0x00, 0x52, 0x06, 0x63, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x42, 0x06, - 0x0a, 0x04, 0x74, 0x79, 0x70, 0x65, 0x1a, 0x6b, 0x0a, 0x08, 0x43, 0x6f, 0x6d, 0x70, 0x6c, 0x65, - 0x74, 0x65, 0x12, 0x14, 0x0a, 0x05, 0x73, 0x74, 0x61, 0x74, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, - 0x0c, 0x52, 0x05, 0x73, 0x74, 0x61, 0x74, 0x65, 0x12, 0x14, 0x0a, 0x05, 0x65, 0x72, 0x72, 0x6f, - 0x72, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x65, 0x72, 0x72, 0x6f, 0x72, 0x12, 0x33, - 0x0a, 0x09, 0x72, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x73, 0x18, 0x03, 0x20, 0x03, 0x28, - 0x0b, 0x32, 0x15, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, - 0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x52, 0x09, 0x72, 0x65, 0x73, 0x6f, 0x75, 0x72, - 0x63, 0x65, 0x73, 0x1a, 0x77, 0x0a, 0x08, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, - 0x24, 0x0a, 0x03, 0x6c, 0x6f, 0x67, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x10, 0x2e, 0x70, - 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x4c, 0x6f, 0x67, 0x48, 0x00, - 0x52, 0x03, 0x6c, 0x6f, 0x67, 0x12, 0x3d, 0x0a, 0x08, 0x63, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, - 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1f, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, - 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, - 0x43, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, 0x65, 0x48, 0x00, 0x52, 0x08, 0x63, 0x6f, 0x6d, 0x70, - 0x6c, 0x65, 0x74, 0x65, 0x42, 0x06, 0x0a, 0x04, 0x74, 0x79, 0x70, 0x65, 0x2a, 0x3f, 0x0a, 0x08, - 0x4c, 0x6f, 0x67, 0x4c, 0x65, 0x76, 0x65, 0x6c, 0x12, 0x09, 0x0a, 0x05, 0x54, 0x52, 0x41, 0x43, - 0x45, 0x10, 0x00, 0x12, 0x09, 0x0a, 0x05, 0x44, 0x45, 0x42, 0x55, 0x47, 0x10, 0x01, 0x12, 0x08, - 0x0a, 0x04, 0x49, 0x4e, 0x46, 0x4f, 0x10, 0x02, 0x12, 0x08, 0x0a, 0x04, 0x57, 0x41, 0x52, 0x4e, - 0x10, 0x03, 0x12, 0x09, 0x0a, 0x05, 0x45, 0x52, 0x52, 0x4f, 0x52, 0x10, 0x04, 0x2a, 0x37, 0x0a, - 0x13, 0x57, 0x6f, 0x72, 0x6b, 0x73, 0x70, 0x61, 0x63, 0x65, 0x54, 0x72, 0x61, 0x6e, 0x73, 0x69, - 0x74, 0x69, 0x6f, 0x6e, 0x12, 0x09, 0x0a, 0x05, 0x53, 0x54, 0x41, 0x52, 0x54, 0x10, 0x00, 0x12, - 0x08, 0x0a, 0x04, 0x53, 0x54, 0x4f, 0x50, 0x10, 0x01, 0x12, 0x0b, 0x0a, 0x07, 0x44, 0x45, 0x53, - 0x54, 0x52, 0x4f, 0x59, 0x10, 0x02, 0x32, 0xa3, 0x01, 0x0a, 0x0b, 0x50, 0x72, 0x6f, 0x76, 0x69, - 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x12, 0x42, 0x0a, 0x05, 0x50, 0x61, 0x72, 0x73, 0x65, 0x12, - 0x1a, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x61, - 0x72, 0x73, 0x65, 0x2e, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x1b, 0x2e, 0x70, 0x72, - 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x61, 0x72, 0x73, 0x65, 0x2e, - 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x30, 0x01, 0x12, 0x50, 0x0a, 0x09, 0x50, 0x72, - 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x12, 0x1e, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, - 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, - 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x1f, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, - 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, - 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x28, 0x01, 0x30, 0x01, 0x42, 0x2d, 0x5a, 0x2b, - 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x63, 0x6f, 0x64, 0x65, 0x72, - 0x2f, 0x63, 0x6f, 0x64, 0x65, 0x72, 0x2f, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, - 0x65, 0x72, 0x73, 0x64, 0x6b, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x06, 0x70, 0x72, 0x6f, - 0x74, 0x6f, 0x33, + 0x52, 0x75, 0x6e, 0x1a, 0x08, 0x0a, 0x06, 0x43, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x1a, 0x80, 0x01, + 0x0a, 0x07, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x34, 0x0a, 0x05, 0x73, 0x74, 0x61, + 0x72, 0x74, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1c, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, + 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, + 0x2e, 0x53, 0x74, 0x61, 0x72, 0x74, 0x48, 0x00, 0x52, 0x05, 0x73, 0x74, 0x61, 0x72, 0x74, 0x12, + 0x37, 0x0a, 0x06, 0x63, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, + 0x1d, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x72, + 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, 0x43, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x48, 0x00, + 0x52, 0x06, 0x63, 0x61, 0x6e, 0x63, 0x65, 0x6c, 0x42, 0x06, 0x0a, 0x04, 0x74, 0x79, 0x70, 0x65, + 0x1a, 0x6b, 0x0a, 0x08, 0x43, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, 0x65, 0x12, 0x14, 0x0a, 0x05, + 0x73, 0x74, 0x61, 0x74, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x05, 0x73, 0x74, 0x61, + 0x74, 0x65, 0x12, 0x14, 0x0a, 0x05, 0x65, 0x72, 0x72, 0x6f, 0x72, 0x18, 0x02, 0x20, 0x01, 0x28, + 0x09, 0x52, 0x05, 0x65, 0x72, 0x72, 0x6f, 0x72, 0x12, 0x33, 0x0a, 0x09, 0x72, 0x65, 0x73, 0x6f, + 0x75, 0x72, 0x63, 0x65, 0x73, 0x18, 0x03, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x15, 0x2e, 0x70, 0x72, + 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, + 0x63, 0x65, 0x52, 0x09, 0x72, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x73, 0x1a, 0x77, 0x0a, + 0x08, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x12, 0x24, 0x0a, 0x03, 0x6c, 0x6f, 0x67, + 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x10, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, + 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x4c, 0x6f, 0x67, 0x48, 0x00, 0x52, 0x03, 0x6c, 0x6f, 0x67, 0x12, + 0x3d, 0x0a, 0x08, 0x63, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, + 0x0b, 0x32, 0x1f, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, + 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, 0x43, 0x6f, 0x6d, 0x70, 0x6c, 0x65, + 0x74, 0x65, 0x48, 0x00, 0x52, 0x08, 0x63, 0x6f, 0x6d, 0x70, 0x6c, 0x65, 0x74, 0x65, 0x42, 0x06, + 0x0a, 0x04, 0x74, 0x79, 0x70, 0x65, 0x2a, 0x3f, 0x0a, 0x08, 0x4c, 0x6f, 0x67, 0x4c, 0x65, 0x76, + 0x65, 0x6c, 0x12, 0x09, 0x0a, 0x05, 0x54, 0x52, 0x41, 0x43, 0x45, 0x10, 0x00, 0x12, 0x09, 0x0a, + 0x05, 0x44, 0x45, 0x42, 0x55, 0x47, 0x10, 0x01, 0x12, 0x08, 0x0a, 0x04, 0x49, 0x4e, 0x46, 0x4f, + 0x10, 0x02, 0x12, 0x08, 0x0a, 0x04, 0x57, 0x41, 0x52, 0x4e, 0x10, 0x03, 0x12, 0x09, 0x0a, 0x05, + 0x45, 0x52, 0x52, 0x4f, 0x52, 0x10, 0x04, 0x2a, 0x37, 0x0a, 0x13, 0x57, 0x6f, 0x72, 0x6b, 0x73, + 0x70, 0x61, 0x63, 0x65, 0x54, 0x72, 0x61, 0x6e, 0x73, 0x69, 0x74, 0x69, 0x6f, 0x6e, 0x12, 0x09, + 0x0a, 0x05, 0x53, 0x54, 0x41, 0x52, 0x54, 0x10, 0x00, 0x12, 0x08, 0x0a, 0x04, 0x53, 0x54, 0x4f, + 0x50, 0x10, 0x01, 0x12, 0x0b, 0x0a, 0x07, 0x44, 0x45, 0x53, 0x54, 0x52, 0x4f, 0x59, 0x10, 0x02, + 0x32, 0xa3, 0x01, 0x0a, 0x0b, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, + 0x12, 0x42, 0x0a, 0x05, 0x50, 0x61, 0x72, 0x73, 0x65, 0x12, 0x1a, 0x2e, 0x70, 0x72, 0x6f, 0x76, + 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x61, 0x72, 0x73, 0x65, 0x2e, 0x52, 0x65, + 0x71, 0x75, 0x65, 0x73, 0x74, 0x1a, 0x1b, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, + 0x6e, 0x65, 0x72, 0x2e, 0x50, 0x61, 0x72, 0x73, 0x65, 0x2e, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, + 0x73, 0x65, 0x30, 0x01, 0x12, 0x50, 0x0a, 0x09, 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, + 0x6e, 0x12, 0x1e, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, + 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, + 0x74, 0x1a, 0x1f, 0x2e, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x2e, + 0x50, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x2e, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, + 0x73, 0x65, 0x28, 0x01, 0x30, 0x01, 0x42, 0x2d, 0x5a, 0x2b, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, + 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x63, 0x6f, 0x64, 0x65, 0x72, 0x2f, 0x63, 0x6f, 0x64, 0x65, 0x72, + 0x2f, 0x70, 0x72, 0x6f, 0x76, 0x69, 0x73, 0x69, 0x6f, 0x6e, 0x65, 0x72, 0x73, 0x64, 0x6b, 0x2f, + 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( diff --git a/provisionersdk/proto/provisioner.proto b/provisionersdk/proto/provisioner.proto index c1fe5ce13ee57..146f2e57400b0 100644 --- a/provisionersdk/proto/provisioner.proto +++ b/provisionersdk/proto/provisioner.proto @@ -152,7 +152,6 @@ message Provision { bool dry_run = 5; } message Cancel { - bool force = 1; } message Request { oneof type { From 1d7f825236560caea4aeae98d0952a901e0d42b8 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 16 Aug 2022 20:00:09 +0300 Subject: [PATCH 08/14] chore: Revert newline in proto --- provisionersdk/proto/provisioner.proto | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/provisionersdk/proto/provisioner.proto b/provisionersdk/proto/provisioner.proto index 146f2e57400b0..ab521256853c0 100644 --- a/provisionersdk/proto/provisioner.proto +++ b/provisionersdk/proto/provisioner.proto @@ -151,8 +151,7 @@ message Provision { bytes state = 4; bool dry_run = 5; } - message Cancel { - } + message Cancel {} message Request { oneof type { Start start = 1; From 5cbb7123fd8eff04b4eacfccf49fc35f4a6ac10d Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 17 Aug 2022 11:35:39 +0300 Subject: [PATCH 09/14] fix: Remove incorrect minimumTerraformVersion variable --- provisioner/terraform/executor.go | 7 ++----- provisioner/terraform/serve.go | 26 ++++++++------------------ 2 files changed, 10 insertions(+), 23 deletions(-) diff --git a/provisioner/terraform/executor.go b/provisioner/terraform/executor.go index 46ba26c63bc67..5c7d7199024c0 100644 --- a/provisioner/terraform/executor.go +++ b/provisioner/terraform/executor.go @@ -116,11 +116,11 @@ func (e executor) checkMinVersion(ctx context.Context) error { if err != nil { return err } - if !v.GreaterThanOrEqual(minimumTerraformVersion) { + if !v.GreaterThanOrEqual(minTerraformVersion) { return xerrors.Errorf( "terraform version %q is too old. required >= %q", v.String(), - minimumTerraformVersion.String()) + minTerraformVersion.String()) } return nil } @@ -435,9 +435,6 @@ func provisionReadAndLog(logr logger, reader io.Reader, done chan<- any) { // If the diagnostic is provided, let's provide a bit more info! logLevel = convertTerraformLogLevel(log.Diagnostic.Severity, logr) - if err != nil { - continue - } err = logr.Log(&proto.Log{Level: logLevel, Output: log.Diagnostic.Detail}) if err != nil { // Not much we can do. We can't log because logging is itself breaking! diff --git a/provisioner/terraform/serve.go b/provisioner/terraform/serve.go index 1a75483bda35d..e45219d97a21b 100644 --- a/provisioner/terraform/serve.go +++ b/provisioner/terraform/serve.go @@ -15,26 +15,16 @@ import ( "github.com/coder/coder/provisionersdk" ) -// This is the exact version of Terraform used internally -// when Terraform is missing on the system. -var terraformVersion = version.Must(version.NewVersion("1.2.1")) -var minTerraformVersion = version.Must(version.NewVersion("1.1.0")) -var maxTerraformVersion = version.Must(version.NewVersion("1.2.1")) - var ( - // The minimum version of Terraform supported by the provisioner. - // Validation came out in 0.13.0, which was released August 10th, 2020. - // https://www.hashicorp.com/blog/announcing-hashicorp-terraform-0-13 - minimumTerraformVersion = func() *version.Version { - v, err := version.NewSemver("0.13.0") - if err != nil { - panic(err) - } - return v - }() -) + // This is the exact version of Terraform used internally + // when Terraform is missing on the system. + terraformVersion = version.Must(version.NewVersion("1.2.1")) + + minTerraformVersion = version.Must(version.NewVersion("1.1.0")) + maxTerraformVersion = version.Must(version.NewVersion("1.2.1")) -var terraformMinorVersionMismatch = xerrors.New("Terraform binary minor version mismatch.") + terraformMinorVersionMismatch = xerrors.New("Terraform binary minor version mismatch.") +) type ServeOptions struct { *provisionersdk.ServeOptions From 3e4005ad10853c35fb7d127f91f3c874b90d8685 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 17 Aug 2022 11:36:18 +0300 Subject: [PATCH 10/14] fix: Add ExitTimeout to terraform.ServeOptions --- provisioner/terraform/provision.go | 8 +------- provisioner/terraform/serve.go | 28 +++++++++++++++++++++++++--- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index 82c8bfc6a7475..25d397e25bf85 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -14,12 +14,6 @@ import ( "github.com/coder/coder/provisionersdk/proto" ) -const ( - // Define how long we will wait for Terraform to exit cleanly - // after receiving an interrupt (if the provision was stopped). - terraformCancelTimeout = 5 * time.Minute -) - // Provision executes `terraform apply` or `terraform plan` for dry runs. func (s *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { request, err := stream.Recv() @@ -54,7 +48,7 @@ func (s *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { // part of graceful server shutdown procedure. Waiting on a // process here should delay provisioner/coder shutdown. select { - case <-time.After(terraformCancelTimeout): + case <-time.After(s.exitTimeout): kill() case <-killCtx.Done(): } diff --git a/provisioner/terraform/serve.go b/provisioner/terraform/serve.go index e45219d97a21b..47a751856f299 100644 --- a/provisioner/terraform/serve.go +++ b/provisioner/terraform/serve.go @@ -4,6 +4,7 @@ import ( "context" "path/filepath" "sync" + "time" "github.com/cli/safeexec" "github.com/hashicorp/go-version" @@ -26,6 +27,10 @@ var ( terraformMinorVersionMismatch = xerrors.New("Terraform binary minor version mismatch.") ) +const ( + defaultExitTimeout = 5 * time.Minute +) + type ServeOptions struct { *provisionersdk.ServeOptions @@ -34,6 +39,17 @@ type ServeOptions struct { BinaryPath string CachePath string Logger slog.Logger + + // ExitTimeout defines how long we will wait for a running Terraform + // command to exit (cleanly) if the provision was stopped. This only + // happens when the command is still running after the provision + // stream is closed. If the provision is canceled via RPC, this + // timeout will not be used. + // + // This is a no-op on Windows where the process can't be interrupted. + // + // Default value: 5 minutes. + ExitTimeout time.Duration } func absoluteBinaryPath(ctx context.Context) (string, error) { @@ -92,10 +108,14 @@ func Serve(ctx context.Context, options *ServeOptions) error { options.BinaryPath = absoluteBinary } } + if options.ExitTimeout == 0 { + options.ExitTimeout = defaultExitTimeout + } return provisionersdk.Serve(ctx, &server{ - binaryPath: options.BinaryPath, - cachePath: options.CachePath, - logger: options.Logger, + binaryPath: options.BinaryPath, + cachePath: options.CachePath, + logger: options.Logger, + exitTimeout: options.ExitTimeout, }, options.ServeOptions) } @@ -107,6 +127,8 @@ type server struct { binaryPath string cachePath string logger slog.Logger + + exitTimeout time.Duration } func (s *server) executor(workdir string) executor { From 685c58326192076bc7213c38f7b36cb8f2777083 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 17 Aug 2022 15:17:08 +0300 Subject: [PATCH 11/14] fix: Add test for interrupt/cancellation --- provisioner/terraform/parse_test.go | 4 +- provisioner/terraform/provision.go | 2 +- provisioner/terraform/provision_test.go | 133 ++++++++++++++++++++++-- provisioner/terraform/serve.go | 8 +- 4 files changed, 133 insertions(+), 14 deletions(-) diff --git a/provisioner/terraform/parse_test.go b/provisioner/terraform/parse_test.go index 0a88ab21d5b78..d00edc8a9b303 100644 --- a/provisioner/terraform/parse_test.go +++ b/provisioner/terraform/parse_test.go @@ -16,7 +16,7 @@ import ( func TestParse(t *testing.T) { t.Parallel() - ctx, api := setupProvisioner(t) + ctx, api := setupProvisioner(t, nil) testCases := []struct { Name string @@ -171,7 +171,7 @@ func TestParse(t *testing.T) { // Write all files to the temporary test directory. directory := t.TempDir() for path, content := range testCase.Files { - err := os.WriteFile(filepath.Join(directory, path), []byte(content), 0600) + err := os.WriteFile(filepath.Join(directory, path), []byte(content), 0o600) require.NoError(t, err) } diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index 25d397e25bf85..f3474184e838d 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -73,7 +73,7 @@ func (s *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { start := request.GetStart() e := s.executor(start.Directory) - if err = e.checkMinVersion(stream.Context()); err != nil { + if err = e.checkMinVersion(ctx); err != nil { return err } if err = logTerraformEnvVars(logr); err != nil { diff --git a/provisioner/terraform/provision_test.go b/provisioner/terraform/provision_test.go index 4b9e203ab4f32..dd6c2b6e01d1d 100644 --- a/provisioner/terraform/provision_test.go +++ b/provisioner/terraform/provision_test.go @@ -5,11 +5,14 @@ package terraform_test import ( "context" "encoding/json" + "fmt" "os" "path/filepath" + "runtime" "sort" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -22,7 +25,15 @@ import ( "github.com/coder/coder/provisionersdk/proto" ) -func setupProvisioner(t *testing.T) (context.Context, proto.DRPCProvisionerClient) { +type provisionerServeOptions struct { + binaryPath string + exitTimeout time.Duration +} + +func setupProvisioner(t *testing.T, opts *provisionerServeOptions) (context.Context, proto.DRPCProvisionerClient) { + if opts == nil { + opts = &provisionerServeOptions{} + } cachePath := t.TempDir() client, server := provisionersdk.TransportPipe() ctx, cancelFunc := context.WithCancel(context.Background()) @@ -39,18 +50,126 @@ func setupProvisioner(t *testing.T) (context.Context, proto.DRPCProvisionerClien ServeOptions: &provisionersdk.ServeOptions{ Listener: server, }, - CachePath: cachePath, - Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug), + BinaryPath: opts.binaryPath, + CachePath: cachePath, + Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug), + ExitTimeout: opts.exitTimeout, }) }() api := proto.NewDRPCProvisionerClient(provisionersdk.Conn(client)) return ctx, api } +func TestProvision_Cancel(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("This test uses interrupts and is not supported on Windows") + } + + t.Parallel() + + cwd, err := os.Getwd() + require.NoError(t, err) + fakeBin := filepath.Join(cwd, "testdata", "bin", "terraform_fake_cancel.sh") + + tests := []struct { + name string + mode string + startSequence []string + wantLog []string + }{ + { + name: "Cancel init", + mode: "init", + startSequence: []string{"init_start"}, + wantLog: []string{"interrupt", "exit"}, + }, + { + name: "Cancel apply", + mode: "apply", + startSequence: []string{"init", "apply_start"}, + wantLog: []string{"interrupt", "exit"}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + binPath := filepath.Join(dir, "terraform") + + // Example: exec /path/to/terrafork_fake_cancel.sh 1.2.1 apply "$@" + content := fmt.Sprintf("#!/bin/sh\nexec %q %s %s \"$@\"\n", fakeBin, terraform.TerraformVersion.String(), tt.mode) + err = os.WriteFile(binPath, []byte(content), 0o755) //#nosec + require.NoError(t, err) + + ctx, api := setupProvisioner(t, &provisionerServeOptions{ + binaryPath: binPath, + exitTimeout: time.Nanosecond, + }) + + response, err := api.Provision(ctx) + require.NoError(t, err) + err = response.Send(&proto.Provision_Request{ + Type: &proto.Provision_Request_Start{ + Start: &proto.Provision_Start{ + Directory: dir, + DryRun: false, + ParameterValues: []*proto.ParameterValue{{ + DestinationScheme: proto.ParameterDestination_PROVISIONER_VARIABLE, + Name: "A", + Value: "example", + }}, + Metadata: &proto.Provision_Metadata{}, + }, + }, + }) + require.NoError(t, err) + + for _, line := range tt.startSequence { + LoopStart: + msg, err := response.Recv() + require.NoError(t, err) + + t.Log(msg.Type) + + log := msg.GetLog() + if log == nil { + goto LoopStart + } + require.NotNil(t, log) + require.Equal(t, line, log.Output) + } + + err = response.Send(&proto.Provision_Request{ + Type: &proto.Provision_Request_Cancel{ + Cancel: &proto.Provision_Cancel{}, + }, + }) + require.NoError(t, err) + + var gotLog []string + for { + msg, err := response.Recv() + require.NoError(t, err) + + if log := msg.GetLog(); log != nil { + gotLog = append(gotLog, log.Output) + } + if c := msg.GetComplete(); c != nil { + require.Contains(t, c.Error, "exit status 1") + break + } + } + require.Equal(t, tt.wantLog, gotLog) + }) + } +} + func TestProvision(t *testing.T) { t.Parallel() - ctx, api := setupProvisioner(t) + ctx, api := setupProvisioner(t, nil) testCases := []struct { Name string @@ -209,7 +328,7 @@ func TestProvision(t *testing.T) { directory := t.TempDir() for path, content := range testCase.Files { - err := os.WriteFile(filepath.Join(directory, path), []byte(content), 0600) + err := os.WriteFile(filepath.Join(directory, path), []byte(content), 0o600) require.NoError(t, err) } @@ -302,11 +421,11 @@ func TestProvision_ExtraEnv(t *testing.T) { t.Setenv("TF_LOG", "INFO") t.Setenv("TF_SUPERSECRET", secretValue) - ctx, api := setupProvisioner(t) + ctx, api := setupProvisioner(t, nil) directory := t.TempDir() path := filepath.Join(directory, "main.tf") - err := os.WriteFile(path, []byte(`resource "null_resource" "A" {}`), 0600) + err := os.WriteFile(path, []byte(`resource "null_resource" "A" {}`), 0o600) require.NoError(t, err) request := &proto.Provision_Request{ diff --git a/provisioner/terraform/serve.go b/provisioner/terraform/serve.go index 47a751856f299..8b24aea20cfa9 100644 --- a/provisioner/terraform/serve.go +++ b/provisioner/terraform/serve.go @@ -17,9 +17,9 @@ import ( ) var ( - // This is the exact version of Terraform used internally - // when Terraform is missing on the system. - terraformVersion = version.Must(version.NewVersion("1.2.1")) + // TerraformVersion is the version of Terraform used internally + // when Terraform is not available on the system. + TerraformVersion = version.Must(version.NewVersion("1.2.1")) minTerraformVersion = version.Must(version.NewVersion("1.1.0")) maxTerraformVersion = version.Must(version.NewVersion("1.2.1")) @@ -96,7 +96,7 @@ func Serve(ctx context.Context, options *ServeOptions) error { installer := &releases.ExactVersion{ InstallDir: options.CachePath, Product: product.Terraform, - Version: terraformVersion, + Version: TerraformVersion, } execPath, err := installer.Install(ctx) From 3303ee1a7ba1887029ca6ee966af8a6bc249d961 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 17 Aug 2022 15:42:42 +0300 Subject: [PATCH 12/14] Allow init to return provision complete response --- provisioner/terraform/provision.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index f3474184e838d..8a858e3bb3bb6 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -113,6 +113,15 @@ func (s *server) Provision(stream proto.DRPCProvisioner_ProvisionStream) error { s.logger.Debug(ctx, "running initialization") err = e.init(ctx, killCtx, logr) if err != nil { + if ctx.Err() != nil { + return stream.Send(&proto.Provision_Response{ + Type: &proto.Provision_Response_Complete{ + Complete: &proto.Provision_Complete{ + Error: err.Error(), + }, + }, + }) + } return xerrors.Errorf("initialize terraform: %w", err) } s.logger.Debug(ctx, "ran initialization") From db41a6518ca16086abc5c8ab5dcae7fd69a15fbb Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 17 Aug 2022 15:46:44 +0300 Subject: [PATCH 13/14] Add testdata script --- .../testdata/bin/terraform_fake_cancel.sh | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100755 provisioner/terraform/testdata/bin/terraform_fake_cancel.sh diff --git a/provisioner/terraform/testdata/bin/terraform_fake_cancel.sh b/provisioner/terraform/testdata/bin/terraform_fake_cancel.sh new file mode 100755 index 0000000000000..faaa59684fedc --- /dev/null +++ b/provisioner/terraform/testdata/bin/terraform_fake_cancel.sh @@ -0,0 +1,60 @@ +#!/bin/sh + +VERSION=$1 +MODE=$2 +shift 2 + +json_print() { + echo "{\"@level\":\"error\",\"@message\":\"$*\"}" +} + +case "$1" in +version) + cat <<-EOF + { + "terraform_version": "${VERSION}", + "platform": "linux_amd64", + "provider_selections": {}, + "terraform_outdated": false + } + EOF + exit 0 + ;; +init) + case "$MODE" in + apply) + echo "init" + ;; + init) + sleep 10 & + sleep_pid=$! + + trap 'echo exit; kill -9 $sleep_pid 2>/dev/null' EXIT + trap 'echo interrupt; exit 1' INT + trap 'echo terminate"; exit 2' TERM + + echo init_start + wait + echo init_end + ;; + esac + ;; +apply) + sleep 10 & + sleep_pid=$! + + trap 'json_print exit; kill -9 $sleep_pid 2>/dev/null' EXIT + trap 'json_print interrupt; exit 1' INT + trap 'json_print terminate"; exit 2' TERM + + json_print apply_start + wait + json_print apply_end + ;; +plan) + echo "plan not supported" + exit 1 + ;; +esac + +exit 0 From 80a4912e9e9426a62929a870daa2d6fc573d4377 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 17 Aug 2022 16:27:58 +0300 Subject: [PATCH 14/14] Remove superfluous require --- provisioner/terraform/provision_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/provisioner/terraform/provision_test.go b/provisioner/terraform/provision_test.go index dd6c2b6e01d1d..e6beffde64332 100644 --- a/provisioner/terraform/provision_test.go +++ b/provisioner/terraform/provision_test.go @@ -137,7 +137,6 @@ func TestProvision_Cancel(t *testing.T) { if log == nil { goto LoopStart } - require.NotNil(t, log) require.Equal(t, line, log.Output) }