Merge branch 'release/v0.2.0'

stumpylog · Jul 20, 2023 · 424f5e4 · 424f5e4
2 parents 9c75a26 + d78835e
commit 424f5e4
Show file tree

Hide file tree

Showing 9 changed files with 230 additions and 141 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,24 @@
-# CHANGELOG
+# Changelog
 
-## 0.1.0
+All notable changes to this project will be documented in this file.
 
-- Initial tagged release of the action
+The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
+and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
+
+## [0.2.0] - 2023-07-20
+
+### Added
+
+- Added increased logging during initial information gathering
+- Added better handling of encountering a rate limit while the action is executing
+
+### Changed
+
+- Bump `httpx` from 0.24.0 to 0.24.1
+- Changelog format updated to [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
+- HTTP connection to the API now uses HTTP/2 if possible
+- Updated `pipenv` from 2023.4.20 to 2023.6.26
+
+## [0.1.0] - 2023-06-30
+
+- Initial versioned release of the actions
diff --git a/Pipfile.lock b/Pipfile.lock
diff --git a/ephemeral/action.yml b/ephemeral/action.yml
@@ -45,7 +45,7 @@ runs:
       name: Install pipenv
       shell: bash
       run: |
-        pip3 --quiet install --user pipenv==2023.4.20
+        pip3 --quiet install --user pipenv==2023.6.26
     -
       name: Install dependencies
       shell: bash

diff --git a/github/base.py b/github/base.py
@@ -7,10 +7,13 @@
 
 """
 import logging
+from http import HTTPStatus
 
 import github_action_utils as gha_utils
 import httpx
 
+from utils.errors import RateLimitError
+
 logger = logging.getLogger(__name__)
 
 
@@ -27,6 +30,7 @@ def __init__(self, token: str) -> None:
         # Create the client for connection pooling, add headers for type
         # version and authorization
         self._client: httpx.Client = httpx.Client(
+            http2=True,
             base_url=self.API_BASE_URL,
             timeout=30.0,
             headers={
@@ -64,7 +68,7 @@ def _read_all_pages(self, endpoint: str, query_params: dict | None = None):
 
         while True:
             resp = self._client.get(endpoint, params=query_params)
-            if resp.status_code == 200:
+            if resp.status_code == HTTPStatus.OK:
                 internal_data += resp.json()
                 if "next" in resp.links:
                     endpoint = resp.links["next"]["url"]
@@ -75,6 +79,15 @@ def _read_all_pages(self, endpoint: str, query_params: dict | None = None):
                 msg = f"Request to {endpoint} return HTTP {resp.status_code}"
                 gha_utils.error(message=msg, title=f"HTTP Error {resp.status_code}")
                 logger.error(msg)
+
+                # If forbidden, check if it is rate limiting
+                if (
+                    resp.status_code == HTTPStatus.FORBIDDEN
+                    and "X-RateLimit-Remaining" in resp.headers
+                ):
+                    remaining = int(resp.headers["X-RateLimit-Remaining"])
+                    if remaining <= 0:
+                        raise RateLimitError
                 resp.raise_for_status()
 
         return internal_data

diff --git a/github/packages.py b/github/packages.py
@@ -2,11 +2,13 @@
 import logging
 import re
 import urllib.parse
+from http import HTTPStatus
 
 import github_action_utils as gha_utils
 
 from github.base import GithubApiBase
 from github.base import GithubEndpointResponse
+from utils.errors import RateLimitError
 
 logger = logging.getLogger(__name__)
 
@@ -123,15 +125,22 @@ def delete(self, package_data: ContainerPackage):
         Deletes the given package version from the GHCR
         """
         resp = self._client.delete(package_data.url)
-        if resp.status_code != 204:
-            msg = (
-                f"Request to delete {package_data.url} returned HTTP {resp.status_code}"
-            )
-            gha_utils.warning(
-                message=msg,
-                title=f"Unexpected delete status: {resp.status_code}",
-            )
-            logger.warning(msg)
+        if resp.status_code != HTTPStatus.NO_CONTENT:
+            # If forbidden, check if it is rate limiting
+            if (
+                resp.status_code == HTTPStatus.FORBIDDEN
+                and "X-RateLimit-Remaining" in resp.headers
+            ):
+                remaining = int(resp.headers["X-RateLimit-Remaining"])
+                if remaining <= 0:
+                    raise RateLimitError
+            else:
+                msg = f"Request to delete {package_data.url} returned HTTP {resp.status_code}"
+                gha_utils.warning(
+                    message=msg,
+                    title=f"Unexpected delete status: {resp.status_code}",
+                )
+                logger.warning(msg)
 
     def restore(
         self,
@@ -147,13 +156,22 @@ def restore(
         )
 
         resp = self._client.post(endpoint)
-        if resp.status_code != 204:
-            msg = f"Request to restore id {id} returned HTTP {resp.status_code}"
-            gha_utils.warning(
-                message=msg,
-                title=f"Unexpected restore status: {resp.status_code}",
-            )
-            logger.warning(msg)
+        if resp.status_code != HTTPStatus.NO_CONTENT:
+            # If forbidden, check if it is rate limiting
+            if (
+                resp.status_code == HTTPStatus.FORBIDDEN
+                and "X-RateLimit-Remaining" in resp.headers
+            ):
+                remaining = int(resp.headers["X-RateLimit-Remaining"])
+                if remaining <= 0:
+                    raise RateLimitError
+            else:
+                msg = f"Request to restore id {id} returned HTTP {resp.status_code}"
+                gha_utils.warning(
+                    message=msg,
+                    title=f"Unexpected restore status: {resp.status_code}",
+                )
+                logger.warning(msg)
 
 
 class GithubContainerRegistryOrgApi(_GithubContainerRegistryApiBase):

diff --git a/main_ephemeral.py b/main_ephemeral.py
@@ -3,6 +3,8 @@
 import logging
 import re
 
+import github_action_utils as gha_utils
+
 from github.branches import GithubBranchApi
 from github.packages import ContainerPackage
 from github.packages import GithubContainerRegistryOrgApi
@@ -13,6 +15,7 @@
 from utils import coerce_to_bool
 from utils import common_args
 from utils import get_log_level
+from utils.errors import RateLimitError
 
 logger = logging.getLogger("image-cleaner")
 
@@ -155,8 +158,10 @@ def _main() -> None:
         config.owner_or_org,
         config.is_org,
     ) as api:
+        logger.info("Getting active packages")
         # Get the active (not deleted) packages
         active_versions = api.active_versions(config.package_name)
+        logger.info(f"{len(active_versions)} active packages")
 
     #
     # Step 2 - Filter the packages to those which are:
@@ -166,6 +171,7 @@ def _main() -> None:
     #
     pkgs_matching_re: list[ContainerPackage] = []
     all_pkgs_tags_to_version: dict[str, ContainerPackage] = {}
+    logger.info("Filtering packages to those matching the regex")
     for pkg in active_versions:
         if pkg.untagged or len(pkg.tags) > 1:
             continue
@@ -184,15 +190,22 @@ def _main() -> None:
     # Step 3 - Gather the packages to remove (those where the source is gone or closed)
     #
     if config.scheme == "branch":
+        logger.info("Looking at branches for deletion considerations")
         tags_to_delete = _get_tag_to_delete_branch(config, pkgs_matching_re)
     elif config.scheme == "pull_request":
+        logger.info("Looking at pull requests for deletion considerations")
         tags_to_delete = _get_tags_to_delete_pull_request(config, pkgs_matching_re)
+    else:
+        # Configuration validation prevents any other option
+        pass
 
     tags_to_keep = list(set(all_pkgs_tags_to_version.keys()) - set(tags_to_delete))
 
     if not len(tags_to_delete):
         logger.info("No images to remove")
         return
+    logger.info(f"Will remove {len(set(tags_to_delete))} tagged packages")
+    logger.info(f"Will keep {len(tags_to_keep)} packages")
 
     #
     # Step 4 - Delete the stale packages
@@ -225,11 +238,14 @@ def _main() -> None:
         for tag in tags_to_keep:
             check_tag_still_valid(config.owner_or_org, config.package_name, tag)
     else:
-        logger.info("Dry run, not checking images")
+        logger.info("Dry run, not checking image manifests")
 
 
 if __name__ == "__main__":
     try:
         _main()
+    except RateLimitError:
+        logger.error("Rate limit hit during execution")
+        gha_utils.error("Rate limit hit during execution")
     finally:
         logging.shutdown()
diff --git a/main_untagged.py b/main_untagged.py
@@ -2,6 +2,8 @@
 
 import logging
 
+import github_action_utils as gha_utils
+
 from github.packages import ContainerPackage
 from github.packages import GithubContainerRegistryOrgApi
 from github.packages import GithubContainerRegistryUserApi
@@ -11,6 +13,7 @@
 from utils import coerce_to_bool
 from utils import common_args
 from utils import get_log_level
+from utils.errors import RateLimitError
 
 logger = logging.getLogger("image-cleaner")
 
@@ -70,10 +73,12 @@ def _main() -> None:
         config.owner_or_org,
         config.is_org,
     ) as api:
+        logger.info("Getting active packages")
         # Get the active (not deleted) packages
         active_versions = api.active_versions(config.package_name)
+        logger.info(f"{len(active_versions)} active packages")
 
-    # Map the tag (eg latest) to its package and simplify the untagged data
+    # Map the tag (e.g. latest) to its package and simplify the untagged data
     # mapping name (which is a digest) to the version
     # These just make it easier to do some lookups later
     tag_to_pkgs: dict[str, ContainerPackage] = {}
@@ -84,6 +89,8 @@ def _main() -> None:
         for tag in pkg.tags:
             tag_to_pkgs[tag] = pkg
 
+    logger.info(f"Found {len(untagged_versions)} packages which look untagged")
+
     #
     # Step 2 - Find actually untagged packages
     #
@@ -119,13 +126,19 @@ def _main() -> None:
             # TODO Make it clear for digests which are multi-tagged (latest, x.x.y)
             # they are not being deleted too
 
+    if not len(untagged_versions):
+        logger.info("Nothing to do")
+        return
+
+    logger.info(
+        f"After multi-arch, there are {len(untagged_versions)} untagged packages",
+    )
+
     #
     # Step 4 - Delete the actually untagged packages
     #
     # Delete the untagged and not pointed at packages
     logger.info(f"Deleting untagged packages of {config.package_name}")
-    if not len(untagged_versions):
-        logger.info("Nothing to do")
     with container_reg_class(
         config.token,
         config.owner_or_org,
@@ -160,5 +173,8 @@ def _main() -> None:
 if __name__ == "__main__":
     try:
         _main()
+    except RateLimitError:
+        logger.error("Rate limit hit during execution")
+        gha_utils.error("Rate limit hit during execution")
     finally:
         logging.shutdown()
diff --git a/untagged/action.yml b/untagged/action.yml
@@ -34,7 +34,7 @@ runs:
       name: Install pipenv
       shell: bash
       run: |
-        pip3 --quiet install --user pipenv==2023.4.20
+        pip3 --quiet install --user pipenv==2023.6.26
     -
       name: Install dependencies
       shell: bash

diff --git a/utils/errors.py b/utils/errors.py
@@ -0,0 +1,2 @@
+class RateLimitError(Exception):
+    pass