Skip to content

Commit

Permalink
Feat(sync): skip sync on non-running pods
Browse files Browse the repository at this point in the history
Fix issue #2360

The goal of this PR is to avoid trying to sync files in a pod which is
not in a running state. It should avoid errors when working on a single
artifact which is deployed in different pods. For example : one running
a webserver and another running a work or a job. This is typically
something that could happend when a microservice is able to run both web
server and a job which make some sort of batch treatments on the database.
  • Loading branch information
seblegall committed Jul 5, 2019
1 parent 1d9bb69 commit 3b2d8e8
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
6 changes: 6 additions & 0 deletions pkg/skaffold/sync/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,12 @@ func Perform(ctx context.Context, image string, files syncMap, cmdFn func(contex
}

for _, p := range pods.Items {

if p.Status.Phase != v1.PodRunning {
logrus.Infof("Skipping sync with pod %s because it's not running", p.Name)
continue
}

for _, c := range p.Spec.Containers {
if c.Image != image {
continue
Expand Down
35 changes: 34 additions & 1 deletion pkg/skaffold/sync/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -511,11 +511,32 @@ var pod = &v1.Pod{
},
}

var nonRunningPod = &v1.Pod{
ObjectMeta: meta_v1.ObjectMeta{
Name: "podname",
Labels: map[string]string{
"app.kubernetes.io/managed-by": "skaffold-dirty",
},
},
Status: v1.PodStatus{
Phase: v1.PodPending,
},
Spec: v1.PodSpec{
Containers: []v1.Container{
{
Name: "container_name",
Image: "gcr.io/k8s-skaffold:123",
},
},
},
}

func TestPerform(t *testing.T) {
var tests = []struct {
description string
image string
files syncMap
pod *v1.Pod
cmdFn func(context.Context, v1.Pod, v1.Container, map[string][]string) []*exec.Cmd
cmdErr error
clientErr error
Expand All @@ -526,13 +547,15 @@ func TestPerform(t *testing.T) {
description: "no error",
image: "gcr.io/k8s-skaffold:123",
files: syncMap{"test.go": {"/test.go"}},
pod: pod,
cmdFn: fakeCmd,
expected: []string{"copy test.go /test.go"},
},
{
description: "cmd error",
image: "gcr.io/k8s-skaffold:123",
files: syncMap{"test.go": {"/test.go"}},
pod: pod,
cmdFn: fakeCmd,
cmdErr: fmt.Errorf(""),
shouldErr: true,
Expand All @@ -541,6 +564,7 @@ func TestPerform(t *testing.T) {
description: "client error",
image: "gcr.io/k8s-skaffold:123",
files: syncMap{"test.go": {"/test.go"}},
pod: pod,
cmdFn: fakeCmd,
clientErr: fmt.Errorf(""),
shouldErr: true,
Expand All @@ -549,6 +573,15 @@ func TestPerform(t *testing.T) {
description: "no copy",
image: "gcr.io/different-pod:123",
files: syncMap{"test.go": {"/test.go"}},
pod: pod,
cmdFn: fakeCmd,
shouldErr: true,
},
{
description: "Skip sync when pod is not running",
image: "gcr.io/k8s-skaffold:123",
files: syncMap{"test.go": {"/test.go"}},
pod: nonRunningPod,
cmdFn: fakeCmd,
shouldErr: true,
},
Expand All @@ -559,7 +592,7 @@ func TestPerform(t *testing.T) {

t.Override(&util.DefaultExecCommand, cmdRecord)
t.Override(&pkgkubernetes.Client, func() (kubernetes.Interface, error) {
return fake.NewSimpleClientset(pod), test.clientErr
return fake.NewSimpleClientset(test.pod), test.clientErr
})

err := Perform(context.Background(), test.image, test.files, test.cmdFn, []string{""})
Expand Down

0 comments on commit 3b2d8e8

Please sign in to comment.