diff --git a/packer/build.go b/packer/build.go index 5c4815d28..df91ccf85 100644 --- a/packer/build.go +++ b/packer/build.go @@ -7,7 +7,7 @@ import "log" type Build interface { Name() string Prepare(Ui) error - Run(Ui, Cache) Artifact + Run(Ui, Cache) (Artifact, error) Cancel() } @@ -61,7 +61,7 @@ func (b *coreBuild) Prepare(ui Ui) (err error) { } // Runs the actual build. Prepare must be called prior to running this. -func (b *coreBuild) Run(ui Ui, cache Cache) Artifact { +func (b *coreBuild) Run(ui Ui, cache Cache) (Artifact, error) { if !b.prepareCalled { panic("Prepare must be called first") } diff --git a/packer/builder.go b/packer/builder.go index 284277b5d..9f73623b9 100644 --- a/packer/builder.go +++ b/packer/builder.go @@ -10,7 +10,7 @@ type Builder interface { Prepare(config interface{}) error // Run is where the actual build should take place. It takes a Build and a Ui. - Run(ui Ui, hook Hook, cache Cache) Artifact + Run(ui Ui, hook Hook, cache Cache) (Artifact, error) // Cancel cancels a possibly running Builder. This should block until // the builder actually cancels and cleans up after itself. diff --git a/packer/builder_test.go b/packer/builder_test.go index a7351e06a..d46946ac6 100644 --- a/packer/builder_test.go +++ b/packer/builder_test.go @@ -16,12 +16,12 @@ func (tb *TestBuilder) Prepare(config interface{}) error { return nil } -func (tb *TestBuilder) Run(ui Ui, h Hook, c Cache) Artifact { +func (tb *TestBuilder) Run(ui Ui, h Hook, c Cache) (Artifact, error) { tb.runCalled = true tb.runHook = h tb.runUi = ui tb.runCache = c - return nil + return nil, nil } func (tb *TestBuilder) Cancel() { diff --git a/packer/rpc/build.go b/packer/rpc/build.go index c0cd5fde4..a816947d2 100644 --- a/packer/rpc/build.go +++ b/packer/rpc/build.go @@ -48,7 +48,7 @@ func (b *build) Prepare(ui packer.Ui) (err error) { return } -func (b *build) Run(ui packer.Ui, cache packer.Cache) packer.Artifact { +func (b *build) Run(ui packer.Ui, cache packer.Cache) (packer.Artifact, error) { // Create and start the server for the UI server := rpc.NewServer() RegisterCache(server, cache) @@ -65,7 +65,7 @@ func (b *build) Run(ui packer.Ui, cache packer.Cache) packer.Artifact { panic(err) } - return Artifact(client) + return Artifact(client), nil } func (b *build) Cancel() { @@ -95,7 +95,10 @@ func (b *BuildServer) Run(args *BuildRunArgs, reply *string) error { return err } - artifact := b.build.Run(&Ui{client}, Cache(client)) + artifact, err := b.build.Run(&Ui{client}, Cache(client)) + if err != nil { + return err + } // Wrap the artifact server := rpc.NewServer() diff --git a/packer/rpc/build_test.go b/packer/rpc/build_test.go index 2890433a9..33d1631c3 100644 --- a/packer/rpc/build_test.go +++ b/packer/rpc/build_test.go @@ -30,11 +30,11 @@ func (b *testBuild) Prepare(ui packer.Ui) error { return nil } -func (b *testBuild) Run(ui packer.Ui, cache packer.Cache) packer.Artifact { +func (b *testBuild) Run(ui packer.Ui, cache packer.Cache) (packer.Artifact, error) { b.runCalled = true b.runCache = cache b.runUi = ui - return testBuildArtifact + return testBuildArtifact, nil } func (b *testBuild) Cancel() { diff --git a/packer/rpc/builder.go b/packer/rpc/builder.go index 6679a025c..4a198dc70 100644 --- a/packer/rpc/builder.go +++ b/packer/rpc/builder.go @@ -30,6 +30,7 @@ type BuilderRunArgs struct { } type BuilderRunResponse struct { + Err error RPCAddress string } @@ -46,7 +47,7 @@ func (b *builder) Prepare(config interface{}) (err error) { return } -func (b *builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) packer.Artifact { +func (b *builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) { // Create and start the server for the Build and UI server := rpc.NewServer() RegisterCache(server, cache) @@ -55,7 +56,7 @@ func (b *builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) packer // Create a server for the response responseL := netListenerInRange(portRangeMin, portRangeMax) - artifactAddress := make(chan string) + runResponseCh := make(chan *BuilderRunResponse) go func() { defer responseL.Close() @@ -72,7 +73,7 @@ func (b *builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) packer log.Panic(err) } - artifactAddress <- response.RPCAddress + runResponseCh <- &response }() args := &BuilderRunArgs{ @@ -81,20 +82,24 @@ func (b *builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) packer } if err := b.client.Call("Builder.Run", args, new(interface{})); err != nil { - panic(err) + return nil, err + } + + response := <-runResponseCh + if response.Err != nil { + return nil, response.Err } - address := <-artifactAddress - if address == "" { - return nil + if response.RPCAddress == "" { + return nil, nil } - client, err := rpc.Dial("tcp", address) + client, err := rpc.Dial("tcp", response.RPCAddress) if err != nil { - panic(err) + return nil, err } - return Artifact(client) + return Artifact(client), nil } func (b *builder) Cancel() { @@ -132,17 +137,24 @@ func (b *BuilderServer) Run(args *BuilderRunArgs, reply *interface{}) error { cache := Cache(client) hook := Hook(client) ui := &Ui{client} - artifact := b.builder.Run(ui, hook, cache) + artifact, responseErr := b.builder.Run(ui, hook, cache) responseAddress := "" - if artifact != nil { + if responseErr == nil && artifact != nil { // Wrap the artifact server := rpc.NewServer() RegisterArtifact(server, artifact) responseAddress = serveSingleConn(server) } - responseWriter.Encode(&BuilderRunResponse{responseAddress}) + if responseErr != nil { + responseErr = NewBasicError(responseErr) + } + + err := responseWriter.Encode(&BuilderRunResponse{responseErr, responseAddress}) + if err != nil { + panic(err) + } }() return nil diff --git a/packer/rpc/builder_test.go b/packer/rpc/builder_test.go index 2b9cc1eaf..e4f412ff6 100644 --- a/packer/rpc/builder_test.go +++ b/packer/rpc/builder_test.go @@ -2,6 +2,7 @@ package rpc import ( "cgl.tideland.biz/asserts" + "errors" "github.com/mitchellh/packer/packer" "net/rpc" "testing" @@ -18,6 +19,7 @@ type testBuilder struct { runUi packer.Ui cancelCalled bool + errRunResult bool nilRunResult bool } @@ -27,16 +29,18 @@ func (b *testBuilder) Prepare(config interface{}) error { return nil } -func (b *testBuilder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) packer.Artifact { +func (b *testBuilder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) { b.runCache = cache b.runCalled = true b.runHook = hook b.runUi = ui - if !b.nilRunResult { - return testBuilderArtifact + if b.errRunResult { + return nil, errors.New("foo") + } else if b.nilRunResult { + return nil, nil } else { - return nil + return testBuilderArtifact, nil } } @@ -70,7 +74,8 @@ func TestBuilderRPC(t *testing.T) { cache := new(testCache) hook := &testHook{} ui := &testUi{} - artifact := bClient.Run(ui, hook, cache) + artifact, err := bClient.Run(ui, hook, cache) + assert.Nil(err, "should have no error") assert.True(b.runCalled, "runs hould be called") if b.runCalled { @@ -89,8 +94,16 @@ func TestBuilderRPC(t *testing.T) { // Test run with nil result b.nilRunResult = true - artifact = bClient.Run(ui, hook, cache) + artifact, err = bClient.Run(ui, hook, cache) assert.Nil(artifact, "should be nil") + assert.Nil(err, "should have no error") + + // Test with an error + b.errRunResult = true + b.nilRunResult = false + artifact, err = bClient.Run(ui, hook, cache) + assert.Nil(artifact, "should be nil") + assert.NotNil(err, "should have error") // Test Cancel bClient.Cancel()