fix problem when "false" values where ignored

and set to default "true" by go-defaults
fixes #135

add couple tests and improve error reporting when pulling error

Author: taraspos

cli/config_test.go

@@ -3,6 +3,7 @@ package cli
 import (
 	"testing"
 
+	defaults "github.com/mcuadros/go-defaults"
 	"github.com/mcuadros/ofelia/core"
 	"github.com/mcuadros/ofelia/middlewares"
 	. "gopkg.in/check.v1"
@@ -36,9 +37,25 @@ func (s *SuiteConfig) TestBuildFromString(c *C) {
 	c.Assert(sh.Jobs, HasLen, 5)
 }
 
+func (s *SuiteConfig) TestJobDefaultsSet(c *C) {
+	j := &RunJobConfig{}
+	j.Pull = "false"
+
+	defaults.SetDefaults(j)
+
+	c.Assert(j.Pull, Equals, "false")
+}
+
+func (s *SuiteConfig) TestJobDefaultsNotSet(c *C) {
+	j := &RunJobConfig{}
+
+	defaults.SetDefaults(j)
+
+	c.Assert(j.Pull, Equals, "true")
+}
+
 func (s *SuiteConfig) TestExecJobBuildEmpty(c *C) {
 	j := &ExecJobConfig{}
-	j.buildMiddlewares()
 
 	c.Assert(j.Middlewares(), HasLen, 0)
 }

core/execjob.go

@@ -3,7 +3,7 @@ package core
 import (
 	"fmt"
 
-	"github.com/fsouza/go-dockerclient"
+	docker "github.com/fsouza/go-dockerclient"
 	"github.com/gobs/args"
 )
 

core/runjob.go

@@ -2,6 +2,7 @@ package core
 
 import (
 	"fmt"
+	"strconv"
 	"time"
 
 	docker "github.com/fsouza/go-dockerclient"
@@ -15,12 +16,19 @@ func init() {
 }
 
 type RunJob struct {
-	BareJob   `mapstructure:",squash"`
-	Client    *docker.Client `json:"-"`
-	User      string         `default:"root"`
-	TTY       bool           `default:"false"`
-	Delete    bool           `default:"true"`
-	Pull      bool           `default:"true"`
+	BareJob `mapstructure:",squash"`
+	Client  *docker.Client `json:"-"`
+	User    string         `default:"root"`
+
+	TTY bool `default:"false"`
+
+	// do not use bool values with "default:true" because if
+	// user would set it to "false" explicitly, it still will be
+	// changed to "true" https://github.com/mcuadros/ofelia/issues/135
+	// so lets use strings here as workaround
+	Delete string `default:"true"`
+	Pull   string `default:"true"`
+
 	Image     string
 	Network   string
 	Container string
@@ -34,35 +42,46 @@ func NewRunJob(c *docker.Client) *RunJob {
 func (j *RunJob) Run(ctx *Context) error {
 	var container *docker.Container
 	var err error
+	pull, _ := strconv.ParseBool(j.Pull)
+
 	if j.Image != "" && j.Container == "" {
 		if err = func() error {
-			var err error
+			var pullError error
 
 			// if Pull option "true"
 			// try pulling image first
-			if j.Pull {
-				if err = j.pullImage(); err == nil {
+			if pull {
+				if pullError = j.pullImage(); pullError == nil {
 					ctx.Log("Pulled image " + j.Image)
 					return nil
 				}
 			}
 
 			// if Pull option "false"
-			// try to find image locally
-			if err = j.searchLocalImage(); err == nil {
+			// try to find image locally first
+			searchErr := j.searchLocalImage()
+			if searchErr == nil {
 				ctx.Log("Found locally image " + j.Image)
 				return nil
 			}
 
-			if !j.Pull && err == ErrLocalImageNotFound {
-				// if couldn't find locally, try to pull
-				if err = j.pullImage(); err == nil {
+			// if couldn't find image locally, still try to pull
+			if !pull && searchErr == ErrLocalImageNotFound {
+				if pullError = j.pullImage(); pullError == nil {
 					ctx.Log("Pulled image " + j.Image)
 					return nil
 				}
 			}
 
-			return err
+			if pullError != nil {
+				return pullError
+			}
+
+			if searchErr != nil {
+				return searchErr
+			}
+
+			return nil
 		}(); err != nil {
 			return err
 		}
@@ -216,7 +235,7 @@ func (j *RunJob) watchContainer(containerID string) error {
 }
 
 func (j *RunJob) deleteContainer(containerID string) error {
-	if !j.Delete {
+	if delete, _ := strconv.ParseBool(j.Delete); !delete {
 		return nil
 	}
 

core/runjob_test.go

@@ -39,7 +39,7 @@ func (s *SuiteRunJob) TestRun(c *C) {
 	job.Command = `echo -a "foo bar"`
 	job.User = "foo"
 	job.TTY = true
-	job.Delete = true
+	job.Delete = "true"
 	job.Network = "foo"
 	job.Name = "test"
 

core/runservice.go

@@ -2,6 +2,7 @@ package core
 
 import (
 	"fmt"
+	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -17,7 +18,11 @@ type RunServiceJob struct {
 	Client  *docker.Client `json:"-"`
 	User    string         `default:"root"`
 	TTY     bool           `default:"false"`
-	Delete  bool           `default:"true"`
+	// do not use bool values with "default:true" because if
+	// user would set it to "false" explicitly, it still will be
+	// changed to "true" https://github.com/mcuadros/ofelia/issues/135
+	// so lets use strings here as workaround
+	Delete  string `default:"true"`
 	Image   string
 	Network string
 }
@@ -194,7 +199,7 @@ func (j *RunServiceJob) findtaskstatus(ctx *Context, taskID string) (int, bool)
 }
 
 func (j *RunServiceJob) deleteService(ctx *Context, svcID string) error {
-	if !j.Delete {
+	if delete, _ := strconv.ParseBool(j.Delete); !delete {
 		return nil
 	}
 

core/runservice_test.go

@@ -52,7 +52,7 @@ func (s *SuiteRunServiceJob) TestRun(c *C) {
 	job.Command = `echo -a foo bar`
 	job.User = "foo"
 	job.TTY = true
-	job.Delete = true
+	job.Delete = "true"
 	job.Network = "foo"
 
 	e := NewExecution()