Update build docker image cache cleanup (#6048)

The current image cache cleanup is not removing many images. Upon examining the cache container registry logs, it appears there are some infrequent pulls of old images which may be made by something other than CI builds (perhaps some automated scan of the registry).
This change adds a minimum access count for images in the cache so that infrequently but periodically accessed images can be removed. The idea is that images used by CI builds that are worth caching will have a higher volume of accesses.

tools/ci_build/clean_docker_image_cache.py

@@ -31,9 +31,11 @@ def parse_args():
         "This assumes a fairly specific setup - an Azure container registry "
         "and a storage account that receives "
         "ContainerRegistryRepositoryEvents logs from that registry. "
-        "The logs are searched to see whether an image was used (pushed or "
-        "pulled) recently enough in order to determine whether the image "
-        "should be kept or cleaned up.")
+        "The logs are searched in order to determine whether images should be "
+        "retained or removed. "
+        "For an image to be retained, it must have been accessed at least N "
+        "times (specified by --cache-min-access-count) over the past K days "
+        "(specified by --cache-history-days).")
 
     parser.add_argument(
         "--container-registry", required=True,
@@ -50,12 +52,16 @@ def parse_args():
         help="The log path pattern in the storage account container.")
 
     parser.add_argument(
-        "--cache-lifetime-days", type=int, default=7,
-        help="How long an image can be cached without being used, in days.")
+        "--cache-history-days", type=int, default=7,
+        help="The length of the cache history in days.")
+
+    parser.add_argument(
+        "--cache-min-access-count", type=int, default=1,
+        help="The minimum access count over the cache history.")
 
     parser.add_argument(
         "--dry-run", action="store_true",
-        help="Do a dry-run and don't actually clean anything up.")
+        help="Do a dry-run and do not remove any images.")
 
     parser.add_argument(
         "--az-path", default="az", help="Path to the az client.")
@@ -129,8 +135,8 @@ def parse_log_line(line, min_datetime):
     return ImageInfo(repo, digest)
 
 
-def get_valid_images_from_logs(log_paths, min_datetime):
-    valid_images = set()  # set of ImageInfo
+def get_valid_images_from_logs(log_paths, min_datetime, min_access_count):
+    image_counts = dict()  # dict of {ImageInfo -> count}
 
     for log_path in log_paths:
         log.debug("Processing log file: {}".format(log_path))
@@ -138,9 +144,9 @@ def get_valid_images_from_logs(log_paths, min_datetime):
             for line in log_file:
                 image_info = parse_log_line(line, min_datetime)
                 if image_info is not None:
-                    valid_images.add(image_info)
+                    image_counts[image_info] = image_counts.get(image_info, 0) + 1
 
-    return valid_images
+    return {image for image, count in image_counts.items() if count >= min_access_count}
 
 
 def get_registry_images(container_registry, az_path):
@@ -171,6 +177,22 @@ def clean_images(container_registry, image_names, az_path):
            parse_output=False)
 
 
+# Note:
+# the log download and parsing could be replaced by a log analytics query
+"""
+let cache_history = 7d;
+let cache_min_access_count = 1;
+ContainerRegistryRepositoryEvents
+| where TimeGenerated >= ago(cache_history)
+| where OperationName in ("Push", "Pull")
+| where ResultDescription == "200"
+| summarize AccessCount = count() by Repository, Digest
+| where AccessCount >= cache_min_access_count
+| project Repository, Digest
+"""
+# need to figure out how run the query the programmatically though
+
+
 def main():
     args = parse_args()
 
@@ -184,12 +206,13 @@ def main():
             tmp_dir,
             args.az_path)
 
-        cache_lifetime = datetime.timedelta(days=args.cache_lifetime_days)
+        cache_history = datetime.timedelta(days=args.cache_history_days)
 
         min_timestamp = \
-            datetime.datetime.now(tz=datetime.timezone.utc) - cache_lifetime
+            datetime.datetime.now(tz=datetime.timezone.utc) - cache_history
 
-        valid_images = get_valid_images_from_logs(log_paths, min_timestamp)
+        valid_images = get_valid_images_from_logs(
+            log_paths, min_timestamp, args.cache_min_access_count)
 
     all_images = get_registry_images(args.container_registry, args.az_path)
 

tools/ci_build/github/azure-pipelines/clean-build-docker-image-cache-pipeline.yml

@@ -2,11 +2,15 @@ parameters:
 - name: DryRun
   type: boolean
   default: false
-  displayName: "Whether to do a dry-run and not actually clean up the cache."
-- name: CacheLifetimeDays
+  displayName: "Do a dry-run and do not remove any images"
+- name: CacheHistoryDays
   type: number
   default: 7
-  displayName: "The lifetime of an unused image in the cache, in days."
+  displayName: "The length of the cache history in days"
+- name: CacheMinAccessCount
+  type: number
+  default: 5
+  displayName: "The minimum access count over the cache history"
 
 variables:
   ${{ if eq(parameters.DryRun, true) }}:
@@ -32,4 +36,5 @@ jobs:
           --container-registry $(buildcache-container-registry) \
           --log-storage-account $(buildcache-log-storage-account) \
           --log-storage-account-container $(buildcache-log-storage-account-container) \
-          --cache-lifetime-days ${{ parameters.CacheLifetimeDays }}
+          --cache-history-days ${{ parameters.CacheHistoryDays }} \
+          --cache-min-access-count ${{ parameters.CacheMinAccessCount }}