fix(backup): use configured imagePullPolicy for backup CronJob

[snecklifter]

Summary

• Read imagePullPolicy from DevWorkspaceOperatorConfig for backup Job containers instead of hardcoding "Always"
• Falls back to "Always" when the field is not explicitly configured, preserving existing default behavior
• Adds unit tests for both configured and default imagePullPolicy scenarios

Fixes #1637

Verification

Configure imagePullPolicy in the DevWorkspaceOperatorConfig:

kubectl patch devworkspaceoperatorconfig devworkspace-operator-config \
  -n devworkspace-controller --type merge \
  -p '{"config":{"workspace":{"imagePullPolicy":"IfNotPresent"}}}'

After a backup Job is created, confirm the policy is applied:

kubectl get job -l controller.devfile.io/devworkspace-backup=true \
  -o jsonpath='{.items[0].spec.template.spec.containers[0].imagePullPolicy}'

Expected output: IfNotPresent

Test plan

• Unit test verifies backup Job uses configured imagePullPolicy (e.g. IfNotPresent)
• Unit test verifies default Always behavior when imagePullPolicy is not set
• All existing backup controller tests pass (go test ./controllers/backupcronjob/...)
• go vet clean

Assisted-by: Claude Code

[openshift-ci]

Hi @snecklifter. Thanks for your PR.

I'm waiting for a devfile member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[coderabbitai]

📝 Walkthrough

Walkthrough

The backup CronJob controller's createBackupJob function previously hard-coded "Always" as the container ImagePullPolicy. A new getImagePullPolicy helper now reads the policy from dwOperatorConfig.Config.Workspace.ImagePullPolicy, defaulting to corev1.PullAlways when unset. Two new tests cover both cases.

Changes

Backup Job ImagePullPolicy from operator config

Layer / File(s)	Summary
getImagePullPolicy helper and backup Job wiring controllers/backupcronjob/backupcronjob_controller.go	Adds getImagePullPolicy(dwOperatorConfig) that returns dwOperatorConfig.Config.Workspace.ImagePullPolicy when non-empty, otherwise corev1.PullAlways. Updates createBackupJob to use this helper instead of the hard-coded "Always" string.
Tests for configured and default ImagePullPolicy controllers/backupcronjob/backupcronjob_controller_test.go	Adds two It blocks: one verifying that IfNotPresent set in operator config is propagated to the generated backup Job's first container, and one verifying that the backup Job defaults to Always when ImagePullPolicy is not configured.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related issues

• Backup CronJob should use DevWorkspaceOperatorConfig imagePullPolicy #1637 — This PR directly implements the acceptance criteria: reads config.workspace.imagePullPolicy from DevWorkspaceOperatorConfig, falls back to Always, and adds the requested unit test for IfNotPresent.
• Backup CronJob should apply DevWorkspaceOperatorConfig podSecurityContext #1636 — The same config-driven pattern applied here (operator config field → backup Job spec) is exactly the pattern requested for podSecurityContext on the backup Job.

Suggested labels

lgtm, approved

Suggested reviewers

• ibuziuk
• btjd
• dkwon17
• rohanKanojia

Poem

🐇 No more "Always" pulling, every single time,
A config check now governs this runtime rhyme.
IfNotPresent skips the registry queue,
The backup Job spec now honors what you configure to.
Hoppy coding, friends — less throttle, more flow! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: using configured imagePullPolicy instead of hardcoding it for the backup CronJob.
Linked Issues check	✅ Passed	The PR implements all coding requirements from issue #1637: reads imagePullPolicy from DevWorkspaceOperatorConfig, defaults to Always when unset, and includes unit tests validating both configured and default behaviors.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #1637: modifying backup controller to use configured imagePullPolicy and adding corresponding test coverage with no unrelated alterations.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rohanKanojia, snecklifter
Once this PR has been reviewed and has the lgtm label, please assign dkwon17 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[tolusha]

/che-ai-assistant ok-pr-readiness

Review is complete. Please check the review comments below.

[tolusha]

PR Readiness Assessment: PR #1654 — fix(backup): use configured imagePullPolicy for backup CronJob

Repository: devfile/devworkspace-operator
Linked Issue: #1637 — Backup CronJob should use DevWorkspaceOperatorConfig imagePullPolicy

---

#	Criterion	Verdict	Notes
1	Problem Statement	PASS	Issue clearly describes the hardcoded imagePullPolicy: Always and its impact on registry rate limiting during backup load tests
2	Reproduction Steps	WARN	Problem is observable by inspecting the code or checking backup Job specs, but no explicit load test reproduction steps provided
3	Expected Behavior After Fix	PASS	Acceptance criteria in issue and PR description clearly state the fix should read from config and fall back to Always when unset
4	Scope of Changes	PASS	Focused 2-file change: controller logic + tests. New helper function getImagePullPolicy() is straightforward
5	Test Evidence	PASS	Comprehensive unit tests for both configured (IfNotPresent) and default (Always) scenarios, existing tests pass, go vet clean
6	Deployment & Verification Notes	WARN	Missing guidance on how to configure imagePullPolicy in DevWorkspaceOperatorConfig and verify the change takes effect in a real cluster, especially under load test conditions

Overall: NEEDS WORK

---

Missing Information

• Deployment verification steps: Add instructions for configuring imagePullPolicy in the DevWorkspaceOperatorConfig resource (e.g., the exact YAML patch or kubectl command)
• Observable verification: Describe how to confirm a backup Job uses the configured policy (e.g., kubectl get job <name> -o jsonpath='{.spec.template.spec.containers[0].imagePullPolicy}')
• Load test validation: Since the issue mentions backup load tests and registry QPS limits, include guidance on how to verify the fix actually mitigates rate limiting (or reference where those load tests are documented)

What's Good

• Clear problem statement: The issue provides exact code location, explains the inconsistency with other workspace containers, and describes the real-world impact (registry rate limiting)
• Well-defined acceptance criteria: The linked issue has explicit checkboxes for expected behavior, fallback logic, and testing requirements
• Solid test coverage: Two new unit tests validate both the configured and default behavior paths, ensuring backward compatibility
• Small, focused change: The fix is a simple refactor that extracts configuration reading into a helper function—easy to review and low risk
• Good PR hygiene: The PR description summarizes changes concisely, references the linked issue, and includes a test plan checklist