Skip to content

Commit

Permalink
Auto sync with Skaffold
Browse files Browse the repository at this point in the history
Signed-off-by: David Gageot <david@gageot.net>
  • Loading branch information
dgageot committed Jan 24, 2020
1 parent fc079c9 commit b599f96
Show file tree
Hide file tree
Showing 16 changed files with 397 additions and 95 deletions.
132 changes: 81 additions & 51 deletions pkg/skaffold/build/buildpacks/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,51 @@ func (f *fakePack) runPack(_ context.Context, _ io.Writer, opts pack.BuildOption
func TestBuild(t *testing.T) {
tests := []struct {
description string
artifact *latest.BuildpackArtifact
artifact *latest.Artifact
tag string
api *testutil.FakeAPIClient
pushImages bool
devMode bool
shouldErr bool
expectedOptions *pack.BuildOptions
}{
{
description: "success",
artifact: &latest.BuildpackArtifact{
Builder: "my/builder",
RunImage: "my/run",
Dependencies: defaultBuildpackDependencies(),
artifact: buildpacksArtifact("my/builder", "my/run"),
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: "dev mode",
artifact: withSync(&latest.Sync{Infer: []string{"**/*"}}, buildpacksArtifact("another/builder", "another/run")),
tag: "img:tag",
api: &testutil.FakeAPIClient{},
devMode: true,
expectedOptions: &pack.BuildOptions{
AppPath: ".",
Builder: "another/builder",
RunImage: "another/run",
Env: map[string]string{
"GOOGLE_DEVMODE": "1",
},
Image: "img:latest",
NoPull: true,
},
tag: "img:tag",
api: &testutil.FakeAPIClient{},
},
{
description: "dev mode but no sync",
artifact: buildpacksArtifact("my/builder", "my/run"),
tag: "img:tag",
api: &testutil.FakeAPIClient{},
devMode: true,
expectedOptions: &pack.BuildOptions{
AppPath: ".",
Builder: "my/builder",
Expand All @@ -68,25 +97,16 @@ func TestBuild(t *testing.T) {
},
{
description: "invalid ref",
artifact: &latest.BuildpackArtifact{
Builder: "my/builder",
RunImage: "my/run",
Dependencies: defaultBuildpackDependencies(),
},
tag: "in valid ref",
api: &testutil.FakeAPIClient{},
shouldErr: true,
artifact: buildpacksArtifact("my/builder", "my/run"),
tag: "in valid ref",
api: &testutil.FakeAPIClient{},
shouldErr: true,
},
{
description: "force pull",
artifact: &latest.BuildpackArtifact{
Builder: "my/builder",
RunImage: "my/run",
ForcePull: true,
Dependencies: defaultBuildpackDependencies(),
},
tag: "img:tag",
api: &testutil.FakeAPIClient{},
artifact: withForcePull(true, buildpacksArtifact("my/builder", "my/run")),
tag: "img:tag",
api: &testutil.FakeAPIClient{},
expectedOptions: &pack.BuildOptions{
AppPath: ".",
Builder: "my/builder",
Expand All @@ -98,29 +118,20 @@ func TestBuild(t *testing.T) {
},
{
description: "push error",
artifact: &latest.BuildpackArtifact{
Builder: "my/builder",
RunImage: "my/run",
Dependencies: defaultBuildpackDependencies(),
},
tag: "img:tag",
pushImages: true,
artifact: buildpacksArtifact("my/builder", "my/run"),
tag: "img:tag",
pushImages: true,
api: &testutil.FakeAPIClient{
ErrImagePush: true,
},
shouldErr: true,
},
{
description: "invalid env",
artifact: &latest.BuildpackArtifact{
Builder: "my/builder",
RunImage: "my/run",
Env: []string{"INVALID"},
Dependencies: defaultBuildpackDependencies(),
},
tag: "img:tag",
api: &testutil.FakeAPIClient{},
shouldErr: true,
artifact: withEnv([]string{"INVALID"}, buildpacksArtifact("my/builder", "my/run")),
tag: "img:tag",
api: &testutil.FakeAPIClient{},
shouldErr: true,
},
}
for _, test := range tests {
Expand All @@ -130,18 +141,13 @@ func TestBuild(t *testing.T) {
t.Override(&runPackBuildFunc, pack.runPack)

test.api.
Add(test.artifact.Builder, "builderImageID").
Add(test.artifact.RunImage, "runImageID").
Add(test.artifact.BuildpackArtifact.Builder, "builderImageID").
Add(test.artifact.BuildpackArtifact.RunImage, "runImageID").
Add("img:latest", "builtImageID")
localDocker := docker.NewLocalDaemon(test.api, nil, false, nil)

builder := NewArtifactBuilder(localDocker, test.pushImages)
_, err := builder.Build(context.Background(), ioutil.Discard, &latest.Artifact{
Workspace: ".",
ArtifactType: latest.ArtifactType{
BuildpackArtifact: test.artifact,
},
}, test.tag)
builder := NewArtifactBuilder(localDocker, test.pushImages, test.devMode)
_, err := builder.Build(context.Background(), ioutil.Discard, test.artifact, test.tag)

t.CheckError(test.shouldErr, err)
if test.expectedOptions != nil {
Expand All @@ -151,8 +157,32 @@ func TestBuild(t *testing.T) {
}
}

func defaultBuildpackDependencies() *latest.BuildpackDependencies {
return &latest.BuildpackDependencies{
Paths: []string{"."},
func buildpacksArtifact(builder, runImage string) *latest.Artifact {
return &latest.Artifact{
Workspace: ".",
ArtifactType: latest.ArtifactType{
BuildpackArtifact: &latest.BuildpackArtifact{
Builder: builder,
RunImage: runImage,
Dependencies: &latest.BuildpackDependencies{
Paths: []string{"."},
},
},
},
}
}

func withForcePull(forcePull bool, artifact *latest.Artifact) *latest.Artifact {
artifact.BuildpackArtifact.ForcePull = forcePull
return artifact
}

func withEnv(env []string, artifact *latest.Artifact) *latest.Artifact {
artifact.BuildpackArtifact.Env = env
return artifact
}

func withSync(sync *latest.Sync, artifact *latest.Artifact) *latest.Artifact {
artifact.Sync = sync
return artifact
}
4 changes: 4 additions & 0 deletions pkg/skaffold/build/buildpacks/lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ func (b *Builder) build(ctx context.Context, out io.Writer, a *latest.Artifact,
return "", errors.Wrap(err, "unable to evaluate env variables")
}

if b.devMode && a.Sync != nil && len(a.Sync.Infer) > 0 {
env = append(env, "GOOGLE_DEVMODE=1")
}

if err := runPackBuildFunc(ctx, out, pack.BuildOptions{
AppPath: workspace,
Builder: builderImage,
Expand Down
45 changes: 45 additions & 0 deletions pkg/skaffold/build/buildpacks/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,48 @@ func (b *Builder) findRunImage(ctx context.Context, a *latest.BuildpackArtifact)

return m.Stack.RunImage.Image, nil
}

type buildMetadata struct {
Bom []bom `json:"bom"`
}

type bom struct {
Metadata bomMetadata `json:"metadata"`
}

type bomMetadata struct {
Sync []sync `json:"devmode.sync"`
}

type sync struct {
Src string `json:"src"`
Dest string `json:"dest"`
Strip string `json:"strip"`
}

// $ docker inspect demo/buildpacks | jq -r '.[].Config.Labels["io.buildpacks.build.metadata"] | fromjson.bom[].metadata["devmode.sync"]'
func SyncRules(labels map[string]string) ([]*latest.SyncRule, error) {
metadataJSON, present := labels["io.buildpacks.build.metadata"]
if !present {
return nil, nil
}

m := buildMetadata{}
if err := json.Unmarshal([]byte(metadataJSON), &m); err != nil {
return nil, err
}

var rules []*latest.SyncRule

for _, b := range m.Bom {
for _, sync := range b.Metadata.Sync {
rules = append(rules, &latest.SyncRule{
Src: sync.Src,
Dest: sync.Dest,
Strip: sync.Strip,
})
}
}

return rules, nil
}
49 changes: 48 additions & 1 deletion pkg/skaffold/build/buildpacks/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func TestFindRunImage(t *testing.T) {
},
}

builder := NewArtifactBuilder(localDocker, false)
builder := NewArtifactBuilder(localDocker, false, false)
runImage, err := builder.findRunImage(context.Background(), test.artifact)

if test.expectedError == "" {
Expand All @@ -110,3 +110,50 @@ func TestFindRunImage(t *testing.T) {
})
}
}

func TestSyncRules(t *testing.T) {
tests := []struct {
description string
labels map[string]string
expectedRules []*latest.SyncRule
shouldErr bool
}{
{
description: "missing labels",
labels: map[string]string{},
},
{
description: "invalid labels",
labels: map[string]string{
"io.buildpacks.build.metadata": "invalid",
},
shouldErr: true,
},
{
description: "valid labels",
labels: map[string]string{
"io.buildpacks.build.metadata": `{
"bom":[{
"metadata":{
"devmode.sync": [
{"src":"src-value1","dest":"dest-value1","strip":"strip-value1"},
{"src":"src-value2","dest":"dest-value2","strip":"strip-value2"}
]
}
}]
}`,
},
expectedRules: []*latest.SyncRule{
{Src: "src-value1", Dest: "dest-value1", Strip: "strip-value1"},
{Src: "src-value2", Dest: "dest-value2", Strip: "strip-value2"},
},
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
rules, err := SyncRules(test.labels)

t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expectedRules, rules)
})
}
}
4 changes: 3 additions & 1 deletion pkg/skaffold/build/buildpacks/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ import "github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
type Builder struct {
localDocker docker.LocalDaemon
pushImages bool
devMode bool
}

// NewArtifactBuilder returns a new buildpack artifact builder
func NewArtifactBuilder(localDocker docker.LocalDaemon, pushImages bool) *Builder {
func NewArtifactBuilder(localDocker docker.LocalDaemon, pushImages, devMode bool) *Builder {
return &Builder{
localDocker: localDocker,
pushImages: pushImages,
devMode: devMode,
}
}
2 changes: 2 additions & 0 deletions pkg/skaffold/build/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type cache struct {
insecureRegistries map[string]bool
cacheFile string
imagesAreLocal bool
devMode bool
}

// DependencyLister fetches a list of dependencies for an artifact
Expand Down Expand Up @@ -85,6 +86,7 @@ func NewCache(runCtx *runcontext.RunContext, imagesAreLocal bool, dependencies D
insecureRegistries: runCtx.InsecureRegistries,
cacheFile: cacheFile,
imagesAreLocal: imagesAreLocal,
devMode: runCtx.DevMode,
}, nil
}

Expand Down
11 changes: 8 additions & 3 deletions pkg/skaffold/build/cache/hash.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ var (
artifactConfigFunction = artifactConfig
)

func getHashForArtifact(ctx context.Context, depLister DependencyLister, a *latest.Artifact) (string, error) {
func getHashForArtifact(ctx context.Context, depLister DependencyLister, a *latest.Artifact, devMode bool) (string, error) {
var inputs []string

// Append the artifact's configuration
config, err := artifactConfigFunction(a)
config, err := artifactConfigFunction(a, devMode)
if err != nil {
return "", errors.Wrapf(err, "getting artifact's configuration for %s", a.ImageName)
}
Expand Down Expand Up @@ -100,12 +100,17 @@ func getHashForArtifact(ctx context.Context, depLister DependencyLister, a *late
return hex.EncodeToString(hasher.Sum(nil)), nil
}

func artifactConfig(a *latest.Artifact) (string, error) {
// TODO(dgageot): when the buildpacks builder image digest changes, we need to change the hash
func artifactConfig(a *latest.Artifact, devMode bool) (string, error) {
buf, err := json.Marshal(a.ArtifactType)
if err != nil {
return "", errors.Wrapf(err, "marshalling the artifact's configuration for %s", a.ImageName)
}

if devMode && a.BuildpackArtifact != nil && a.Sync != nil && len(a.Sync.Infer) > 0 {
return string(buf) + ".DEV", nil
}

return string(buf), nil
}

Expand Down
Loading

0 comments on commit b599f96

Please sign in to comment.