ore aws upload does not override existing AMIs/snapshots

[miabbott]

Maybe I am misunderstanding what the --force flag should do, but it doesn't appear to function the way it is described.

--force                    overwrite existing S3 object rather than reusing

Namely, if I use the --force flag when trying to upload a vmdk to AWS, if the name of the uploaded image is the same, the existing image is reused. I would expect that with the --force flag, the image would always get re-uploaded/overwritten, but this does not appear to happen.

### Initial upload

$ coreos-assembler buildextend-aws --build 42.80.20190729.4 --region us-east-1 --bucket s3://rhcos-ci/testing/amis
...
+ ore aws upload --region us-east-1 --bucket s3://rhcos-ci/testing/amis --ami-name rhcos-42.80.20190729.4 --name rhcos-42.80.20190729.4 --ami-description "OpenShift 4 42.80.20190729.4" --file tmp/buildpost-aws/rht
2019-07-29 15:00:07.677076 I | platform/api/aws: uploading s3://rhcos-ci/testing/amis/rhcos-42.80.20190729.4.vmdk
2019-07-29 15:00:23.078125 I | platform/api/aws: created snapshot import task import-snap-0a90d36ee59fb59e0
2019-07-29 15:10:26.944972 I | platform/api/aws: deleting s3://rhcos-ci/testing/amis/rhcos-42.80.20190729.4.vmdk

### Use --force with same image name, AMI name, file name (expecting no reuse)

$ ore upload --force --region us-east-1 --bucket s3://rhcos-ci/testing/amis --ami-name rhcos-42.80.20190729.4 --name rhcos-42.80.20190729.4 --ami-description "OpenShift 4 42.80.20190729.4" --file rhcos-42.80.20190729.4-aws.vmdk --disk-size-inspect --delete-object
2019-07-29 11:50:05.305003 I | platform/api/aws: found existing snapshot snap-05c8783a3c27820b8
2019-07-29 11:50:05.305071 I | platform/api/aws: deleting s3://rhcos-ci/testing/amis/rhcos-42.80.20190729.4-aws.vmdk
2019-07-29 11:50:06.245980 I | platform/api/aws: found existing image ami-0ea3a0a1c565b986c, reusing
{"HVM":"ami-0ea3a0a1c565b986c","SnapshotID":"snap-05c8783a3c27820b8","S3Object":"s3://rhcos-ci/testing/amis/rhcos-42.80.20190729.4-aws.vmdk"}

### Use --force with same image/AMI name, but different file (expecting no reuse)

$ cp rhcos-42.80.20190729.4-aws.vmdk rhcos-42.80.20190729.4-miabbott-aws.vmdk
$ ore aws upload --force --region us-east-1 --bucket s3://rhcos-ci/testing/amis --ami-name rhco
s-42.80.20190729.4 --name rhcos-42.80.20190729.4 --ami-description "OpenShift 4 42.80.20190729.4" --file rhcos-42.80.20190729.4-miabbott-aws.vmdk --disk-size-inspect --delete-object
2019-07-29 11:15:59.395574 I | platform/api/aws: found existing snapshot snap-05c8783a3c27820b8
2019-07-29 11:15:59.395645 I | platform/api/aws: deleting s3://rhcos-ci/testing/amis/rhcos-42.80.20190729.4-miabbott-aws.vmdk
2019-07-29 11:16:00.240589 I | platform/api/aws: found existing image ami-0ea3a0a1c565b986c, reusing
{"HVM":"ami-0ea3a0a1c565b986c","SnapshotID":"snap-05c8783a3c27820b8","S3Object":"s3://rhcos-ci/testing/amis/rhcos-42.80.20190729.4-miabbott-aws.vmdk"}

[arithx]

I agree that the --force flag isn't obvious in what it's doing, it requires additional context on what is actually being done under the hood for image uploads. Essentially that flag is currently only targeted at overwriting the S3 objects, this notably excludes snapshots & AMIs.

The process for uploading an image follows the flow: upload bits to S3 -> create a snapshot from S3 -> create AMI from snapshot. Both ore & plume have some extra code that allows it to re-use bits throughout to allow for idempotency.

AMI names are intended to be unique (although some of the AWS APIs don't/didn't respect that) and as such it makes sense that we might want to be able to create a new AMI with the same name (which would involve deleting both the AMI & the underlying snapshot).

The actual fix would probably look something like adding code inside of the ore aws upload command that when --force is specified checks if an AMI exists of the same name (using the FindImage method) and then deleting both the AMI & the underlying snapshot (the getImageSnapshotID method exists which can find the underlying snapshot for a given AMI). If the deletion of the snapshot fails then we would want to print a warning with the snapshot ID but continue (as it's possible for multiple AMIs to be using the same underlying snapshot), which means we'd also need to add a ore aws delete-snapshot command to allow users to be able to properly clean-up their AWS environment.

Is this something that is urgently needed for your current work or can you work around it for now (specifying a different AMI name will bypass it in the interim)?

[miabbott]

Is this something that is urgently needed for your current work or can you work around it for now (specifying a different AMI name will bypass it in the interim)?

Nope, not urgent, just a nice to have. Just something I've noticed while doing some testing and was looking for some context.

[cgwalters]

This behavior directly conflicts with cosa, since "failed" builds will reuse version numbers. See coreos/coreos-assembler#668

[arithx]

@cgwalters ah yeah, that's true.

I'll try to get some spare cycles in the next week or two to hack on it; if anyone would like to pick up the work I'd also be happy to review it and/or answer any questions.

[cgwalters]

PR in #1039