fix : reset workspace storage defaults when PVC config removed

[rohanKanojia]

What does this PR do?

Previously, removing a workspace PVC Config (e.g., perUserStrategyPvcConfig) did not reset workspaceConfig fields. As a result, stale values in persisted instead of reverting to defaults. See my comment on this issue eclipse-che/che#23502 (comment)

This change explicitly resets:

• StorageAccessMode to {ReadWriteOnce}
• StorageClassName to nil
• DefaultStorageSize to nil

before applying any PVC config, ensuring that removing PVC config restores default workspace storage settings.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

eclipse-che/che#23502

How to test this PR?

1. Deploy the operator:

OpenShift

./build/scripts/olm/test-catalog-from-sources.sh

or

build/scripts/docker-run.sh /bin/bash -c "
  oc login \
    --token=<...> \
    --server=<...> \
    --insecure-skip-tls-verify=true && \
  build/scripts/olm/test-catalog-from-sources.sh
"

on Minikube

./build/scripts/minikube-tests/test-operator-from-sources.sh

2. In the CheCluster configuration, set:

kubectl patch checluster eclipse-che -n eclipse-che --type merge -p '{"spec":{"devEnvironments":{"storage":{"perUserStrategyPvcConfig":{"storageAccessMode":["ReadWriteMany"]}}}}}'

checluster.org.eclipse.che/eclipse-che patched

3. Check DWOC, it should have updated storageAccessMode:

oc get dwoc -n eclipse-che -o jsonpath='{.items[*].config.workspace.storageAccessMode}' 

["ReadWriteMany"]

4. Then remove perUserStrategyPvcConfig:

kubectl patch checluster eclipse-che -n eclipse-che --type merge -p '{"spec":{"devEnvironments":{"storage":{"perUserStrategyPvcConfig":null}}}}'

checluster.org.eclipse.che/eclipse-che patched

5. Check DWOC, it should be reset to default value:

oc get dwoc -n eclipse-che -o jsonpath='{.items[*].config.workspace.storageAccessMode}'

["ReadWriteOnce"]

Common Test Scenarios

• Deploy Eclipse Che
• Start an empty workspace
• Open terminal and build/run an image
• Stop a workspace
• Check operator logs for reconciliation errors or infinite reconciliation loops

PR Checklist

As the author of this Pull Request I made sure that:

• The Eclipse Contributor Agreement is valid
• Code produced is complete
• Code builds without errors
• Tests are covering the bugfix
• The repository devfile is up to date and works
• Relevant user documentation updated
• Relevant contributing documentation updated
• CI/CD changes implemented, documented and communicated

Reviewers

Reviewers, please comment how you tested the PR when approving it.

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[tolusha]

Please, add a constant for default access mode

[rohanKanojia]

Sorry, I've added it to constants.go and reference it here.

[dkwon17]

What happens if we set

workspaceConfig.StorageAccessMode = nil

instead?

Would the DWO set the default value in that case?

[rohanKanojia]

No, according to the current logic:

1. If the workspace PVC Config (e.g., perUserStrategyPvcConfig) is defined and it has StorageAccessMode nil, we assign it as it is (nil in this case).

storage:
      perUserStrategyPvcConfig:
        storageAccessMode: null
      pvcStrategy: per-user

2. If the workspace PVC config is omitted like this, we assign a default value:

storage:
        pvcStrategy: per-user

Do you think we should add a nil or empty array check here? But since it's explicitly specified by user shall we accept it as it is?

[dkwon17]

Basically what I want to avoid is to define and use DefaultWorkspaceStorageAccessMode = []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce} in che-operator.

In line 179 if we have:

workspaceConfig.StorageAccessMode = nil

instead of:

workspaceConfig.StorageAccessMode = constants.DefaultWorkspaceStorageAccessMode

would it mean that if perUserStrategyPvcConfig is not defined, then thestorageAccessMode field will be unset/removed in the DWOC?

If it's removed in the DWOC, then the DevWorkspace operator should use the default value for storageAccessMode, which IMHO is preferable than defining constants.DefaultWorkspaceStorageAccessMode in che-operator

[rohanKanojia]

you're right. Let me update it accordingly.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.92%. Comparing base (162a83d) to head (c162c82).
⚠️ Report is 14 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2061      +/-   ##
==========================================
+ Coverage   49.60%   49.92%   +0.32%     
==========================================
  Files          92       94       +2     
  Lines       12311    12552     +241     
==========================================
+ Hits         6107     6267     +160     
- Misses       5773     5858      +85     
+ Partials      431      427       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dkwon17, rohanKanojia, tolusha

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