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

fix: don't abort PVC size calculation early if volume size not defined #1163

Merged
merged 1 commit into from
Aug 10, 2023

Conversation

AObuchow
Copy link
Collaborator

@AObuchow AObuchow commented Aug 9, 2023

What does this PR do?

The following rules are supposed to be used when computing the per-workspace PVC size:

  1. If all volumes in a devworkspace specify their size, the computed PVC size will be used.
  2. If all volumes in a devworkspace either specify their size or are ephemeral, the computed PVC size will be used.
  3. If at least one volume in a devworkspace specifies its size, and the computed PVC size is greater than the default per-workspace PVC size, the computed PVC size will be used.

Prior to this PR, rule 3 was not being respected in cases where a volume did not define its size, but later volumes (in the volume array) did define their sizes.

This commit prevents aborting the PVC size calculation too early, and modifies the test case that was relevant to rule 3 to ensure the case from #1162 is accounted for.

What issues does this PR fix or reference?

Fix #1162

Is it tested? How?

  1. Install DWO using the changes from this PR
  2. Apply the following reproducer devworkspace:
kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: pvc-size-reproducer
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: true
          command:
           - "tail"
           - "-f"
           - "/dev/null"
          volumeMounts:
            - name: no-size-volume
              path: /home/user/no-size-volume
            - name: sized-volume
              path: /home/user/sized-volume
      - name: no-size-volume
        volume: {}
      - name: sized-volume
        volume:
          size: 10Gi
  1. Verify that the PVC created for the devworkspace has a claim size of 10Gi rather than the default 5Gi.

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#1162

The following rules are supposed to be used when computing the per-workspace PVC size:
1. If all volumes in a devworkspace specify their size, the computed PVC size will be used.
2. If all volumes in a devworkspace either specify their size or are ephemeral, the computed PVC size will be used.
3. If at least one volume in a devworkspace specifies its size, and the computed PVC size is greater than the default per-workspace PVC size, the computed PVC size will be used.

Prior to this commit, rule 3 was not being respected in cases where a volume did not define its size, but later volumes (in the volume array)
did define their sizes.

This commit prevents aborting the PVC size calculation too early, and modifies the test case that was relevant to rule 3 to ensure this case is accounted for.

Signed-off-by: Andrew Obuchowicz <[email protected]>
@openshift-ci
Copy link

openshift-ci bot commented Aug 9, 2023

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@openshift-ci openshift-ci bot added the approved label Aug 9, 2023
@AObuchow AObuchow merged commit 39be751 into devfile:main Aug 10, 2023
4 checks passed
@AObuchow AObuchow deleted the volume_size_calc_fix branch August 10, 2023 17:10
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.

Per workspace PVC size calculation can abort early and miss some volumes
2 participants