From f8d64f55a15e561291e74247c33cc1389b55eaf9 Mon Sep 17 00:00:00 2001 From: Jeremy Cline Date: May 20 2024 20:12:42 +0000 Subject: Implement image cleanup policy for Azure This runs image cleanup after processing a new Azure image. The policy is arbitrarily as follows: - For each image definition (Fedora 40, for example), only the 7 latest images which are marked as "excluded from latest" are kept. Images that are not excluded are kept until their EOL is reached. - Images which have an end-of-life date older than the current time are removed. - Any image definitions which have no images in them are removed. - Rawhide is treated as a special case; only the latest 7 are kept regardless of whether they are marked as "excluded from latest" or not. --- diff --git a/fedora_cloud_image_uploader/handler.py b/fedora_cloud_image_uploader/handler.py index aac16b9..273af71 100644 --- a/fedora_cloud_image_uploader/handler.py +++ b/fedora_cloud_image_uploader/handler.py @@ -8,6 +8,8 @@ import time from typing import NamedTuple import ansible_runner +from azure import identity as az_identity +from azure.mgmt.compute import ComputeManagementClient from fedfind import exceptions as ff_exceptions from fedfind import helpers as ff_helpers from fedfind import release as ff_release @@ -257,3 +259,87 @@ class Uploader: } playbook = os.path.join(PLAYBOOKS, "azure.yml") self.run_playbook(playbook, variables, workdir) + try: + self.azure_cleanup_images() + except Exception: + _log.exception("Unable to clean up Azure images") + + def azure_cleanup_images(self): + """ + Remove old images from the Azure Compute Gallery. + """ + subscription = os.environ["AZURE_SUBSCRIPTION_ID"] + creds = az_identity.ClientSecretCredential( + tenant_id=os.environ["AZURE_TENANT"], + client_id=os.environ["AZURE_CLIENT_ID"], + client_secret=os.environ["AZURE_SECRET"], + ) + compute_client = ComputeManagementClient( + credential=creds, + subscription_id=subscription, + api_version="2023-07-03", + ) + + resource_group = self.conf["azure"]["resource_group_name"] + gallery_name = self.conf["azure"]["gallery_name"] + _log.info("Querying image definitions in gallery %s", gallery_name) + for image_definition in compute_client.gallery_images.list_by_gallery( + resource_group_name=resource_group, gallery_name=gallery_name + ): + end_of_life_images = [] + excluded_images = [] + _log.info("Querying image versions in definition %s", image_definition.name) + image_versions = list( + compute_client.gallery_image_versions.list_by_gallery_image( + resource_group_name=resource_group, + gallery_name=gallery_name, + gallery_image_name=image_definition.name, + ) + ) + for image_version in image_versions: + if ( + image_version.publishing_profile.exclude_from_latest + or "rawhide" in image_definition.name.lower() + ): + excluded_images.append(image_version) + if ( + image_version.publishing_profile.end_of_life_date + and image_version.publishing_profile.end_of_life_date + < datetime.datetime.now(datetime.UTC) + ): + end_of_life_images.append(image_version) + excluded_images.sort( + key=lambda version: version.publishing_profile.published_date, reverse=True + ) + + _log.info( + "Removing %d out of %d images from %s", + max(len(excluded_images) - 7, 0), + len(excluded_images), + image_definition.name, + ) + # Save the latest week of images that have been excluded from latest + for image in excluded_images[7:]: + compute_client.gallery_image_versions.begin_delete( + resource_group, gallery_name, image_definition.name, image.name + ) + _log.info( + f"Deleted image {image.name} (excluded from latest) from " + f"{image_definition.name} since 7 newer versions exist" + ) + for image in end_of_life_images: + compute_client.gallery_image_versions.begin_delete( + resource_group, gallery_name, image_definition.name, image.name + ) + _log.info( + f"Deleted image {image.name} from {image_definition.name} " + "since the image is end-of-life" + ) + + if len(image_versions) == 0: + compute_client.gallery_images.begin_delete( + resource_group, gallery_name, image_definition.name + ) + _log.info( + f"Deleted image definition {image_definition.name} since it has no versions" + ) diff --git a/tests/conftest.py b/tests/conftest.py index 7b9d1f8..c063d91 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,8 @@ import os +from unittest import mock import pytest +from fedora_messaging import config as fm_config @pytest.fixture(scope="module") @@ -14,3 +16,38 @@ def vcr_config(fixtures_dir): "record_mode": "once", "cassette_library_dir": os.path.join(fixtures_dir, "cassettes"), } + + +@pytest.fixture(scope="module") +def azure_env_vars(): + """Provide the environment variables needed to authenticate to Azure via SecretCredentials.""" + with mock.patch.dict( + os.environ, + { + "AZURE_SUBSCRIPTION_ID": "sub-id", + "AZURE_TENANT": "tenant-id", + "AZURE_CLIENT_ID": "client-id", + "AZURE_SECRET": "secret", + }, + ): + yield + + +@pytest.fixture(scope="module") +def azure_fm_conf(): + """Provide a minimal config for Azure in the fedora-messaging configuration dictionary.""" + with mock.patch.dict( + fm_config.conf["consumer_config"], + { + "azure": { + "location": "eastus", + "resource_group_name": "fedora-cloud", + "gallery_name": "Fedora", + "gallery_description": "The Fedora compute gallery.", + "storage_account_name": "fedoraimageuploads", + "storage_container_name": "vhds", + "target_regions": {}, + } + }, + ): + yield diff --git a/tests/test_handler.py b/tests/test_handler.py index b5cf94a..c1708be 100644 --- a/tests/test_handler.py +++ b/tests/test_handler.py @@ -1,11 +1,19 @@ +import datetime import json import logging import os +import random import tempfile +from datetime import timedelta from unittest import mock import fedfind.release import pytest +from azure.mgmt.compute.v2023_07_03.models import ( + GalleryImage, + GalleryImageVersion, + GalleryImageVersionPublishingProfile, +) from fedora_messaging import config, exceptions, message from freezegun import freeze_time from requests.exceptions import RequestException @@ -237,3 +245,147 @@ def test_download_image(): image["checksums"]["sha256"] = "nottherightone" with pytest.raises(exceptions.Nack): ret = consumer.download_image(image, tempdir, decompress=True) + + +@mock.patch("fedora_cloud_image_uploader.handler.ComputeManagementClient", autospec=True) +def test_azure_old_excluded_images(mock_az_client, azure_env_vars, azure_fm_conf): + """ + Test the image cleanup policy for removing all images excluded from latest except the + latest 7. + """ + + consumer = Uploader() + image_definition = GalleryImage(location="eastus") + image_definition.name = "Fedora-40" + now = datetime.datetime.now(datetime.UTC) + image_versions = [] + for v in random.sample(range(14), 14): + publish_profile = GalleryImageVersionPublishingProfile( + exclude_from_latest=True, end_of_life_date=now + timedelta(days=7) + ) + publish_profile.published_date = now + timedelta(seconds=v) + version = GalleryImageVersion(location="eastus", publishing_profile=publish_profile) + version.name = f"40.1.{v}" + image_versions.append(version) + client = mock_az_client.return_value + client.gallery_images.list_by_gallery.return_value = [image_definition] + client.gallery_image_versions.list_by_gallery_image.side_effect = [image_versions] + consumer.azure_cleanup_images() + + expected_calls = [ + mock.call("fedora-cloud", "Fedora", "Fedora-40", f"40.1.{v}") for v in range(7) + ] + expected_calls.reverse() + actual_calls = [call for call in client.gallery_image_versions.begin_delete.call_args_list] + assert expected_calls == actual_calls + + +@mock.patch("fedora_cloud_image_uploader.handler.ComputeManagementClient", autospec=True) +def test_azure_end_of_life(mock_az_client, azure_env_vars, azure_fm_conf): + """Test the image cleanup policy for removing end-of-life images.""" + + consumer = Uploader() + image_definition = GalleryImage(location="eastus") + image_definition.name = "Fedora-40" + now = datetime.datetime.now(datetime.UTC) + image_versions = [] + for v in range(1, 5): + publish_profile = GalleryImageVersionPublishingProfile( + exclude_from_latest=False, end_of_life_date=now + timedelta(days=v - 2) + ) + publish_profile.published_date = now - timedelta(days=v) + version = GalleryImageVersion(location="eastus", publishing_profile=publish_profile) + version.name = f"40.1.{v}" + image_versions.append(version) + client = mock_az_client.return_value + client.gallery_images.list_by_gallery.return_value = [image_definition] + client.gallery_image_versions.list_by_gallery_image.side_effect = [image_versions] + consumer.azure_cleanup_images() + + expected_calls = [ + mock.call("fedora-cloud", "Fedora", "Fedora-40", "40.1.2"), + mock.call("fedora-cloud", "Fedora", "Fedora-40", "40.1.1"), + ] + expected_calls.reverse() + actual_calls = [call for call in client.gallery_image_versions.begin_delete.call_args_list] + assert expected_calls == actual_calls + + +@mock.patch("fedora_cloud_image_uploader.handler.ComputeManagementClient", autospec=True) +def test_azure_empty_definitions(mock_az_client, azure_env_vars, azure_fm_conf): + """ + Test the image cleanup policy for definitions with no more images. + + This keeps the definitions tidy after a version EOLs and all image versions are removed. + """ + + consumer = Uploader() + image_definition = GalleryImage(location="eastus") + image_definition.name = "Fedora-40" + client = mock_az_client.return_value + client.gallery_images.list_by_gallery.return_value = [image_definition] + client.gallery_image_versions.list_by_gallery_image.return_value = [] + consumer.azure_cleanup_images() + + client.gallery_images.begin_delete.assert_called_once_with( + "fedora-cloud", "Fedora", "Fedora-40" + ) + + +@mock.patch("fedora_cloud_image_uploader.handler.ComputeManagementClient", autospec=True) +def test_azure_old_included_images(mock_az_client, azure_env_vars, azure_fm_conf): + """ + Test the image cleanup policy keeps all images included in latest that aren't EOL + """ + + consumer = Uploader() + image_definition = GalleryImage(location="eastus") + image_definition.name = "Fedora-40" + now = datetime.datetime.now(datetime.UTC) + image_versions = [] + for v in random.sample(range(14), 14): + publish_profile = GalleryImageVersionPublishingProfile( + exclude_from_latest=False, end_of_life_date=now + timedelta(days=7) + ) + publish_profile.published_date = now + timedelta(seconds=v) + version = GalleryImageVersion(location="eastus", publishing_profile=publish_profile) + version.name = f"40.1.{v}" + image_versions.append(version) + client = mock_az_client.return_value + client.gallery_images.list_by_gallery.return_value = [image_definition] + client.gallery_image_versions.list_by_gallery_image.side_effect = [image_versions] + consumer.azure_cleanup_images() + + assert client.gallery_image_versions.begin_delete.call_count == 0 + + +@mock.patch("fedora_cloud_image_uploader.handler.ComputeManagementClient", autospec=True) +def test_azure_rawhide_images(mock_az_client, azure_env_vars, azure_fm_conf): + """ + Test the image cleanup policy keeps only 7 rawhide images even if included in latest + """ + + consumer = Uploader() + image_definition = GalleryImage(location="eastus") + image_definition.name = "Fedora-Rawhide" + now = datetime.datetime.now(datetime.UTC) + image_versions = [] + for v in random.sample(range(14), 14): + publish_profile = GalleryImageVersionPublishingProfile( + exclude_from_latest=False, end_of_life_date=now + timedelta(days=7) + ) + publish_profile.published_date = now + timedelta(seconds=v) + version = GalleryImageVersion(location="eastus", publishing_profile=publish_profile) + version.name = f"41.1.{v}" + image_versions.append(version) + client = mock_az_client.return_value + client.gallery_images.list_by_gallery.return_value = [image_definition] + client.gallery_image_versions.list_by_gallery_image.side_effect = [image_versions] + consumer.azure_cleanup_images() + + expected_calls = [ + mock.call("fedora-cloud", "Fedora", "Fedora-Rawhide", f"41.1.{v}") for v in range(7) + ] + expected_calls.reverse() + actual_calls = [call for call in client.gallery_image_versions.begin_delete.call_args_list] + assert expected_calls == actual_calls