fix(sync): add validation for namespaces and handle empty pods

GoogleContainerTools · Dec 6, 2024 · 25a48dc · 25a48dc
1 parent 8103806
commit 25a48dc
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 43 deletions.
diff --git a/pkg/skaffold/sync/sync.go b/pkg/skaffold/sync/sync.go
@@ -328,6 +328,10 @@ func Perform(ctx context.Context, image string, files syncMap, cmdFn func(contex
 		return nil
 	}
 
+	if len(namespaces) == 0 {
+		return fmt.Errorf("no namespaces provided for syncing")
+	}
+
 	errs, ctx := errgroup.WithContext(ctx)
 
 	client, err := kubernetesclient.Client(kubeContext)
@@ -340,10 +344,16 @@ func Perform(ctx context.Context, image string, files syncMap, cmdFn func(contex
 		pods, err := client.CoreV1().Pods(ns).List(ctx, metav1.ListOptions{
 			FieldSelector: fmt.Sprintf("status.phase=%s", v1.PodRunning),
 		})
+
 		if err != nil {
 			return fmt.Errorf("getting pods for namespace %q: %w", ns, err)
 		}
 
+		if len(pods.Items) == 0 {
+			log.Entry(ctx).Warnf("no running pods found in namespace %q", ns)
+			continue
+		}
+
 		for _, p := range pods.Items {
 			for _, c := range p.Spec.Containers {
 				if c.Image != image {

diff --git a/pkg/skaffold/sync/sync_test.go b/pkg/skaffold/sync/sync_test.go
@@ -903,7 +903,7 @@ func fakeCmd(ctx context.Context, _ v1.Pod, _ v1.Container, files syncMap) *exec
 	return exec.CommandContext(ctx, "copy", args...)
 }
 
-var pod = &v1.Pod{
+var runningPod = &v1.Pod{
 	ObjectMeta: metav1.ObjectMeta{
 		Name: "podname",
 	},
@@ -921,58 +921,64 @@ var pod = &v1.Pod{
 }
 
 func TestPerform(t *testing.T) {
-	tests := []struct {
-		description string
-		image       string
-		files       syncMap
-		pod         *v1.Pod
-		cmdFn       func(context.Context, v1.Pod, v1.Container, syncMap) *exec.Cmd
-		cmdErr      error
-		clientErr   error
-		expected    []string
-		shouldErr   bool
+	tests := map[string]struct {
+		image     string
+		files     syncMap
+		pod       *v1.Pod
+		cmdFn     func(context.Context, v1.Pod, v1.Container, syncMap) *exec.Cmd
+		cmdErr    error
+		clientErr error
+		expected  []string
+		shouldErr bool
 	}{
-		{
-			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"},
+		"no error": {
+			image:    "gcr.io/k8s-skaffold:123",
+			files:    syncMap{"test.go": {"/test.go"}},
+			pod:      runningPod,
+			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,
+		"cmd error": {
+			image:     "gcr.io/k8s-skaffold:123",
+			files:     syncMap{"test.go": {"/test.go"}},
+			pod:       runningPod,
+			cmdFn:     fakeCmd,
+			cmdErr:    fmt.Errorf(""),
+			shouldErr: true,
 		},
-		{
-			description: "client error",
-			image:       "gcr.io/k8s-skaffold:123",
-			files:       syncMap{"test.go": {"/test.go"}},
-			pod:         pod,
-			cmdFn:       fakeCmd,
-			clientErr:   fmt.Errorf(""),
-			shouldErr:   true,
+		"client error": {
+			image:     "gcr.io/k8s-skaffold:123",
+			files:     syncMap{"test.go": {"/test.go"}},
+			pod:       runningPod,
+			cmdFn:     fakeCmd,
+			clientErr: fmt.Errorf(""),
+			shouldErr: true,
 		},
-		{
-			description: "no copy",
-			image:       "gcr.io/different-pod:123",
-			files:       syncMap{"test.go": {"/test.go"}},
-			pod:         pod,
-			cmdFn:       fakeCmd,
-			shouldErr:   true,
+		"pod not running": {
+			image:     "gcr.io/k8s-skaffold:123",
+			files:     syncMap{"test.go": {"/test.go"}},
+			pod:       nil,
+			cmdFn:     fakeCmd,
+			shouldErr: true,
+		},
+		"no copy": {
+			image:     "gcr.io/different-pod:123",
+			files:     syncMap{"test.go": {"/test.go"}},
+			pod:       runningPod,
+			cmdFn:     fakeCmd,
+			shouldErr: true,
 		},
 	}
-	for _, test := range tests {
-		testutil.Run(t, test.description, func(t *testutil.T) {
+	for name, test := range tests {
+		testutil.Run(t, name, func(t *testutil.T) {
 			cmdRecord := &TestCmdRecorder{err: test.cmdErr}
 
 			t.Override(&util.DefaultExecCommand, cmdRecord)
 			t.Override(&client.Client, func(string) (kubernetes.Interface, error) {
+				if test.pod == nil {
+					return fake.NewSimpleClientset(), nil
+				}
+
 				return fake.NewSimpleClientset(test.pod), test.clientErr
 			})