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

csi: move volume claim release into volumewatcher #7708

Closed
wants to merge 1 commit into from

Conversation

tgross
Copy link
Member

@tgross tgross commented Apr 13, 2020

Work in progress for #7629, for design discussion with @langmartin

  • this steals borrows liberally from the existing nomad/deploymentwatcher package
  • this avoids tying up scheduling workers by immediately sending volume claim workloads into their own loop, rather than blocking the scheduling workers in the core GC job doing things like talking to CSI controllers

open items:

  • fix some of the old tests that I've broken here
  • unit tests for registration/deregistration of watchers (especially after claims are fully released)
  • unit tests for batching logic
  • make sure it works in e2e (I've tested with hostpath and it works, but that doesn't tell us a lot)
  • periodic job for iterating over batches of volumes to GC, similar to current periodic job GC (edit: in the interest of keeping this changeset manageable, I'm going to implement this in a separate PR)

@tgross tgross added this to the 0.11.1 milestone Apr 13, 2020
@tgross tgross self-assigned this Apr 14, 2020
@tgross tgross force-pushed the f-csi-volume-watcher branch 2 times, most recently from 08b6d42 to 6eeba83 Compare April 20, 2020 14:32
@tgross tgross force-pushed the f-csi-volume-watcher branch 2 times, most recently from 9edaa8a to 3ec871e Compare April 20, 2020 20:26
@tgross tgross force-pushed the f-csi-volume-watcher branch 2 times, most recently from 783b25d to 8f24510 Compare April 21, 2020 14:34
@tgross tgross modified the milestones: 0.11.1, 0.11.2 Apr 22, 2020
This changeset avoids tying up scheduling workers by immediately
sending volume claim release workloads into their own loop, rather
than blocking in the core GC job doing expensive things like talking
to CSI plugins.
@tgross tgross force-pushed the f-csi-volume-watcher branch from 8f24510 to 664d990 Compare April 22, 2020 14:02
@tgross
Copy link
Member Author

tgross commented Apr 22, 2020

Per a conversation with @langmartin I'm going to close this draft and break it into two changesets for reviewability's sake. I might rebase this on just the volumewatcher changes later and reopen

@github-actions
Copy link

github-actions bot commented Jan 9, 2023

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant