adding tests

Makefile

@@ -28,19 +28,39 @@ githooks:
 	echo 'make fmt; git add `git diff --name-only --cached`' > .git/hooks/pre-commit
 	chmod +x .git/hooks/pre-commit
 
-mockgen:
+_mockgen:
 	mockgen \
 		-destination pkg/test-mocks/config/resolver.go \
-		-package config github.com/replicatedcom/ship/pkg/lifecycle/render/config \
+		-package config \
+		github.com/replicatedcom/ship/pkg/lifecycle/render/config \
 		Resolver
 	mockgen \
 		-destination pkg/test-mocks/config/daemon.go \
-		-package config github.com/replicatedcom/ship/pkg/lifecycle/render/config \
+		-package config \
+		github.com/replicatedcom/ship/pkg/lifecycle/render/config \
 		Daemon
 	mockgen \
 		-destination pkg/test-mocks/planner/planner_mock.go \
-		-package planner github.com/replicatedcom/ship/pkg/lifecycle/render/planner \
+		-package planner \
+		github.com/replicatedcom/ship/pkg/lifecycle/render/planner \
 		Planner
+	mockgen \
+		-destination pkg/test-mocks/docker/image_saver_mock.go \
+		-package docker \
+		github.com/replicatedcom/ship/pkg/lifecycle/render/docker \
+		ImageSaver
+	mockgen \
+		-destination pkg/test-mocks/docker/image_manager_mock.go \
+		-package docker \
+		github.com/replicatedcom/ship/pkg/lifecycle/render/docker \
+		ImageManager
+	mockgen \
+		-destination pkg/test-mocks/docker/pull_url_resovler_mock.go \
+		-package docker \
+		github.com/replicatedcom/ship/pkg/lifecycle/render/docker \
+		PullURLResolver
+
+mockgen: _mockgen fmt
 
 dep:
 	dep ensure

Millfile

@@ -0,0 +1,5 @@
+### Configs
+watch("**", "!.git/**")
+
+### Commands to execute
+sh("make _test | grep -B 10 FAIL || :")

pkg/lifecycle/render/docker/image.go

@@ -8,11 +8,34 @@ import (
 	"github.com/docker/distribution/reference"
 	"github.com/pkg/errors"
 
+	"github.com/go-kit/kit/log"
+	"github.com/go-kit/kit/log/level"
 	"github.com/replicatedcom/ship/pkg/api"
+	"github.com/replicatedcom/ship/pkg/logger"
+	"github.com/spf13/viper"
 )
 
-func ResolvePullUrl(asset *api.DockerAsset, meta api.ReleaseMetadata) (string, error) {
+type PullURLResolver interface {
+	ResolvePullURL(asset *api.DockerAsset, meta api.ReleaseMetadata) (string, error)
+}
+
+var _ PullURLResolver = &URLResolver{}
+
+type URLResolver struct {
+	Logger log.Logger
+}
+
+func URLResolverFromViper(v *viper.Viper) PullURLResolver {
+	return &URLResolver{
+		Logger: logger.FromViper(v),
+	}
+}
+
+func (r *URLResolver) ResolvePullURL(asset *api.DockerAsset, meta api.ReleaseMetadata) (string, error) {
+	debug := level.Debug(r.Logger)
+
 	if asset.Source == "replicated" || asset.Source == "public" || asset.Source == "" {
+		debug.Log("event", "image.rewrite.skip", "source", asset.Source)
 		return asset.Image, nil
 	}
 

pkg/lifecycle/render/docker/image_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/replicatedcom/ship/pkg/api"
+	"github.com/replicatedcom/ship/pkg/test-mocks/logger"
 )
 
 func TestResolveImageName(t *testing.T) {
@@ -90,31 +91,31 @@ func TestResolvePullUrl(t *testing.T) {
 		ExpectUrl string
 	}
 	cases := []testcase{
-		testcase{
+		{
 			Name: "replicated private image",
 			Asset: api.DockerAsset{
 				Image:  "registry.replicated.com/library/retraced-api:1.1.12-slim-20180329",
 				Source: "replicated",
 			},
 			ExpectUrl: "registry.replicated.com/library/retraced-api:1.1.12-slim-20180329",
 		},
-		testcase{
+		{
 			Name: "public image with host name",
 			Asset: api.DockerAsset{
 				Image:  "quay.io/awesome/redis:1.1",
 				Source: "public",
 			},
 			ExpectUrl: "quay.io/awesome/redis:1.1",
 		},
-		testcase{
+		{
 			Name: "private proxied image without host name",
 			Asset: api.DockerAsset{
 				Image:  "replicated/www:3",
 				Source: "dockerhub",
 			},
 			ExpectUrl: fmt.Sprintf("%s/awesomeapp/jjzpr9u62gaz2.www:3", replicatedRegistry()),
 		},
-		testcase{
+		{
 			Name: "private proxied image with host name",
 			Asset: api.DockerAsset{
 				Image:  "quay.io/redacted/chatops:f3c689e",
@@ -125,13 +126,13 @@ func TestResolvePullUrl(t *testing.T) {
 	}
 	meta := api.ReleaseMetadata{
 		Images: []api.Image{
-			api.Image{
+			{
 				URL:      "replicated/www:3",
 				Source:   "dockerhub",
 				AppSlug:  "awesomeapp",
 				ImageKey: "jjzpr9u62gaz2",
 			},
-			api.Image{
+			{
 				URL:      "quay.io/redacted/chatops:f3c689e",
 				Source:   "quayio",
 				AppSlug:  "awesomeapp",
@@ -142,7 +143,8 @@ func TestResolvePullUrl(t *testing.T) {
 
 	for _, test := range cases {
 		t.Run(test.Name, func(t *testing.T) {
-			url, err := ResolvePullUrl(&test.Asset, meta)
+			r := &URLResolver{Logger: &logger.TestLogger{T: t}}
+			url, err := r.ResolvePullURL(&test.Asset, meta)
 			require.New(t).NoError(err)
 			require.New(t).Equal(test.ExpectUrl, url)
 		})

pkg/lifecycle/render/docker/save.go

@@ -10,48 +10,76 @@ import (
 	"github.com/pkg/errors"
 
 	"github.com/docker/docker/api/types"
-	"github.com/docker/docker/client"
+	docker "github.com/docker/docker/client"
 	"github.com/go-kit/kit/log"
 	"github.com/go-kit/kit/log/level"
+	"github.com/replicatedcom/ship/pkg/logger"
+	"github.com/spf13/viper"
 )
 
+// ImageSaver saves an image
+type ImageSaver interface {
+	SaveImage(ctx context.Context, opts SaveOpts) chan interface{}
+}
+
+var _ ImageManager = &docker.Client{}
+
+// ImageManager represents a subset of the docker client interface
+type ImageManager interface {
+	ImagePull(ctx context.Context, refStr string, options types.ImagePullOptions) (io.ReadCloser, error)
+	ImageTag(ctx context.Context, source, target string) error
+	ImageSave(ctx context.Context, imageIDs []string) (io.ReadCloser, error)
+}
+
 type SaveOpts struct {
 	PullUrl   string
 	SaveUrl   string
 	IsPrivate bool
 	Filename  string
 	Username  string
 	Password  string
-	Logger    log.Logger
 }
 
-func SaveImage(ctx context.Context, saveOpts SaveOpts) chan interface{} {
+var _ ImageSaver = &DockerSaver{}
+
+// DockerSaver implementes ImageSaver via a docker client
+type DockerSaver struct {
+	Logger log.Logger
+	client ImageManager
+}
+
+func SaverFromViper(v *viper.Viper) (*DockerSaver, error) {
+	client, err := docker.NewEnvClient()
+	if err != nil {
+		return nil, errors.Wrap(err, "initialize docker client")
+	}
+
+	return &DockerSaver{
+		Logger: logger.FromViper(v),
+		client: client,
+	}, nil
+}
+
+func (s *DockerSaver) SaveImage(ctx context.Context, saveOpts SaveOpts) chan interface{} {
 	ch := make(chan interface{})
 	go func() {
 		defer close(ch)
-		if err := saveImage(ctx, saveOpts, ch); err != nil {
+		if err := s.saveImage(ctx, saveOpts, ch); err != nil {
 			ch <- err
 		}
 	}()
 	return ch
 }
 
-func saveImage(ctx context.Context, saveOpts SaveOpts, progressCh chan interface{}) error {
-	debug := level.Debug(log.With(saveOpts.Logger, "method", "saveImage", "image", saveOpts.SaveUrl))
+func (s *DockerSaver) saveImage(ctx context.Context, saveOpts SaveOpts, progressCh chan interface{}) error {
+	debug := level.Debug(log.With(s.Logger, "method", "saveImage", "image", saveOpts.SaveUrl))
 
 	authOpts := types.AuthConfig{}
 	if saveOpts.IsPrivate {
 		authOpts.Username = saveOpts.Username
 		authOpts.Password = saveOpts.Password
 	}
 
-	debug.Log("stage", "create.client")
-
-	cli, err := client.NewEnvClient()
-	if err != nil {
-		return errors.Wrapf(err, "create docker client")
-	}
-
 	debug.Log("stage", "make.auth")
 
 	authString, err := makeAuthValue(authOpts)
@@ -64,15 +92,15 @@ func saveImage(ctx context.Context, saveOpts SaveOpts, progressCh chan interface
 	pullOpts := types.ImagePullOptions{
 		RegistryAuth: authString,
 	}
-	progressReader, err := cli.ImagePull(ctx, saveOpts.PullUrl, pullOpts)
+	progressReader, err := s.client.ImagePull(ctx, saveOpts.PullUrl, pullOpts)
 	if err != nil {
 		return errors.Wrapf(err, "pull image %s", saveOpts.PullUrl)
 	}
 	copyDockerProgress(progressReader, progressCh)
 
 	if saveOpts.PullUrl != saveOpts.SaveUrl {
 		debug.Log("stage", "tag", "old.tag", saveOpts.PullUrl, "new.tag", saveOpts.SaveUrl)
-		err := cli.ImageTag(ctx, saveOpts.PullUrl, saveOpts.SaveUrl)
+		err := s.client.ImageTag(ctx, saveOpts.PullUrl, saveOpts.SaveUrl)
 		if err != nil {
 			return errors.Wrapf(err, "tag image %s -> %s", saveOpts.PullUrl, saveOpts.SaveUrl)
 		}
@@ -88,7 +116,7 @@ func saveImage(ctx context.Context, saveOpts SaveOpts, progressCh chan interface
 
 	debug.Log("stage", "save")
 
-	imageReader, err := cli.ImageSave(ctx, []string{saveOpts.SaveUrl})
+	imageReader, err := s.client.ImageSave(ctx, []string{saveOpts.SaveUrl})
 	if err != nil {
 		return errors.Wrapf(err, "save image %s", saveOpts.SaveUrl)
 	}

pkg/lifecycle/render/planner/build.go

@@ -25,6 +25,7 @@ type buildProgress struct {
 
 // Build builds a plan in memory from assets+resolved config
 func (p *CLIPlanner) Build(assets []api.Asset, configGroups []libyaml.ConfigGroup, meta api.ReleaseMetadata, templateContext map[string]interface{}) Plan {
+
 	defer p.Daemon.ClearProgress()
 
 	debug := level.Debug(log.With(p.Logger, "step.type", "render", "phase", "plan"))
@@ -43,7 +44,7 @@ func (p *CLIPlanner) Build(assets []api.Asset, configGroups []libyaml.ConfigGrou
 		} else if asset.Docker != nil {
 			asset.Docker.Dest = filepath.Join("installer", asset.Docker.Dest)
 			debug.Log("event", "asset.resolve", "asset.type", "docker")
-			plan = append(plan, p.dockerStep(asset.Docker, meta, templateContext))
+			plan = append(plan, p.dockerStep(asset.Docker, meta))
 		} else {
 			debug.Log("event", "asset.resolve.fail", "asset", fmt.Sprintf("%#v", asset))
 		}
@@ -102,7 +103,7 @@ func (p *CLIPlanner) inlineStep(inline *api.InlineAsset, configGroups []libyaml.
 	}
 }
 
-func (p *CLIPlanner) dockerStep(asset *api.DockerAsset, meta api.ReleaseMetadata, templateContext map[string]interface{}) Step {
+func (p *CLIPlanner) dockerStep(asset *api.DockerAsset, meta api.ReleaseMetadata) Step {
 	debug := level.Debug(log.With(p.Logger, "step.type", "render", "render.phase", "execute", "asset.type", "docker", "dest", asset.Dest, "description", asset.Description))
 	return Step{
 		Dest:        asset.Dest,
@@ -116,23 +117,22 @@ func (p *CLIPlanner) dockerStep(asset *api.DockerAsset, meta api.ReleaseMetadata
 				return errors.Wrapf(err, "write directory to %s", asset.Dest)
 			}
 
-			pullUrl, err := docker.ResolvePullUrl(asset, meta)
+			pullUrl, err := p.URLResolver.ResolvePullURL(asset, meta)
 			if err != nil {
 				return errors.Wrapf(err, "resolve pull url")
 			}
 
 			// first try with registry secret
-			// TODO remove this once registry is updated
+			// TODO remove this once registry is updated to read installation ID
 			registrySecretSaveOpts := docker.SaveOpts{
 				PullUrl:   pullUrl,
 				SaveUrl:   asset.Image,
 				IsPrivate: asset.Source != "public" && asset.Source != "",
 				Filename:  asset.Dest,
 				Username:  meta.CustomerID,
 				Password:  meta.RegistrySecret,
-				Logger:    p.Logger,
 			}
-			ch := docker.SaveImage(ctx, registrySecretSaveOpts)
+			ch := p.Saver.SaveImage(ctx, registrySecretSaveOpts)
 			saveError := p.watchProgress(ch, debug)
 
 			if saveError == nil {
@@ -151,10 +151,9 @@ func (p *CLIPlanner) dockerStep(asset *api.DockerAsset, meta api.ReleaseMetadata
 				Filename:  asset.Dest,
 				Username:  meta.CustomerID,
 				Password:  p.Viper.GetString("installation-id"),
-				Logger:    p.Logger,
 			}
 
-			ch = docker.SaveImage(ctx, installationIdSaveOpts)
+			ch = p.Saver.SaveImage(ctx, installationIdSaveOpts)
 			saveError = p.watchProgress(ch, debug)
 
 			if saveError != nil {