search

devfile / devworkspace-operator

Apache License 2.0
61 stars 55 forks source link

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

Closed AObuchow closed 6 months ago

AObuchow commented 6 months ago

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

codecov[bot] commented 6 months ago

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 :warning:
controllers/workspace/devworkspace_controller.go 0.00% 0 Missing and 1 partial :warning:
pkg/provision/storage/asyncStorage.go 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

openshift-ci[bot] commented 6 months ago

[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: - ~~[OWNERS](https://github.com/devfile/devworkspace-operator/blob/main/OWNERS)~~ [AObuchow,dkwon17] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment