From a7d9632d5e4c2ffc2b56044c14a37657502aa7a1 Mon Sep 17 00:00:00 2001 From: Yevgeniy Brikman Date: Wed, 14 Sep 2016 23:02:35 +0100 Subject: [PATCH 1/3] Delete command should not error out if there are no existing snapshots --- delete_command.go | 9 ++++++--- integration_create_delete_test.go | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/delete_command.go b/delete_command.go index 645df10..724f672 100644 --- a/delete_command.go +++ b/delete_command.go @@ -103,6 +103,12 @@ func deleteSnapshots(c DeleteCommand) error { if err != nil { return err } + + if len(images) == 0 { + c.Ui.Info("NO ACTION TAKEN. There are no existing snapshots of instance " + c.InstanceId + " to delete.") + return nil + } + // Check that at least the --require-at-least number of AMIs exists // - Note that even if this passes, we still want to avoid deleting so many AMIs that we go below the threshold if len(images) <= c.RequireAtLeast { @@ -237,9 +243,6 @@ func findImages(instanceId string, svc *ec2.EC2) ([]*ec2.Image, error) { } else if err != nil { return noImages, err } - if len(resp.Images) == 0 { - return noImages, errors.New("No AMIs were found for EC2 instance \"" + instanceId + "\"") - } return resp.Images, nil } diff --git a/integration_create_delete_test.go b/integration_create_delete_test.go index 66a399e..7a4476a 100644 --- a/integration_create_delete_test.go +++ b/integration_create_delete_test.go @@ -87,6 +87,23 @@ func TestDeleteRespectsAtLeast(t *testing.T) { verifySnapshotWorks(snapshotId, svc, logger, t) } +// An integration test that runs an EC2 instance, does not take any snapshots of it, and then calls the delete_command +// to ensure that it exits gracefully even if no snapshots exist. +func TestDeleteHandlesNoSnapshots(t *testing.T) { + t.Parallel() + + logger, ui := createLoggerAndUi("TestDeleteHandlesNoSnapshots") + session := session.New(&aws.Config{Region: aws.String(AWS_REGION_FOR_TESTING)}) + svc := ec2.New(session) + + instance, instanceName := launchInstance(svc, logger, t) + defer terminateInstance(instance, svc, logger, t) + waitForInstanceToStart(instance, svc, logger, t) + + // Set atLeast to 1 to ensure the snapshot, which is the only one that exists, does not get deleted + deleteSnapshotForInstance(instanceName, "0h", 0, ui, logger, t) +} + func TestCreateWithInvalidInstanceName(t *testing.T) { t.Parallel() From 299008b06993d0a8f16dd081552e4a382982965e Mon Sep 17 00:00:00 2001 From: Yevgeniy Brikman Date: Wed, 14 Sep 2016 23:29:04 +0100 Subject: [PATCH 2/3] Make sure invalid instance ids still return an error --- delete_command.go | 8 ++++++++ integration_create_delete_test.go | 1 - 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/delete_command.go b/delete_command.go index 724f672..45ee0d5 100644 --- a/delete_command.go +++ b/delete_command.go @@ -97,6 +97,14 @@ func deleteSnapshots(c DeleteCommand) error { return err } c.InstanceId = instanceId + } else { + result, err := svc.DescribeInstances(&ec2.DescribeInstancesInput{InstanceIds: []*string{aws.String(c.InstanceId)}}) + if err != nil { + return err + } + if len(result.Reservations) == 0 || len(result.Reservations[0].Instances) == 0 { + return fmt.Errorf("Could not find an instance with id %s", c.InstanceId) + } } images, err := findImages(c.InstanceId, svc) diff --git a/integration_create_delete_test.go b/integration_create_delete_test.go index 7a4476a..ef440c5 100644 --- a/integration_create_delete_test.go +++ b/integration_create_delete_test.go @@ -100,7 +100,6 @@ func TestDeleteHandlesNoSnapshots(t *testing.T) { defer terminateInstance(instance, svc, logger, t) waitForInstanceToStart(instance, svc, logger, t) - // Set atLeast to 1 to ensure the snapshot, which is the only one that exists, does not get deleted deleteSnapshotForInstance(instanceName, "0h", 0, ui, logger, t) } From 2bad794e19cab6f6fcdf49faf32d8572d44b54b6 Mon Sep 17 00:00:00 2001 From: Yevgeniy Brikman Date: Wed, 14 Sep 2016 23:36:28 +0100 Subject: [PATCH 3/3] Update version number and CHANGELOG --- CHANGELOG.md | 5 +++++ main.go | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9a020e..a731a91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +# 0.5.2 (September 14, 2016) + +* BUG: The ec2-snapper `delete` command now gracefully exits if no snapshots exist for the given instance rather than + exiting with an error. + # 0.5.1 (September 11, 2016) * ENHANCEMENT: ec2-snapper now also tags the EBS volume snapshots it creates as part of the process of creating an AMI. diff --git a/main.go b/main.go index d20b192..44d31c7 100644 --- a/main.go +++ b/main.go @@ -15,7 +15,7 @@ func main() { } // CLI stuff - c := cli.NewCLI("ec2-snapper", "0.5.1") + c := cli.NewCLI("ec2-snapper", "0.5.2") c.Args = os.Args[1:] c.Commands = map[string]cli.CommandFactory{