[WIP] Add snapshot import to amazon-import #104
Conversation
This patch adds the possibility of importing and image to amazon by using the import snapshot api, which has lower requirements than the image import. It resuses the current post-process method but diverges once we need to import the image. The image import is untouched, but the new snapshot import does something similar to image. It uploads the artifact to s3, then imports it as snapshot and finally registers the snapshot as a new ami. Signed-off-by: Itxaka <igarcia@suse.com>
|
|
|
Missing:
Done:
|
|
@sylviamoss Just the first pass, before I go further, I think this is a good step. Will await any comments before continuing. Have a nice weekend! |
sylviamoss
left a comment
sylviamoss
left a comment
There was a problem hiding this comment.
This is looking good so far! I make some small suggestions. I haven't tested it though.
So, my understanding is that importing from a snapshot is an easier option to register an AMI as it has fewer requirements to do so, right? In the end, the post-processor will produce two artifacts then, a snapshot and an AMI.
We need to make sure to document the advantages here, so people won't get confused about which option they should go for it.
| if p.config.FromSnapshotDeviceName == "" { | ||
| p.config.FromSnapshotDeviceName = "/dev/sda" | ||
| } |
There was a problem hiding this comment.
What do you think about moving this to inside the FromSnapshot condition? For better readability of the fact that this is only used and the default only matters if FromSnapshot is true.
| }) | ||
|
|
||
| if err != nil { | ||
| return "", fmt.Errorf("Failed to start import from s3://%s/%s: %s", p.config.S3Bucket, p.config.S3Key, err) |
There was a problem hiding this comment.
To differentiate the error messages:
| return "", fmt.Errorf("Failed to start import from s3://%s/%s: %s", p.config.S3Bucket, p.config.S3Key, err) | |
| return "", fmt.Errorf("Failed to start snapshot import from s3://%s/%s: %s", p.config.S3Bucket, p.config.S3Key, err) |
|
|
||
| importTaskId := importStart.ImportTaskId | ||
|
|
||
| ui.Message(fmt.Sprintf("Started import of s3://%s/%s, task id %s", p.config.S3Bucket, p.config.S3Key, *importTaskId)) |
There was a problem hiding this comment.
| ui.Message(fmt.Sprintf("Started import of s3://%s/%s, task id %s", p.config.S3Bucket, p.config.S3Key, *importTaskId)) | |
| ui.Message(fmt.Sprintf("Started snapshot import of s3://%s/%s, task id %s", p.config.S3Bucket, p.config.S3Key, *importTaskId)) |
| importResult, err2 = ec2conn.DescribeImportSnapshotTasks(&ec2.DescribeImportSnapshotTasksInput{ | ||
| ImportTaskIds: []*string{ | ||
| importTaskId, | ||
| }, | ||
| }) | ||
|
|
||
| statusMessage := "Error retrieving status message" | ||
|
|
||
| if err2 == nil { | ||
| statusMessage = *importResult.ImportSnapshotTasks[0].SnapshotTaskDetail.StatusMessage | ||
| } | ||
| return "", fmt.Errorf("Import task %s failed with status message: %s, error: %s", *importTaskId, statusMessage, err) |
There was a problem hiding this comment.
Simplifying this a little bit:
| importResult, err2 = ec2conn.DescribeImportSnapshotTasks(&ec2.DescribeImportSnapshotTasksInput{ | |
| ImportTaskIds: []*string{ | |
| importTaskId, | |
| }, | |
| }) | |
| statusMessage := "Error retrieving status message" | |
| if err2 == nil { | |
| statusMessage = *importResult.ImportSnapshotTasks[0].SnapshotTaskDetail.StatusMessage | |
| } | |
| return "", fmt.Errorf("Import task %s failed with status message: %s, error: %s", *importTaskId, statusMessage, err) | |
| if importResult, describeErr := ec2conn.DescribeImportSnapshotTasks(&ec2.DescribeImportSnapshotTasksInput{ | |
| ImportTaskIds: []*string{ | |
| importTaskId, | |
| }, | |
| }); describeErr != nil { | |
| return "", fmt.Errorf("Import task %s failed with status message: %s, error: %s", *importTaskId, *importResult.ImportSnapshotTasks[0].SnapshotTaskDetail.StatusMessage, err) | |
| } | |
| return "", fmt.Errorf("Import task %s failed with status message: Error retrieving status message, error: %s", *importTaskId, err) |
|
|
||
| func (p *PostProcessor) importSnapshot(ui packersdk.Ui, ctx context.Context, ec2conn *ec2.EC2) (string, error) { | ||
| var err error | ||
| var err2 error |
There was a problem hiding this comment.
Because of the code simplification I proposed below this can be removed
| var err2 error |
| }) | ||
|
|
||
| if err != nil { | ||
| return "", fmt.Errorf("Failed to find import task %s: %s", *importTaskId, err) |
There was a problem hiding this comment.
| return "", fmt.Errorf("Failed to find import task %s: %s", *importTaskId, err) | |
| return "", fmt.Errorf("Failed to find import snapshot task %s: %s", *importTaskId, err) |
| // Check it was actually completed | ||
| if *importResult.ImportSnapshotTasks[0].SnapshotTaskDetail.Status != "completed" { | ||
| // The most useful error message is from the job itself | ||
| return "", fmt.Errorf("Import task %s failed: %s", *importTaskId, *importResult.ImportSnapshotTasks[0].SnapshotTaskDetail.StatusMessage) |
There was a problem hiding this comment.
| return "", fmt.Errorf("Import task %s failed: %s", *importTaskId, *importResult.ImportSnapshotTasks[0].SnapshotTaskDetail.StatusMessage) | |
| return "", fmt.Errorf("Import snapshot task %s failed: %s", *importTaskId, *importResult.ImportSnapshotTasks[0].SnapshotTaskDetail.StatusMessage) |
| return "", fmt.Errorf("Import task %s failed: %s", *importTaskId, *importResult.ImportSnapshotTasks[0].SnapshotTaskDetail.StatusMessage) | ||
| } | ||
|
|
||
| ui.Message(fmt.Sprintf("Import task %s complete", *importTaskId)) |
There was a problem hiding this comment.
| ui.Message(fmt.Sprintf("Import task %s complete", *importTaskId)) | |
| ui.Message(fmt.Sprintf("Import snapshot task %s completed", *importTaskId)) |
|
Thanks for the comments! Not stale, Im just a bit strapped on time :) |
|
Currently leaving for vacation, will pick this up once Im back in August! |
|
Sounds good! |
|
unfortunately I wont have any more time to dedicate to this, sorry about that @sylviamoss :/ |
|
Why was this never merged? This feature is required especially when attempting to upload Linux distro that aren't supported during vmimport [Example: Rocky Linux 9] |
|
See above, my bandwidth to test and continue developing this was zero, so I could not continue on it. Good thing the code is in there, feel free to pick it up and with the comments from Silvia send a PR to have this merged 😀 |
5 participants
This patch adds the possibility of importing and image to amazon by
using the import snapshot api, which has lower requirements than the
image import.
It resuses the current post-process method but diverges once we need to
import the image. The image import is untouched, but the new snapshot
import does something similar to image. It uploads the artifact to s3,
then imports it as snapshot and finally registers the snapshot as a new
ami.
Closes #103
Signed-off-by: Itxaka igarcia@suse.com