feat: Use labels on PVCs for better reconciling

mkuznyetsov commented 4 months ago

What does this PR do?

Add new labels to indicate PVCs that are in use by Devworkspace Operator. This allows to trigger workspace reconcile, that must be triggered upon PVC deletion. and if it's a common PVC that had a name configured by external operator config, only those workspaces that would have the same PVC name should be reconciled

What issues does this PR fix or reference?

https://github.com/devfile/devworkspace-operator/issues/920

Is it tested? How?

Steps to test the new & old behavior with sample DevWorkspaces (DW)

1) install & run a current version of DWO:

https://github.com/devfile/devworkspace-operator/?tab=readme-ov-file#run-controller-locally

2) add some Devworkspaces and a devworkspace:

DW with per-user storage

cat <<'EOF' | kubectl apply -n $NAMESPACE -f - 
kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: old-dw-per-user
spec:
  started: true
  routingClass: 'basic'
  template:
    components:
      - name: web-terminal
        container:
          image: quay.io/wto/web-terminal-tooling:next
          memoryRequest: 256Mi
          memoryLimit: 512Mi
          mountSources: true
          command:
           - "tail"
           - "-f"
           - "/dev/null"
EOF

DW with per-workspace storage

cat <<'EOF' | kubectl apply -n $NAMESPACE -f - 
kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
name: old-dw-per-workspace
spec:
started: true
routingClass: 'basic'
template:
attributes:
  controller.devfile.io/storage-type: per-workspace
components:
  - name: basic-volume
    volume: 
      size: 5Gi
  - name: web-terminal
    container:
      image: quay.io/wto/web-terminal-tooling:next
      memoryRequest: 256Mi
      memoryLimit: 512Mi
      mountSources: true
      command:
       - "tail"
       - "-f"
       - "/dev/null"
EOF

External operator config, that is configured with custom PVC name

cat <<'EOF' | kubectl apply -f - 
apiVersion: controller.devfile.io/v1alpha1
config:
  enableExperimentalFeatures: true
  routing:
    clusterHostSuffix: 192.168.49.2.nip.io
    defaultRoutingClass: basic
  workspace:
    defaultStorageSize:
      common: 6Gi
      perWorkspace: 4Gi
    imagePullPolicy: Always
    pvcName: old-custom-pvc-name
kind: DevWorkspaceOperatorConfig
metadata:
  name: external-config
  namespace: default
EOF

cat <<'EOF' | kubectl apply -n $NAMESPACE -f - 
kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: old-dw-per-user-external
spec:
  started: true
  routingClass: 'basic'
  template:
    attributes:
      controller.devfile.io/devworkspace-config:
        name: external-config
        namespace: default
      controller.devfile.io/storage-type: common
    components:
      - name: web-terminal
        container:
          image: quay.io/wto/web-terminal-tooling:next
          memoryRequest: 256Mi
          memoryLimit: 512Mi
          mountSources: true
          command:
           - "tail"
           - "-f"
           - "/dev/null"
EOF

Inspect PVC objects, that are created for these repos.

kubectl describe pvc -n $NAMESPACE | grep "Labels:" -A 3 -B 6

They WILL NOT have the label "controller.devfile.io/devworkspace_pvc_type".
Only per-workspace PVC will have "controller.devfile.io/devworkspace_id" label

1) build and run a new version of operator from this pull-request: export NAMESPACE="devworkspace-controler" ... make run

1) Create DWs from step TODO:

DW with per-user storage

cat <<'EOF' | kubectl apply -n $NAMESPACE -f - 
kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: new-dw-per-user
spec:
  started: true
  routingClass: 'basic'
  template:
    components:
      - name: web-terminal
        container:
          image: quay.io/wto/web-terminal-tooling:next
          memoryRequest: 256Mi
          memoryLimit: 512Mi
          mountSources: true
          command:
           - "tail"
           - "-f"
           - "/dev/null"
EOF

DW with per-workspace storage

cat <<'EOF' | kubectl apply -n $NAMESPACE -f - 
kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
name: new-dw-per-workspace
spec:
started: true
routingClass: 'basic'
template:
attributes:
  controller.devfile.io/storage-type: per-workspace
components:
  - name: basic-volume
    volume: 
      size: 5Gi
  - name: web-terminal
    container:
      image: quay.io/wto/web-terminal-tooling:next
      memoryRequest: 256Mi
      memoryLimit: 512Mi
      mountSources: true
      command:
       - "tail"
       - "-f"
       - "/dev/null"
EOF

Another external operator config, that is configured with custom PVC name

cat <<'EOF' | kubectl apply -f - 
apiVersion: controller.devfile.io/v1alpha1
config:
  enableExperimentalFeatures: true
  routing:
    clusterHostSuffix: 192.168.49.2.nip.io
    defaultRoutingClass: basic
  workspace:
    defaultStorageSize:
      common: 20Gi
      perWorkspace: 29Gi
    imagePullPolicy: Always
    pvcName: new-custom-pvc-name
kind: DevWorkspaceOperatorConfig
metadata:
  name: new-external-config
  namespace: default
EOF

cat <<'EOF' | kubectl apply -n $NAMESPACE -f - 
kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: new-dw-per-user-external
spec:
  started: true
  routingClass: 'basic'
  template:
    attributes:
      controller.devfile.io/devworkspace-config:
        name: new-external-config
        namespace: default
      controller.devfile.io/storage-type: common
    components:
      - name: web-terminal
        container:
          image: quay.io/wto/web-terminal-tooling:next
          memoryRequest: 256Mi
          memoryLimit: 512Mi
          mountSources: true
          command:
           - "tail"
           - "-f"
           - "/dev/null"
EOF

kubectl describe pvc -n $NAMESPACE | grep "Labels:" -A 3 -B 6

Observe a label "controller.devfile.io/devworkspace_pvc_type" added to certain kubernetes PVC objects:

claim-devworkspace PVC will not have this label, since this PVC was created with old DWO. Even new DWO
old-custom-pvc-name PVC will not have this label, but new-custom-pvc-name will have it
same for per-workspace, only the PVC for workspace started with new operator will have the type label

Now start deleting PVCs one after another with kubectl delete pvc And see that workspaces are reconciled respectivelly.

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

dkwon17 commented 4 months ago

I will review this PR

AObuchow commented 3 months ago

@mkuznyetsov sorry about the failing CI tests, that's due to https://github.com/devfile/devworkspace-operator/issues/1242 (not related to your PR). I've opened https://github.com/devfile/devworkspace-operator/pull/1243 to resolve this.

AObuchow commented 2 months ago

@mkuznyetsov when you get a chance, please rebase your PR onto the main branch so that the Validate PR GH Action will pass btw :)

AObuchow commented 1 month ago

@mkuznyetsov I think this PR is good to merge :) Before we do so, can you squash your commits with a git rebase --autosquash please? I also think a705e11 (#1233) could be squashed into 08e12bc (#1233).

Additionally, the commit description of 08e12bc (#1233) could maybe be re-worded to something like: feat: label PVCs by storage strategy

openshift-ci[bot] commented 1 month ago

[APPROVALNOTIFIER] This PR is APPROVED

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

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] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

AObuchow commented 1 month ago

Congrats on having your first PR merged to the project @mkuznyetsov :tada: Amazing work :)

devfile / devworkspace-operator