Skip to content

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

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

Closed
tolusha wants to merge 1 commit into from
+5 −0
Closed

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

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
5 changes: 5 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 @@ -417,6 +417,11 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
reqLogger.Info("Skipping init-persistent-home container: persistent home is disabled")
continue
}
// Skip if workspace does not need persistent home (e.g. ephemeral storage)
if !home.NeedsPersistentHomeDirectory(workspace) {

@tolusha tolusha Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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 tolusha Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

reqLogger.Info("Skipping init-persistent-home container: workspace does not need persistent home directory")
continue
}
// Skip if init container is explicitly disabled
if disableHomeInit {
reqLogger.Info("Skipping init-persistent-home container: DisableInitContainer is true")
Expand Down
Loading