Merge pull request #1482 from dgageot/new-tagging

New tagging mechanism

docs/content/en/docs/how-tos/templating/_index.md

@@ -20,9 +20,5 @@ List of fields that support templating:
 
 List of variables that are available for templating: 
 
-* all environment variables passed to the Skaffold process as startup 
-* `IMAGE_NAME` - the artifacts' image name - the [image name rewriting](/docs/concepts/#image-repository-handling) acts after the template was calculated  
-* `DIGEST` - the image digest calculated by the docker registry after pushing the image 
-* if `DIGEST` is of format `algo:hex`, `DIGEST_ALGO` and `DIGEST_HEX` parts correspond to the parts of the string otherwise `DIGEST_HEX`=`DIGEST` is set
-
-
+* all environment variables passed to the skaffold process as startup 
+* `IMAGE_NAME` - the artifacts' image name - the [image name rewriting](/docs/concepts/#image-repository-handling) acts after the template was calculated

integration/examples/annotated-skaffold.yaml

@@ -19,9 +19,6 @@ build:
     # The template is compiled and executed against the current environment,
     # with those variables injected:
     #   IMAGE_NAME   |  Name of the image being built, as supplied in the artifacts section.
-    #   DIGEST       |  Digest of the newly built image. For eg. `sha256:27ffc7f352665cc50ae3cbcc4b2725e36062f1b38c611b6f95d6df9a7510de23`.
-    #   DIGEST_ALGO  |  Algorithm used by the digest: For eg. `sha256`.
-    #   DIGEST_HEX   |  Digest of the newly built image. For eg. `27ffc7f352665cc50ae3cbcc4b2725e36062f1b38c611b6f95d6df9a7510de23`.
     # Example
     # envTemplate:
     #  template: "{{.RELEASE}}-{{.IMAGE_NAME}}"

pkg/skaffold/build/gcb/cloud_build.go

@@ -29,7 +29,6 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/gcp"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/sources"
@@ -49,6 +48,11 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, tagger tag.Tagger, a
 }
 
 func (b *Builder) buildArtifactWithCloudBuild(ctx context.Context, out io.Writer, tagger tag.Tagger, artifact *latest.Artifact) (string, error) {
+	tag, err := tagger.GenerateFullyQualifiedImageName(artifact.Workspace, artifact.ImageName)
+	if err != nil {
+		return "", errors.Wrap(err, "generating tag")
+	}
+
 	client, err := google.DefaultClient(ctx, cloudbuild.CloudPlatformScope)
 	if err != nil {
 		return "", errors.Wrap(err, "getting google client")
@@ -86,7 +90,7 @@ func (b *Builder) buildArtifactWithCloudBuild(ctx context.Context, out io.Writer
 		return "", errors.Wrap(err, "checking bucket is in correct project")
 	}
 
-	desc, err := b.buildDescription(artifact, cbBucket, buildObject)
+	desc, err := b.buildDescription(artifact, tag, cbBucket, buildObject)
 	if err != nil {
 		return "", errors.Wrap(err, "could not create build description")
 	}
@@ -152,22 +156,8 @@ watch:
 		return "", errors.Wrap(err, "cleaning up source tar after build")
 	}
 	logrus.Infof("Deleted object %s", buildObject)
-	builtTag := fmt.Sprintf("%s@%s", artifact.ImageName, digest)
-	logrus.Infof("Image built at %s", builtTag)
-
-	newTag, err := tagger.GenerateFullyQualifiedImageName(artifact.Workspace, tag.Options{
-		ImageName: artifact.ImageName,
-		Digest:    digest,
-	})
-	if err != nil {
-		return "", errors.Wrap(err, "generating tag")
-	}
-
-	if err := docker.AddTag(builtTag, newTag); err != nil {
-		return "", errors.Wrap(err, "tagging image")
-	}
 
-	return newTag, nil
+	return tag + "@" + digest, nil
 }
 
 func getBuildID(op *cloudbuild.Operation) (string, error) {

pkg/skaffold/build/gcb/desc.go

@@ -24,8 +24,8 @@ import (
 	cloudbuild "google.golang.org/api/cloudbuild/v1"
 )
 
-func (b *Builder) buildDescription(artifact *latest.Artifact, bucket, object string) (*cloudbuild.Build, error) {
-	steps, err := b.buildSteps(artifact)
+func (b *Builder) buildDescription(artifact *latest.Artifact, tag, bucket, object string) (*cloudbuild.Build, error) {
+	steps, err := b.buildSteps(artifact, tag)
 	if err != nil {
 		return nil, err
 	}
@@ -39,7 +39,7 @@ func (b *Builder) buildDescription(artifact *latest.Artifact, bucket, object str
 			},
 		},
 		Steps:  steps,
-		Images: []string{artifact.ImageName},
+		Images: []string{tag},
 		Options: &cloudbuild.BuildOptions{
 			DiskSizeGb:  b.DiskSizeGb,
 			MachineType: b.MachineType,
@@ -48,19 +48,19 @@ func (b *Builder) buildDescription(artifact *latest.Artifact, bucket, object str
 	}, nil
 }
 
-func (b *Builder) buildSteps(artifact *latest.Artifact) ([]*cloudbuild.BuildStep, error) {
+func (b *Builder) buildSteps(artifact *latest.Artifact, tag string) ([]*cloudbuild.BuildStep, error) {
 	switch {
 	case artifact.DockerArtifact != nil:
-		return b.dockerBuildSteps(artifact.ImageName, artifact.DockerArtifact), nil
+		return b.dockerBuildSteps(artifact.DockerArtifact, tag), nil
 
 	case artifact.BazelArtifact != nil:
 		return nil, errors.New("skaffold can't build a bazel artifact with Google Cloud Build")
 
 	case artifact.JibMavenArtifact != nil:
-		return b.jibMavenBuildSteps(artifact.ImageName, artifact.JibMavenArtifact), nil
+		return b.jibMavenBuildSteps(artifact.JibMavenArtifact, tag), nil
 
 	case artifact.JibGradleArtifact != nil:
-		return b.jibGradleBuildSteps(artifact.ImageName, artifact.JibGradleArtifact), nil
+		return b.jibGradleBuildSteps(artifact.JibGradleArtifact, tag), nil
 
 	default:
 		return nil, fmt.Errorf("undefined artifact type: %+v", artifact.ArtifactType)

pkg/skaffold/build/gcb/desc_test.go

@@ -33,7 +33,7 @@ func TestBuildBazelDescriptionFail(t *testing.T) {
 	builder := Builder{
 		GoogleCloudBuild: &latest.GoogleCloudBuild{},
 	}
-	_, err := builder.buildDescription(artifact, "bucket", "object")
+	_, err := builder.buildDescription(artifact, "tag", "bucket", "object")
 
 	testutil.CheckError(t, true, err)
 }

pkg/skaffold/build/gcb/docker.go

@@ -22,7 +22,7 @@ import (
 	cloudbuild "google.golang.org/api/cloudbuild/v1"
 )
 
-func (b *Builder) dockerBuildSteps(imageName string, artifact *latest.DockerArtifact) []*cloudbuild.BuildStep {
+func (b *Builder) dockerBuildSteps(artifact *latest.DockerArtifact, tag string) []*cloudbuild.BuildStep {
 	var steps []*cloudbuild.BuildStep
 
 	for _, cacheFrom := range artifact.CacheFrom {
@@ -32,7 +32,7 @@ func (b *Builder) dockerBuildSteps(imageName string, artifact *latest.DockerArti
 		})
 	}
 
-	args := append([]string{"build", "--tag", imageName, "-f", artifact.DockerfilePath})
+	args := append([]string{"build", "--tag", tag, "-f", artifact.DockerfilePath})
 	args = append(args, docker.GetBuildArgs(artifact)...)
 	args = append(args, ".")
 

pkg/skaffold/build/gcb/docker_test.go

@@ -27,7 +27,6 @@ import (
 
 func TestDockerBuildDescription(t *testing.T) {
 	artifact := &latest.Artifact{
-		ImageName: "nginx",
 		ArtifactType: latest.ArtifactType{
 			DockerArtifact: &latest.DockerArtifact{
 				DockerfilePath: "Dockerfile",
@@ -47,7 +46,7 @@ func TestDockerBuildDescription(t *testing.T) {
 			Timeout:     "10m",
 		},
 	}
-	desc, err := builder.buildDescription(artifact, "bucket", "object")
+	desc, err := builder.buildDescription(artifact, "nginx", "bucket", "object")
 
 	expected := cloudbuild.Build{
 		LogsBucket: "bucket",
@@ -61,7 +60,7 @@ func TestDockerBuildDescription(t *testing.T) {
 			Name: "docker/docker",
 			Args: []string{"build", "--tag", "nginx", "-f", "Dockerfile", "--build-arg", "arg1=value1", "--build-arg", "arg2", "."},
 		}},
-		Images: []string{artifact.ImageName},
+		Images: []string{"nginx"},
 		Options: &cloudbuild.BuildOptions{
 			DiskSizeGb:  100,
 			MachineType: "n1-standard-1",
@@ -83,7 +82,7 @@ func TestPullCacheFrom(t *testing.T) {
 			DockerImage: "docker/docker",
 		},
 	}
-	steps := builder.dockerBuildSteps("nginx2", artifact)
+	steps := builder.dockerBuildSteps(artifact, "nginx2")
 
 	expected := []*cloudbuild.BuildStep{{
 		Name: "docker/docker",

pkg/skaffold/build/gcb/jib.go

@@ -23,16 +23,16 @@ import (
 )
 
 // TODO(dgageot): check that `package` is bound to `jib:build`
-func (b *Builder) jibMavenBuildSteps(imageName string, artifact *latest.JibMavenArtifact) []*cloudbuild.BuildStep {
+func (b *Builder) jibMavenBuildSteps(artifact *latest.JibMavenArtifact, tag string) []*cloudbuild.BuildStep {
 	return []*cloudbuild.BuildStep{{
 		Name: b.MavenImage,
-		Args: jib.GenerateMavenArgs("dockerBuild", imageName, artifact, b.skipTests),
+		Args: jib.GenerateMavenArgs("dockerBuild", tag, artifact, b.skipTests),
 	}}
 }
 
-func (b *Builder) jibGradleBuildSteps(imageName string, artifact *latest.JibGradleArtifact) []*cloudbuild.BuildStep {
+func (b *Builder) jibGradleBuildSteps(artifact *latest.JibGradleArtifact, tag string) []*cloudbuild.BuildStep {
 	return []*cloudbuild.BuildStep{{
 		Name: b.GradleImage,
-		Args: jib.GenerateGradleArgs("jibDockerBuild", imageName, artifact, b.skipTests),
+		Args: jib.GenerateGradleArgs("jibDockerBuild", tag, artifact, b.skipTests),
 	}}
 }

pkg/skaffold/build/gcb/jib_test.go

@@ -41,7 +41,7 @@ func TestJibMavenBuildSteps(t *testing.T) {
 			},
 			skipTests: tt.skipTests,
 		}
-		steps := builder.jibMavenBuildSteps("img", artifact)
+		steps := builder.jibMavenBuildSteps(artifact, "img")
 
 		expected := []*cloudbuild.BuildStep{{
 			Name: "maven:3.6.0",
@@ -69,7 +69,7 @@ func TestJibGradleBuildSteps(t *testing.T) {
 			},
 			skipTests: tt.skipTests,
 		}
-		steps := builder.jibGradleBuildSteps("img", artifact)
+		steps := builder.jibGradleBuildSteps(artifact, "img")
 
 		expected := []*cloudbuild.BuildStep{{
 			Name: "gradle:5.1.1",

pkg/skaffold/build/kaniko/kaniko.go

@@ -22,7 +22,6 @@ import (
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/pkg/errors"
 )
@@ -47,27 +46,15 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, tagger tag.Tagger, a
 }
 
 func (b *Builder) buildArtifactWithKaniko(ctx context.Context, out io.Writer, tagger tag.Tagger, artifact *latest.Artifact) (string, error) {
-	initialTag, err := b.run(ctx, out, artifact)
-	if err != nil {
-		return "", errors.Wrapf(err, "kaniko build for [%s]", artifact.ImageName)
-	}
-
-	digest, err := docker.RemoteDigest(initialTag)
-	if err != nil {
-		return "", errors.Wrap(err, "getting digest")
-	}
-
-	tag, err := tagger.GenerateFullyQualifiedImageName(artifact.Workspace, tag.Options{
-		ImageName: artifact.ImageName,
-		Digest:    digest,
-	})
+	tag, err := tagger.GenerateFullyQualifiedImageName(artifact.Workspace, artifact.ImageName)
 	if err != nil {
 		return "", errors.Wrap(err, "generating tag")
 	}
 
-	if err := docker.AddTag(initialTag, tag); err != nil {
-		return "", errors.Wrap(err, "tagging image")
+	digest, err := b.run(ctx, out, artifact, tag)
+	if err != nil {
+		return "", errors.Wrapf(err, "kaniko build for [%s]", artifact.ImageName)
 	}
 
-	return tag, nil
+	return tag + "@" + digest, nil
 }

pkg/skaffold/build/kaniko/run.go

@@ -31,17 +31,14 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func (b *Builder) run(ctx context.Context, out io.Writer, artifact *latest.Artifact) (string, error) {
+func (b *Builder) run(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, error) {
 	if artifact.DockerArtifact == nil {
 		return "", errors.New("kaniko builder supports only Docker artifacts")
 	}
 
-	initialTag := util.RandomID()
-	imageDst := fmt.Sprintf("%s:%s", artifact.ImageName, initialTag)
-
 	// Prepare context
 	s := sources.Retrieve(b.KanikoBuild)
-	context, err := s.Setup(ctx, out, artifact, initialTag)
+	context, err := s.Setup(ctx, out, artifact, util.RandomID())
 	if err != nil {
 		return "", errors.Wrap(err, "setting up build context")
 	}
@@ -51,7 +48,7 @@ func (b *Builder) run(ctx context.Context, out io.Writer, artifact *latest.Artif
 	args := []string{
 		"--dockerfile", artifact.DockerArtifact.DockerfilePath,
 		"--context", context,
-		"--destination", imageDst,
+		"--destination", tag,
 		"-v", logLevel().String()}
 	args = append(args, b.AdditionalFlags...)
 	args = append(args, docker.GetBuildArgs(artifact.DockerArtifact)...)
@@ -96,5 +93,5 @@ func (b *Builder) run(ctx context.Context, out io.Writer, artifact *latest.Artif
 
 	waitForLogs()
 
-	return imageDst, nil
+	return docker.RemoteDigest(tag)
 }

pkg/skaffold/build/local/bazel.go

@@ -36,10 +36,10 @@ import (
 	"github.com/pkg/errors"
 )
 
-func (b *Builder) buildBazel(ctx context.Context, out io.Writer, workspace string, a *latest.Artifact) (string, error) {
+func (b *Builder) buildBazel(ctx context.Context, out io.Writer, workspace string, a *latest.BazelArtifact, tag string) (string, error) {
 	args := []string{"build"}
-	args = append(args, a.BazelArtifact.BuildArgs...)
-	args = append(args, a.BazelArtifact.BuildTarget)
+	args = append(args, a.BuildArgs...)
+	args = append(args, a.BuildTarget)
 
 	// FIXME: is it possible to apply b.skipTests?
 	cmd := exec.CommandContext(ctx, "bazel", args...)
@@ -50,20 +50,18 @@ func (b *Builder) buildBazel(ctx context.Context, out io.Writer, workspace strin
 		return "", errors.Wrap(err, "running command")
 	}
 
-	bazelBin, err := bazelBin(ctx, workspace, a.BazelArtifact)
+	bazelBin, err := bazelBin(ctx, workspace, a)
 	if err != nil {
 		return "", errors.Wrap(err, "getting path of bazel-bin")
 	}
 
-	tarPath := filepath.Join(bazelBin, buildTarPath(a.BazelArtifact.BuildTarget))
+	tarPath := filepath.Join(bazelBin, buildTarPath(a.BuildTarget))
 
 	if b.pushImages {
-		uniqueTag := a.ImageName + ":" + util.RandomID()
-		return pushImage(tarPath, uniqueTag)
+		return pushImage(tarPath, tag)
 	}
 
-	ref := buildImageTag(a.BazelArtifact.BuildTarget)
-	return b.loadImage(ctx, out, tarPath, ref)
+	return b.loadImage(ctx, out, tarPath, a, tag)
 }
 
 func pushImage(tarPath, tag string) (string, error) {
@@ -89,18 +87,23 @@ func pushImage(tarPath, tag string) (string, error) {
 	return docker.RemoteDigest(tag)
 }
 
-func (b *Builder) loadImage(ctx context.Context, out io.Writer, tarPath string, ref string) (string, error) {
+func (b *Builder) loadImage(ctx context.Context, out io.Writer, tarPath string, a *latest.BazelArtifact, tag string) (string, error) {
 	imageTar, err := os.Open(tarPath)
 	if err != nil {
 		return "", errors.Wrap(err, "opening image tarball")
 	}
 	defer imageTar.Close()
 
-	imageID, err := b.localDocker.Load(ctx, out, imageTar, ref)
+	bazelTag := buildImageTag(a.BuildTarget)
+	imageID, err := b.localDocker.Load(ctx, out, imageTar, bazelTag)
 	if err != nil {
 		return "", errors.Wrap(err, "loading image into docker daemon")
 	}
 
+	if err := b.localDocker.Tag(ctx, imageID, tag); err != nil {
+		return "", errors.Wrap(err, "tagging the image")
+	}
+
 	return imageID, nil
 }