From 2a9f64af9b0254474c7ba076d61656838056322f Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Sep 12 2024 21:52:51 +0000 Subject: Drop release numbers, repos and arches from message topics I don't believe message topics should encode properties this specific. It makes it awkward for consumers - they can't just subscribe to "messages about aws cloud publishing", they have to subscribe to "messages about aws cloud publishing for Cloud_Base for Fedora 41 on x86_64", "messages about aws cloud publishing for Cloud_Base for Fedora 41 on aarch64", "messages about aws cloud publishing for Cloud_Base for Fedora 42 on x86_64" and on and on and on, and keep updating it for new releases. It makes things awkward for the code in wikitcms that writes the AMI download tables in Cloud test pages - with fedimg it could just find a single message per compose with all the relevant info in it which would always be under the same topic, now it needs at least two messages under different topics. Also, I believe it breaks the schema stuff to some degree, because the topic names of the schemas we define are not the same as the topic names of the messages we publish. I believe this kind of detail belongs in the message bodies, not the topics. This rejigs things so we publish messages just with the topics from the schema classes, and the message bodies contain any information which was previously only in the topic. Signed-off-by: Adam Williamson --- diff --git a/fedora-image-uploader-messages/fedora_image_uploader_messages/publish.py b/fedora-image-uploader-messages/fedora_image_uploader_messages/publish.py index 867b1ea..ffe2725 100644 --- a/fedora-image-uploader-messages/fedora_image_uploader_messages/publish.py +++ b/fedora-image-uploader-messages/fedora_image_uploader_messages/publish.py @@ -53,12 +53,22 @@ class AwsPublishedV1(_PublishedV1): "the value is the AMI ID." ), }, + "release": { + "type": "integer", + "description": "The release number associated with the image.", + }, + "subvariant": { + "type": "string", + "description": "The subvariant of the inmage (e.g. Cloud_Base).", + }, }, "required": [ "architecture", "compose_id", "image_name", "regions", + "release", + "subvariant", ], } @@ -129,6 +139,14 @@ class AzurePublishedV1(_PublishedV1): ), "items": {"type": "string"}, }, + "release": { + "type": "integer", + "description": "The release number associated with the image.", + }, + "subvariant": { + "type": "string", + "description": "The subvariant of the inmage (e.g. Cloud_Base).", + }, }, "required": [ "architecture", @@ -137,6 +155,8 @@ class AzurePublishedV1(_PublishedV1): "image_version_name", "image_resource_id", "regions", + "release", + "subvariant", ], } @@ -186,6 +206,10 @@ class ContainerPublishedV1(_PublishedV1): "items": {"type": "string"}, "description": "The registries where the container was published.", }, + "release": { + "type": "integer", + "description": "The release number associated with the image.", + }, "repository": { "type": "string", "description": "The repository where the container was published.", @@ -198,7 +222,7 @@ class ContainerPublishedV1(_PublishedV1): ), }, }, - "required": ["architectures", "compose_id", "registries", "repository", "tags"], + "required": ["architectures", "compose_id", "registries", "release", "repository", "tags"], } @property diff --git a/fedora-image-uploader/fedora_image_uploader/handler.py b/fedora-image-uploader/fedora_image_uploader/handler.py index 4403b8d..8cc11b3 100644 --- a/fedora-image-uploader/fedora_image_uploader/handler.py +++ b/fedora-image-uploader/fedora_image_uploader/handler.py @@ -249,17 +249,11 @@ class Uploader: _run(("buildah", "rmi", manref), failok=True) message = ContainerPublishedV1( - topic=".".join( - [ - ContainerPublishedV1.topic, - repo, - str(ffrel.relnum), - ] - ), body={ "architectures": self.container_repos[repo], "compose_id": ffrel.cid, "registries": [r["url"] for r in self.conf["container"]["registries"]], + "release": ffrel.relnum, "repository": repo, "tags": tags, }, @@ -605,14 +599,13 @@ class Uploader: raise fm_exceptions.Nack() message = AwsPublishedV1( - topic=".".join( - [AwsPublishedV1.topic, image["subvariant"], str(ffrel.relnum), image["arch"]] - ), body={ "architecture": image["arch"], "compose_id": ffrel.cid, "image_name": ami_name, "regions": regions_to_amis, + "release": ffrel.relnum, + "subvariant": image["subvariant"], }, ) if self.conf["aws"].get("publish_amqp_messages", False): @@ -786,9 +779,6 @@ class Uploader: ) message = AzurePublishedV1( - topic=".".join( - [AzurePublishedV1.topic, image["subvariant"], str(ffrel.relnum), image["arch"]] - ), body={ "architecture": image["arch"], "compose_id": ffrel.cid, @@ -800,6 +790,8 @@ class Uploader: f"{gallery_image_name}/Versions/{image_version}" ), "regions": [r["name"] for r in self.conf["azure"]["target_regions"]], + "release": ffrel.relnum, + "subvariant": image["subvariant"], }, ) # Gate publishing behind a feature flag so we can roll out updates while getting diff --git a/fedora-image-uploader/tests/test_handler.py b/fedora-image-uploader/tests/test_handler.py index 19dfd45..4da32f6 100644 --- a/fedora-image-uploader/tests/test_handler.py +++ b/fedora-image-uploader/tests/test_handler.py @@ -162,13 +162,14 @@ def test_containers(mock_subrun, _mock_login, mock_missing, fixtures_dir, caplog consumer.handlers = [consumer.handle_container] expected_messages = [ ContainerPublishedV1( - topic=(f"fedora_image_uploader.published.v1.container.{repo}.{relnum}"), + topic=("fedora_image_uploader.published.v1.container"), body={ "architectures": arches, "compose_id": msg.body["compose_id"], "registries": [r["url"] for r in consumer.conf["container"]["registries"]], "repository": repo, "tags": [relnum, alias], + "release": int(relnum), }, ) for repo, arches in expected_images.items() @@ -355,7 +356,7 @@ def test_azure_messages(fixtures_dir, azure_fm_conf, azure_env_vars): consumer.handlers = [consumer.handle_azure] expected_messages = ( AzurePublishedV1( - topic="fedora_image_uploader.published.v1.azure.Cloud_Base.40.aarch64", + topic="fedora_image_uploader.published.v1.azure", body={ "architecture": "aarch64", "compose_id": "Fedora-Cloud-40-20240910.0", @@ -366,10 +367,12 @@ def test_azure_messages(fixtures_dir, azure_fm_conf, azure_env_vars): "Fedora-Cloud-40-Arm64/Versions/40.20240910.0" ), "regions": [], + "subvariant": "Cloud_Base", + "release": 40, }, ), AzurePublishedV1( - topic="fedora_image_uploader.published.v1.azure.Cloud_Base.40.x86_64", + topic="fedora_image_uploader.published.v1.azure", body={ "architecture": "x86_64", "compose_id": "Fedora-Cloud-40-20240910.0", @@ -380,6 +383,8 @@ def test_azure_messages(fixtures_dir, azure_fm_conf, azure_env_vars): "Fedora-Cloud-40-x64/Versions/40.20240910.0" ), "regions": [], + "subvariant": "Cloud_Base", + "release": 40, }, ), ) @@ -727,21 +732,25 @@ def test_aws_messages(_mock_client, fixtures_dir): expected_messages = [ AwsPublishedV1( - topic="fedora_image_uploader.published.v1.aws.Cloud_Base.40.aarch64", + topic="fedora_image_uploader.published.v1.aws", body={ "architecture": "aarch64", "compose_id": "Fedora-40-20240414.0", "image_name": "Fedora-Cloud-Base-AmazonEC2.aarch64-40-1.14", "regions": {"us-east-1": "ami-0123"}, + "subvariant": "Cloud_Base", + "release": 40, }, ), AwsPublishedV1( - topic="fedora_image_uploader.published.v1.aws.Cloud_Base.40.x86_64", + topic="fedora_image_uploader.published.v1.aws", body={ "architecture": "x86_64", "compose_id": "Fedora-40-20240414.0", "image_name": "Fedora-Cloud-Base-AmazonEC2.x86_64-40-1.14", "regions": {"us-east-1": "ami-0123"}, + "subvariant": "Cloud_Base", + "release": 40, }, ), ]