Skip to content

fix: default init container imagePullPolicy to IfNotPresent #1692

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dkwon17 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: default init container imagePullPolicy to IfNotPresent #1692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions controllers/workspace/devworkspace_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
177 changes: 177 additions & 0 deletions controllers/workspace/devworkspace_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
1 change: 1 addition & 0 deletions pkg/config/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ var defaultConfig = &v1alpha1.OperatorConfiguration{
ContainerSecurityContext: nil, // Set per-platform in setDefaultContainerSecurityContext()
DefaultTemplate: nil,
ProjectCloneConfig: &v1alpha1.ProjectCloneConfig{
ImagePullPolicy: corev1.PullIfNotPresent,

@coderabbitai coderabbitai Bot Aug 12, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Update the public fallback documentation.

Line 60 makes an unset workspace.projectClone.imagePullPolicy resolve to IfNotPresent. apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go:395-407 still states that an unset value uses .config.workspace.imagePullPolicy. Update the API and generated CRD documentation so users do not expect workspace.imagePullPolicy: Always to affect project-clone.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/config/defaults.go` at line 60, Update the public API documentation for
workspace.projectClone.imagePullPolicy in the relevant type definition and
regenerated CRD output to state that an unset value defaults to IfNotPresent,
rather than inheriting workspace.imagePullPolicy. Ensure the generated
documentation matches the API source and explicitly avoids implying that
workspace.imagePullPolicy: Always affects project-clone.

@rohanKanojia rohanKanojia Aug 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

devworkspace_controller.go:396 : this else is unreachable after the defaults.go change. workspace.Config comes from ResolveConfigForWorkspace → mergeConfig, which only overwrites
non-empty values, so ProjectCloneConfig.ImagePullPolicy is never "".

@coderabbitai coderabbitai Bot Aug 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline controllers/workspace/devworkspace_controller.go --items all --match 'ResolveConfigForWorkspace|ImagePullPolicy|InitContainer' || true
ast-grep outline pkg/config --items all --match 'ResolveConfigForWorkspace|mergeConfig' || true

printf '\n--- controller policy references ---\n'
rg -n -C 8 'ProjectCloneConfig\.ImagePullPolicy|ImagePullPolicy' controllers/workspace/devworkspace_controller.go

printf '\n--- configuration resolution and merge references ---\n'
rg -n -C 8 'func\s+(ResolveConfigForWorkspace|mergeConfig)\s*\(' pkg/config

printf '\n--- configuration assignment references ---\n'
rg -n -C 5 'workspace\.Config|\.Config\s*=' controllers pkg --glob '*.go'

printf '\n--- relevant tests ---\n'
rg -n -C 5 'ProjectCloneConfig.*ImagePullPolicy|ImagePullPolicy.*ProjectCloneConfig|PullIfNotPresent' controllers/workspace pkg/config --glob '*_test.go'

Length of output: 67843

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- workspace configuration resolution ---'
sed -n '110,150p' controllers/workspace/devworkspace_controller.go

printf '%s\n' '--- ResolveConfigForWorkspace ---'
sed -n '59,115p' pkg/config/sync.go

printf '%s\n' '--- global configuration initialization and default merge ---'
rg -n -C 10 'defaultConfig|internalConfig|SetGlobalConfig|mergeConfig\(' pkg/config --glob '*.go'

printf '%s\n' '--- mergeConfig ProjectCloneConfig behavior ---'
sed -n '250,430p' pkg/config/sync.go

printf '%s\n' '--- configuration-related tests that establish default merging ---'
rg -n -C 6 'defaultConfig|ProjectCloneConfig|ResolveConfigForWorkspace|mergeConfig' pkg/config --glob '*_test.go'

Length of output: 91908

@rohanKanojia, confirmed.

internalConfig starts from defaultConfig, and mergeConfig does not replace ProjectCloneConfig.ImagePullPolicy with an empty value. ResolveConfigForWorkspace also merges an external configuration over internalConfig.

Therefore, workspace.Config.Workspace.ProjectCloneConfig.ImagePullPolicy is always non-empty. The fallback at devworkspace_controller.go:396-398 is unreachable.

This also confirms the documentation finding. The API documentation must not state that project-clone inherits workspace.imagePullPolicy.

🧬 Code Graph Analysis Results

apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go:395-407

ProjectCloneConfig struct {
	// Image is the container image to use for cloning projects
	Image string `json:"image,omitempty"`
	// ImagePullPolicy configures the imagePullPolicy for the project clone container.
	// If undefined, the general setting .config.workspace.imagePullPolicy is used instead.
	ImagePullPolicy corev1.PullPolicy `json:"imagePullPolicy,omitempty"`
	// Resources defines the resource (cpu, memory) limits and requests for the project
	// clone container. To explicitly not specify a limit or request, define the resource
	// quantity as zero ('0')
	Resources *corev1.ResourceRequirements `json:"resources,omitempty"`
	// Env allows defining additional environment variables for the project clone container.
	Env []corev1.EnvVar `json:"env,omitempty"`
}

apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go:167-174

WorkspaceConfig struct {
	// ProjectCloneConfig defines configuration related to the project clone init container
	// that is used to clone git projects into the DevWorkspace.
	ProjectCloneConfig *ProjectCloneConfig `json:"projectClone,omitempty"`
	// RestoreConfig defines configuration related to the workspace restore init container
	// that is used to restore workspace data from a backup image.
	RestoreConfig *RestoreConfig `json:"restore,omitempty"`

apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go:175-181

	// ImagePullPolicy defines the imagePullPolicy used for containers in a DevWorkspace
	// For additional information, see Kubernetes documentation for imagePullPolicy. If
	// not specified, the default value of "Always" is used.
	// +kubebuilder:validation:Enum=IfNotPresent;Always;Never
	ImagePullPolicy string `json:"imagePullPolicy,omitempty"`
✏️ Learnings added 
Learnt from: rohanKanojia
Repo: devfile/devworkspace-operator PR: 1692
File: pkg/config/defaults.go:60-60
Timestamp: 2026-08-13T07:01:37.222Z
Learning: In `pkg/config/sync.go`, `internalConfig` is initialized from `defaultConfig`, and `mergeConfig` only overwrites `WorkspaceConfig.ProjectCloneConfig.ImagePullPolicy` when the incoming value is non-empty. `ResolveConfigForWorkspace` merges external DevWorkspaceOperatorConfig values over `internalConfig`. Therefore, after `pkg/config/defaults.go` sets `ProjectCloneConfig.ImagePullPolicy` to `corev1.PullIfNotPresent`, the fallback branch in `controllers/workspace/devworkspace_controller.go` that handles an empty project-clone image pull policy is unreachable.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

Resources: &corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceMemory: resource.MustParse("1Gi"),
Expand Down
1 change: 1 addition & 0 deletions pkg/config/sync_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading