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

feat: Mark the VM as deleted #124

Merged
merged 1 commit into from
Oct 11, 2021

Conversation

yitsushi
Copy link
Contributor

@yitsushi yitsushi commented Oct 8, 2021

What this PR does / why we need it:

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.

Which issue(s) this PR fixes:
Fixes #74

Special notes for your reviewer:
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.

Checklist:

  • squashed commits
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

Release note:

Mark the VM as deleted in the spec instead of deleting it on the Delete gRPC endpoint.

@yitsushi yitsushi added the kind/feature New feature or request label Oct 8, 2021
@yitsushi yitsushi requested a review from richardcase October 8, 2021 12:55
core/application/app.go Outdated Show resolved Hide resolved
yitsushi added a commit to yitsushi/flintlock that referenced this pull request Oct 8, 2021
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 yitsushi force-pushed the 74-mark-vm-as-deleted branch from 4f4329d to dca8227 Compare October 8, 2021 14:09
@codecov-commenter
Copy link

Codecov Report

Merging #124 (4f4329d) into main (ca14a16) will increase coverage by 0.30%.
The diff coverage is 83.33%.

❗ Current head 4f4329d differs from pull request most recent head dca8227. Consider uploading reports for the commit dca8227 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #124      +/-   ##
==========================================
+ Coverage   40.09%   40.40%   +0.30%     
==========================================
  Files          32       32              
  Lines        1444     1448       +4     
==========================================
+ Hits          579      585       +6     
+ Misses        789      788       -1     
+ Partials       76       75       -1     
Impacted Files Coverage Δ
core/application/commands.go 64.93% <83.33%> (+1.92%) ⬆️
pkg/queue/queue.go 100.00% <0.00%> (+6.06%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b5ff28...dca8227. Read the comment docs.

@yitsushi yitsushi merged commit 7d0f857 into liquidmetal-dev:main Oct 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delete from API should mark for deletion not delete
3 participants