Add automount functionality for PVCs #544

amisevsk · 2021-08-09T16:41:02Z

What does this PR do?

Adds the ability to mount PVCs to all DevWorkspaces by applying the controller.devfile.io/mount-to-devworkspace label (similarly to configmaps/secrets). PVCs are mounted to the path specified by controller.devfile.io/mount-path, or /tmp/<claim-name> if annotation is not present.

PVCs can be mounted read-only (to support read-only-many volumes)

The first three commits in this PR are just moving stuff around to make things sensible:

Move controllers/workspace/provision to pkg/provision/workspace so that all "provisioning" code is in same place
Split controllers/workspace/env into pkg/provision/workspace and pkg/constants
Move automount-processing code into subpackage pkg/provision/workspace/automount

What issues does this PR fix or reference?

Closes #503

Is it tested? How?

Create a PVC on your cluster, e.g.

cat <<EOF | kubectl apply -f -
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: test-pvc
  labels:
    controller.devfile.io/mount-to-devworkspace: "true"
  annotations:
    controller.devfile.io/mount-path: "/testing/"
    controller.devfile.io/read-only: "false" # Change to "true" to test read-only mounts 
spec:
  accessModes:
  - ReadWriteOnce
  resources:
    requests:
      storage: 1Gi
  storageClassName: standard
EOF

and create a DevWorkspace. PVC should be mounted to /testing

If mounted read-write, try creating a file there, stopping the current workspace, and creating a new one.

PR Checklist

E2E tests pass (when PR is ready, comment /test v7-devworkspaces-operator-e2e, v7-devworkspace-happy-path to trigger)
- v7-devworkspaces-operator-e2e: DevWorkspace e2e test
- v7-devworkspace-happy-path: DevWorkspace e2e test

sleshchenko

I haven't tested but changes seem to be pretty straightforward

sleshchenko · 2021-08-11T13:21:54Z

controllers/workspace/finalize.go

 	"github.com/go-logr/logr"
 	coputil "github.com/redhat-cop/operator-utils/pkg/util"
 	corev1 "k8s.io/api/core/v1"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	"github.com/devfile/devworkspace-operator/pkg/provision/storage"
+	provision "github.com/devfile/devworkspace-operator/pkg/provision/workspace"


really minor. Maybe it makes sense to drop provision alias, or make it wsProvision to avoid confusion with provision/storage...

Yeah, I had a bit of trouble with this since we're having some aliasing issues 😄

Can't use workspace as we name the DevWorkspace instance that

provision isn't ideal since it's completely unrelated to the package (as you point out)

workspaceprovision is too long

I think wsprovision makes sense. I'll update the PR -- do we want to settle on wsProvision or wsprovision? We kind of use both (k8serrors vs k8sErrors) but I think the standard is all-lowercase.

I'm ok with any

sleshchenko · 2021-08-11T13:24:38Z

pkg/provision/storage/cleanup.go

+	"github.com/devfile/devworkspace-operator/pkg/common"
+	"github.com/devfile/devworkspace-operator/pkg/config"
+	"github.com/devfile/devworkspace-operator/pkg/constants"
+	workspace2 "github.com/devfile/devworkspace-operator/pkg/provision/workspace"


IDE's footprint is found

I just wish it would warn me it was doing this 😄

Thanks for catching it.

amisevsk · 2021-08-11T21:00:13Z

/test v7-devworkspaces-operator-e2e, v7-devworkspace-happy-path

pkg/provision/storage/shared.go

pkg/provision/storage/provisioner.go

pkg/provision/storage/ephemeralStorage.go

pkg/provision/storage/commonStorage_test.go

pkg/provision/storage/commonStorage.go

pkg/provision/storage/asyncstorage/configuration.go

pkg/provision/storage/asyncstorage/configmap.go

pkg/provision/storage/asyncStorage.go

pkg/provision/metadata/metadata.go

pkg/provision/config/config.go

openshift-ci · 2021-08-11T21:08:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amisevsk, JPinkney, sleshchenko

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~~ [JPinkney,amisevsk,sleshchenko]

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

openshift-ci · 2021-08-12T04:43:43Z

New changes are detected. LGTM label has been removed.

Move all provisioning code (i.e. code that creates resources on-cluster) to the same place Signed-off-by: Angel Misevski <[email protected]>

Split common env var package to merge it into existing pkg/ structure: * Constants (env var names) to pkg/constants/env.go * Function providing common env vars to pkg/provision/workspace to sit alongside alongside where it's used. Signed-off-by: Angel Misevski <[email protected]>

Since we're adding more features to automounting, it makes sense to move functionality for getting configmaps/secrets/pvcs to a separate package from workspace provisioning. This commit splits the only large automount file into files for common functionality, managing secrets, and managing configmaps Signed-off-by: Angel Misevski <[email protected]>

Add ability to automatically mount PVCs to DevWorkspace pods based on the automount label. Automounted PVCs require a mountPath specified via the mount-path annotation. Signed-off-by: Angel Misevski <[email protected]>

Add annotation 'controller.devfile.io/read-only' to enable automounting PVCs with read-only access. Signed-off-by: Angel Misevski <[email protected]>

This is a really annoying change to make, since 1. VS Code will remove "unused" imports and reimport the unaliased package name on save, *even* if you use find-replace 2. Goland will let you refactor the import alias, but will basically find-replace throughout the file to do so, meaning that "workspace" gets replaced with "wsprovision" *in comments*. Signed-off-by: Angel Misevski <[email protected]>

amisevsk · 2021-08-12T05:16:00Z

/test v7-devworkspaces-operator-e2e, v7-devworkspace-happy-path

openshift-ci · 2021-08-12T06:20:03Z

@amisevsk: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Rerun command
ci/prow/v7-devworkspace-happy-path	`449fd7f`	link	`/test v7-devworkspace-happy-path`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

amisevsk requested review from sleshchenko and JPinkney August 9, 2021 16:41

openshift-ci bot added the approved label Aug 9, 2021

amisevsk force-pushed the automount-pvcs branch from b8086e3 to ef738fd Compare August 10, 2021 14:55

amisevsk mentioned this pull request Aug 10, 2021

Update GitHub Action for PRs to use go 1.15.13 and old version of addlicense #548

Merged

3 tasks

amisevsk force-pushed the automount-pvcs branch from ef738fd to 3b04f82 Compare August 10, 2021 20:18

sleshchenko approved these changes Aug 11, 2021

View reviewed changes

openshift-ci bot assigned sleshchenko Aug 11, 2021

openshift-ci bot added the lgtm label Aug 11, 2021

amisevsk force-pushed the automount-pvcs branch from 3b04f82 to 5ed1840 Compare August 11, 2021 15:39

openshift-ci bot removed the lgtm label Aug 11, 2021

amisevsk force-pushed the automount-pvcs branch from 5ed1840 to 4f4af21 Compare August 11, 2021 15:48

JPinkney approved these changes Aug 11, 2021

View reviewed changes

openshift-ci bot assigned JPinkney Aug 11, 2021

openshift-ci bot added the lgtm label Aug 11, 2021

openshift-ci bot added the needs-rebase label Aug 11, 2021

openshift-ci bot removed the lgtm label Aug 12, 2021

amisevsk force-pushed the automount-pvcs branch from 020af58 to 33d4fb3 Compare August 12, 2021 04:47

openshift-ci bot removed the needs-rebase label Aug 12, 2021

amisevsk added 6 commits August 12, 2021 01:13

Move controllers/workspace/provision package to pkg/provision/workspace

953782f

Move all provisioning code (i.e. code that creates resources on-cluster) to the same place Signed-off-by: Angel Misevski <[email protected]>

Add capability to automount PVCs in read-only mode

36f8452

Add annotation 'controller.devfile.io/read-only' to enable automounting PVCs with read-only access. Signed-off-by: Angel Misevski <[email protected]>

amisevsk force-pushed the automount-pvcs branch from 33d4fb3 to 449fd7f Compare August 12, 2021 05:15

amisevsk merged commit 17cdf4b into devfile:main Aug 12, 2021

amisevsk deleted the automount-pvcs branch August 12, 2021 18:07

l0rd mentioned this pull request Feb 20, 2022

Ability to mount the same PV across mutiple workspaces (one workspace running at a time) eclipse-che/che#15652

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add automount functionality for PVCs #544

Add automount functionality for PVCs #544

amisevsk commented Aug 9, 2021

sleshchenko left a comment

sleshchenko Aug 11, 2021

amisevsk Aug 11, 2021

sleshchenko Aug 11, 2021

sleshchenko Aug 11, 2021

amisevsk Aug 11, 2021

amisevsk commented Aug 11, 2021

openshift-ci bot commented Aug 11, 2021

openshift-ci bot commented Aug 12, 2021

amisevsk commented Aug 12, 2021

openshift-ci bot commented Aug 12, 2021

Add automount functionality for PVCs #544

Add automount functionality for PVCs #544

Conversation

amisevsk commented Aug 9, 2021

What does this PR do?

What issues does this PR fix or reference?

Is it tested? How?

PR Checklist

sleshchenko left a comment

Choose a reason for hiding this comment

sleshchenko Aug 11, 2021

Choose a reason for hiding this comment

amisevsk Aug 11, 2021

Choose a reason for hiding this comment

sleshchenko Aug 11, 2021

Choose a reason for hiding this comment

sleshchenko Aug 11, 2021

Choose a reason for hiding this comment

amisevsk Aug 11, 2021

Choose a reason for hiding this comment

amisevsk commented Aug 11, 2021

openshift-ci bot commented Aug 11, 2021

openshift-ci bot commented Aug 12, 2021

amisevsk commented Aug 12, 2021

openshift-ci bot commented Aug 12, 2021