Skip to content

Support using a stub drive of a deleted container #243

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

Closed
sipsma opened this issue Aug 7, 2019 · 10 comments · Fixed by #244
Closed

Support using a stub drive of a deleted container #243

sipsma opened this issue Aug 7, 2019 · 10 comments · Fixed by #244

Comments

@sipsma
Copy link
Contributor

sipsma commented Aug 7, 2019

As part of #230 @kzys noticed that if a container is deleted, the stub drive for it cannot be re-used. This results in situations like the following:

  1. Call CreateVM with ContainerCount set to 2
  2. Start container A
  3. Start container B, let it exit and then Delete it.
  4. Try to start container C, it will fail due to being out of stub drives

In theory, once container B is deleted, the stub drive is no longer needed and should be able to be used by container C. The implementation would need an update to allow this. To be clear, this is just re-use of stub-drives within the context of a single VM (not across different VMs, which creates a bunch of other concurrency related issues).

@Zyqsempai
Copy link
Contributor

@sipsma Can I help with that issue? Sounds interesting.

@sipsma
Copy link
Contributor Author

sipsma commented Oct 30, 2019

@Zyqsempai This issue was opened as an idea and while I think it's a good feature for us to have, it's something I'd want to run by others on the team first to make sure everyone else is on board with making this change now. I will ask others to post any concerns here or will update if there are no concerns.

In general though, it will definitely be a fairly involved change but really appreciate all the contributions you've made lately and would be happy for you to help out if we decide to go forward with it! Will get back on the issue in the next few days

@Zyqsempai
Copy link
Contributor

@sipsma Got it, if there is any other issue I can help you with, just ping me, will be glad to work on something!

@sipsma
Copy link
Contributor Author

sipsma commented Nov 6, 2019

@Zyqsempai there are no concerns from the team on making this change.

There is an unrelated issue that I have a draft for which may require refactoring of stub drives (#296). I would like to make that change first in order to minimize conflict resolution with the changes required here. I will try to get that draft finalized and merged in the next week, at which time you should be able to get going on this change if still interested! I'll update this issue at that time.

@sipsma
Copy link
Contributor Author

sipsma commented Nov 15, 2019

@Zyqsempai Just an update, I sent out PR #332 to address #296. Once that's merged, you should be good to start working on this issue if still interested. Apologies for the delays, I ran into #325 in the midst of #332, which derailed me for a few days.

@sipsma
Copy link
Contributor Author

sipsma commented Nov 19, 2019

@Zyqsempai I just merged #332, so this issue should be ready to be worked on! Let us know if you're still interested in handling it

@Zyqsempai
Copy link
Contributor

Zyqsempai commented Nov 19, 2019

@sipsma For sure I am! Thanks for letting me know!

@Zyqsempai
Copy link
Contributor

@sipsma So here is the brief plan:
I will add new method to the StubDriveHandler -> Release(taskID string), by taskID i will find stubDrive in usedDrives and than I need to clean it up (this is the most important part) and then will just push it back into the freeDrives.
So the only question here is how to clean up stub properly, should I unmount it first, and then just remove it and create new one? Or somehow clean the content?

@sipsma
Copy link
Contributor Author

sipsma commented Nov 20, 2019

@Zyqsempai Basic idea sounds right to me! For cleaning up the stubs, I think something like the following should work:

  1. Add a new method UnmountDrive to our internal-only service DriverMounter, which will allow us to unmount a drive inside the VM.
  2. Call that new UnmountDrive after we've successfully deleted a task, so somewhere around here. If UnmountDrive succeeds, then we can patch the drive back to the original stubfile and return it to freeDrives like you said.

Let me know if that makes sense

@sipsma
Copy link
Contributor Author

sipsma commented Dec 5, 2019

Closed via #339

@sipsma sipsma closed this as completed Dec 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants