fix(home): skip persistent home init container for ephemeral workspaces by tolusha · Pull Request #1651 · devfile/devworkspace-operator

tolusha · 2026-06-17T12:52:43Z

When a DevWorkspaceOperatorConfig defines an init-persistent-home entry in workspace.initContainers, the config merge logic only checked whether persistent home was globally enabled (PersistUserHomeEnabled) but not whether the specific workspace actually needs it. Ephemeral workspaces passed this check, causing an init container to be injected that references a non-existent persistent-home volume and potentially has no image — resulting in an invalid Deployment.

Add a NeedsPersistentHomeDirectory check to also skip the config-level init container when the workspace storage strategy does not support home persistence (e.g. ephemeral storage).

Assisted-by: Claude Code

What issues does this PR fix or reference?

eclipse-che/che#23884

Is it tested? How?

TODO

PR Checklist

E2E tests pass (when PR is ready, comment /test v8-devworkspace-operator-e2e, v8-che-happy-path to trigger)
- v8-devworkspace-operator-e2e: DevWorkspace e2e test
- v8-che-happy-path: Happy path for verification integration with Che

When a DevWorkspaceOperatorConfig defines an init-persistent-home entry in workspace.initContainers, the config merge logic only checked whether persistent home was globally enabled (PersistUserHomeEnabled) but not whether the specific workspace actually needs it. Ephemeral workspaces passed this check, causing an init container to be injected that references a non-existent persistent-home volume and potentially has no image — resulting in an invalid Deployment. Add a NeedsPersistentHomeDirectory check to also skip the config-level init container when the workspace storage strategy does not support home persistence (e.g. ephemeral storage). Fixes: eclipse-che/che#23884 Assisted-by: Claude Code Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Anatolii Bazko <abazko@redhat.com>

openshift-ci · 2026-06-17T12:52:50Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tolusha
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

coderabbitai · 2026-06-17T12:53:37Z

Warning

Review limit reached

@tolusha, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 40 minutes and 22 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 87aee401-f239-4c38-8481-0c026aaf21e9

📥 Commits

Reviewing files that changed from the base of the PR and between 32c3d0a and 7d776fe.

📒 Files selected for processing (1)

controllers/workspace/devworkspace_controller.go

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/ephemeral-persistent-home-init-container

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

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

tolusha · 2026-06-17T12:56:01Z

Hi! I'm che-ai-assistant — I help with your pull requests.

Available commands:

/che-ai-assistant generate-che-doc — Generate a documentation PR based on this PR's changes
/che-ai-assistant ok-pr-review — Run a comprehensive PR review (summary, code review, deep review, impact analysis)
/che-ai-assistant help — Show this help message

tolusha · 2026-06-17T12:56:17Z

/che-ai-assistant ok-pr-review

tolusha

Review Summary

This is a well-crafted bug fix that correctly addresses the issue. The fix is minimal, surgical, and reuses existing abstractions appropriately. The main concern is test coverage, which you've already acknowledged as TODO.

Positive Observations

The fix correctly delegates to NeedsPersistentHomeDirectory() rather than duplicating logic
The log message is distinct and diagnostic-friendly
Commit message thoroughly explains root cause and fix
Design is sound - aligns the config-level guard logic with the Devfile-level path

Key Findings

I've added inline comments with specific recommendations. The most important is adding test coverage for the ephemeral storage case.

Review conducted using ok-pr-review

tolusha · 2026-06-17T13:18:39Z

 					continue
 				}
+				// Skip if workspace does not need persistent home (e.g. ephemeral storage)
+				if !home.NeedsPersistentHomeDirectory(workspace) {


Add test coverage for ephemeral storage case

The PR description marks testing as "TODO". This is a bug fix in the main reconciliation loop - the exact kind of change that benefits most from a unit test proving ephemeral workspaces skip the init container.

The existing pkg/library/home/persistentHome_test.go tests NeedsPersistentHomeDirectory itself, but there's no test verifying the controller's filtering logic applies it correctly. More importantly, none of the 10 test fixtures in testdata/persistent-home/ test the ephemeral storage branch (storageStrategySupportsPersistentHome returning false).

Recommendation: Add a test fixture like noop-if-ephemeral-storage.yaml to pkg/library/home/testdata/persistent-home/ with:

Workspace that has controller.devfile.io/storage-type: ephemeral in its attributes

Persistent home enabled in config

Expected output: workspace unchanged (no home volume added)

This is low-effort (YAML file only) and closes the only real coverage gap.

tolusha · 2026-06-17T13:19:00Z

 					continue
 				}
+				// Skip if workspace does not need persistent home (e.g. ephemeral storage)
+				if !home.NeedsPersistentHomeDirectory(workspace) {


Minor note - redundant check

home.NeedsPersistentHomeDirectory(workspace) internally calls home.PersistUserHomeEnabled(workspace) (see persistentHome.go:96), which is already checked on line 418 above.

This is harmless - the function is cheap and the double-check doesn't affect correctness. The current approach preserves distinct log messages for each skip reason, which aids debugging. Not a defect, just worth noting for future maintainers.

tolusha requested review from akurinnoy, btjd, dkwon17, ibuziuk and rohanKanojia as code owners June 17, 2026 12:52

tolusha commented Jun 17, 2026

View reviewed changes

tolusha closed this Jun 17, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(home): skip persistent home init container for ephemeral workspaces#1651

fix(home): skip persistent home init container for ephemeral workspaces#1651
tolusha wants to merge 1 commit into
mainfrom
fix/ephemeral-persistent-home-init-container

tolusha commented Jun 17, 2026 •

edited

Loading

Uh oh!

openshift-ci Bot commented Jun 17, 2026

Uh oh!

coderabbitai Bot commented Jun 17, 2026

Review limit reached

Uh oh!

tolusha commented Jun 17, 2026

Uh oh!

tolusha commented Jun 17, 2026

Uh oh!

tolusha left a comment

Uh oh!

tolusha Jun 17, 2026

Uh oh!

tolusha Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

tolusha commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What issues does this PR fix or reference?

Is it tested? How?

PR Checklist

Uh oh!

openshift-ci Bot commented Jun 17, 2026

Uh oh!

coderabbitai Bot commented Jun 17, 2026

Review limit reached

Uh oh!

tolusha commented Jun 17, 2026

Uh oh!

tolusha commented Jun 17, 2026

Uh oh!

tolusha left a comment

Choose a reason for hiding this comment

Review Summary

Positive Observations

Key Findings

Uh oh!

tolusha Jun 17, 2026

Choose a reason for hiding this comment

Add test coverage for ephemeral storage case

Uh oh!

tolusha Jun 17, 2026

Choose a reason for hiding this comment

Minor note - redundant check

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

tolusha commented Jun 17, 2026 •

edited

Loading