Skip to content
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.

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

Ensure home persistence can only be enabled when workspace requires persistent storage #1241

Merged

Conversation

AObuchow
Copy link
Collaborator

What does this PR do?

This PR makes 2 changes:

Ensure only the storage strategies which support home persistence result in the persistent home devfile volume being added to a devworkspace.

The storage strategies which support home persistence are: per-user/common, perworkspace & async. The ephemeral storage strategy does not support home persistence.

Previously, we were trying to disable home persistence for the ephemeral storage strategy by calling provisioner.NeedsStorage(), however, the implementation of this function for the per-workspace PVC strategy always returns false and thus disabled home persistence when using per-workspace storage.

Now, we simply disable home persistence if the devworkspace is using the ephemeral storage strategy.

Ensure home persistence is only enabled for workspaces which actually require persistent storage.

There are cases where a devworkspace could use the per-workspace, per-user or async storage strategy, but not actually require persistent storage. For example, a devworkspace with no volume components and no components which mount project sources. In these cases, we should not add a persistent home devfile volume to the devworkspace.

What issues does this PR fix or reference?

Fix #1238

Is it tested? How?

Test the per-workspace storage strategy can be used with persistUserHome

  1. Set config.workspace.persistUserHome: true in the DWOC: kubectl edit dwoc -n $NAMESPACE
apiVersion: controller.devfile.io/v1alpha1                                                                                                                                 
config:                                                                                                                                                                    
  routing:                                                                                                                                                                 
    clusterHostSuffix: 192.168.49.2.nip.io                                                                                                                                 
    defaultRoutingClass: basic                                                                                                                                             
  workspace:                                                                                                                                                               
    imagePullPolicy: Always                                                                                                                                                
+    persistUserHome:                                                                                                                                                       
+      enabled: true                                                                                                                                                        
kind: DevWorkspaceOperatorConfig 
  1. Create a devworkspace that uses the per-workspace storage strategy, for example: kubectl apply -f ./samples/per-workspace-storage.yaml
  2. Once the devworkspace deployment is created, ensure the the workspace's pod has the persistent home volumeMount:
(...)
    volumeMounts:                                                                                                                                                          
    - mountPath: /checode                                                                                                                                                  
      name: storage-workspace4915e1b2cc1e4575                                                                                                                              
      subPath: checode                                                                                                                                                     
+    - mountPath: /home/user/                                                                                                                                               
+      name: storage-workspace4915e1b2cc1e4575                                                                                                                              
      subPath: persistent-home                                                                                                                                             
    - mountPath: /projects                                                                                                                                                 
      name: storage-workspace4915e1b2cc1e4575                                                                                                                              
      subPath: projects                                                                                                                                                    
    - mountPath: /devworkspace-metadata                                                                                                                                    
      name: workspace-metadata                                                                                                                                             
      readOnly: true                                                                                                                                                       
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount                                                                                                             
      name: kube-api-access-8bmpn                                                                                                                                          
      readOnly: true   
(...)

Ensure persistent home volumeMount is not created when the workspace does not need storage

  1. Set config.workspace.persistUserHome: true in the DWOC: kubectl edit dwoc -n $NAMESPACE
  2. Apply a devworkspace that uses the per-user or per-workspace storage strategy, but does not need need persistent storage. It should have no devfile volumes, and none of it's components have mountSources: true. For example:
kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: plain-devworkspace
spec:
  started: true
  routingClass: 'basic'
  template:
    attributes:
        controller.devfile.io/storage-type: per-workspace
    components:
      - name: web-terminal
        container:
          image: quay.io/wto/web-terminal-tooling:next
          memoryRequest: 256Mi
          memoryLimit: 512Mi
          mountSources: false
          command:
           - "tail"
           - "-f"
           - "/dev/null"
  1. After the devworkspace deployment is created, ensure the devworkspace pod does not have a volumeMount for /home/user/:
    volumeMounts:                                                                                                                                                          
    - mountPath: /devworkspace-metadata                                                                                                                                    
      name: workspace-metadata                                                                                                                                             
      readOnly: true                                                                                                                                                       
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount                                                                                                             
      name: kube-api-access-x4g72                                                                                                                                          
      readOnly: true     

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

Fix devfile#1238

Ensure only the storage strategies which support home persistence
can result in the persistent home devfile volume being added to a devworkspace.

The storage strategies which support home persistence are:
- per-user/common
- perworkspace
- async

The ephemeral storage strategy does not support home persistence.

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
Ensure a persistent home devfile volume will only be added to a devworkspace
which actually needs persistent storage. This includes the devworkspace
which mount project sources to any containers and have non ephemeral volumes.

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
@AObuchow AObuchow force-pushed the persistent-home-per-workspace-storage-fix branch from 3c37fbf to 60d584c Compare March 15, 2024 04:20
Copy link

codecov bot commented Mar 15, 2024

Codecov Report

Attention: Patch coverage is 81.48148% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 52.53%. Comparing base (8f3eafb) to head (60d584c).
Report is 4 commits behind head on main.

Files Patch % Lines
pkg/library/home/persistentHome.go 57.14% 2 Missing and 1 partial ⚠️
controllers/workspace/devworkspace_controller.go 0.00% 0 Missing and 1 partial ⚠️
pkg/provision/storage/asyncStorage.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1241      +/-   ##
==========================================
+ Coverage   52.48%   52.53%   +0.05%     
==========================================
  Files          84       84              
  Lines        7636     7642       +6     
==========================================
+ Hits         4008     4015       +7     
+ Misses       3335     3334       -1     
  Partials      293      293              

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

Copy link

openshift-ci bot commented Mar 18, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AObuchow, dkwon17

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@AObuchow AObuchow merged commit 54f9871 into devfile:main Mar 20, 2024
10 checks passed
@AObuchow AObuchow deleted the persistent-home-per-workspace-storage-fix branch March 20, 2024 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

persistUserHome does not work with per-workspace storage strategy
2 participants