Support cloud pods in snapshot save cmd#250
Conversation
anisaoshafi
changed the title
anisaoshafi
changed the title
anisaoshafi
requested review from
carole-lavillonniere and
silv-io
as code owners
|
:fyi: Taking a look as soon as I resolve the license check issue, currently locked out of the CLI. |
| return save(ctx, rt, containers, sink, | ||
| "Saving snapshot...", | ||
| func() { | ||
| sink.Emit(output.MessageEvent{Severity: output.SeveritySuccess, Text: fmt.Sprintf("Snapshot saved to %s", displayPath(dest, cwd, home))}) |
There was a problem hiding this comment.
issue: This conflates "pod" (the remote) with the snapshot name, reading snap1 as the pod name.
# BEFORE
✔︎ Snapshot saved to pod "snap1"Closer to spec wording would be...
# AFTER
Snapshot saved to pod:snap1This way once we add support for s3:// and oras:// the same structure can be used.
# pod
Snapshot saved to pod:snap1
# s3
Snapshot saved to s3://bucket/snap1
# oras
Snapshot saved to oras://snap1| // cloud: prefix also catches cloud:// | ||
| input: "cloud://my-pod", | ||
| wantRemoteErr: true, | ||
| // pod:// is also accepted |
There was a problem hiding this comment.
question: The spec so far defines pod: as a remote name, not a scheme. Do you suggest to add pod:// as also accepted prefix? See relevant discussion. Happy to consider adding it, but I think it's easier to start strict and loosen later if needed.
| type PodSaveResult struct { | ||
| Version int | ||
| Services []string | ||
| Size int64 | ||
| } |
There was a problem hiding this comment.
praise: Thanks for bootstrapping the save result output. 🙏 Adding some screenshots for future reference.
| Saving | Saved |
|---|---|
 |
 |
| type PodSaveResult struct { | ||
| Version int | ||
| Services []string | ||
| Size int64 | ||
| } |
There was a problem hiding this comment.
suggestion: Kinda miss this when saving snapshot locally. 😁 Can be part of a follow up PR, and size can be skipped if it's not trivial to fetch. Thoughts?
# BEFORE
$ lstk snapshot save snap1
✔︎ Snapshot saved to ./snap1.zip
# AFTER
$ lstk snapshot save snap1
✔︎ Snapshot saved to ./snap1.zip
• Services: s3
• Size: 62.9 KB2 participants
Added support for cloud pods in existing
lstk snapshot savecommand.