From 4a34ec7b367e1abd295a32bce8b96db39198322b Mon Sep 17 00:00:00 2001 From: Halvard Skogsrud Date: Fri, 3 Dec 2021 17:16:12 +1100 Subject: [PATCH] fix(ko): Do not print image name to stdout With this change, the `ko` builder no longer prints the image name to `stdout` by default. Originally, this was added to the `ko` builder to mimic the behavior of the `ko` CLI. Other Skaffold image builders do not print the image name in this way. The reason this is useful for existing `ko` CLI users is that some documented workflows rely on capturing the image name from `stdout`, see https://github.com/google/ko/blob/v0.9.3/README.md#build-an-image After some investigation, the better option seems to be to rely on the existing `--quiet` and `--output` Skaffold flags to format the output of the image name(s). This change also updates the `ko` builder documentation to show existing `ko` CLI users how to capture the image name by providing an appropriate Go template to `--output`. Fixes: #6835 Closes: #6836 Tracking: #6041 --- .../en/docs/pipeline-stages/builders/ko.md | 26 ++++++++++++++++--- integration/ko_test.go | 12 +-------- pkg/skaffold/build/ko/build.go | 1 - pkg/skaffold/build/ko/build_test.go | 20 +++++--------- 4 files changed, 31 insertions(+), 28 deletions(-) diff --git a/docs/content/en/docs/pipeline-stages/builders/ko.md b/docs/content/en/docs/pipeline-stages/builders/ko.md index c5969414676..0ba5656a6c6 100644 --- a/docs/content/en/docs/pipeline-stages/builders/ko.md +++ b/docs/content/en/docs/pipeline-stages/builders/ko.md @@ -213,6 +213,11 @@ Useful tips for existing `ko` users: [`default-repo` functionality]({{< relref "/docs/environment/image-registries" >}}). The ko builder does _not_ read the `KO_DOCKER_REPO` environment variable. +- Image naming follows the + [Skaffold image naming strategy]({{< relref "/docs/environment/image-registries" >}}). + Skaffold removes the `ko://` prefix, if present, before determining the image + name. + - The ko builder supports reading [base image configuration](https://github.com/google/ko#overriding-base-images) from the `.ko.yaml` file. If you already configure your base images using @@ -242,8 +247,6 @@ Useful tips for existing `ko` users: cat << EOF >> skaffold.yaml local: concurrency: 0 - tagPolicy: - sha256: {} deploy: kubectl: manifests: @@ -279,6 +282,23 @@ To achieve the same using Skaffold's ko builder, use the `flags` field in skaffold build ``` +#### Capturing image name from `stdout` + +If you want Skaffold to print out the full image name and digest (and nothing +else) to `stdout`, similar to what `ko build` does, use the +[`--quiet` and `--output` flags]({{< relref "/docs/references/cli#skaffold-build" >}}). +These flags enable you to capture the full image references in an environment +variable or redirect to a file, e.g.: + +```shell +skaffold build --quiet --output='{{range .Builds}}{{.Tag}}{{end}}' > out.txt +``` + +Note that Skaffold produces a JSON file with the image names if you run +`skaffold build` with the `--file-output` flag. You can then use this flag as +input to `skaffold render` to render Kubernetes manifests. For details on how +to do this, see the next section. + #### Rendering Kubernetes manifests When you use the Skaffold ko builder, Skaffold takes care of replacing the @@ -311,7 +331,7 @@ Or you can perform the action as two steps: first `build` the images, then ```shell skaffold build --file-output artifacts.json --push -skaffold render --build-artifacts artifacts.json --offline --output out.yaml +skaffold render --build-artifacts artifacts.json --digest-source none --offline --output out.yaml ``` Specify the location of your Kubernetes manifests in `skaffold.yaml`: diff --git a/integration/ko_test.go b/integration/ko_test.go index a35b6a454ee..d4d43169171 100644 --- a/integration/ko_test.go +++ b/integration/ko_test.go @@ -17,7 +17,6 @@ limitations under the License. package integration import ( - "bytes" "context" "fmt" "io/ioutil" @@ -25,10 +24,8 @@ import ( "net/http/httptest" "path/filepath" "runtime" - "strings" "testing" - "github.com/google/go-cmp/cmp" "github.com/google/go-containerregistry/pkg/crane" "github.com/google/go-containerregistry/pkg/registry" "github.com/google/go-containerregistry/pkg/v1/random" @@ -60,7 +57,6 @@ func TestBuildAndPushKoImageProgrammatically(t *testing.T) { // Build the artifact b := ko.NewArtifactBuilder(nil, true, config.RunModes.Build, nil) - var imageFullNameBuffer bytes.Buffer artifact := &latestV1.Artifact{ ArtifactType: latestV1.ArtifactType{ KoArtifact: &latestV1.KoArtifact{ @@ -70,16 +66,10 @@ func TestBuildAndPushKoImageProgrammatically(t *testing.T) { Workspace: exampleAppDir, } imageName := fmt.Sprintf("%s/%s", registryAddr, "skaffold-ko") - digest, err := b.Build(context.Background(), &imageFullNameBuffer, artifact, imageName) + _, err = b.Build(context.Background(), nil, artifact, imageName) if err != nil { t.Fatalf("b.Build(): %+v", err) } - - wantImageFullName := fmt.Sprintf("%s@%s", imageName, digest) - gotImageFullName := strings.TrimSuffix(imageFullNameBuffer.String(), "\n") - if diff := cmp.Diff(wantImageFullName, gotImageFullName); diff != "" { - t.Errorf("image name mismatch (-want +got):\n%s", diff) - } } // registryServerWithImage starts a local registry and pushes a random image. diff --git a/pkg/skaffold/build/ko/build.go b/pkg/skaffold/build/ko/build.go index 7d826d9254f..6417bb0d521 100644 --- a/pkg/skaffold/build/ko/build.go +++ b/pkg/skaffold/build/ko/build.go @@ -52,7 +52,6 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, a *latestV1.Artifact if err != nil { return "", fmt.Errorf("could not build and publish ko image %q: %w", a.ImageName, err) } - fmt.Fprintln(out, imageRef.Name()) return b.getImageIdentifier(ctx, imageRef, ref) } diff --git a/pkg/skaffold/build/ko/build_test.go b/pkg/skaffold/build/ko/build_test.go index 30c5c87b345..54a325cbd54 100644 --- a/pkg/skaffold/build/ko/build_test.go +++ b/pkg/skaffold/build/ko/build_test.go @@ -17,9 +17,7 @@ limitations under the License. package ko import ( - "bytes" "context" - "strings" "testing" "github.com/docker/docker/client" @@ -34,32 +32,32 @@ import ( ) // TestBuild doesn't actually build (or publish) any container images, because -// it's a unit test. Instead, it only verifies that the Build() function prints -// the image name to the out io.Writer and returns the image identifier. +// it's a unit test. Instead, it only verifies that the Build() returns the +// image identifier. func TestBuild(t *testing.T) { tests := []struct { description string pushImages bool - expectedRef string + imageRef string expectedImageIdentifier string }{ { description: "pushed image with tag", pushImages: true, - expectedRef: "registry.example.com/repo/image1:tag1", + imageRef: "registry.example.com/repo/image1:tag1", expectedImageIdentifier: "tag1", }, { description: "sideloaded image", pushImages: false, - expectedRef: "registry.example.com/repo/image2:any", + imageRef: "registry.example.com/repo/image2:any", expectedImageIdentifier: "ab737430e80b", }, } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { importPath := "ko://github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/ko" // this package - b := stubKoArtifactBuilder(test.expectedRef, test.expectedImageIdentifier, test.pushImages, importPath) + b := stubKoArtifactBuilder(test.imageRef, test.expectedImageIdentifier, test.pushImages, importPath) artifact := &latestV1.Artifact{ ArtifactType: latestV1.ArtifactType{ @@ -67,12 +65,8 @@ func TestBuild(t *testing.T) { }, ImageName: importPath, } - var outBuffer bytes.Buffer - gotImageIdentifier, err := b.Build(context.Background(), &outBuffer, artifact, test.expectedRef) + gotImageIdentifier, err := b.Build(context.Background(), nil, artifact, test.imageRef) t.CheckNoError(err) - - imageNameOut := strings.TrimSuffix(outBuffer.String(), "\n") - t.CheckDeepEqual(test.expectedRef, imageNameOut) t.CheckDeepEqual(test.expectedImageIdentifier, gotImageIdentifier) }) }