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

Annotate DWO-created PVC's #920

Closed
AObuchow opened this issue Sep 6, 2022 · 2 comments
Closed

Annotate DWO-created PVC's #920

AObuchow opened this issue Sep 6, 2022 · 2 comments
Assignees
Labels
good first issue Good for newcomers
Milestone

Comments

@AObuchow
Copy link
Collaborator

AObuchow commented Sep 6, 2022

Description

Currently, to trigger reconciles when the common PVC is deleted, we are relying on filtering PVC's by their name (checking if the deleted PVC's name is equal to the common PVC name).

With #918, the common PVC name which used to be only set in the global DWOC can now be set on a per-workspace basis. However, the PVC handler function does not have access to the common PVC name set for the workspace's configuration.

Instead, it would be better to add a DWO-specific annotation to the PVC spec and filter for this annotation when checking if a reconcile should be triggered when the common PVC is updated.

Additional context

Limitation number 3 of #918 (comment):

There is a handler that checks if the common PVC was deleted. Currently, it filters for PVC's related to DWO by checking the PVC name against the global configs PVC name. In the future, this limitation can be worked around by applying a DWO-specific annotation to all PVC's created by DWO, and filtering by that annotation.

@AObuchow AObuchow self-assigned this Sep 6, 2022
@l0rd l0rd added the sprint/current Is assigned to issues which are planned to work on in the current team sprint label Sep 20, 2022
@ibuziuk ibuziuk mentioned this issue Oct 11, 2022
67 tasks
@ibuziuk ibuziuk mentioned this issue Nov 2, 2022
73 tasks
@ibuziuk ibuziuk mentioned this issue Nov 22, 2022
68 tasks
@ibuziuk ibuziuk mentioned this issue Dec 13, 2022
82 tasks
@ibuziuk ibuziuk removed the sprint/current Is assigned to issues which are planned to work on in the current team sprint label Jan 10, 2023
@ibuziuk ibuziuk added the good first issue Good for newcomers label Jan 10, 2023
@mkuznyetsov mkuznyetsov self-assigned this Dec 12, 2023
@AObuchow
Copy link
Collaborator Author

Though this issue involves specifically annotating DWO-created PVC's so that changes to PVC with custom names (using an external DWOC) can trigger workspace reconciles, @mkuznyetsov has brought up a good point regarding the implications of setting the common PVC name in an external DWOC.

I wanted to document this topic here, for the time being:

Consider the case where two workspaces A and B exist in a namespace. Workspace A uses the common PVC strategy and uses the global DWOC (i.e. it does not use an external DWOC). Workspace B uses the common PVC strategy and an external DWOC where the PVC name field is configured.

Upon deletion of workspace B, the common PVC cleanup job will not actually delete the PVC which has a custom name. This is because the PVC cleanup logic will detect that there are other workspaces still using the common PVC strategy (in this case, workspace A).

To resolve this case, we'd have to find out exactly which workspaces are using the custom-named PVC. To do this, we'd have to add extra logic for the common PVC cleanup logic:

  1. Check if the workspace being deleted defines a custom PVC name
  2. If a custom PVC name is being used, we need to get all the devworkspace's in the namespace, and for each devworkspace, get their external DWOC's from the cluster (if they exist) and check if they are using the same PVC name from step 1.
  3. If no other workspaces are using a PVC with the same name, the PVC can safely be deleted.

This adds quite a bit of complexity to the common PVC cleanup logic, however. Moreover, @amisevsk pointed out that the DWOC's PVCName field warns that changing the PVC name after workspaces have been created will lead to dangling PVCs that need to be manually deleted.

Thus, I think it makes more sense to simply document that changing the PVC name when using an external DWOC can result in dangling PVCs that need to be manually deleted.

@AObuchow
Copy link
Collaborator Author

Fixed in #1233

@AObuchow AObuchow added this to the v0.28.x milestone May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants