From b0091ea560123ac54ea78bcaaf8b17dd4a88736c Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 5 Oct 2018 10:42:52 +0200 Subject: [PATCH 01/11] Fix errors Signed-off-by: David Gageot --- pkg/skaffold/sync/sync.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/skaffold/sync/sync.go b/pkg/skaffold/sync/sync.go index d53ab842b0b..ed5a1fd0bff 100644 --- a/pkg/skaffold/sync/sync.go +++ b/pkg/skaffold/sync/sync.go @@ -51,7 +51,7 @@ func NewItem(a *latest.Artifact, e watch.Events, builds []build.Artifact) (*Item toDelete, err := intersect(a.Workspace, a.Sync, e.Deleted) if err != nil { - return nil, errors.Wrap(err, "intersecting sync map and added, modified files") + return nil, errors.Wrap(err, "intersecting sync map and deleted files") } if toCopy == nil || toDelete == nil { @@ -60,7 +60,7 @@ func NewItem(a *latest.Artifact, e watch.Events, builds []build.Artifact) (*Item tag := latestTag(a.ImageName, builds) if tag == "" { - return nil, fmt.Errorf("Could not find latest tag for image %s in builds: %s", a.ImageName, builds) + return nil, fmt.Errorf("could not find latest tag for image %s in builds: %v", a.ImageName, builds) } return &Item{ From faecc291fa235863b3e3574cf83df045676f504b Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 5 Oct 2018 10:43:11 +0200 Subject: [PATCH 02/11] Not needed --- pkg/skaffold/sync/sync.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/skaffold/sync/sync.go b/pkg/skaffold/sync/sync.go index ed5a1fd0bff..589e84c81c9 100644 --- a/pkg/skaffold/sync/sync.go +++ b/pkg/skaffold/sync/sync.go @@ -40,7 +40,7 @@ type Item struct { func NewItem(a *latest.Artifact, e watch.Events, builds []build.Artifact) (*Item, error) { // If there are no changes, short circuit and don't sync anything - if !e.HasChanged() || a.Sync == nil || len(a.Sync) == 0 { + if !e.HasChanged() || len(a.Sync) == 0 { return nil, nil } From fea9ad1872ecba84d9c145ce863fd82d80fd8c2b Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 5 Oct 2018 10:46:32 +0200 Subject: [PATCH 03/11] Add a comment Signed-off-by: David Gageot --- pkg/skaffold/sync/sync.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/skaffold/sync/sync.go b/pkg/skaffold/sync/sync.go index 589e84c81c9..f94bcb9c0ac 100644 --- a/pkg/skaffold/sync/sync.go +++ b/pkg/skaffold/sync/sync.go @@ -54,6 +54,7 @@ func NewItem(a *latest.Artifact, e watch.Events, builds []build.Artifact) (*Item return nil, errors.Wrap(err, "intersecting sync map and deleted files") } + // Something went wrong, don't sync, rebuild. if toCopy == nil || toDelete == nil { return nil, nil } From 7f42c6354461edd7b8fdb5b8efd0241a54e7b597 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 5 Oct 2018 10:48:35 +0200 Subject: [PATCH 04/11] Simplify code Signed-off-by: David Gageot --- pkg/skaffold/runner/runner.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/skaffold/runner/runner.go b/pkg/skaffold/runner/runner.go index e9aac715715..8cfe7f71e51 100644 --- a/pkg/skaffold/runner/runner.go +++ b/pkg/skaffold/runner/runner.go @@ -259,8 +259,7 @@ func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*la } if s != nil { changed.AddResync(s) - } - if s == nil { + } else { changed.AddRebuild(a.artifact) } } From d860c54f27b439642a8e57d8d6a5ebc78fa5d120 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 5 Oct 2018 10:49:58 +0200 Subject: [PATCH 05/11] Improve logs Signed-off-by: David Gageot --- pkg/skaffold/kubernetes/sync.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/pkg/skaffold/kubernetes/sync.go b/pkg/skaffold/kubernetes/sync.go index 511f35563b2..b8d587a356d 100644 --- a/pkg/skaffold/kubernetes/sync.go +++ b/pkg/skaffold/kubernetes/sync.go @@ -35,12 +35,22 @@ import ( type KubectlSyncer struct{} func (k *KubectlSyncer) Sync(s *sync.Item) error { - if err := perform(s.Image, s.Copy, copyFileFn); err != nil { - return errors.Wrap(err, "copying files") + if len(s.Copy) > 0 { + logrus.Infoln("Copying files:", s.Copy) + + if err := perform(s.Image, s.Copy, copyFileFn); err != nil { + return errors.Wrap(err, "copying files") + } } - if err := perform(s.Image, s.Delete, deleteFileFn); err != nil { - return errors.Wrap(err, "deleting files") + + if len(s.Delete) > 0 { + logrus.Infoln("Deleting files:", s.Delete) + + if err := perform(s.Image, s.Delete, deleteFileFn); err != nil { + return errors.Wrap(err, "deleting files") + } } + return nil } @@ -61,7 +71,6 @@ func labelSelector() string { } func perform(image string, files map[string]string, cmdFn func(v1.Pod, v1.Container, string, string) *exec.Cmd) error { - logrus.Info("Syncing files:", files) client, err := Client() if err != nil { return errors.Wrap(err, "getting k8s client") From 97b0ea1dbadc42405adda58e5e0b177d03330861 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 5 Oct 2018 11:16:11 +0200 Subject: [PATCH 06/11] Pass context Signed-off-by: David Gageot --- pkg/skaffold/kubernetes/sync.go | 21 ++++++++++++--------- pkg/skaffold/kubernetes/sync_test.go | 11 +++++++---- pkg/skaffold/runner/runner.go | 7 ++++--- pkg/skaffold/runner/runner_test.go | 2 +- pkg/skaffold/sync/sync.go | 3 ++- pkg/skaffold/sync/sync_test.go | 1 - 6 files changed, 26 insertions(+), 19 deletions(-) diff --git a/pkg/skaffold/kubernetes/sync.go b/pkg/skaffold/kubernetes/sync.go index b8d587a356d..a98f58453a6 100644 --- a/pkg/skaffold/kubernetes/sync.go +++ b/pkg/skaffold/kubernetes/sync.go @@ -17,6 +17,7 @@ limitations under the License. package kubernetes import ( + "context" "fmt" "os/exec" "strings" @@ -34,11 +35,11 @@ import ( type KubectlSyncer struct{} -func (k *KubectlSyncer) Sync(s *sync.Item) error { +func (k *KubectlSyncer) Sync(ctx context.Context, s *sync.Item) error { if len(s.Copy) > 0 { logrus.Infoln("Copying files:", s.Copy) - if err := perform(s.Image, s.Copy, copyFileFn); err != nil { + if err := perform(ctx, s.Image, s.Copy, copyFileFn); err != nil { return errors.Wrap(err, "copying files") } } @@ -46,7 +47,7 @@ func (k *KubectlSyncer) Sync(s *sync.Item) error { if len(s.Delete) > 0 { logrus.Infoln("Deleting files:", s.Delete) - if err := perform(s.Image, s.Delete, deleteFileFn); err != nil { + if err := perform(ctx, s.Image, s.Delete, deleteFileFn); err != nil { return errors.Wrap(err, "deleting files") } } @@ -54,12 +55,12 @@ func (k *KubectlSyncer) Sync(s *sync.Item) error { return nil } -func deleteFileFn(pod v1.Pod, container v1.Container, src, dst string) *exec.Cmd { - return exec.Command("kubectl", "exec", pod.Name, "--namespace", pod.Namespace, "-c", container.Name, "--", "rm", "-rf", dst) +func deleteFileFn(ctx context.Context, pod v1.Pod, container v1.Container, src, dst string) *exec.Cmd { + return exec.CommandContext(ctx, "kubectl", "exec", pod.Name, "--namespace", pod.Namespace, "-c", container.Name, "--", "rm", "-rf", dst) } -func copyFileFn(pod v1.Pod, container v1.Container, src, dst string) *exec.Cmd { - return exec.Command("kubectl", "cp", src, fmt.Sprintf("%s/%s:%s", pod.Namespace, pod.Name, dst), "-c", container.Name) +func copyFileFn(ctx context.Context, pod v1.Pod, container v1.Container, src, dst string) *exec.Cmd { + return exec.CommandContext(ctx, "kubectl", "cp", src, fmt.Sprintf("%s/%s:%s", pod.Namespace, pod.Name, dst), "-c", container.Name) } func labelSelector() string { @@ -70,17 +71,19 @@ func labelSelector() string { return strings.Join(reqs, ",") } -func perform(image string, files map[string]string, cmdFn func(v1.Pod, v1.Container, string, string) *exec.Cmd) error { +func perform(ctx context.Context, image string, files map[string]string, cmdFn func(context.Context, v1.Pod, v1.Container, string, string) *exec.Cmd) error { client, err := Client() if err != nil { return errors.Wrap(err, "getting k8s client") } + pods, err := client.CoreV1().Pods("").List(meta_v1.ListOptions{ LabelSelector: labelSelector(), }) if err != nil { return errors.Wrap(err, "getting pods") } + for _, p := range pods.Items { for _, c := range p.Spec.Containers { if c.Image == image { @@ -88,7 +91,7 @@ func perform(image string, files map[string]string, cmdFn func(v1.Pod, v1.Contai for src, dst := range files { src, dst := src, dst e.Go(func() error { - cmd := cmdFn(p, c, src, dst) + cmd := cmdFn(ctx, p, c, src, dst) return util.RunCmd(cmd) }) } diff --git a/pkg/skaffold/kubernetes/sync_test.go b/pkg/skaffold/kubernetes/sync_test.go index 2cc70021dc0..84bcd8e0436 100644 --- a/pkg/skaffold/kubernetes/sync_test.go +++ b/pkg/skaffold/kubernetes/sync_test.go @@ -17,6 +17,7 @@ limitations under the License. package kubernetes import ( + "context" "fmt" "os/exec" "strings" @@ -49,8 +50,8 @@ func (t *TestCmdRecorder) RunCmdOut(cmd *exec.Cmd) ([]byte, error) { return nil, t.RunCmd(cmd) } -func fakeCmd(p v1.Pod, c v1.Container, src, dst string) *exec.Cmd { - return exec.Command("copy", src, dst) +func fakeCmd(ctx context.Context, p v1.Pod, c v1.Container, src, dst string) *exec.Cmd { + return exec.CommandContext(ctx, "copy", src, dst) } var pod = &v1.Pod{ @@ -76,7 +77,7 @@ func TestPerform(t *testing.T) { description string image string files map[string]string - cmdFn func(v1.Pod, v1.Container, string, string) *exec.Cmd + cmdFn func(context.Context, v1.Pod, v1.Container, string, string) *exec.Cmd cmdErr error clientErr error expected []string @@ -125,7 +126,9 @@ func TestPerform(t *testing.T) { } util.DefaultExecCommand = cmdRecord - err := perform(test.image, test.files, test.cmdFn) + + err := perform(context.Background(), test.image, test.files, test.cmdFn) + testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, cmdRecord.cmds) }) } diff --git a/pkg/skaffold/runner/runner.go b/pkg/skaffold/runner/runner.go index 8cfe7f71e51..e313d6fc45f 100644 --- a/pkg/skaffold/runner/runner.go +++ b/pkg/skaffold/runner/runner.go @@ -30,6 +30,7 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/kaniko" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/local" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/color" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker" @@ -270,12 +271,12 @@ func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*la return ErrorConfigurationChanged case len(changed.needsResync) > 0: for _, s := range changed.needsResync { - if err := r.Syncer.Sync(s); err != nil { + color.Default.Fprintf(out, "Syncing %d files for %s\n", len(s.Copy)+len(s.Delete), s.Image) + + if err := r.Syncer.Sync(ctx, s); err != nil { logrus.Warnln("Skipping build and deploy due to sync error:", err) return nil } - logrus.Infof("Synced %d files for %s", len(s.Copy)+len(s.Delete), s.Image) - logrus.Debugf("Synced files for %s...\nCopied: %s\nDeleted: %s\n", s.Image, s.Copy, s.Delete) } case len(changed.needsRebuild) > 0: bRes, err := r.Build(ctx, out, r.Tagger, changed.needsRebuild) diff --git a/pkg/skaffold/runner/runner_test.go b/pkg/skaffold/runner/runner_test.go index 8ab01d131a1..467ce9f5871 100644 --- a/pkg/skaffold/runner/runner_test.go +++ b/pkg/skaffold/runner/runner_test.go @@ -123,7 +123,7 @@ func NewTestSyncer() *TestSyncer { } } -func (t *TestSyncer) Sync(s *sync.Item) error { +func (t *TestSyncer) Sync(ctx context.Context, s *sync.Item) error { if t.err != nil { return t.err } diff --git a/pkg/skaffold/sync/sync.go b/pkg/skaffold/sync/sync.go index f94bcb9c0ac..d5849480a79 100644 --- a/pkg/skaffold/sync/sync.go +++ b/pkg/skaffold/sync/sync.go @@ -17,6 +17,7 @@ limitations under the License. package sync import ( + "context" "fmt" "path" "path/filepath" @@ -29,7 +30,7 @@ import ( ) type Syncer interface { - Sync(s *Item) error + Sync(context.Context, *Item) error } type Item struct { diff --git a/pkg/skaffold/sync/sync_test.go b/pkg/skaffold/sync/sync_test.go index ea310375177..175c36a5a02 100644 --- a/pkg/skaffold/sync/sync_test.go +++ b/pkg/skaffold/sync/sync_test.go @@ -21,7 +21,6 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/watch" "github.com/GoogleContainerTools/skaffold/testutil" ) From 34ddc9669f9e2e0c6d70f6e58d86543b1ea0297b Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 5 Oct 2018 11:40:33 +0200 Subject: [PATCH 07/11] Add an error if not all the files were synced Signed-off-by: David Gageot --- pkg/skaffold/kubernetes/sync.go | 41 ++++++++++++++++++---------- pkg/skaffold/kubernetes/sync_test.go | 1 + 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/pkg/skaffold/kubernetes/sync.go b/pkg/skaffold/kubernetes/sync.go index a98f58453a6..6e7e92737f0 100644 --- a/pkg/skaffold/kubernetes/sync.go +++ b/pkg/skaffold/kubernetes/sync.go @@ -37,7 +37,7 @@ type KubectlSyncer struct{} func (k *KubectlSyncer) Sync(ctx context.Context, s *sync.Item) error { if len(s.Copy) > 0 { - logrus.Infoln("Copying files:", s.Copy) + logrus.Infoln("Copying files:", s.Copy, "to", s.Image) if err := perform(ctx, s.Image, s.Copy, copyFileFn); err != nil { return errors.Wrap(err, "copying files") @@ -45,7 +45,7 @@ func (k *KubectlSyncer) Sync(ctx context.Context, s *sync.Item) error { } if len(s.Delete) > 0 { - logrus.Infoln("Deleting files:", s.Delete) + logrus.Infoln("Deleting files:", s.Delete, "from", s.Image) if err := perform(ctx, s.Image, s.Delete, deleteFileFn); err != nil { return errors.Wrap(err, "deleting files") @@ -84,22 +84,35 @@ func perform(ctx context.Context, image string, files map[string]string, cmdFn f return errors.Wrap(err, "getting pods") } + performed := 0 for _, p := range pods.Items { for _, c := range p.Spec.Containers { - if c.Image == image { - var e errgroup.Group - for src, dst := range files { - src, dst := src, dst - e.Go(func() error { - cmd := cmdFn(ctx, p, c, src, dst) - return util.RunCmd(cmd) - }) - } - if err := e.Wait(); err != nil { - return errors.Wrap(err, "syncing files") - } + if c.Image != image { + continue + } + + var e errgroup.Group + for src, dst := range files { + src, dst := src, dst + e.Go(func() error { + cmd := cmdFn(ctx, p, c, src, dst) + if err := util.RunCmd(cmd); err != nil { + return err + } + + performed++ + return nil + }) + } + if err := e.Wait(); err != nil { + return errors.Wrap(err, "syncing files") } } } + + if performed != len(files) { + return errors.New("couldn't sync all the files") + } + return nil } diff --git a/pkg/skaffold/kubernetes/sync_test.go b/pkg/skaffold/kubernetes/sync_test.go index 84bcd8e0436..36910ee3755 100644 --- a/pkg/skaffold/kubernetes/sync_test.go +++ b/pkg/skaffold/kubernetes/sync_test.go @@ -111,6 +111,7 @@ func TestPerform(t *testing.T) { image: "gcr.io/different-pod:123", files: map[string]string{"test.go": "/test.go"}, cmdFn: fakeCmd, + shouldErr: true, }, } From 1017663b3c8567d92fa731b130c8fbdf0ba5bc6f Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 5 Oct 2018 11:53:59 +0200 Subject: [PATCH 08/11] Pods are not labeled by Skaffold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can’t use skaffold labels to narrow down the list of pods. Signed-off-by: David Gageot --- pkg/skaffold/kubernetes/sync.go | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/pkg/skaffold/kubernetes/sync.go b/pkg/skaffold/kubernetes/sync.go index 6e7e92737f0..34934291e93 100644 --- a/pkg/skaffold/kubernetes/sync.go +++ b/pkg/skaffold/kubernetes/sync.go @@ -20,9 +20,7 @@ import ( "context" "fmt" "os/exec" - "strings" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/sync" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" "github.com/sirupsen/logrus" @@ -63,23 +61,13 @@ func copyFileFn(ctx context.Context, pod v1.Pod, container v1.Container, src, ds return exec.CommandContext(ctx, "kubectl", "cp", src, fmt.Sprintf("%s/%s:%s", pod.Namespace, pod.Name, dst), "-c", container.Name) } -func labelSelector() string { - var reqs []string - for k, v := range constants.Labels.DefaultLabels { - reqs = append(reqs, fmt.Sprintf("%s=%s", k, v)) - } - return strings.Join(reqs, ",") -} - func perform(ctx context.Context, image string, files map[string]string, cmdFn func(context.Context, v1.Pod, v1.Container, string, string) *exec.Cmd) error { client, err := Client() if err != nil { return errors.Wrap(err, "getting k8s client") } - pods, err := client.CoreV1().Pods("").List(meta_v1.ListOptions{ - LabelSelector: labelSelector(), - }) + pods, err := client.CoreV1().Pods("").List(meta_v1.ListOptions{}) if err != nil { return errors.Wrap(err, "getting pods") } From 81495237eb61397cb038c7c3c22629c995e60ea7 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 5 Oct 2018 11:59:26 +0200 Subject: [PATCH 09/11] Make the code go-routine safe Signed-off-by: David Gageot --- pkg/skaffold/kubernetes/sync.go | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/pkg/skaffold/kubernetes/sync.go b/pkg/skaffold/kubernetes/sync.go index 34934291e93..3693653eef5 100644 --- a/pkg/skaffold/kubernetes/sync.go +++ b/pkg/skaffold/kubernetes/sync.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "os/exec" + "sync/atomic" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/sync" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" @@ -72,33 +73,35 @@ func perform(ctx context.Context, image string, files map[string]string, cmdFn f return errors.Wrap(err, "getting pods") } - performed := 0 + var performed int32 + var e errgroup.Group + for _, p := range pods.Items { for _, c := range p.Spec.Containers { if c.Image != image { continue } - var e errgroup.Group for src, dst := range files { - src, dst := src, dst + cmd := cmdFn(ctx, p, c, src, dst) + e.Go(func() error { - cmd := cmdFn(ctx, p, c, src, dst) if err := util.RunCmd(cmd); err != nil { return err } - performed++ + atomic.AddInt32(&performed, 1) return nil }) } - if err := e.Wait(); err != nil { - return errors.Wrap(err, "syncing files") - } } } - if performed != len(files) { + if err := e.Wait(); err != nil { + return errors.Wrap(err, "syncing files") + } + + if int(performed) != len(files) { return errors.New("couldn't sync all the files") } From c53bb8643ad048332a9a73e2b76c014948e4e02f Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 5 Oct 2018 16:44:00 +0200 Subject: [PATCH 10/11] Remove duplication --- pkg/skaffold/kubernetes/sync.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/skaffold/kubernetes/sync.go b/pkg/skaffold/kubernetes/sync.go index 3693653eef5..24a1046419d 100644 --- a/pkg/skaffold/kubernetes/sync.go +++ b/pkg/skaffold/kubernetes/sync.go @@ -35,20 +35,16 @@ import ( type KubectlSyncer struct{} func (k *KubectlSyncer) Sync(ctx context.Context, s *sync.Item) error { - if len(s.Copy) > 0 { - logrus.Infoln("Copying files:", s.Copy, "to", s.Image) + logrus.Infoln("Copying files:", s.Copy, "to", s.Image) - if err := perform(ctx, s.Image, s.Copy, copyFileFn); err != nil { - return errors.Wrap(err, "copying files") - } + if err := perform(ctx, s.Image, s.Copy, copyFileFn); err != nil { + return errors.Wrap(err, "copying files") } - if len(s.Delete) > 0 { - logrus.Infoln("Deleting files:", s.Delete, "from", s.Image) + logrus.Infoln("Deleting files:", s.Delete, "from", s.Image) - if err := perform(ctx, s.Image, s.Delete, deleteFileFn); err != nil { - return errors.Wrap(err, "deleting files") - } + if err := perform(ctx, s.Image, s.Delete, deleteFileFn); err != nil { + return errors.Wrap(err, "deleting files") } return nil @@ -63,6 +59,10 @@ func copyFileFn(ctx context.Context, pod v1.Pod, container v1.Container, src, ds } func perform(ctx context.Context, image string, files map[string]string, cmdFn func(context.Context, v1.Pod, v1.Container, string, string) *exec.Cmd) error { + if len(files) == 0 { + return nil + } + client, err := Client() if err != nil { return errors.Wrap(err, "getting k8s client") From 3e8f198c69914461c8db6aada55b8036eb9cc2f4 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 5 Oct 2018 17:03:03 +0200 Subject: [PATCH 11/11] Deal with files synced to multiple pods. Sync files in a sequential manner, it makes the code simpler and anyway, the typical use case will be to sync one file with one pod. Signed-off-by: David Gageot --- pkg/skaffold/kubernetes/sync.go | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/pkg/skaffold/kubernetes/sync.go b/pkg/skaffold/kubernetes/sync.go index 24a1046419d..4e442c62921 100644 --- a/pkg/skaffold/kubernetes/sync.go +++ b/pkg/skaffold/kubernetes/sync.go @@ -20,14 +20,12 @@ import ( "context" "fmt" "os/exec" - "sync/atomic" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/sync" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" - "github.com/sirupsen/logrus" - "golang.org/x/sync/errgroup" "github.com/pkg/errors" + "github.com/sirupsen/logrus" "k8s.io/api/core/v1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -73,8 +71,7 @@ func perform(ctx context.Context, image string, files map[string]string, cmdFn f return errors.Wrap(err, "getting pods") } - var performed int32 - var e errgroup.Group + synced := map[string]bool{} for _, p := range pods.Items { for _, c := range p.Spec.Containers { @@ -84,24 +81,16 @@ func perform(ctx context.Context, image string, files map[string]string, cmdFn f for src, dst := range files { cmd := cmdFn(ctx, p, c, src, dst) + if err := util.RunCmd(cmd); err != nil { + return err + } - e.Go(func() error { - if err := util.RunCmd(cmd); err != nil { - return err - } - - atomic.AddInt32(&performed, 1) - return nil - }) + synced[src] = true } } } - if err := e.Wait(); err != nil { - return errors.Wrap(err, "syncing files") - } - - if int(performed) != len(files) { + if len(synced) != len(files) { return errors.New("couldn't sync all the files") }