Merge pull request #1428 from dgageot/digest-push-and-build

Get digest on push and imageID on build

pkg/skaffold/build/local/bazel.go

@@ -62,12 +62,12 @@ func (b *Builder) buildBazel(ctx context.Context, out io.Writer, workspace strin
 	}
 	defer resp.Body.Close()
 
-	err = docker.StreamDockerMessages(out, resp.Body)
+	err = docker.StreamDockerMessages(out, resp.Body, nil)
 	if err != nil {
 		return "", errors.Wrap(err, "reading from image load response")
 	}
 
-	return docker.Digest(ctx, b.api, buildImageTag(a.BuildTarget))
+	return docker.ImageID(ctx, b.api, buildImageTag(a.BuildTarget))
 }
 
 func bazelBin(ctx context.Context, workspace string) (string, error) {

pkg/skaffold/build/local/docker.go

@@ -50,11 +50,9 @@ func (b *Builder) buildDocker(ctx context.Context, out io.Writer, workspace stri
 		if err := util.RunCmd(cmd); err != nil {
 			return "", errors.Wrap(err, "running build")
 		}
-	} else {
-		if err := docker.BuildArtifact(ctx, out, b.api, workspace, a, initialTag); err != nil {
-			return "", errors.Wrap(err, "running build")
-		}
+
+		return docker.ImageID(ctx, b.api, initialTag)
 	}
 
-	return docker.Digest(ctx, b.api, initialTag)
+	return docker.Build(ctx, out, b.api, workspace, a, initialTag)
 }

pkg/skaffold/build/local/jib_gradle.go

@@ -44,7 +44,7 @@ func (b *Builder) buildJibGradleToDocker(ctx context.Context, out io.Writer, wor
 		return "", err
 	}
 
-	return docker.Digest(ctx, b.api, skaffoldImage)
+	return docker.ImageID(ctx, b.api, skaffoldImage)
 }
 
 func (b *Builder) buildJibGradleToRegistry(ctx context.Context, out io.Writer, workspace string, artifact *latest.Artifact) (string, error) {

pkg/skaffold/build/local/jib_maven.go

@@ -52,7 +52,7 @@ func (b *Builder) buildJibMavenToDocker(ctx context.Context, out io.Writer, work
 		return "", err
 	}
 
-	return docker.Digest(ctx, b.api, skaffoldImage)
+	return docker.ImageID(ctx, b.api, skaffoldImage)
 }
 
 func (b *Builder) buildJibMavenToRegistry(ctx context.Context, out io.Writer, workspace string, artifact *latest.Artifact) (string, error) {

pkg/skaffold/build/local/local.go

@@ -103,7 +103,7 @@ func (b *Builder) retagAndPush(ctx context.Context, out io.Writer, initialTag st
 	}
 
 	if b.pushImages {
-		if err := docker.RunPush(ctx, out, b.api, newTag); err != nil {
+		if _, err := docker.RunPush(ctx, out, b.api, newTag); err != nil {
 			return errors.Wrap(err, "pushing")
 		}
 	}

pkg/skaffold/docker/image.go

@@ -18,6 +18,7 @@ package docker
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
@@ -39,8 +40,18 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-// BuildArtifact performs a docker build and returns nothing
-func BuildArtifact(ctx context.Context, out io.Writer, cli APIClient, workspace string, a *latest.DockerArtifact, initialTag string) error {
+// PushResult gives the information on an image that has been pushed.
+type PushResult struct {
+	Digest string
+}
+
+// BuildResult gives the information on an image that has been built.
+type BuildResult struct {
+	ID string
+}
+
+// Build performs a docker build and returns the imageID.
+func Build(ctx context.Context, out io.Writer, cli APIClient, workspace string, a *latest.DockerArtifact, initialTag string) (string, error) {
 	logrus.Debugf("Running docker build: context: %s, dockerfile: %s", workspace, a.DockerfilePath)
 
 	// Like `docker build`, we ignore the errors
@@ -69,33 +80,90 @@ func BuildArtifact(ctx context.Context, out io.Writer, cli APIClient, workspace
 		Target:      a.Target,
 	})
 	if err != nil {
-		return errors.Wrap(err, "docker build")
+		return "", errors.Wrap(err, "docker build")
 	}
 	defer resp.Body.Close()
 
-	return StreamDockerMessages(out, resp.Body)
+	var imageID string
+	auxCallback := func(msg jsonmessage.JSONMessage) {
+		if msg.Aux == nil {
+			return
+		}
+
+		var result BuildResult
+		if err := json.Unmarshal(*msg.Aux, &result); err != nil {
+			logrus.Debugln("Unable to parse build output:", err)
+			return
+		}
+		imageID = result.ID
+	}
+
+	if err := StreamDockerMessages(out, resp.Body, auxCallback); err != nil {
+		return "", err
+	}
+
+	if imageID == "" {
+		// Maybe this version of Docker doesn't return the digest of the image
+		// that has been built.
+		imageID, err = ImageID(ctx, cli, initialTag)
+		if err != nil {
+			return "", errors.Wrap(err, "getting digest")
+		}
+	}
+
+	return imageID, nil
 }
 
 // StreamDockerMessages streams formatted json output from the docker daemon
 // TODO(@r2d4): Make this output much better, this is the bare minimum
-func StreamDockerMessages(dst io.Writer, src io.Reader) error {
+func StreamDockerMessages(dst io.Writer, src io.Reader, auxCallback func(jsonmessage.JSONMessage)) error {
 	fd, _ := term.GetFdInfo(dst)
-	return jsonmessage.DisplayJSONMessagesStream(src, dst, fd, false, nil)
+	return jsonmessage.DisplayJSONMessagesStream(src, dst, fd, false, auxCallback)
 }
 
-func RunPush(ctx context.Context, out io.Writer, cli APIClient, ref string) error {
+// RunPush pushes an image reference to a registry. Returns the image digest.
+func RunPush(ctx context.Context, out io.Writer, cli APIClient, ref string) (string, error) {
 	registryAuth, err := encodedRegistryAuth(ctx, cli, DefaultAuthHelper, ref)
 	if err != nil {
-		return errors.Wrapf(err, "getting auth config for %s", ref)
+		return "", errors.Wrapf(err, "getting auth config for %s", ref)
 	}
+
 	rc, err := cli.ImagePush(ctx, ref, types.ImagePushOptions{
 		RegistryAuth: registryAuth,
 	})
 	if err != nil {
-		return errors.Wrap(err, "pushing image to repository")
+		return "", errors.Wrap(err, "pushing image to repository")
 	}
 	defer rc.Close()
-	return StreamDockerMessages(out, rc)
+
+	var digest string
+	auxCallback := func(msg jsonmessage.JSONMessage) {
+		if msg.Aux == nil {
+			return
+		}
+
+		var result PushResult
+		if err := json.Unmarshal(*msg.Aux, &result); err != nil {
+			logrus.Debugln("Unable to parse push output:", err)
+			return
+		}
+		digest = result.Digest
+	}
+
+	if err := StreamDockerMessages(out, rc, auxCallback); err != nil {
+		return "", err
+	}
+
+	if digest == "" {
+		// Maybe this version of Docker doesn't return the digest of the image
+		// that has been pushed.
+		digest, err = RemoteDigest(ref)
+		if err != nil {
+			return "", errors.Wrap(err, "getting digest")
+		}
+	}
+
+	return digest, nil
 }
 
 func AddTag(src, target string) error {
@@ -131,10 +199,8 @@ func addTag(ref name.Reference, targetRef name.Reference, auth authn.Authenticat
 	return remote.Write(targetRef, img, auth, t)
 }
 
-// Digest returns the image digest for a corresponding reference.
-// The digest is of the form
-// sha256:<image_id>
-func Digest(ctx context.Context, cli APIClient, ref string) (string, error) {
+// ImageID returns the image ID for a corresponding reference.
+func ImageID(ctx context.Context, cli APIClient, ref string) (string, error) {
 	image, _, err := cli.ImageInspectWithRaw(ctx, ref)
 	if err != nil {
 		if client.IsErrNotFound(err) {

pkg/skaffold/docker/image_test.go

@@ -38,22 +38,21 @@ func TestMain(m *testing.M) {
 
 func TestRunPush(t *testing.T) {
 	var tests = []struct {
-		description string
-		imageName   string
-		api         testutil.FakeAPIClient
-		shouldErr   bool
+		description    string
+		imageName      string
+		api            testutil.FakeAPIClient
+		expectedDigest string
+		shouldErr      bool
 	}{
 		{
 			description: "push",
 			imageName:   "gcr.io/scratchman",
-		},
-		{
-			description: "no error pushing non canonical tag",
-			imageName:   "noncanonicalscratchman",
-		},
-		{
-			description: "no error pushing canonical tag",
-			imageName:   "canonical/name",
+			api: testutil.FakeAPIClient{
+				TagToImageID: map[string]string{
+					"gcr.io/scratchman": "sha256:abcab",
+				},
+			},
+			expectedDigest: "sha256:abcab",
 		},
 		{
 			description: "stream error",
@@ -74,14 +73,14 @@ func TestRunPush(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
-			err := RunPush(context.Background(), ioutil.Discard, &test.api, test.imageName)
+			digest, err := RunPush(context.Background(), ioutil.Discard, &test.api, test.imageName)
 
-			testutil.CheckError(t, test.shouldErr, err)
+			testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expectedDigest, digest)
 		})
 	}
 }
 
-func TestRunBuildArtifact(t *testing.T) {
+func TestRunBuild(t *testing.T) {
 	var tests = []struct {
 		description string
 		expected    string
@@ -109,24 +108,24 @@ func TestRunBuildArtifact(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
-			err := BuildArtifact(context.Background(), ioutil.Discard, &test.api, ".", &latest.DockerArtifact{}, "finalimage")
+			_, err := Build(context.Background(), ioutil.Discard, &test.api, ".", &latest.DockerArtifact{}, "finalimage")
 
 			testutil.CheckError(t, test.shouldErr, err)
 		})
 	}
 }
 
-func TestDigest(t *testing.T) {
+func TestImageID(t *testing.T) {
 	var tests = []struct {
 		description string
-		imageName   string
+		ref         string
 		api         testutil.FakeAPIClient
 		expected    string
 		shouldErr   bool
 	}{
 		{
 			description: "get digest",
-			imageName:   "identifier:latest",
+			ref:         "identifier:latest",
 			api: testutil.FakeAPIClient{
 				TagToImageID: map[string]string{
 					"identifier:latest": "sha256:123abc",
@@ -136,23 +135,23 @@ func TestDigest(t *testing.T) {
 		},
 		{
 			description: "image inspect error",
-			imageName:   "test",
+			ref:         "test",
 			api: testutil.FakeAPIClient{
 				ErrImageInspect: true,
 			},
 			shouldErr: true,
 		},
 		{
 			description: "not found",
-			imageName:   "somethingelse",
+			ref:         "somethingelse",
 			expected:    "",
 		},
 	}
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
-			digest, err := Digest(context.Background(), &test.api, test.imageName)
+			imageID, err := ImageID(context.Background(), &test.api, test.ref)
 
-			testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, digest)
+			testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, imageID)
 		})
 	}
 }

testutil/fake_image_api.go

@@ -44,11 +44,12 @@ type errReader struct{}
 
 func (f errReader) Read([]byte) (int, error) { return 0, fmt.Errorf("") }
 
-func (f *FakeAPIClient) body() io.ReadCloser {
+func (f *FakeAPIClient) body(digest string) io.ReadCloser {
 	if f.ErrStream {
 		return ioutil.NopCloser(&errReader{})
 	}
-	return ioutil.NopCloser(strings.NewReader(""))
+
+	return ioutil.NopCloser(strings.NewReader(fmt.Sprintf(`{"aux":{"digest":"%s"}}`, digest)))
 }
 
 func (f *FakeAPIClient) ImageBuild(_ context.Context, _ io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) {
@@ -71,7 +72,7 @@ func (f *FakeAPIClient) ImageBuild(_ context.Context, _ io.Reader, options types
 	}
 
 	return types.ImageBuildResponse{
-		Body: f.body(),
+		Body: f.body(imageID),
 	}, nil
 }
 
@@ -109,12 +110,14 @@ func (f *FakeAPIClient) ImageTag(_ context.Context, image, ref string) error {
 	return nil
 }
 
-func (f *FakeAPIClient) ImagePush(context.Context, string, types.ImagePushOptions) (io.ReadCloser, error) {
+func (f *FakeAPIClient) ImagePush(_ context.Context, ref string, _ types.ImagePushOptions) (io.ReadCloser, error) {
 	if f.ErrImagePush {
 		return nil, fmt.Errorf("")
 	}
 
-	return f.body(), nil
+	digest := f.TagToImageID[ref]
+
+	return f.body(digest), nil
 }
 
 func (f *FakeAPIClient) Info(context.Context) (types.Info, error) {