Merge pull request #2251 from mitchellh/b-warn-prov

packer: HookProvision errors if no communicator

common/step_provision.go

@@ -23,9 +23,11 @@ type StepProvision struct {
 func (s *StepProvision) Run(state multistep.StateBag) multistep.StepAction {
 	comm := s.Comm
 	if comm == nil {
-		comm = state.Get("communicator").(packer.Communicator)
+		raw, ok := state.Get("communicator").(packer.Communicator)
+		if ok {
+			comm = raw.(packer.Communicator)
+		}
 	}
-
 	hook := state.Get("hook").(packer.Hook)
 	ui := state.Get("ui").(packer.Ui)
 

packer/build_test.go

@@ -202,7 +202,7 @@ func TestBuild_Run(t *testing.T) {
 	}
 
 	// Verify provisioners run
-	dispatchHook.Run(HookProvision, nil, nil, 42)
+	dispatchHook.Run(HookProvision, nil, new(MockCommunicator), 42)
 	prov := build.provisioners[0].provisioner.(*MockProvisioner)
 	if !prov.ProvCalled {
 		t.Fatal("should be called")

packer/builder_mock.go

@@ -43,7 +43,7 @@ func (tb *MockBuilder) Run(ui Ui, h Hook, c Cache) (Artifact, error) {
 	}
 
 	if h != nil {
-		if err := h.Run(HookProvision, ui, nil, nil); err != nil {
+		if err := h.Run(HookProvision, ui, new(MockCommunicator), nil); err != nil {
 			return nil, err
 		}
 	}

packer/provisioner.go

@@ -38,6 +38,18 @@ type ProvisionHook struct {
 
 // Runs the provisioners in order.
 func (h *ProvisionHook) Run(name string, ui Ui, comm Communicator, data interface{}) error {
+	// Shortcut
+	if len(h.Provisioners) == 0 {
+		return nil
+	}
+
+	if comm == nil {
+		return fmt.Errorf(
+			"No communicator found for provisioners! This is usually because the\n" +
+				"`communicator` config was set to \"none\". If you have any provisioners\n" +
+				"then a communicator is required. Please fix this to continue.")
+	}
+
 	defer func() {
 		h.lock.Lock()
 		defer h.lock.Unlock()

packer/provisioner_test.go

@@ -19,7 +19,7 @@ func TestProvisionHook(t *testing.T) {
 	pB := &MockProvisioner{}
 
 	ui := testUi()
-	var comm Communicator = nil
+	var comm Communicator = new(MockCommunicator)
 	var data interface{} = nil
 
 	hook := &ProvisionHook{
@@ -37,6 +37,24 @@ func TestProvisionHook(t *testing.T) {
 	}
 }
 
+func TestProvisionHook_nilComm(t *testing.T) {
+	pA := &MockProvisioner{}
+	pB := &MockProvisioner{}
+
+	ui := testUi()
+	var comm Communicator = nil
+	var data interface{} = nil
+
+	hook := &ProvisionHook{
+		Provisioners: []Provisioner{pA, pB},
+	}
+
+	err := hook.Run("foo", ui, comm, data)
+	if err == nil {
+		t.Fatal("should error")
+	}
+}
+
 func TestProvisionHook_cancel(t *testing.T) {
 	var lock sync.Mutex
 	order := make([]string, 0, 2)
@@ -59,7 +77,7 @@ func TestProvisionHook_cancel(t *testing.T) {
 
 	finished := make(chan struct{})
 	go func() {
-		hook.Run("foo", nil, nil, nil)
+		hook.Run("foo", nil, new(MockCommunicator), nil)
 		close(finished)
 	}()
 
@@ -74,7 +92,7 @@ func TestProvisionHook_cancel(t *testing.T) {
 	<-finished
 
 	// Verify order
-	if order[0] != "cancel" || order[1] != "prov" {
+	if len(order) != 2 || order[0] != "cancel" || order[1] != "prov" {
 		t.Fatalf("bad: %#v", order)
 	}
 }