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

Update restore process to use the appropriate locations (snapshot, destination for volume create) when doing PV restores #808

Closed
skriss opened this issue Aug 29, 2018 · 5 comments

Comments

@skriss
Copy link
Contributor

commented Aug 29, 2018

Ark currently assumes there's only one location for snapshots and creating new disks. The restore process (the controller and restore package) will need to be updated to inspect the relevant Backup and use the right locations.

  • pkg/restore will get a list of the VolumeSnapshot objects that are associated with the backup being restored
  • restore will either use backup.spec.volumeSnapshotLocations or the list of VSLs from the previous step to identify the locations where snapshots exist
  • restore will instantiate a BlockStore per VSL and use it to execute CreateVolumeFromSnapshot
  • TODO: figure out UX around how we figure out where the nodes are/where the volume should be created. Currently, we always restore into the same AZ as the original backed-up volume, but that's not ideal. (also consider how this changes in v0.11 with replication)
@skriss

This comment has been minimized.

Copy link
Contributor Author

commented Oct 11, 2018

@wwitzel3 @carlisia - since we're switching to storing snapshot info in VolumeSnapshot API objects a/o v0.10, I'm wondering if we want be backwards-compatible for restores - so, if there's an existing backup in object storage, that has .status.volumeBackupInfo, should Ark v0.10 support restoring it? I'm tempted to say yes, as long as there's only a single VolumeSnapshotLocation configured, so we trivially know where the snapshots are located. I think if we don't do this, we're going to potentially get a lot of issues/complaints about not being compatible. I think we can plan to remove this code for v1.0. WDYT?

@wwitzel3

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2018

I think this makes sense. When you say single VolumeSnapshotLocation, you mean, only one provider configured? I know we only allow one per provider, so just wanted to be sure I'm understanding you right.

Would we want to give the 0.10 client a way to specify the provider in that case? Since it would be being removed in 1.0 or even 0.11? So log/warn that the restore is using the old format if there is a single provider, log/error and provide a CLI example of specifying a provider for the restore? --deprecated-restore-provider ? Really just thinking out loud here.

I think going through the exercise of deprecating and rolling off of an API/file format would be good practice in general.

@skriss

This comment has been minimized.

Copy link
Contributor Author

commented Oct 12, 2018

@wwitzel3 have you made any progress on this? Some of it is going to need to change based on the discussion we just had so just trying to get a sense of where it is. Happy to help out if needed.

@wwitzel3

This comment has been minimized.

Copy link
Contributor

commented Oct 12, 2018

Yeah, so based on what we talked about for #863, should I base this off #922?

@skriss

This comment has been minimized.

Copy link
Contributor Author

commented Oct 12, 2018

Yes, you'll want to use the BackupStore's GetBackupVolumeSnapshots to get the snapshot & location info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.