ensure context cancel is never nil
The context used for Stop is more appropriately tied to the lifetime of the provisioner rather than a call to the ProvisionResource method. In some cases Stop can be called before ProvisionResource, causing a panic the provisioner. Rather than adding nil checks to the CancelFunc call for Stop, create a base context to use for cancellation with both Stop and Close methods.
This commit is contained in:
parent
d175e67bc9
commit
026ba5b013
|
@ -6,7 +6,6 @@ import (
|
||||||
"fmt"
|
"fmt"
|
||||||
"io/ioutil"
|
"io/ioutil"
|
||||||
"os"
|
"os"
|
||||||
"sync"
|
|
||||||
|
|
||||||
"github.com/hashicorp/terraform/communicator"
|
"github.com/hashicorp/terraform/communicator"
|
||||||
"github.com/hashicorp/terraform/configs/configschema"
|
"github.com/hashicorp/terraform/configs/configschema"
|
||||||
|
@ -16,13 +15,17 @@ import (
|
||||||
)
|
)
|
||||||
|
|
||||||
func New() provisioners.Interface {
|
func New() provisioners.Interface {
|
||||||
return &provisioner{}
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
|
return &provisioner{
|
||||||
|
ctx: ctx,
|
||||||
|
cancel: cancel,
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
type provisioner struct {
|
type provisioner struct {
|
||||||
// this stored from the running context, so that Stop() can
|
// We store a context here tied to the lifetime of the provisioner.
|
||||||
// cancel the transfer
|
// This allows the Stop method to cancel any in-flight requests.
|
||||||
mu sync.Mutex
|
ctx context.Context
|
||||||
cancel context.CancelFunc
|
cancel context.CancelFunc
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -71,11 +74,6 @@ func (p *provisioner) ValidateProvisionerConfig(req provisioners.ValidateProvisi
|
||||||
}
|
}
|
||||||
|
|
||||||
func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceRequest) (resp provisioners.ProvisionResourceResponse) {
|
func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceRequest) (resp provisioners.ProvisionResourceResponse) {
|
||||||
p.mu.Lock()
|
|
||||||
ctx, cancel := context.WithCancel(context.Background())
|
|
||||||
p.cancel = cancel
|
|
||||||
p.mu.Unlock()
|
|
||||||
|
|
||||||
comm, err := communicator.New(req.Connection)
|
comm, err := communicator.New(req.Connection)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
resp.Diagnostics = resp.Diagnostics.Append(err)
|
resp.Diagnostics = resp.Diagnostics.Append(err)
|
||||||
|
@ -94,7 +92,7 @@ func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceReques
|
||||||
|
|
||||||
// Begin the file copy
|
// Begin the file copy
|
||||||
dst := req.Config.GetAttr("destination").AsString()
|
dst := req.Config.GetAttr("destination").AsString()
|
||||||
if err := copyFiles(ctx, comm, src, dst); err != nil {
|
if err := copyFiles(p.ctx, comm, src, dst); err != nil {
|
||||||
resp.Diagnostics = resp.Diagnostics.Append(err)
|
resp.Diagnostics = resp.Diagnostics.Append(err)
|
||||||
return resp
|
return resp
|
||||||
}
|
}
|
||||||
|
@ -178,12 +176,11 @@ func copyFiles(ctx context.Context, comm communicator.Communicator, src, dst str
|
||||||
}
|
}
|
||||||
|
|
||||||
func (p *provisioner) Stop() error {
|
func (p *provisioner) Stop() error {
|
||||||
p.mu.Lock()
|
|
||||||
defer p.mu.Unlock()
|
|
||||||
p.cancel()
|
p.cancel()
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func (p *provisioner) Close() error {
|
func (p *provisioner) Close() error {
|
||||||
|
p.cancel()
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
|
@ -95,3 +95,10 @@ func TestResourceProvider_Validate_bad_to_many_src(t *testing.T) {
|
||||||
t.Fatal("Should have errors")
|
t.Fatal("Should have errors")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Validate that Stop can Close can be called even when not provisioning.
|
||||||
|
func TestResourceProvisioner_StopClose(t *testing.T) {
|
||||||
|
p := New()
|
||||||
|
p.Stop()
|
||||||
|
p.Close()
|
||||||
|
}
|
||||||
|
|
|
@ -7,7 +7,6 @@ import (
|
||||||
"os"
|
"os"
|
||||||
"os/exec"
|
"os/exec"
|
||||||
"runtime"
|
"runtime"
|
||||||
"sync"
|
|
||||||
|
|
||||||
"github.com/armon/circbuf"
|
"github.com/armon/circbuf"
|
||||||
"github.com/hashicorp/terraform/configs/configschema"
|
"github.com/hashicorp/terraform/configs/configschema"
|
||||||
|
@ -24,13 +23,17 @@ const (
|
||||||
)
|
)
|
||||||
|
|
||||||
func New() provisioners.Interface {
|
func New() provisioners.Interface {
|
||||||
return &provisioner{}
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
|
return &provisioner{
|
||||||
|
ctx: ctx,
|
||||||
|
cancel: cancel,
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
type provisioner struct {
|
type provisioner struct {
|
||||||
// this stored from the running context, so that Stop() can cancel the
|
// We store a context here tied to the lifetime of the provisioner.
|
||||||
// command
|
// This allows the Stop method to cancel any in-flight requests.
|
||||||
mu sync.Mutex
|
ctx context.Context
|
||||||
cancel context.CancelFunc
|
cancel context.CancelFunc
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -68,11 +71,6 @@ func (p *provisioner) ValidateProvisionerConfig(req provisioners.ValidateProvisi
|
||||||
}
|
}
|
||||||
|
|
||||||
func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceRequest) (resp provisioners.ProvisionResourceResponse) {
|
func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceRequest) (resp provisioners.ProvisionResourceResponse) {
|
||||||
p.mu.Lock()
|
|
||||||
ctx, cancel := context.WithCancel(context.Background())
|
|
||||||
p.cancel = cancel
|
|
||||||
p.mu.Unlock()
|
|
||||||
|
|
||||||
command := req.Config.GetAttr("command").AsString()
|
command := req.Config.GetAttr("command").AsString()
|
||||||
if command == "" {
|
if command == "" {
|
||||||
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("local-exec provisioner command must be a non-empty string"))
|
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("local-exec provisioner command must be a non-empty string"))
|
||||||
|
@ -127,7 +125,7 @@ func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceReques
|
||||||
cmdEnv = append(cmdEnv, env...)
|
cmdEnv = append(cmdEnv, env...)
|
||||||
|
|
||||||
// Setup the command
|
// Setup the command
|
||||||
cmd := exec.CommandContext(ctx, cmdargs[0], cmdargs[1:]...)
|
cmd := exec.CommandContext(p.ctx, cmdargs[0], cmdargs[1:]...)
|
||||||
cmd.Stderr = pw
|
cmd.Stderr = pw
|
||||||
cmd.Stdout = pw
|
cmd.Stdout = pw
|
||||||
// Dir specifies the working directory of the command.
|
// Dir specifies the working directory of the command.
|
||||||
|
@ -165,7 +163,7 @@ func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceReques
|
||||||
// copyOutput goroutine will just hang out until exit.
|
// copyOutput goroutine will just hang out until exit.
|
||||||
select {
|
select {
|
||||||
case <-copyDoneCh:
|
case <-copyDoneCh:
|
||||||
case <-ctx.Done():
|
case <-p.ctx.Done():
|
||||||
}
|
}
|
||||||
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
@ -178,13 +176,12 @@ func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceReques
|
||||||
}
|
}
|
||||||
|
|
||||||
func (p *provisioner) Stop() error {
|
func (p *provisioner) Stop() error {
|
||||||
p.mu.Lock()
|
|
||||||
defer p.mu.Unlock()
|
|
||||||
p.cancel()
|
p.cancel()
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func (p *provisioner) Close() error {
|
func (p *provisioner) Close() error {
|
||||||
|
p.cancel()
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -197,3 +197,10 @@ BAR 1 true`
|
||||||
t.Errorf("wrong output\ngot: %s\nwant: %s", got, want)
|
t.Errorf("wrong output\ngot: %s\nwant: %s", got, want)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Validate that Stop can Close can be called even when not provisioning.
|
||||||
|
func TestResourceProvisioner_StopClose(t *testing.T) {
|
||||||
|
p := New()
|
||||||
|
p.Stop()
|
||||||
|
p.Close()
|
||||||
|
}
|
||||||
|
|
|
@ -10,7 +10,6 @@ import (
|
||||||
"log"
|
"log"
|
||||||
"os"
|
"os"
|
||||||
"strings"
|
"strings"
|
||||||
"sync"
|
|
||||||
|
|
||||||
"github.com/hashicorp/terraform/communicator"
|
"github.com/hashicorp/terraform/communicator"
|
||||||
"github.com/hashicorp/terraform/communicator/remote"
|
"github.com/hashicorp/terraform/communicator/remote"
|
||||||
|
@ -21,13 +20,17 @@ import (
|
||||||
)
|
)
|
||||||
|
|
||||||
func New() provisioners.Interface {
|
func New() provisioners.Interface {
|
||||||
return &provisioner{}
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
|
return &provisioner{
|
||||||
|
ctx: ctx,
|
||||||
|
cancel: cancel,
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
type provisioner struct {
|
type provisioner struct {
|
||||||
// this stored from the running context, so that Stop() can cancel the
|
// We store a context here tied to the lifetime of the provisioner.
|
||||||
// command
|
// This allows the Stop method to cancel any in-flight requests.
|
||||||
mu sync.Mutex
|
ctx context.Context
|
||||||
cancel context.CancelFunc
|
cancel context.CancelFunc
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -82,11 +85,6 @@ func (p *provisioner) ValidateProvisionerConfig(req provisioners.ValidateProvisi
|
||||||
}
|
}
|
||||||
|
|
||||||
func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceRequest) (resp provisioners.ProvisionResourceResponse) {
|
func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceRequest) (resp provisioners.ProvisionResourceResponse) {
|
||||||
p.mu.Lock()
|
|
||||||
ctx, cancel := context.WithCancel(context.Background())
|
|
||||||
p.cancel = cancel
|
|
||||||
p.mu.Unlock()
|
|
||||||
|
|
||||||
comm, err := communicator.New(req.Connection)
|
comm, err := communicator.New(req.Connection)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
resp.Diagnostics = resp.Diagnostics.Append(err)
|
resp.Diagnostics = resp.Diagnostics.Append(err)
|
||||||
|
@ -104,7 +102,7 @@ func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceReques
|
||||||
}
|
}
|
||||||
|
|
||||||
// Copy and execute each script
|
// Copy and execute each script
|
||||||
if err := runScripts(ctx, req.UIOutput, comm, scripts); err != nil {
|
if err := runScripts(p.ctx, req.UIOutput, comm, scripts); err != nil {
|
||||||
resp.Diagnostics = resp.Diagnostics.Append(err)
|
resp.Diagnostics = resp.Diagnostics.Append(err)
|
||||||
return resp
|
return resp
|
||||||
}
|
}
|
||||||
|
@ -113,13 +111,12 @@ func (p *provisioner) ProvisionResource(req provisioners.ProvisionResourceReques
|
||||||
}
|
}
|
||||||
|
|
||||||
func (p *provisioner) Stop() error {
|
func (p *provisioner) Stop() error {
|
||||||
p.mu.Lock()
|
|
||||||
defer p.mu.Unlock()
|
|
||||||
p.cancel()
|
p.cancel()
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func (p *provisioner) Close() error {
|
func (p *provisioner) Close() error {
|
||||||
|
p.cancel()
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -254,3 +254,10 @@ func TestProvisionerTimeout(t *testing.T) {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Validate that Stop can Close can be called even when not provisioning.
|
||||||
|
func TestResourceProvisioner_StopClose(t *testing.T) {
|
||||||
|
p := New()
|
||||||
|
p.Stop()
|
||||||
|
p.Close()
|
||||||
|
}
|
||||||
|
|
Loading…
Reference in New Issue