fix(RELEASE-1491): cleanup_tags supports multiple repos

this PR fixes the case when the image has multiple
repositories and ends up not cleaning tags of all
of them.

Signed-off-by: Leandro Mendes <lmendes@redhat.com>

Author: theflockers

pyxis/cleanup_tags.py

@@ -40,6 +40,7 @@ def parse_arguments() -> argparse.Namespace:  # pragma: no cover
         ),
         help="Pyxis Graphql endpoint",
     )
+    parser.add_argument("--repository", required=True, help="Repository to cleanup tags from")
     parser.add_argument(
         "image_id",
         help="Pyxis Container Image ID",
@@ -58,14 +59,15 @@ def parse_arguments() -> argparse.Namespace:  # pragma: no cover
 def cleanup_tags_with_retry(
     graphql_api: str,
     image_id: str,
+    target_repository: str,
     retries: int = 3,
     backoff_factor: float = 5.0,
 ):
     last_err = RuntimeError()
     for attempt in range(retries):
         try:
             time.sleep(backoff_factor * attempt)
-            cleanup_tags(graphql_api, image_id)
+            cleanup_tags(graphql_api, image_id, target_repository)
             return
         except RuntimeError as e:
             LOGGER.warning(f"Attempt {attempt+1} failed.")
@@ -74,13 +76,14 @@ def cleanup_tags_with_retry(
     raise last_err
 
 
-def cleanup_tags(graphql_api, image_id: str):
+def cleanup_tags(graphql_api, image_id: str, target_repository: str):
     image = get_image(graphql_api, image_id)
 
-    registry, repository, tags = get_rh_registry_image_properties(image)
+    registry, repository, tags = get_rh_registry_image_properties(image, target_repository)
 
     LOGGER.info(f"Image id: {image['_id']}")
     LOGGER.info(f"Image architecture: {image['architecture']}")
+    LOGGER.info(f"Repository: {repository}")
     LOGGER.info(f"Image tags: {tags}")
 
     images_for_cleanup = {}
@@ -102,7 +105,7 @@ def cleanup_tags(graphql_api, image_id: str):
                 images_for_cleanup[id] = candidate
 
     LOGGER.info(f"Found {len(images_for_cleanup)} images for cleanup.")
-    update_images(graphql_api, tags, images_for_cleanup)
+    update_images(graphql_api, tags, images_for_cleanup, repository)
 
 
 def get_image(graphql_api: str, image_id: str) -> dict:
@@ -137,19 +140,23 @@ def get_image(graphql_api: str, image_id: str) -> dict:
     return image
 
 
-def get_rh_registry_image_properties(image: Dict):
+def get_rh_registry_image_properties(image: Dict, target_repository: str):
     """Get the registry.access.redhat.com repository properties of the image
     needed to search for related images.
 
-    Returns (registry, repository, tags)
+    :return: (registry, repository, tags)
     """
     for repo in image["repositories"]:
-        if repo["registry"] == "registry.access.redhat.com":
+        if (
+            repo["registry"] == "registry.access.redhat.com"
+            and repo["repository"] == target_repository
+        ):
             if repo["tags"] is None:
                 tags = []
             else:
                 tags = [tag["name"] for tag in repo["tags"]]
             return repo["registry"], repo["repository"], tags
+
     raise RuntimeError(
         "Cannot find the registry.access.redhat.com repository entry for the image"
     )
@@ -246,7 +253,7 @@ def get_candidates_for_cleanup(
     return images
 
 
-def update_images(graphql_api: str, tags: List[str], images: Dict):
+def update_images(graphql_api: str, tags: List[str], images: Dict, target_repository: str):
     """Update images to remove unwanted tags from them
 
     For each image in `images` it will remove all `tags`
@@ -257,13 +264,16 @@ def update_images(graphql_api: str, tags: List[str], images: Dict):
         LOGGER.info(f"Updating image {image['_id']} with architecture {image['architecture']}")
         LOGGER.info("Repositories and tags before update:")
         for repository in image["repositories"]:
-            repo_tags = [tag["name"] for tag in repository["tags"]]
+            repo_tags = [tag["name"] for tag in repository.get("tags") or []]
             LOGGER.info(f"  {repository['registry']}/{repository['repository']}: {repo_tags}")
         for i in range(len(image["repositories"])):
-            repo_tags = image["repositories"][i]["tags"]
-            image["repositories"][i]["tags"] = [
-                tag for tag in repo_tags if tag["name"] not in tags
-            ]
+            # clean up tags only only from the given repository
+            if image["repositories"][i]["repository"] == target_repository:
+                repo_tags = image["repositories"][i]["tags"]
+                image["repositories"][i]["tags"] = [
+                    tag for tag in repo_tags if tag["name"] not in tags
+                ]
+
         # When we load the images for patching, we request all fields of
         # the ContainerRepository objects because otherwise we might remove some data with
         # the update. But that means that fields that are not used will be null/None and
@@ -354,9 +364,9 @@ def main():  # pragma: no cover
     LOGGER.debug(f"Pyxis GraphQL API: {args.pyxis_graphql_api}")
 
     if args.retry:
-        cleanup_tags_with_retry(args.pyxis_graphql_api, args.image_id)
+        cleanup_tags_with_retry(args.pyxis_graphql_api, args.image_id, args.repository)
     else:
-        cleanup_tags(args.pyxis_graphql_api, args.image_id)
+        cleanup_tags(args.pyxis_graphql_api, args.image_id, args.repository)
 
 
 if __name__ == "__main__":  # pragma: no cover

pyxis/test_cleanup_tags.py

@@ -21,17 +21,17 @@
 @patch("cleanup_tags.cleanup_tags")
 def test_cleanup_tags_with_retry__success(mock_cleanup_tags):
     """cleanup_tags succeeds on first attempt"""
-    cleanup_tags_with_retry(GRAPHQL_API, IMAGE_ID)
+    cleanup_tags_with_retry(GRAPHQL_API, IMAGE_ID, REPOSITORY)
 
-    mock_cleanup_tags.assert_called_once_with(GRAPHQL_API, IMAGE_ID)
+    mock_cleanup_tags.assert_called_once_with(GRAPHQL_API, IMAGE_ID, REPOSITORY)
 
 
 @patch("cleanup_tags.cleanup_tags")
 def test_cleanup_tags_with_retry__success_after_one_attempt(mock_cleanup_tags):
     """cleanup_tags succeeds after one retry"""
     mock_cleanup_tags.side_effect = [RuntimeError("error"), None]
 
-    cleanup_tags_with_retry(GRAPHQL_API, IMAGE_ID, backoff_factor=0)
+    cleanup_tags_with_retry(GRAPHQL_API, IMAGE_ID, REPOSITORY, backoff_factor=0)
 
     assert mock_cleanup_tags.call_count == 2
 
@@ -42,7 +42,7 @@ def test_cleanup_tags_with_retry__fails(mock_cleanup_tags):
     mock_cleanup_tags.side_effect = RuntimeError("error")
 
     with pytest.raises(RuntimeError):
-        cleanup_tags_with_retry(GRAPHQL_API, IMAGE_ID, retries=2, backoff_factor=0)
+        cleanup_tags_with_retry(GRAPHQL_API, IMAGE_ID, REPOSITORY, retries=2, backoff_factor=0)
 
     assert mock_cleanup_tags.call_count == 2
 
@@ -72,7 +72,7 @@ def test_cleanup_tags__success(
         [image1, image3],
     ]
 
-    cleanup_tags(GRAPHQL_API, "1111")
+    cleanup_tags(GRAPHQL_API, "1111", REPOSITORY)
 
     mock_get_image.assert_called_once_with(GRAPHQL_API, "1111")
     assert mock_get_candidates_for_cleanup.call_args_list == [
@@ -81,7 +81,7 @@ def test_cleanup_tags__success(
         call(GRAPHQL_API, REGISTRY, REPOSITORY, "9.4-1111"),
     ]
     mock_update_images.assert_called_once_with(
-        GRAPHQL_API, ["latest", "9.4", "9.4-1111"], {image2["_id"]: image2}
+        GRAPHQL_API, ["latest", "9.4", "9.4-1111"], {image2["_id"]: image2}, REPOSITORY
     )
 
 
@@ -108,15 +108,17 @@ def test_cleanup_tags__nothing_to_cleanup(
         [image1, image2],
     ]
 
-    cleanup_tags(GRAPHQL_API, "1111")
+    cleanup_tags(GRAPHQL_API, "1111", REPOSITORY)
 
     mock_get_image.assert_called_once_with(GRAPHQL_API, "1111")
     assert mock_get_candidates_for_cleanup.call_args_list == [
         call(GRAPHQL_API, REGISTRY, REPOSITORY, "latest"),
         call(GRAPHQL_API, REGISTRY, REPOSITORY, "9.4"),
         call(GRAPHQL_API, REGISTRY, REPOSITORY, "9.4-1111"),
     ]
-    mock_update_images.assert_called_once_with(GRAPHQL_API, ["latest", "9.4", "9.4-1111"], {})
+    mock_update_images.assert_called_once_with(
+        GRAPHQL_API, ["latest", "9.4", "9.4-1111"], {}, REPOSITORY
+    )
 
 
 @patch("pyxis.graphql_query")
@@ -137,7 +139,7 @@ def test_get_rh_registry_image_properties__success():
     """
     image = generate_image("1111", "amd64", ["latest"])
 
-    registry, repository, tags = get_rh_registry_image_properties(image)
+    registry, repository, tags = get_rh_registry_image_properties(image, REPOSITORY)
 
     assert registry == REGISTRY
     assert repository == REPOSITORY
@@ -152,13 +154,25 @@ def test_get_rh_registry_image_properties__no_tags():
     image["repositories"][0]["tags"] = None
     image["repositories"][1]["tags"] = None
 
-    registry, repository, tags = get_rh_registry_image_properties(image)
+    registry, repository, tags = get_rh_registry_image_properties(image, REPOSITORY)
 
     assert registry == REGISTRY
     assert repository == REPOSITORY
     assert tags == []
 
 
+def test_get_rh_registry_image_properties__multiple_images__repository_set__success():
+    """Basic scenario where the function parses the image with multiple repositories
+    and returns the expected values
+    """
+    image = generate_image("1111", "amd64", ["latest"], True)
+    registry, repository, tags = get_rh_registry_image_properties(image, REPOSITORY)
+
+    assert registry == REGISTRY
+    assert repository == REPOSITORY
+    assert tags == ["latest"]
+
+
 def test_get_rh_registry_image_properties__failure():
     """The Red Hat registry repository is not found in the image,
     so an exception is raised
@@ -176,7 +190,7 @@ def test_get_rh_registry_image_properties__failure():
     ]
 
     with pytest.raises(RuntimeError):
-        get_rh_registry_image_properties(image)
+        get_rh_registry_image_properties(image, REPOSITORY)
 
 
 @patch("pyxis.graphql_query")
@@ -203,16 +217,26 @@ def test_update_images__success(mock_update_image):
     """Happy path scenario:
     There are 2 images on input and both have the correct tags removed
     """
+
+    lasting_tags = {
+        "image1": [{"name": "latest"}, {"name": "9.4"}, {"name": "9.4-1111"}],
+        "image2": [{"name": "9.4"}, {"name": "9.4-2222"}],
+    }
+
     image1 = generate_image("1111", "amd64", ["latest", "9.4", "9.4-1111"])
     image1_new = generate_image("1111", "amd64", ["9.4-1111"])
+    image1_new["repositories"][0]["tags"] = lasting_tags["image1"]
+
     image2 = generate_image("2222", "amd64", ["9.4", "9.4-2222"])
     image2_new = generate_image("2222", "amd64", ["9.4-2222"])
+    image2_new["repositories"][0]["tags"] = lasting_tags["image2"]
+
     images = {
         image1["_id"]: image1,
         image2["_id"]: image2,
     }
 
-    update_images(GRAPHQL_API, ["latest", "9.4", "9.4-0000"], images)
+    update_images(GRAPHQL_API, ["latest", "9.4", "9.4-0000"], images, REPOSITORY)
 
     assert mock_update_image.call_args_list == [
         call(GRAPHQL_API, image1_new),
@@ -252,7 +276,7 @@ def test_remove_none_values__success():
     assert remove_none_values(data) == expected_result
 
 
-def generate_image(id, architecture, tags):
+def generate_image(id, architecture, tags, multiple_repos=False):
     image = {
         "_id": id,
         "architecture": architecture,
@@ -269,6 +293,16 @@ def generate_image(id, architecture, tags):
             },
         ],
     }
+
+    if multiple_repos is not False:
+        image["repositories"].append(
+            {
+                "registry": REGISTRY,
+                "repository": "redhat-nonprod/myproduct----myimage",
+                "tags": [{"name": tag} for tag in tags],
+            }
+        )
+
     return image