Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vendor pack CLI code to build with Buildpacks #3445

Merged
merged 4 commits into from
Jan 16, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ require (
github.com/aws/aws-sdk-go v1.25.33 // indirect
github.com/blang/semver v3.5.1+incompatible
github.com/bmatcuk/doublestar v1.2.2
github.com/buildpacks/pack v0.6.0
github.com/creack/pty v1.1.9 // indirect
github.com/docker/cli v0.0.0-20191212191748-ebca1413117a
github.com/docker/distribution v2.7.1+incompatible
Expand Down Expand Up @@ -69,8 +70,6 @@ require (
github.com/krishicks/yaml-patch v0.0.10
github.com/markbates/inflect v1.0.4 // indirect
github.com/mattbaird/jsonpatch v0.0.0-20171005235357-81af80346b1a // indirect
github.com/mattn/go-colorable v0.1.4 // indirect
github.com/mattn/go-isatty v0.0.10 // indirect
github.com/mitchellh/go-homedir v1.1.0
github.com/moby/buildkit v0.6.3
github.com/onsi/ginkgo v1.10.3 // indirect
Expand Down
105 changes: 61 additions & 44 deletions go.sum

Large diffs are not rendered by default.

59 changes: 40 additions & 19 deletions pkg/skaffold/build/buildpacks/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,35 @@ package buildpacks

import (
"context"
"io"
"io/ioutil"
"testing"

"github.com/buildpacks/pack"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/testutil"
)

type fakePack struct {
Opts pack.BuildOptions
}

func (f *fakePack) runPack(_ context.Context, _ io.Writer, opts pack.BuildOptions) error {
f.Opts = opts
return nil
}

func TestBuild(t *testing.T) {
tests := []struct {
description string
artifact *latest.BuildpackArtifact
tag string
api *testutil.FakeAPIClient
pushImages bool
shouldErr bool
description string
artifact *latest.BuildpackArtifact
tag string
api *testutil.FakeAPIClient
pushImages bool
shouldErr bool
expectedOptions *pack.BuildOptions
}{
{
description: "success",
Expand All @@ -44,6 +57,14 @@ func TestBuild(t *testing.T) {
},
tag: "img:tag",
api: &testutil.FakeAPIClient{},
expectedOptions: &pack.BuildOptions{
AppPath: ".",
Builder: "my/builder",
RunImage: "my/run",
Env: map[string]string{},
Image: "img:latest",
NoPull: true,
},
},
{
description: "invalid ref",
Expand All @@ -66,20 +87,14 @@ func TestBuild(t *testing.T) {
},
tag: "img:tag",
api: &testutil.FakeAPIClient{},
},
{
description: "force pull error",
artifact: &latest.BuildpackArtifact{
Builder: "my/builder",
RunImage: "my/run",
ForcePull: true,
Dependencies: defaultBuildpackDependencies(),
expectedOptions: &pack.BuildOptions{
AppPath: ".",
Builder: "my/builder",
RunImage: "my/run",
Env: map[string]string{},
Image: "img:latest",
NoPull: false,
},
tag: "img:tag",
api: &testutil.FakeAPIClient{
ErrImagePull: true,
},
shouldErr: true,
},
{
description: "push error",
Expand Down Expand Up @@ -111,6 +126,9 @@ func TestBuild(t *testing.T) {
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.NewTempDir().Touch("file").Chdir()
pack := &fakePack{}
t.Override(&runPackBuildFunc, pack.runPack)

test.api.
Add(test.artifact.Builder, "builderImageID").
Add(test.artifact.RunImage, "runImageID").
Expand All @@ -126,6 +144,9 @@ func TestBuild(t *testing.T) {
}, test.tag)

t.CheckError(test.shouldErr, err)
if test.expectedOptions != nil {
t.CheckDeepEqual(*test.expectedOptions, pack.Opts)
}
})
}
}
Expand Down
140 changes: 55 additions & 85 deletions pkg/skaffold/build/buildpacks/lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,21 @@ package buildpacks

import (
"context"
"fmt"
"io"
"path/filepath"
"time"
"strings"

"github.com/docker/docker/api/types/mount"
"github.com/buildpacks/pack"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/misc"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
)

// For testing
var (
runPackBuildFunc = runPackBuild
)

func (b *Builder) build(ctx context.Context, out io.Writer, a *latest.Artifact, tag string) (string, error) {
Expand All @@ -48,17 +50,30 @@ func (b *Builder) build(ctx context.Context, out io.Writer, a *latest.Artifact,

builderImage := artifact.Builder
logrus.Debugln("Builder image", builderImage)
if err := b.pull(ctx, out, builderImage, artifact.ForcePull); err != nil {
return "", err
// If ForcePull is false: we pull the image only if it's not there already.
// If ForcePull is true: we will let `pack` always pull.
// Ideally, we add a `--pullIdNotPresent` option to upstream `pack`.
dgageot marked this conversation as resolved.
Show resolved Hide resolved
if !artifact.ForcePull {
if err := b.pull(ctx, out, builderImage); err != nil {
return "", err
Comment on lines +56 to +58
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is confusing: if we aren't force-pulling then we pull the image? Doesn't pack do this anyways? Or this needs a comment to explain why.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pack is either pulling always or pulling never. This is trying to pullIfNotPresent. I'll update the code with a comment.

}
}

runImage, err := b.findRunImage(ctx, artifact, builderImage)
if err != nil {
return "", err
}
logrus.Debugln("Run image", runImage)
if err := b.pull(ctx, out, runImage, artifact.ForcePull); err != nil {
return "", err
runImage := artifact.RunImage
if !artifact.ForcePull {
// If ForcePull is false: we pull the image only if it's not there already.
// If ForcePull is true: we will let `pack` always pull.
// Ideally, we add a `--pullIdNotPresent` option to upstream `pack`.
dgageot marked this conversation as resolved.
Show resolved Hide resolved
var err error
runImage, err = b.findRunImage(ctx, artifact, builderImage)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't pack already do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pack is either pulling always or pulling never. This is trying to pullIfNotPresent. I'll update the code with a comment.

if err != nil {
return "", err
}
logrus.Debugln("Run image", runImage)

if err := b.pull(ctx, out, runImage); err != nil {
return "", err
}
}

logrus.Debugln("Evaluate env variables")
Expand All @@ -67,89 +82,44 @@ func (b *Builder) build(ctx context.Context, out io.Writer, a *latest.Artifact,
return "", errors.Wrap(err, "unable to evaluate env variables")
}

logrus.Debugln("Get dependencies")
deps, err := GetDependencies(ctx, workspace, artifact)
if err != nil {
if err := runPackBuildFunc(ctx, out, pack.BuildOptions{
AppPath: workspace,
Builder: builderImage,
RunImage: runImage,
Env: envMap(env),
Image: latest,
NoPull: !artifact.ForcePull,
}); err != nil {
return "", err
}

var paths []string
for _, dep := range deps {
paths = append(paths, filepath.Join(workspace, dep))
}

copyWorkspace := func(ctx context.Context, container string) error {
uid := 1000
gid := 1000
modTime := time.Date(1980, time.January, 1, 0, 0, 1, 0, time.UTC)
return latest, nil
}

return b.localDocker.CopyToContainer(ctx, container, "/workspace", workspace, paths, uid, gid, modTime)
func runPackBuild(ctx context.Context, out io.Writer, opts pack.BuildOptions) error {
packClient, err := pack.NewClient(pack.WithLogger(NewLogger(out)))
if err != nil {
return errors.Wrap(err, "unable to create pack client")
}

// These volumes store the state shared between build steps.
// After the build, they are deleted.
cacheID := util.RandomID()
packWorkspace := volume(mount.TypeVolume, fmt.Sprintf("pack-%s.workspace", cacheID), "/workspace")
layers := volume(mount.TypeVolume, fmt.Sprintf("pack-%s.layers", cacheID), "/layers")

// These volumes are kept after the build and shared with all the builds.
// They handle the caching of layer both for build time and run time.
buildCache := volume(mount.TypeVolume, "pack-cache-skaffold.build", "/cache")
launchCache := volume(mount.TypeVolume, "pack-cache-skaffold.launch", "/launch-cache")
return packClient.Build(ctx, opts)
}

// Some steps need access to the Docker socket to load/save images.
dockerSocket := volume(mount.TypeBind, "/var/run/docker.sock", "/var/run/docker.sock")
func envMap(env []string) map[string]string {
kv := make(map[string]string)

defer func() {
// Don't use ctx. It might have been cancelled by Ctrl-C
if err := b.localDocker.VolumeRemove(context.Background(), packWorkspace.Source, true); err != nil {
logrus.Warnf("unable to delete the docker volume [%s]", packWorkspace.Source)
}
if err := b.localDocker.VolumeRemove(context.Background(), layers.Source, true); err != nil {
logrus.Warnf("unable to delete the docker volume [%s]", layers.Source)
}
}()

if err := b.localDocker.ContainerRun(ctx, out,
docker.ContainerRun{
Image: builderImage,
Command: []string{"/lifecycle/detector"},
BeforeStart: copyWorkspace,
Mounts: []mount.Mount{packWorkspace, layers},
Env: env,
}, docker.ContainerRun{
Image: builderImage,
Command: []string{"sh", "-c", "/lifecycle/restorer -path /cache && /lifecycle/analyzer -daemon " + latest},
User: "root",
Mounts: []mount.Mount{packWorkspace, layers, buildCache, dockerSocket},
}, docker.ContainerRun{
Image: builderImage,
Command: []string{"/lifecycle/builder"},
Mounts: []mount.Mount{packWorkspace, layers},
Env: env,
}, docker.ContainerRun{
Image: builderImage,
Command: []string{"sh", "-c", "/lifecycle/exporter -daemon -image " + runImage + " -launch-cache /launch-cache " + latest + " && /lifecycle/cacher -path /cache"},
User: "root",
Mounts: []mount.Mount{packWorkspace, layers, launchCache, buildCache, dockerSocket},
},
); err != nil {
return "", err
for _, e := range env {
parts := strings.SplitN(e, "=", 2)
kv[parts[0]] = parts[1]
}

return latest, nil
}

func volume(mountType mount.Type, source, target string) mount.Mount {
return mount.Mount{Type: mountType, Source: source, Target: target}
return kv
}

// pull makes sure the given image is pre-pulled.
func (b *Builder) pull(ctx context.Context, out io.Writer, image string, force bool) error {
if force || !b.localDocker.ImageExists(ctx, image) {
if err := b.localDocker.Pull(ctx, out, image); err != nil {
return err
}
func (b *Builder) pull(ctx context.Context, out io.Writer, image string) error {
if b.localDocker.ImageExists(ctx, image) {
return nil
}
return nil
return b.localDocker.Pull(ctx, out, image)
}
62 changes: 62 additions & 0 deletions pkg/skaffold/build/buildpacks/logger.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
Copyright 2019 The Skaffold Authors

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package buildpacks

import (
"io"

"github.com/sirupsen/logrus"

"github.com/buildpacks/pack/logging"
)

// logger exists to meet the requirements of the pack logger.
type logger struct {
dgageot marked this conversation as resolved.
Show resolved Hide resolved
*logrus.Logger
out io.Writer
}

func NewLogger(out io.Writer) logging.Logger {
return &logger{
Logger: logrus.StandardLogger(),
out: out,
}
}

func (l *logger) Debug(msg string) {
l.Logger.Debug(msg)
}

func (l *logger) Info(msg string) {
l.Logger.Info(msg)
}

func (l *logger) Warn(msg string) {
l.Logger.Warn(msg)
}

func (l *logger) Error(msg string) {
l.Logger.Error(msg)
}

func (l *logger) Writer() io.Writer {
return l.out
}

func (l *logger) IsVerbose() bool {
return false
}
Loading