From fd8bd97cb0240a6607e5a2e17ebdaa9227918841 Mon Sep 17 00:00:00 2001 From: David Kwon Date: Wed, 12 Aug 2026 11:25:30 -0400 Subject: [PATCH] fix: default init container imagePullPolicy to IfNotPresent Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 Signed-off-by: David Kwon --- .../workspace/devworkspace_controller.go | 3 + .../workspace/devworkspace_controller_test.go | 177 ++++++++++++++++++ pkg/config/defaults.go | 1 + pkg/config/sync_test.go | 1 + 4 files changed, 182 insertions(+) diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index 4c92cd297..375016951 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -424,6 +424,9 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request continue } } + if container.ImagePullPolicy == "" { + container.ImagePullPolicy = corev1.PullIfNotPresent + } patches = append(patches, container) } diff --git a/controllers/workspace/devworkspace_controller_test.go b/controllers/workspace/devworkspace_controller_test.go index 26bee1609..df850e6d5 100644 --- a/controllers/workspace/devworkspace_controller_test.go +++ b/controllers/workspace/devworkspace_controller_test.go @@ -1555,6 +1555,183 @@ var _ = Describe("DevWorkspace Controller", func() { }) + Context("Init container imagePullPolicy", func() { + const testURL = "test-url" + + BeforeEach(func() { + workspacecontroller.SetupHttpClientsForTesting(&http.Client{ + Transport: &testutil.TestRoundTripper{ + Data: map[string]testutil.TestResponse{ + fmt.Sprintf("%s/healthz", testURL): { + StatusCode: http.StatusOK, + }, + }, + }, + }) + }) + + AfterEach(func() { + deleteDevWorkspace(devWorkspaceName) + workspacecontroller.SetupHttpClientsForTesting(getBasicTestHttpClient()) + }) + + It("Defaults project-clone imagePullPolicy to IfNotPresent", func() { + createDevWorkspace(devWorkspaceName, "test-devworkspace.yaml") + devworkspace := getExistingDevWorkspace(devWorkspaceName) + workspaceID := devworkspace.Status.DevWorkspaceId + + By("Manually making Routing ready to continue") + markRoutingReady(testURL, common.DevWorkspaceRoutingName(workspaceID)) + + deploy := &appsv1.Deployment{} + deployNN := namespacedName(common.DeploymentName(workspaceID), testNamespace) + Eventually(func() error { + return k8sClient.Get(ctx, deployNN, deploy) + }, timeout, interval).Should(Succeed(), "Getting workspace deployment from cluster") + + var projectClone *corev1.Container + for i := range deploy.Spec.Template.Spec.InitContainers { + if deploy.Spec.Template.Spec.InitContainers[i].Name == projects.ProjectClonerContainerName { + projectClone = &deploy.Spec.Template.Spec.InitContainers[i] + } + } + Expect(projectClone).NotTo(BeNil(), "project-clone init container should be present") + Expect(projectClone.ImagePullPolicy).To(Equal(corev1.PullIfNotPresent), "project-clone should default to IfNotPresent") + }) + + It("Uses DWOC override for project-clone imagePullPolicy", func() { + config.SetGlobalConfigForTesting(&controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + ProjectCloneConfig: &controllerv1alpha1.ProjectCloneConfig{ + ImagePullPolicy: corev1.PullAlways, + }, + }, + }) + defer config.SetGlobalConfigForTesting(nil) + + createDevWorkspace(devWorkspaceName, "test-devworkspace.yaml") + devworkspace := getExistingDevWorkspace(devWorkspaceName) + workspaceID := devworkspace.Status.DevWorkspaceId + + By("Manually making Routing ready to continue") + markRoutingReady(testURL, common.DevWorkspaceRoutingName(workspaceID)) + + deploy := &appsv1.Deployment{} + deployNN := namespacedName(common.DeploymentName(workspaceID), testNamespace) + Eventually(func() error { + return k8sClient.Get(ctx, deployNN, deploy) + }, timeout, interval).Should(Succeed(), "Getting workspace deployment from cluster") + + var projectClone *corev1.Container + for i := range deploy.Spec.Template.Spec.InitContainers { + if deploy.Spec.Template.Spec.InitContainers[i].Name == projects.ProjectClonerContainerName { + projectClone = &deploy.Spec.Template.Spec.InitContainers[i] + } + } + Expect(projectClone).NotTo(BeNil(), "project-clone init container should be present") + Expect(projectClone.ImagePullPolicy).To(Equal(corev1.PullAlways), "project-clone should use DWOC override") + }) + + It("Defaults DWOC init container imagePullPolicy to IfNotPresent", func() { + config.SetGlobalConfigForTesting(&controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + InitContainers: []corev1.Container{ + { + Name: "test-init", + Image: "busybox:latest", + Command: []string{"sh", "-c", "echo hello"}, + }, + }, + }, + }) + defer config.SetGlobalConfigForTesting(nil) + + createDevWorkspace(devWorkspaceName, "test-devworkspace.yaml") + devworkspace := getExistingDevWorkspace(devWorkspaceName) + workspaceID := devworkspace.Status.DevWorkspaceId + + By("Manually making Routing ready to continue") + markRoutingReady(testURL, common.DevWorkspaceRoutingName(workspaceID)) + + deploy := &appsv1.Deployment{} + deployNN := namespacedName(common.DeploymentName(workspaceID), testNamespace) + Eventually(func() error { + return k8sClient.Get(ctx, deployNN, deploy) + }, timeout, interval).Should(Succeed(), "Getting workspace deployment from cluster") + + var testInit *corev1.Container + for i := range deploy.Spec.Template.Spec.InitContainers { + if deploy.Spec.Template.Spec.InitContainers[i].Name == "test-init" { + testInit = &deploy.Spec.Template.Spec.InitContainers[i] + } + } + Expect(testInit).NotTo(BeNil(), "test-init container should be present") + Expect(testInit.ImagePullPolicy).To(Equal(corev1.PullIfNotPresent), "DWOC init container should default to IfNotPresent") + }) + + It("Preserves explicit imagePullPolicy on DWOC init containers", func() { + config.SetGlobalConfigForTesting(&controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + InitContainers: []corev1.Container{ + { + Name: "test-init", + Image: "busybox:latest", + Command: []string{"sh", "-c", "echo hello"}, + ImagePullPolicy: corev1.PullAlways, + }, + }, + }, + }) + defer config.SetGlobalConfigForTesting(nil) + + createDevWorkspace(devWorkspaceName, "test-devworkspace.yaml") + devworkspace := getExistingDevWorkspace(devWorkspaceName) + workspaceID := devworkspace.Status.DevWorkspaceId + + By("Manually making Routing ready to continue") + markRoutingReady(testURL, common.DevWorkspaceRoutingName(workspaceID)) + + deploy := &appsv1.Deployment{} + deployNN := namespacedName(common.DeploymentName(workspaceID), testNamespace) + Eventually(func() error { + return k8sClient.Get(ctx, deployNN, deploy) + }, timeout, interval).Should(Succeed(), "Getting workspace deployment from cluster") + + var testInit *corev1.Container + for i := range deploy.Spec.Template.Spec.InitContainers { + if deploy.Spec.Template.Spec.InitContainers[i].Name == "test-init" { + testInit = &deploy.Spec.Template.Spec.InitContainers[i] + } + } + Expect(testInit).NotTo(BeNil(), "test-init container should be present") + Expect(testInit.ImagePullPolicy).To(Equal(corev1.PullAlways), "Explicit imagePullPolicy should be preserved") + }) + + It("Keeps workspace container imagePullPolicy as Always", func() { + createDevWorkspace(devWorkspaceName, "test-devworkspace.yaml") + devworkspace := getExistingDevWorkspace(devWorkspaceName) + workspaceID := devworkspace.Status.DevWorkspaceId + + By("Manually making Routing ready to continue") + markRoutingReady(testURL, common.DevWorkspaceRoutingName(workspaceID)) + + deploy := &appsv1.Deployment{} + deployNN := namespacedName(common.DeploymentName(workspaceID), testNamespace) + Eventually(func() error { + return k8sClient.Get(ctx, deployNN, deploy) + }, timeout, interval).Should(Succeed(), "Getting workspace deployment from cluster") + + var devContainer *corev1.Container + for i := range deploy.Spec.Template.Spec.Containers { + if deploy.Spec.Template.Spec.Containers[i].Name == "web-terminal" { + devContainer = &deploy.Spec.Template.Spec.Containers[i] + } + } + Expect(devContainer).NotTo(BeNil(), "web-terminal container should be present") + Expect(devContainer.ImagePullPolicy).To(Equal(corev1.PullAlways), "Workspace containers should still default to Always") + }) + }) + Context("Edge cases", func() { It("Allows Kubernetes and Container components to share same target port on endpoint", func() { diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index 7b299ac38..176ba6559 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -57,6 +57,7 @@ var defaultConfig = &v1alpha1.OperatorConfiguration{ ContainerSecurityContext: nil, // Set per-platform in setDefaultContainerSecurityContext() DefaultTemplate: nil, ProjectCloneConfig: &v1alpha1.ProjectCloneConfig{ + ImagePullPolicy: corev1.PullIfNotPresent, Resources: &corev1.ResourceRequirements{ Limits: corev1.ResourceList{ corev1.ResourceMemory: resource.MustParse("1Gi"), diff --git a/pkg/config/sync_test.go b/pkg/config/sync_test.go index c01cada4a..b937c84e4 100644 --- a/pkg/config/sync_test.go +++ b/pkg/config/sync_test.go @@ -99,6 +99,7 @@ func TestMergesAllFieldsFromClusterConfig(t *testing.T) { func(_ *dw.DevWorkspaceTemplateSpecContent, c fuzz.Continue) {}, // Ensure no empty strings are generated as they cause default values to be used func(s *string, c fuzz.Continue) { *s = "a" + c.RandString() }, + func(p *corev1.PullPolicy, c fuzz.Continue) { *p = corev1.PullPolicy("a" + c.RandString()) }, // The only valid deployment strategies are Recreate and RollingUpdate func(deploymentStrategy *appsv1.DeploymentStrategyType, c fuzz.Continue) { if c.Int()%2 == 0 {