From 4a402b01cfdd8ffe8947e384471bd7a31faa6d86 Mon Sep 17 00:00:00 2001 From: mickeypash Date: Mon, 23 Oct 2017 19:12:49 +0100 Subject: [PATCH 1/7] Correct response when trying to delete a volume that is attached to an EC2 instance. Created a VolumeInUse error and did a simple check on the delete_volume method. --- moto/ec2/exceptions.py | 9 +++++++++ moto/ec2/models.py | 5 +++++ 2 files changed, 14 insertions(+) diff --git a/moto/ec2/exceptions.py b/moto/ec2/exceptions.py index e5432baf7..ae279d5b2 100644 --- a/moto/ec2/exceptions.py +++ b/moto/ec2/exceptions.py @@ -244,6 +244,15 @@ class InvalidVolumeAttachmentError(EC2ClientError): .format(volume_id, instance_id)) +class VolumeInUseError(EC2ClientError): + + def __init__(self, volume_id, instance_id): + super(VolumeInUseError, self).__init__( + "VolumeInUse", + "Volume {0} is currently attached to {1}" + .format(volume_id, instance_id)) + + class InvalidDomainError(EC2ClientError): def __init__(self, domain): diff --git a/moto/ec2/models.py b/moto/ec2/models.py index 7fa7e1009..011258520 100755 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -45,6 +45,7 @@ from .exceptions import ( InvalidAMIAttributeItemValueError, InvalidSnapshotIdError, InvalidVolumeIdError, + VolumeInUseError, InvalidVolumeAttachmentError, InvalidDomainError, InvalidAddressError, @@ -1813,6 +1814,10 @@ class EBSBackend(object): def delete_volume(self, volume_id): if volume_id in self.volumes: + volume = self.volumes[volume_id] + instance_id = volume.attachment.instance.id + if volume.attachment is not None: + raise VolumeInUseError(volume_id, instance_id) return self.volumes.pop(volume_id) raise InvalidVolumeIdError(volume_id) From d5b841fb6c4e8fdb4c94bd9becc74441a96ec6da Mon Sep 17 00:00:00 2001 From: mickeypash Date: Mon, 13 Nov 2017 19:58:21 +0000 Subject: [PATCH 2/7] Fixing volume.attachment is None --- moto/ec2/models.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index bad32d653..b9cbe0407 100755 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -1812,9 +1812,8 @@ class EBSBackend(object): def delete_volume(self, volume_id): if volume_id in self.volumes: volume = self.volumes[volume_id] - instance_id = volume.attachment.instance.id if volume.attachment is not None: - raise VolumeInUseError(volume_id, instance_id) + raise VolumeInUseError(volume_id, volume.attachment.instance.id) return self.volumes.pop(volume_id) raise InvalidVolumeIdError(volume_id) From 41b1482b595cef56ff9b0b49380a86555cb7e6cc Mon Sep 17 00:00:00 2001 From: mickeypash Date: Sat, 20 Jul 2019 21:36:21 +0100 Subject: [PATCH 3/7] Simplify conditional --- moto/ec2/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index 79838147e..f91835581 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -2064,7 +2064,7 @@ class EBSBackend(object): def delete_volume(self, volume_id): if volume_id in self.volumes: volume = self.volumes[volume_id] - if volume.attachment is not None: + if volume.attachment: raise VolumeInUseError(volume_id, volume.attachment.instance.id) return self.volumes.pop(volume_id) raise InvalidVolumeIdError(volume_id) From 231b1000571c1720655989c82760116402643935 Mon Sep 17 00:00:00 2001 From: mickeypash Date: Fri, 3 Apr 2020 01:50:17 +0100 Subject: [PATCH 4/7] Add test scaffold. Currently broken --- tests/test_ec2/test_elastic_block_store.py | 38 ++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/test_ec2/test_elastic_block_store.py b/tests/test_ec2/test_elastic_block_store.py index 3c7e17ec8..0e39d2069 100644 --- a/tests/test_ec2/test_elastic_block_store.py +++ b/tests/test_ec2/test_elastic_block_store.py @@ -13,6 +13,7 @@ from freezegun import freeze_time import sure # noqa from moto import mock_ec2_deprecated, mock_ec2 +from moto.ec2.exceptions import VolumeInUseError from moto.ec2.models import OWNER_ID @@ -53,6 +54,43 @@ def test_create_and_delete_volume(): cm.exception.request_id.should_not.be.none +@mock_ec2_deprecated +def test_delete_attached_volume(): + conn = boto.ec2.connect_to_region("us-east-1") + reservation = conn.run_instances("ami-1234abcd") + # create an instance + instance = reservation.instances[0] + # create a volume + volume = conn.create_volume(80, "us-east-1a") + # attach volume to instance + volume.attach(instance.id, "/dev/sdh") + + volume.update() + volume.volume_state().should.equal("in-use") + volume.attachment_state().should.equal("attached") + + volume.attach_data.instance_id.should.equal(instance.id) + + # attempt to delete volume + # assert raises VolumeInUseError + with assert_raises(VolumeInUseError) as ex: + volume.delete() + ex.exception.error_code.should.equal("VolumeInUse") + ex.exception.status.should.equal(400) + ex.exception.message.should.equal(f"Volume {volume.id} is currently attached to {instance_id}") + + volume.detach() + + volume.update() + volume.volume_state().should.equal("available") + + volume.delete() + + all_volumes = conn.get_all_volumes() + my_volume = [item for item in all_volumes if item.id == volume.id] + my_volume.should.have.length_of(0) + + @mock_ec2_deprecated def test_create_encrypted_volume_dryrun(): conn = boto.ec2.connect_to_region("us-east-1") From 76b9cbe16d76decef8becad643c1426dba2c927d Mon Sep 17 00:00:00 2001 From: mickeypash Date: Fri, 3 Apr 2020 02:14:14 +0100 Subject: [PATCH 5/7] Fix test --- tests/test_ec2/test_elastic_block_store.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_ec2/test_elastic_block_store.py b/tests/test_ec2/test_elastic_block_store.py index 0e39d2069..1182610e8 100644 --- a/tests/test_ec2/test_elastic_block_store.py +++ b/tests/test_ec2/test_elastic_block_store.py @@ -13,7 +13,6 @@ from freezegun import freeze_time import sure # noqa from moto import mock_ec2_deprecated, mock_ec2 -from moto.ec2.exceptions import VolumeInUseError from moto.ec2.models import OWNER_ID @@ -73,11 +72,11 @@ def test_delete_attached_volume(): # attempt to delete volume # assert raises VolumeInUseError - with assert_raises(VolumeInUseError) as ex: + with assert_raises(EC2ResponseError) as ex: volume.delete() ex.exception.error_code.should.equal("VolumeInUse") ex.exception.status.should.equal(400) - ex.exception.message.should.equal(f"Volume {volume.id} is currently attached to {instance_id}") + ex.exception.message.should.equal(f"Volume {volume.id} is currently attached to {instance.id}") volume.detach() From d3367b8a90b25fa2fab323889f72717054e63d54 Mon Sep 17 00:00:00 2001 From: mickeypash Date: Fri, 3 Apr 2020 02:27:46 +0100 Subject: [PATCH 6/7] Black formatting --- moto/ec2/exceptions.py | 5 ++--- tests/test_ec2/test_elastic_block_store.py | 4 +++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/moto/ec2/exceptions.py b/moto/ec2/exceptions.py index 4df507a0d..5af4690ae 100644 --- a/moto/ec2/exceptions.py +++ b/moto/ec2/exceptions.py @@ -232,12 +232,11 @@ class InvalidVolumeAttachmentError(EC2ClientError): class VolumeInUseError(EC2ClientError): - def __init__(self, volume_id, instance_id): super(VolumeInUseError, self).__init__( "VolumeInUse", - "Volume {0} is currently attached to {1}" - .format(volume_id, instance_id)) + "Volume {0} is currently attached to {1}".format(volume_id, instance_id), + ) class InvalidDomainError(EC2ClientError): diff --git a/tests/test_ec2/test_elastic_block_store.py b/tests/test_ec2/test_elastic_block_store.py index 1182610e8..ac9c7e3d9 100644 --- a/tests/test_ec2/test_elastic_block_store.py +++ b/tests/test_ec2/test_elastic_block_store.py @@ -76,7 +76,9 @@ def test_delete_attached_volume(): volume.delete() ex.exception.error_code.should.equal("VolumeInUse") ex.exception.status.should.equal(400) - ex.exception.message.should.equal(f"Volume {volume.id} is currently attached to {instance.id}") + ex.exception.message.should.equal( + f"Volume {volume.id} is currently attached to {instance.id}" + ) volume.detach() From a6864f483db1c1b292098a67381c6f20437a8dd2 Mon Sep 17 00:00:00 2001 From: mickeypash Date: Fri, 3 Apr 2020 14:17:55 +0100 Subject: [PATCH 7/7] Use Python 2 format --- tests/test_ec2/test_elastic_block_store.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_ec2/test_elastic_block_store.py b/tests/test_ec2/test_elastic_block_store.py index ac9c7e3d9..4bd2a8dfa 100644 --- a/tests/test_ec2/test_elastic_block_store.py +++ b/tests/test_ec2/test_elastic_block_store.py @@ -77,7 +77,7 @@ def test_delete_attached_volume(): ex.exception.error_code.should.equal("VolumeInUse") ex.exception.status.should.equal(400) ex.exception.message.should.equal( - f"Volume {volume.id} is currently attached to {instance.id}" + "Volume {0} is currently attached to {1}".format(volume.id, instance.id) ) volume.detach()