Add new Auto sync option and tie into jib impl

Author: loosebazooka

pkg/skaffold/build/jib/sync.go

@@ -53,17 +53,21 @@ type JSONSyncEntry struct {
 	Dest string `json:"dest"`
 }
 
-func InitSync(ctx context.Context, workspace string, a *latest.JibArtifact) error {
+var InitSync = initSync
+
+func initSync(ctx context.Context, workspace string, a *latest.JibArtifact) error {
 	syncMap, err := getSyncMapFunc(ctx, workspace, a)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "failed to initialize sync state for %s", workspace)
 	}
 	syncLists[getProjectKey(workspace, a)] = *syncMap
 	return nil
 }
 
+var GetSyncDiff = getSyncDiff
+
 // returns toCopy, toDelete, error
-func GetSyncDiff(ctx context.Context, workspace string, a *latest.JibArtifact, e filemon.Events) (map[string][]string, map[string][]string, error) {
+func getSyncDiff(ctx context.Context, workspace string, a *latest.JibArtifact, e filemon.Events) (map[string][]string, map[string][]string, error) {
 	// no deletions allowed
 	if len(e.Deleted) != 0 {
 		// change into logging

pkg/skaffold/runner/dev.go

@@ -145,7 +145,7 @@ func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*la
 					return build.DependenciesForArtifact(ctx, artifact, r.runCtx.InsecureRegistries)
 				},
 				func(e filemon.Events) {
-					s, err := sync.NewItem(artifact, e, r.builds, r.runCtx.InsecureRegistries)
+					s, err := sync.NewItem(ctx, artifact, e, r.builds, r.runCtx.InsecureRegistries)
 					switch {
 					case err != nil:
 						logrus.Warnf("error adding dirty artifact to changeset: %s", err.Error())
@@ -187,6 +187,11 @@ func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*la
 
 	logrus.Infoln("List generated in", time.Since(start))
 
+	// Init Sync State
+	if err := sync.Init(ctx, artifacts); err != nil {
+		return errors.Wrap(err, "exiting dev mode because initializing sync state failed")
+	}
+
 	// First build
 	if _, err := r.BuildAndTest(ctx, out, artifacts); err != nil {
 		return errors.Wrap(err, "exiting dev mode because first build failed")

pkg/skaffold/schema/defaults/defaults.go

@@ -217,8 +217,15 @@ func setDefaultWorkspace(a *latest.Artifact) {
 }
 
 func setDefaultSync(a *latest.Artifact) {
-	if a.Sync != nil && len(a.Sync.Manual) == 0 && len(a.Sync.Infer) == 0 {
-		a.Sync.Infer = []string{"**/*"}
+	if a.Sync != nil {
+		if len(a.Sync.Manual) == 0 && len(a.Sync.Infer) == 0 && a.Sync.Auto == nil {
+			switch {
+			case a.JibArtifact != nil:
+				a.Sync.Auto = &latest.Auto{}
+			default:
+				a.Sync.Infer = []string{"**/*"}
+			}
+		}
 	}
 }
 

pkg/skaffold/schema/defaults/defaults_test.go

@@ -58,6 +58,13 @@ func TestSetDefaults(t *testing.T) {
 						},
 						Sync: &latest.Sync{},
 					},
+					{
+						ImageName: "fifth",
+						ArtifactType: latest.ArtifactType{
+							JibArtifact: &latest.JibArtifact{},
+						},
+						Sync: &latest.Sync{},
+					},
 				},
 			},
 		},
@@ -83,6 +90,9 @@ func TestSetDefaults(t *testing.T) {
 	testutil.CheckDeepEqual(t, []string{"."}, cfg.Build.Artifacts[3].BuildpackArtifact.Dependencies.Paths)
 	testutil.CheckDeepEqual(t, []string(nil), cfg.Build.Artifacts[3].BuildpackArtifact.Dependencies.Ignore)
 	testutil.CheckDeepEqual(t, []string{"**/*"}, cfg.Build.Artifacts[3].Sync.Infer)
+
+	testutil.CheckDeepEqual(t, "fifth", cfg.Build.Artifacts[4].ImageName)
+	testutil.CheckDeepEqual(t, &latest.Auto{}, cfg.Build.Artifacts[4].Sync.Auto)
 }
 
 func TestSetDefaultsOnCluster(t *testing.T) {

pkg/skaffold/schema/latest/config.go

@@ -593,6 +593,10 @@ type Sync struct {
 	// The container destination is inferred by the builder.
 	// Currently only available for docker artifacts.
 	Infer []string `yaml:"infer,omitempty" yamltags:"oneOf=sync"`
+
+	// Auto delegates discovery of sync rules to the build system.
+	// Currently not available on any builders.
+	Auto *Auto `yaml:"auto,omitempty" yamltags:"oneOf=sync"`
 }
 
 // SyncRule specifies which local files to sync to remote folders.
@@ -612,6 +616,9 @@ type SyncRule struct {
 	Strip string `yaml:"strip,omitempty"`
 }
 
+// Auto cannot be customized.
+type Auto struct{}
+
 // Profile is used to override any `build`, `test` or `deploy` configuration.
 type Profile struct {
 	// Name is a unique profile name.

pkg/skaffold/sync/sync.go

@@ -33,6 +33,7 @@ import (
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/buildpacks"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/jib"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/filemon"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
@@ -47,7 +48,7 @@ var (
 	SyncMap    = syncMapForArtifact
 )
 
-func NewItem(a *latest.Artifact, e filemon.Events, builds []build.Artifact, insecureRegistries map[string]bool) (*Item, error) {
+func NewItem(ctx context.Context, a *latest.Artifact, e filemon.Events, builds []build.Artifact, insecureRegistries map[string]bool) (*Item, error) {
 	if !e.HasChanged() || a.Sync == nil {
 		return nil, nil
 	}
@@ -83,6 +84,9 @@ func NewItem(a *latest.Artifact, e filemon.Events, builds []build.Artifact, inse
 	case len(a.Sync.Infer) > 0:
 		return inferredSyncItem(a, tag, e, insecureRegistries)
 
+	case a.Sync.Auto != nil:
+		return autoSyncItem(ctx, a, e, builds)
+
 	default:
 		return nil, nil
 	}
@@ -169,6 +173,30 @@ func syncMapForArtifact(a *latest.Artifact, insecureRegistries map[string]bool)
 	}
 }
 
+func autoSyncItem(ctx context.Context, a *latest.Artifact, e filemon.Events, builds []build.Artifact) (*Item, error) {
+	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)
+	}
+
+	switch {
+	case a.JibArtifact != nil:
+		toCopy, toDelete, err := jib.GetSyncDiff(ctx, a.Workspace, a.JibArtifact, e)
+		if err != nil {
+			return nil, err
+		}
+		if toCopy == nil && toDelete == nil {
+			// do a rebuild
+			return nil, nil
+		}
+		return &Item{Image: tag, Copy: toCopy, Delete: toDelete}, nil
+
+	default:
+		// TODO: this error does appear a little late in the build, perhaps it could surface at first run, rather than first sync?
+		return nil, fmt.Errorf("Sync: Auto is not supported by the build of %s", a.ImageName)
+	}
+}
+
 func latestTag(image string, builds []build.Artifact) string {
 	for _, build := range builds {
 		if build.ImageName == image {
@@ -338,3 +366,19 @@ func Perform(ctx context.Context, image string, files syncMap, cmdFn func(contex
 
 	return errs.Wait()
 }
+
+func Init(ctx context.Context, artifacts []*latest.Artifact) error {
+	for _, a := range artifacts {
+		if a.Sync == nil {
+			continue
+		}
+
+		if a.Sync.Auto != nil && a.JibArtifact != nil {
+			err := jib.InitSync(ctx, a.Workspace, a.JibArtifact)
+			if err != nil {
+				return errors.Wrapf(err, "failed to initialize sync state for %s", a.ImageName)
+			}
+		}
+	}
+	return nil
+}

pkg/skaffold/sync/sync_test.go

@@ -23,12 +23,14 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/pkg/errors"
 	v1 "k8s.io/api/core/v1"
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/fake"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/jib"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/filemon"
 	pkgkubernetes "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
@@ -37,6 +39,7 @@ import (
 )
 
 func TestNewSyncItem(t *testing.T) {
+	ctx := context.Background()
 	tests := []struct {
 		description  string
 		artifact     *latest.Artifact
@@ -677,14 +680,48 @@ func TestNewSyncItem(t *testing.T) {
 				Delete: map[string][]string{},
 			},
 		},
+
+		// Auto with Jib
+		{
+			description: "auto with jib",
+			artifact: &latest.Artifact{
+				ImageName: "test",
+				Workspace: ".",
+				Sync: &latest.Sync{
+					Auto: &latest.Auto{},
+				},
+				ArtifactType: latest.ArtifactType{
+					JibArtifact: &latest.JibArtifact{},
+				},
+			},
+			evt: filemon.Events{
+				Added: []string{"this actually doesn't matter"},
+			},
+			builds: []build.Artifact{
+				{
+					ImageName: "test",
+					Tag:       "test:123",
+				},
+			},
+			expected: &Item{
+				Image: "test:123",
+				Copy: map[string][]string{
+					"file.class": {"/some/file.class"},
+				},
+				Delete: nil,
+			},
+		},
 	}
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
 			t.Override(&WorkingDir, func(string, map[string]bool) (string, error) { return test.workingDir, nil })
 			t.Override(&SyncMap, func(*latest.Artifact, map[string]bool) (map[string][]string, error) { return test.dependencies, nil })
 			t.Override(&Labels, func(string, map[string]bool) (map[string]string, error) { return test.labels, nil })
+			t.Override(&jib.GetSyncDiff, func(context.Context, string, *latest.JibArtifact, filemon.Events) (map[string][]string, map[string][]string, error) {
+				return map[string][]string{"file.class": {"/some/file.class"}}, nil, nil
+			})
 
-			actual, err := NewItem(test.artifact, test.evt, test.builds, nil)
+			actual, err := NewItem(ctx, test.artifact, test.evt, test.builds, nil)
 
 			t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expected, actual)
 		})
@@ -947,3 +984,58 @@ func TestSyncMap(t *testing.T) {
 		})
 	}
 }
+
+func TestInit(t *testing.T) {
+	ctx := context.Background()
+	tests := []struct {
+		description string
+		artifact    *latest.Artifact
+		shouldInit  bool
+		initErrors  bool
+	}{
+		{
+			description: "sync off",
+			artifact:    &latest.Artifact{},
+			shouldInit:  false,
+		},
+		{
+			description: "sync on, auto off",
+			artifact:    &latest.Artifact{Sync: &latest.Sync{}},
+			shouldInit:  false,
+		},
+		{
+			description: "sync on, auto on, non-jib",
+			artifact:    &latest.Artifact{Sync: &latest.Sync{Auto: &latest.Auto{}}},
+			shouldInit:  false,
+		},
+		{
+			description: "sync on, auto on, jib",
+			artifact:    &latest.Artifact{ArtifactType: latest.ArtifactType{JibArtifact: &latest.JibArtifact{}}, Sync: &latest.Sync{Auto: &latest.Auto{}}},
+			shouldInit:  true,
+			initErrors:  false,
+		},
+		{
+			description: "sync on, auto on, jib, init fails",
+			artifact:    &latest.Artifact{ArtifactType: latest.ArtifactType{JibArtifact: &latest.JibArtifact{}}, Sync: &latest.Sync{Auto: &latest.Auto{}}},
+			shouldInit:  true,
+			initErrors:  true,
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			isCalled := false
+			t.Override(&jib.InitSync, func(ctx context.Context, workspace string, a *latest.JibArtifact) error {
+				isCalled = true
+				if test.initErrors {
+					return errors.New("intentional test failure")
+				}
+				return nil
+			})
+
+			artifacts := []*latest.Artifact{test.artifact}
+			err := Init(ctx, artifacts)
+			t.CheckDeepEqual(test.shouldInit, isCalled)
+			t.CheckError(test.initErrors, err)
+		})
+	}
+}