Skip to content

Commit

Permalink
Get working dir from image and use as relative directory if necessary
Browse files Browse the repository at this point in the history
  • Loading branch information
Priya Wadhwa committed Feb 28, 2019
2 parents b7ef0c7 + 49ef4ce commit 9d56c22
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 50 deletions.
2 changes: 1 addition & 1 deletion pkg/skaffold/docker/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func (l *localDaemon) ConfigFile(ctx context.Context, image string) (*v1.ConfigF
return nil, err
}
} else {
cfg, err = retrieveRemoteConfig(image)
cfg, err = RetrieveRemoteConfig(image)
if err != nil {
return nil, errors.Wrap(err, "getting remote config")
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/skaffold/docker/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ func RemoteDigest(identifier string) (string, error) {
return h.String(), nil
}

func retrieveRemoteConfig(identifier string) (*v1.ConfigFile, error) {
// RetrieveRemoteConfig retrieves the remote config file for an image
func RetrieveRemoteConfig(identifier string) (*v1.ConfigFile, error) {
img, err := remoteImage(identifier)
if err != nil {
return nil, errors.Wrap(err, "getting image")
Expand Down
8 changes: 8 additions & 0 deletions pkg/skaffold/runner/dev_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/sync"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/watch"
"github.com/GoogleContainerTools/skaffold/testutil"
)
Expand Down Expand Up @@ -327,6 +328,13 @@ func TestDevSync(t *testing.T) {

for _, test := range tests {
t.Run(test.description, func(t *testing.T) {
originalWorkingDir := sync.WorkingDir
sync.WorkingDir = func(image string) (string, error) {
return "/", nil
}
defer func() {
sync.WorkingDir = originalWorkingDir
}()
runner := createRunner(t, test.testBench)
runner.Watcher = &TestWatcher{
events: test.watchEvents,
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/sync/kubectl/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func copyFileFn(ctx context.Context, pod v1.Pod, container v1.Container, files m
// Use "m" flag to touch the files as they are copied.
reader, writer := io.Pipe()
copy := exec.CommandContext(ctx, "kubectl", "exec", pod.Name, "--namespace", pod.Namespace, "-c", container.Name, "-i",
"--", "tar", "xmf", "-", "-C", "/", "--no-same-owner")
"--", "tar", "xmf", "-", "--no-same-owner")
copy.Stdin = reader
go func() {
defer writer.Close()
Expand Down
53 changes: 35 additions & 18 deletions pkg/skaffold/sync/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ import (
"fmt"
"os/exec"
"path/filepath"
"strings"

"github.com/bmatcuk/doublestar"
"github.com/sirupsen/logrus"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
Expand All @@ -36,6 +36,11 @@ import (
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

var (
// For testing
WorkingDir = retrieveWorkingDir
)

type Syncer interface {
Sync(context.Context, *Item) error
}
Expand All @@ -52,12 +57,22 @@ func NewItem(a *latest.Artifact, e watch.Events, builds []build.Artifact) (*Item
return nil, nil
}

toCopy, err := intersect(a.Workspace, a.Sync, append(e.Added, e.Modified...))
tag := latestTag(a.ImageName, builds)
if tag == "" {
return nil, fmt.Errorf("could not find latest tag for image %s in builds: %v", a.ImageName, builds)
}

wd, err := WorkingDir(tag)
if err != nil {
return nil, errors.Wrapf(err, "retrieving working dir for %s", tag)
}

toCopy, err := intersect(a.Workspace, a.Sync, append(e.Added, e.Modified...), wd)
if err != nil {
return nil, errors.Wrap(err, "intersecting sync map and added, modified files")
}

toDelete, err := intersect(a.Workspace, a.Sync, e.Deleted)
toDelete, err := intersect(a.Workspace, a.Sync, e.Deleted, wd)
if err != nil {
return nil, errors.Wrap(err, "intersecting sync map and deleted files")
}
Expand All @@ -67,18 +82,24 @@ func NewItem(a *latest.Artifact, e watch.Events, builds []build.Artifact) (*Item
return nil, nil
}

tag := latestTag(a.ImageName, builds)
if tag == "" {
return nil, fmt.Errorf("could not find latest tag for image %s in builds: %v", a.ImageName, builds)
}

return &Item{
Image: tag,
Copy: toCopy,
Delete: toDelete,
}, nil
}

func retrieveWorkingDir(image string) (string, error) {
cf, err := docker.RetrieveRemoteConfig(image)
if err != nil {
return "", errors.Wrap(err, "retrieving remote config")
}
if cf.Config.WorkingDir == "" {
return "/", nil
}
return cf.Config.WorkingDir, nil
}

func latestTag(image string, builds []build.Artifact) string {
for _, build := range builds {
if build.ImageName == image {
Expand All @@ -88,7 +109,7 @@ func latestTag(image string, builds []build.Artifact) string {
return ""
}

func intersect(context string, syncMap map[string]string, files []string) (map[string]string, error) {
func intersect(context string, syncMap map[string]string, files []string, workingDir string) (map[string]string, error) {
ret := map[string]string{}

for _, f := range files {
Expand All @@ -103,18 +124,14 @@ func intersect(context string, syncMap map[string]string, files []string) (map[s
return nil, errors.Wrapf(err, "pattern error for %s", relPath)
}
if match {
staticPath := strings.Split(filepath.FromSlash(p), "*")[0]
if filepath.IsAbs(dst) {
dst = filepath.ToSlash(filepath.Join(dst, filepath.Base(relPath)))
} else {
dst = filepath.ToSlash(filepath.Join(workingDir, dst, filepath.Base(relPath)))
}
// Every file must match at least one sync pattern, if not we'll have to
// skip the entire sync
matches = true
// If the source has special match characters,
// the destination must be a directory
// The path package must be used here to enforce slashes,
// since the destination is always a linux filesystem.
if util.HasMeta(p) {
relPathDynamic := strings.TrimPrefix(relPath, staticPath)
dst = filepath.ToSlash(filepath.Join(dst, filepath.Base(relPathDynamic)))
}
ret[f] = dst
}
}
Expand Down
53 changes: 24 additions & 29 deletions pkg/skaffold/sync/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func TestNewSyncItem(t *testing.T) {
builds []build.Artifact
shouldErr bool
expected *Item
workingDir string
}{
{
description: "match copy",
Expand Down Expand Up @@ -142,10 +143,11 @@ func TestNewSyncItem(t *testing.T) {
evt: watch.Events{
Modified: []string{filepath.Join("node", "src/app/server/server.js")},
},
workingDir: "/",
expected: &Item{
Image: "test:123",
Copy: map[string]string{
filepath.Join("node", "src/app/server/server.js"): "src/server.js",
filepath.Join("node", "src/app/server/server.js"): "/src/server.js",
},
Delete: map[string]string{},
},
Expand Down Expand Up @@ -193,6 +195,11 @@ func TestNewSyncItem(t *testing.T) {
Added: []string{"main.go"},
Deleted: []string{"index.html"},
},
builds: []build.Artifact{
{
Tag: "placeholder",
},
},
},
{
description: "not delete syncable",
Expand All @@ -206,6 +213,11 @@ func TestNewSyncItem(t *testing.T) {
Added: []string{"index.html"},
Deleted: []string{"some/other/file"},
},
builds: []build.Artifact{
{
Tag: "placeholder",
},
},
},
{
description: "err bad pattern",
Expand Down Expand Up @@ -245,6 +257,7 @@ func TestNewSyncItem(t *testing.T) {
},
Workspace: ".",
},
workingDir: "/some/dir",
builds: []build.Artifact{
{
ImageName: "test",
Expand All @@ -257,33 +270,7 @@ func TestNewSyncItem(t *testing.T) {
expected: &Item{
Image: "test:123",
Copy: map[string]string{
filepath.Join("dir1", "dir2/node.js"): "node.js",
},
Delete: map[string]string{},
},
},
{
description: "copy subdirectory",
artifact: &latest.Artifact{
ImageName: "test",
Sync: map[string]string{
"**/*.py": ".",
},
Workspace: "python",
},
builds: []build.Artifact{
{
ImageName: "test",
Tag: "test:123",
},
},
evt: watch.Events{
Modified: []string{filepath.Join("src", "app.py")},
},
expected: &Item{
Image: "test:123",
Copy: map[string]string{
filepath.Join("src", "app.py"): "app.py",
filepath.Join("dir1", "dir2/node.js"): "/some/dir/node.js",
},
Delete: map[string]string{},
},
Expand All @@ -292,6 +279,13 @@ func TestNewSyncItem(t *testing.T) {

for _, test := range tests {
t.Run(test.description, func(t *testing.T) {
originalWorkingDir := WorkingDir
WorkingDir = func(image string) (string, error) {
return test.workingDir, nil
}
defer func() {
WorkingDir = originalWorkingDir
}()
actual, err := NewItem(test.artifact, test.evt, test.builds)
testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, actual)
})
Expand All @@ -305,6 +299,7 @@ func TestIntersect(t *testing.T) {
syncPatterns map[string]string
files []string
context string
workingDir string
expected map[string]string
shouldErr bool
}{
Expand Down Expand Up @@ -347,7 +342,7 @@ func TestIntersect(t *testing.T) {

for _, test := range tests {
t.Run(test.description, func(t *testing.T) {
actual, err := intersect(test.context, test.syncPatterns, test.files)
actual, err := intersect(test.context, test.syncPatterns, test.files, test.workingDir)
testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, actual)
})
}
Expand Down

0 comments on commit 9d56c22

Please sign in to comment.