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

Per workspace PVC size calculation can abort early and miss some volumes #1162

Closed
amisevsk opened this issue Aug 8, 2023 · 0 comments · Fixed by #1163
Closed

Per workspace PVC size calculation can abort early and miss some volumes #1162

amisevsk opened this issue Aug 8, 2023 · 0 comments · Fixed by #1163
Assignees
Milestone

Comments

@amisevsk
Copy link
Collaborator

amisevsk commented Aug 8, 2023

Description

In computing the per-workspace PVC size, the following rules are supposed to be used

  • If all volumes in a devworkspace specify their size, the computed PVC size will be used.
  • If all volumes in a devworkspace either specify their size or are ephemeral, the computed PVC size will be used.
  • 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.

However, in some cases, the third option is followed, as collecting all defined PVC sizes exits early after encountering a volume that does not define a size.

How To Reproduce

It depends on iteration order for PVC volumes, though the following DevWorkspace should reproduce

kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: plain-devworkspace
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"
          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

(we would expect the per-workspace PVC to be 10Gi instead of the default 5Gi here)

Additional context

Original PR: #1020
Related issue: https://issues.redhat.com/browse/CRW-4609

@amisevsk amisevsk added this to the v0.23.x milestone Aug 8, 2023
AObuchow added a commit to AObuchow/devworkspace-operator that referenced this issue Aug 9, 2023
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 it's 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 <aobuchow@redhat.com>
AObuchow added a commit to AObuchow/devworkspace-operator that referenced this issue Aug 9, 2023
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 <aobuchow@redhat.com>
AObuchow added a commit that referenced this issue Aug 10, 2023
Fix #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 <aobuchow@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants