Skip to content

Conversation

domendobnikar
Copy link
Collaborator

@domendobnikar domendobnikar commented Apr 26, 2023

As a user I would like to be able to clone from VM level snapshots using “hypercore” collection.

  • Cloning
  • Docs

Testing

  • Integration
  • Units

@domendobnikar domendobnikar self-assigned this Apr 26, 2023
@domendobnikar domendobnikar marked this pull request as draft April 26, 2023 07:01
@domendobnikar domendobnikar mentioned this pull request Apr 26, 2023
6 tasks
- integration tests to clone
- doc fragment to changelogs
@domendobnikar domendobnikar requested a review from justinc1 April 26, 2023 13:19
@domendobnikar domendobnikar marked this pull request as ready for review April 26, 2023 13:19
@domendobnikar
Copy link
Collaborator Author

domendobnikar commented Apr 26, 2023

Copy link
Collaborator

@justinc1 justinc1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor changes are suggested.

A major change is requirement to clone VM from automated snapshot. This makes things complicated, since automated snapshot have duplicated labels. We can simulate this case by creating two USER snapshots with duplicated label.

Nonunique labels were reason we used source_snapshot_uuid as snapshot identificator in #179 (sorry, not in main yet). Discussion about how to identify snapshot was long, and there are other alternatives (snapshot serial, label, date as "latest snapshot created before date X" etc). But all those alternatives are more complicated to implement.

By using source_snapshot_uuid as parameter name we can implement something nicer later, in backward compatible way. I believe vm_snapshot_info can be used to list snapshosts with specific label, then user can assert in playbook a single snapshot was returned, or can decide which of N snapshots to use.

Copy link
Collaborator

@justinc1 justinc1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is trivial typo in integration test. Atherwise, all good. Ty.

@domendobnikar
Copy link
Collaborator Author

Added duplicate snapshot label cloning to integration tests.
Fixed typos in vm_clone.

@domendobnikar
Copy link
Collaborator Author

Copy link
Collaborator

@justinc1 justinc1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TY.

@justinc1 justinc1 merged commit 022eff4 into main May 15, 2023
@justinc1 justinc1 deleted the snapshot-clone branch May 15, 2023 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants