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.

Sign up for GitHub

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

Jump to bottom

Delete from API should mark for deletion not delete #74

Closed
richardcase opened this issue Aug 24, 2021 · 1 comment · Fixed by #124
Closed

Delete from API should mark for deletion not delete #74

richardcase opened this issue Aug 24, 2021 · 1 comment · Fixed by #124
Assignees
@yitsushi
Labels
area/api Indicates an issue or PR relates to the APIs kind/feature New feature or request

Comments

@richardcase
Copy link
Member

Describe the solution you'd like:
When requesting that a microvm is deleted via the API the spec for the VM should be marked as deleted but the content should remain.

The reconciliation loop should do the ultimate delete after the microvm has been removed.

Why do you want this feature:

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]
@richardcase richardcase added kind/feature New feature or request area/reignite labels Aug 24, 2021
@richardcase richardcase added this to the Milestone 1 - re:ignite milestone Aug 24, 2021
@richardcase richardcase added area/api Indicates an issue or PR relates to the APIs and removed area/reignite labels Sep 9, 2021
@richardcase
Copy link
Member Author

  • Accept delete grpc request
  • Get spec and set deletion indicator (save)
  • Raise event to kick off reconciliation

@yitsushi yitsushi self-assigned this Oct 8, 2021
yitsushi added a commit to yitsushi/flintlock that referenced this issue Oct 8, 2021
@yitsushi
feat: Mark VM as deleted
08e73e6 
Mark the VM in the spec as deleted and let the reconciliation loop
handle it.

For mocking purpose, I introduced a port.HasTime interface. This
interface requires only one function: `SetClock(func() time.Time)`. By
defualt on app creation with `New()` it's `time.Now`, but while writing
tests, it's hard to make deep comparisons with dynamic data.

Potentially it could be solved with extra mocking layers all around `app`, but
this is a clean and easy solution and can be reused later if we have to
add more logic where we want to use the current timestamp.

fixes liquidmetal-dev#74
@yitsushi yitsushi mentioned this issue Oct 8, 2021
Merged
4 tasks
yitsushi added a commit to yitsushi/flintlock that referenced this issue Oct 8, 2021
@yitsushi
feat: Mark VM as deleted
dca8227 
Mark the VM in the spec as deleted and let the reconciliation loop
handle it.

For mocking purpose, I introduced a Clock in ports.Collection.

Changed the event to Update event from Delete event. For the
reconciliation loop, they are the same. Let me know if we still want to
use the Delete event.

fixes liquidmetal-dev#74
yitsushi added a commit that referenced this issue Oct 11, 2021
@yitsushi
feat: Mark VM as deleted (#124)
7d0f857 
Mark the VM in the spec as deleted and let the reconciliation loop
handle it.

For mocking purpose, I introduced a Clock in ports.Collection.

Changed the event to Update event from Delete event. For the
reconciliation loop, they are the same.

fixes #74
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@yitsushi yitsushi

Labels
area/api Indicates an issue or PR relates to the APIs kind/feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: Mark the VM as deleted yitsushi/flintlock
2 participants
@yitsushi @richardcase