diff --git a/pycloudlib/azure/cloud.py b/pycloudlib/azure/cloud.py index b2388f03..719f99e6 100644 --- a/pycloudlib/azure/cloud.py +++ b/pycloudlib/azure/cloud.py @@ -637,7 +637,7 @@ def delete_image(self, image_id, **kwargs): if delete_poller.status() == "Succeeded": if image_id in self.registered_images: del self.registered_images[image_id] - self._log.debug("Image %s was deleted", image_id) + self._record_image_deletion(image_id) else: self._log.debug( "Error deleting %s. Status: %d", @@ -1102,12 +1102,13 @@ class compatibility. raise InstanceNotFoundError(resource_id=instance_id) - def snapshot(self, instance, clean=True, delete_provisioned_user=True, **kwargs): + def snapshot(self, instance, *, clean=True, keep=False, delete_provisioned_user=True, **kwargs): """Snapshot an instance and generate an image from it. Args: instance: Instance to snapshot clean: Run instance clean method before taking snapshot + keep: keep the snapshot after the cloud instance is cleaned up delete_provisioned_user: Deletes the last provisioned user kwargs: Other named arguments specific to this implementation @@ -1139,7 +1140,11 @@ def snapshot(self, instance, clean=True, delete_provisioned_user=True, **kwargs) image_id = image.id image_name = image.name - self.created_images.append(image_id) + self._store_snapshot_info( + snapshot_id=image_id, + snapshot_name=image_name, + keep_snapshot=keep, + ) self.registered_images[image_id] = { "name": image_name, diff --git a/pycloudlib/cloud.py b/pycloudlib/cloud.py index b627395c..0d2ea8c5 100644 --- a/pycloudlib/cloud.py +++ b/pycloudlib/cloud.py @@ -19,6 +19,7 @@ ) from pycloudlib.instance import BaseInstance from pycloudlib.key import KeyPair +from pycloudlib.types import ImageInfo from pycloudlib.util import ( get_timestamped_tag, log_exception_list, @@ -47,7 +48,8 @@ def __init__( config_file: path to pycloudlib configuration file """ self.created_instances: List[BaseInstance] = [] - self.created_images: List[str] = [] + self.created_images: List[ImageInfo] = [] + self.preserved_images: List[ImageInfo] = [] # each dict will hold an id and name self._log = logging.getLogger("{}.{}".format(__name__, self.__class__.__name__)) self.config = self._check_and_get_config(config_file, required_values) @@ -177,12 +179,13 @@ def launch( raise NotImplementedError @abstractmethod - def snapshot(self, instance, clean=True, **kwargs): + def snapshot(self, instance, *, clean=True, keep=False, **kwargs): """Snapshot an instance and generate an image from it. Args: instance: Instance to snapshot clean: run instance clean method before taking snapshot + keep: keep the snapshot after the cloud instance is cleaned up Returns: An image id @@ -204,11 +207,18 @@ def clean(self) -> List[Exception]: instance.delete() except Exception as e: exceptions.append(e) - for image_id in self.created_images: + for image_info in self.created_images: try: - self.delete_image(image_id) + self.delete_image(image_id=image_info.image_id) except Exception as e: exceptions.append(e) + for image_info in self.preserved_images: + # noop - just log that we're not cleaning up these images + self._log.info( + "Preserved image %s [id:%s] is NOT being cleaned up.", + image_info.image_name, + image_info.image_id, + ) return exceptions def list_keys(self): @@ -359,3 +369,72 @@ def _get_ssh_keys( private_key_path=private_key_path, name=name, ) + + def _store_snapshot_info( + self, + snapshot_id: str, + snapshot_name: str, + keep_snapshot: bool, + ) -> ImageInfo: + """ + Save the snapshot information for later cleanup depending on the keep_snapshot argument. + + This method saves the snapshot information to either `created_images` or `preserved_images` + based on the value of `keep_snapshot`. These lists are used by the `BaseCloud`'s `clean()` + method to manage snapshots during cleanup. The snapshot information is also logged in a + consistent format so that individual clouds do NOT need to worry about logging. + + Args: + snapshot_id (str): ID of the snapshot (used later to delete the snapshot). + snapshot_name (str): Name of the snapshot (for user reference). + keep_snapshot (bool): Whether to keep the snapshot after the cloud instance is cleaned up. + + Returns: + ImageInfo: An ImageInfo object containing the snapshot information. + """ + image_info = ImageInfo( + image_id=snapshot_id, + image_name=snapshot_name, + ) + if not keep_snapshot: + self.created_images.append(image_info) + self._log.info( + "Created temporary snapshot %s", + image_info, + ) + else: + self.preserved_images.append(image_info) + self._log.info( + "Created permanent snapshot %s", + image_info, + ) + return image_info + + def _record_image_deletion(self, image_id: str): + """ + Record the deletion of an image. + + This method should be called after an image is successfully deleted. + It will remove the image from the list of created_images or preserved_images + so that the cloud does not attempt to re-clean it up later. It will also log + the deletion of the image. + + :param image_id: ID of the image that was deleted + """ + if match := [i for i in self.created_images if i.image_id == image_id]: + deleted_image = match[0] + self.created_images.remove(deleted_image) + self._log.debug( + "Snapshot %s has been deleted. Will no longer need to be cleaned up later.", + deleted_image, + ) + elif match := [i for i in self.preserved_images if i.image_id == image_id]: + deleted_image = match[0] + self.preserved_images.remove(deleted_image) + self._log.debug( + "Snapshot %s has been deleted. This snapshot was taken with keep=True, " + "but since it has been manually deleted, it will not be preserved.", + deleted_image, + ) + else: + self._log.debug("Deleted image %s", image_id) diff --git a/pycloudlib/ec2/cloud.py b/pycloudlib/ec2/cloud.py index 9048fdac..513ea581 100644 --- a/pycloudlib/ec2/cloud.py +++ b/pycloudlib/ec2/cloud.py @@ -295,6 +295,8 @@ def delete_image(self, image_id, **kwargs): self._log.debug("removing custom snapshot %s", snapshot_id) self.client.delete_snapshot(SnapshotId=snapshot_id) + self._record_image_deletion(image_id) + def delete_key(self, name): """Delete an uploaded key. @@ -417,12 +419,13 @@ def list_keys(self): keypair_names.append(keypair["KeyName"]) return keypair_names - def snapshot(self, instance, clean=True): + def snapshot(self, instance, *, clean=True, keep=False, **kwargs): """Snapshot an instance and generate an image from it. Args: instance: Instance to snapshot clean: run instance clean method before taking snapshot + keep: keep the snapshot after the cloud instance is cleaned up Returns: An image id @@ -441,7 +444,12 @@ def snapshot(self, instance, clean=True): ) image_ami_edited = response["ImageId"] image = self.resource.Image(image_ami_edited) - self.created_images.append(image.id) + + self._store_snapshot_info( + snapshot_id=image.id, + snapshot_name=image.name, + keep_snapshot=keep, + ) self._wait_for_snapshot(image) _tag_resource(image, self.tag) diff --git a/pycloudlib/gce/cloud.py b/pycloudlib/gce/cloud.py index f334a09c..44d40d20 100644 --- a/pycloudlib/gce/cloud.py +++ b/pycloudlib/gce/cloud.py @@ -315,6 +315,7 @@ def delete_image(self, image_id, **kwargs): raise_on_error(operation) except GoogleAPICallError as e: raise_on_error(e) + self._record_image_deletion(image_id) def get_instance( self, @@ -428,12 +429,13 @@ def launch( self.created_instances.append(instance) return instance - def snapshot(self, instance: GceInstance, clean=True, **kwargs): + def snapshot(self, instance: GceInstance, *, clean=True, keep=False, **kwargs): """Snapshot an instance and generate an image from it. Args: instance: Instance to snapshot clean: run instance clean method before taking snapshot + keep: keep the snapshot after the cloud instance is cleaned up Returns: An image id @@ -471,7 +473,11 @@ def snapshot(self, instance: GceInstance, clean=True, **kwargs): self._wait_for_operation(operation) image_id = "projects/{}/global/images/{}".format(self.project, snapshot_name) - self.created_images.append(image_id) + self._store_snapshot_info( + snapshot_name=snapshot_name, + snapshot_id=image_id, + keep_snapshot=keep, + ) return image_id def _wait_for_operation(self, operation, operation_type="global", sleep_seconds=300): diff --git a/pycloudlib/ibm/cloud.py b/pycloudlib/ibm/cloud.py index a75c135b..9ff3ac99 100644 --- a/pycloudlib/ibm/cloud.py +++ b/pycloudlib/ibm/cloud.py @@ -13,7 +13,7 @@ from pycloudlib.cloud import BaseCloud from pycloudlib.config import ConfigFile -from pycloudlib.errors import InvalidTagNameError +from pycloudlib.errors import InvalidTagNameError, ResourceNotFoundError, ResourceType from pycloudlib.ibm._util import get_first as _get_first from pycloudlib.ibm._util import iter_resources as _iter_resources from pycloudlib.ibm._util import wait_until as _wait_until @@ -130,7 +130,9 @@ def delete_image(self, image_id: str, **kwargs): self._client.delete_image(image_id).get_result() except ApiException as e: if "does not exist" not in str(e): - raise + raise ResourceNotFoundError(ResourceType.IMAGE, image_id) from e + else: + self._record_image_deletion(image_id) def released_image(self, release, *, arch: str = "amd64", **kwargs): """ID of the latest released image for a particular release. @@ -312,12 +314,13 @@ def launch( return instance - def snapshot(self, instance: IBMInstance, clean: bool = True, **kwargs) -> str: + def snapshot(self, instance: IBMInstance, *, clean=True, keep=False, **kwargs) -> str: """Snapshot an instance and generate an image from it. Args: instance: Instance to snapshot clean: run instance clean method before taking snapshot + keep: keep the snapshot after the cloud instance is cleaned up Returns: An image id @@ -347,7 +350,11 @@ def snapshot(self, instance: IBMInstance, clean: bool = True, **kwargs) -> str: f"Snapshot not available after {timeout_seconds} seconds. Check IBM VPC console." ), ) - self.created_images.append(snapshot_id) + self._store_snapshot_info( + snapshot_name=str(image_prototype["name"]), + snapshot_id=snapshot_id, + keep_snapshot=keep, + ) return snapshot_id def list_keys(self) -> List[str]: diff --git a/pycloudlib/ibm_classic/cloud.py b/pycloudlib/ibm_classic/cloud.py index b9d89b5d..82bb6f84 100644 --- a/pycloudlib/ibm_classic/cloud.py +++ b/pycloudlib/ibm_classic/cloud.py @@ -81,6 +81,7 @@ def delete_image(self, image_id: str, **kwargs): ) from e except SoftLayer.SoftLayerAPIError as e: raise IBMClassicException(f"Error deleting image {image_id}") from e + self._record_image_deletion(image_id) def released_image(self, release, *, disk_size: str = "25G", **kwargs): """ID (globalIdentifier) of the latest released image for a particular release. @@ -267,7 +268,9 @@ def launch( def snapshot( self, instance, + *, clean=True, + keep=False, note: Optional[str] = None, **kwargs, ): @@ -276,6 +279,7 @@ def snapshot( Args: instance: Instance to snapshot clean: run instance clean method before taking snapshot + keep: keep the snapshot after the cloud instance is cleaned up note: optional note to add to the snapshot Returns: @@ -290,10 +294,10 @@ def snapshot( name=f"{self.tag}-snapshot", notes=note, ) - self._log.info( - "Successfully created snapshot '%s' with ID: %s", - snapshot_result["name"], - snapshot_result["id"], + self._store_snapshot_info( + snapshot_name=snapshot_result["name"], + snapshot_id=snapshot_result["id"], + keep_snapshot=keep, ) return snapshot_result["id"] diff --git a/pycloudlib/lxd/cloud.py b/pycloudlib/lxd/cloud.py index 75ec9228..b4de7c59 100644 --- a/pycloudlib/lxd/cloud.py +++ b/pycloudlib/lxd/cloud.py @@ -394,11 +394,10 @@ def delete_image(self, image_id, **kwargs): image_id: string, LXD image fingerprint """ self._log.debug("Deleting image: '%s'", image_id) - subp(["lxc", "image", "delete", image_id]) - self._log.debug("Deleted %s", image_id) + self._record_image_deletion(image_id) - def snapshot(self, instance, clean=True, name=None): + def snapshot(self, instance: LXDInstance, *, clean=True, keep=False, name=None): # type: ignore """Take a snapshot of the passed in instance for use as image. :param instance: The instance to create an image from @@ -413,7 +412,11 @@ def snapshot(self, instance, clean=True, name=None): instance.clean() snapshot_name = instance.snapshot(name) - self.created_snapshots.append(snapshot_name) + self._store_snapshot_info( + snapshot_name=snapshot_name, + snapshot_id=snapshot_name, + keep_snapshot=keep, + ) return snapshot_name # pylint: disable=broad-except @@ -425,13 +428,6 @@ def clean(self) -> List[Exception]: """ exceptions = super().clean() - for snapshot in self.created_snapshots: - try: - subp(["lxc", "image", "delete", snapshot]) - except RuntimeError as e: - if "Image not found" not in str(e): - exceptions.append(e) - for profile in self.created_profiles: try: subp(["lxc", "profile", "delete", profile]) diff --git a/pycloudlib/oci/cloud.py b/pycloudlib/oci/cloud.py index 0078148a..a37c4cb8 100644 --- a/pycloudlib/oci/cloud.py +++ b/pycloudlib/oci/cloud.py @@ -135,6 +135,7 @@ def delete_image(self, image_id, **kwargs): image_id: string, id of the image to delete """ self.compute_client.delete_image(image_id, **kwargs) + self._record_image_deletion(image_id) def released_image(self, release, operating_system="Canonical Ubuntu"): """Get the released image. @@ -365,15 +366,17 @@ def find_compatible_subnet(self, networking_config: NetworkingConfig) -> str: ) return subnet_id - def snapshot(self, instance, clean=True, name=None): + def snapshot(self, instance, *, clean=True, keep=False, name=None): """Snapshot an instance and generate an image from it. Args: instance: Instance to snapshot clean: run instance clean method before taking snapshot - name: (Optional) Name of created image + keep: Keep the image after the cloud instance is cleaned up + name: Name of created image + Returns: - An image object + The image id of the snapshot """ if clean: instance.clean() @@ -392,7 +395,11 @@ def snapshot(self, instance, clean=True, name=None): desired_state="AVAILABLE", ) - self.created_images.append(image_data.id) + self._store_snapshot_info( + snapshot_name=image_data.display_name, + snapshot_id=image_data.id, + keep_snapshot=keep, + ) return image_data.id diff --git a/pycloudlib/openstack/cloud.py b/pycloudlib/openstack/cloud.py index ca009a1e..f9861048 100644 --- a/pycloudlib/openstack/cloud.py +++ b/pycloudlib/openstack/cloud.py @@ -56,6 +56,7 @@ def delete_image(self, image_id, **kwargs): image_id: string, id of the image to delete """ self.conn.delete_image(image_id, wait=True) + self._record_image_deletion(image_id) def released_image(self, release, **kwargs): """Not supported for openstack.""" @@ -171,12 +172,13 @@ def launch( self.created_instances.append(instance) return instance - def snapshot(self, instance, clean=True, **kwargs): + def snapshot(self, instance, *, clean=True, keep=False, **kwargs): """Snapshot an instance and generate an image from it. Args: instance: Instance to snapshot clean: run instance clean method before taking snapshot + keep: keep the snapshot after the cloud instance is cleaned up Returns: An image id @@ -188,7 +190,11 @@ def snapshot(self, instance, clean=True, **kwargs): image = self.conn.create_image_snapshot( "{}-snapshot".format(self.tag), instance.server.id, wait=True ) - self.created_images.append(image.id) + self._store_snapshot_info( + snapshot_name=image.name, + snapshot_id=image.id, + keep_snapshot=keep, + ) return image.id def use_key(self, public_key_path, private_key_path=None, name=None): diff --git a/pycloudlib/qemu/cloud.py b/pycloudlib/qemu/cloud.py index 76939ff2..8743a168 100644 --- a/pycloudlib/qemu/cloud.py +++ b/pycloudlib/qemu/cloud.py @@ -107,6 +107,7 @@ def delete_image(self, image_id, **kwargs): image_file = Path(image_id) if image_file.exists(): image_file.unlink() + self._record_image_deletion(image_id) else: self._log.debug("Cannot delete image %s as it does not exist", image_file) @@ -542,12 +543,13 @@ def launch( return instance - def snapshot(self, instance: QemuInstance, clean=True, **kwargs) -> str: + def snapshot(self, instance: QemuInstance, *, clean=True, keep=False, **kwargs) -> str: """Snapshot an instance and generate an image from it. Args: instance: Instance to snapshot clean: run instance clean method before taking snapshot + keep: keep the snapshot after the cloud instance is cleaned up Returns: An image id @@ -596,7 +598,11 @@ def snapshot(self, instance: QemuInstance, clean=True, **kwargs) -> str: snapshot_path, instance.instance_path, ) - self.created_images.append(str(snapshot_path)) + self._store_snapshot_info( + snapshot_name=snapshot_path.stem, + snapshot_id=str(snapshot_path), + keep_snapshot=keep, + ) return str(snapshot_path) diff --git a/pycloudlib/types.py b/pycloudlib/types.py index 81ee878b..a60b012b 100644 --- a/pycloudlib/types.py +++ b/pycloudlib/types.py @@ -65,3 +65,38 @@ def to_dict(self) -> dict: "networking_type": self.networking_type.value, "private": self.private, } + + +@dataclass +class ImageInfo: + """Dataclass that represents an image on any given cloud.""" + + image_id: str + image_name: str + + def __str__(self): + """Return a human readable string representation of the image.""" + return f"{self.image_name} [id: {self.image_id}]" + + def __repr__(self): + """Return a string representation of the image.""" + return f"ImageInfo(id={self.image_id}, name={self.image_name})" + + def __eq__(self, other): + """ + Check if two ImageInfo objects represent the same image. + + Only the id is used for comparison since this should be the unique identifier for an image. + """ + # Allow for comparing an ImageInfo object with just an ID string + if isinstance(other, str): + return self.image_id == other + # Do not allow for comparing with other types + if not isinstance(other, ImageInfo): + return False + # Check if the image IDs are the same when comparing two ImageInfo objects + return self.image_id == other.image_id + + def __dict__(self): + """Return a dictionary representation of the image.""" + return {"image_id": self.image_id, "image_name": self.image_name} diff --git a/pycloudlib/vmware/cloud.py b/pycloudlib/vmware/cloud.py index 4eb71a14..f5e3ad3c 100644 --- a/pycloudlib/vmware/cloud.py +++ b/pycloudlib/vmware/cloud.py @@ -100,6 +100,8 @@ def delete_image(self, image_id, **kwargs): except subprocess.CalledProcessError as e: if "not found" not in str(e): raise + else: + self._record_image_deletion(image_id) def daily_image(self, release: str, **kwargs): """Return released_image for VMWare. @@ -220,12 +222,13 @@ def launch( instance.start() return instance - def snapshot(self, instance, clean=True, **kwargs): + def snapshot(self, instance, *, clean=True, keep=False, **kwargs): """Snapshot an instance and generate an image from it. Args: instance: Instance to snapshot clean: run instance clean method before taking snapshot + keep: keep the snapshot after the cloud instance is cleaned up Returns: An image id @@ -246,6 +249,10 @@ def snapshot(self, instance, clean=True, **kwargs): check=True, ) - self.created_images.append(image_name) + self._store_snapshot_info( + snapshot_name=image_name, + snapshot_id=image_name, + keep_snapshot=keep, + ) return image_name diff --git a/tests/unit_tests/test_cloud.py b/tests/unit_tests/test_cloud.py index 158e6703..a9cf44c3 100644 --- a/tests/unit_tests/test_cloud.py +++ b/tests/unit_tests/test_cloud.py @@ -40,7 +40,7 @@ def get_instance(self, instance_id): def launch(self, image_id, instance_type=None, user_data=None, **kwargs): """Skeletal launch.""" - def snapshot(self, instance, clean=True, **kwargs): + def snapshot(self, instance, *, clean=True, keep=False, **kwargs): """Skeletal snapshot.""" def list_keys(self): @@ -237,3 +237,85 @@ def test_validate_tag(self, tag: str, rules_failed: List[str]): assert tag in str(exc_info.value) for rule in rules_failed: assert rule in str(exc_info.value) + + +class TestSnapshotHelpers: + """ + Tests covering both the _store_snapshot_info and _record_image_deletion methods of BaseCloud. + """ + + @pytest.fixture + def cloud(self): + """Fixture to create a CloudSubclass instance for testing.""" + return CloudSubclass(tag="tag", timestamp_suffix=False, config_file=StringIO(CONFIG)) + + def test_store_snapshot_info_temporary(self, cloud, caplog): + """Test storing snapshot info as temporary.""" + snapshot_id = "snap-123" + snapshot_name = "snapshot-temp" + keep_snapshot = False + + caplog.set_level(logging.DEBUG) + image_info = cloud._store_snapshot_info(snapshot_id, snapshot_name, keep_snapshot) + + assert image_info.image_id == snapshot_id + assert image_info.image_name == snapshot_name + assert image_info in cloud.created_images + assert image_info not in cloud.preserved_images + assert f"Created temporary snapshot {image_info}" in caplog.text + + def test_store_snapshot_info_permanent(self, cloud, caplog): + """Test storing snapshot info as permanent.""" + snapshot_id = "snap-456" + snapshot_name = "snapshot-perm" + keep_snapshot = True + + caplog.set_level(logging.DEBUG) + image_info = cloud._store_snapshot_info(snapshot_id, snapshot_name, keep_snapshot) + + assert image_info.image_id == snapshot_id + assert image_info.image_name == snapshot_name + assert image_info not in cloud.created_images + assert image_info in cloud.preserved_images + assert f"Created permanent snapshot {image_info}" in caplog.text + + def test_record_image_deletion_created_image(self, cloud, caplog): + """Test recording deletion of a created image.""" + snapshot_id = "snap-789" + snapshot_name = "snapshot-created" + keep_snapshot = False + + image_info = cloud._store_snapshot_info(snapshot_id, snapshot_name, keep_snapshot) + caplog.set_level(logging.DEBUG) + cloud._record_image_deletion(snapshot_id) + + assert image_info not in cloud.created_images + assert image_info not in cloud.preserved_images + assert ( + f"Snapshot {image_info} has been deleted. Will no longer need to be cleaned up later." + in caplog.text + ) + + def test_record_image_deletion_preserved_image(self, cloud, caplog): + """Test recording deletion of a preserved image.""" + snapshot_id = "snap-101" + snapshot_name = "snapshot-preserved" + keep_snapshot = True + + image_info = cloud._store_snapshot_info(snapshot_id, snapshot_name, keep_snapshot) + caplog.set_level(logging.DEBUG) + cloud._record_image_deletion(snapshot_id) + + assert image_info not in cloud.created_images + assert image_info not in cloud.preserved_images + assert ( + f"Snapshot {image_info} has been deleted. This snapshot was taken with keep=True, " + "but since it has been manually deleted, it will not be preserved." + ) in caplog.text + + def test_record_image_deletion_nonexistent_image(self, cloud, caplog): + """Test recording deletion of a non-existent image.""" + snapshot_id = "snap-999" + caplog.set_level(logging.DEBUG) + cloud._record_image_deletion(snapshot_id) + assert f"Deleted image {snapshot_id}" in caplog.text