snok / container-retention-policy

GitHub action for pruning old GHCR container image versions.
MIT License
178 stars 29 forks source link

Integrate fetching of multi-platform image digests into the action #90

Open sondrelg opened 1 month ago

sondrelg commented 1 month ago

Issue for tracking the progress of "real" multi-platform image support, as outlined in https://github.com/snok/container-retention-policy/issues/78#issuecomment-2212537081.

sondrelg commented 1 month ago

@rcdailey, I think I've found a pretty reliable and cheap way of fetching digests. Just wanted to check with you whether this output looks informative enough for your use-case?

2024-07-09T20:16:09.723085Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-5-2 package_name="container-retention-policy"
2024-07-09T20:16:09.724184Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-4-3 package_name="container-retention-policy"
2024-07-09T20:16:09.725224Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-3 package_name="container-retention-policy"
2024-07-09T20:16:09.728401Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-2-1 package_name="container-retention-policy"
2024-07-09T20:16:09.729285Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-3-1 package_name="container-retention-policy"
2024-07-09T20:16:09.730076Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-1-1 package_name="container-retention-policy"
2024-07-09T20:16:09.730627Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-2-3 package_name="container-retention-policy"
2024-07-09T20:16:09.734102Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-3-2 package_name="container-retention-policy"
2024-07-09T20:16:09.735524Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-2 package_name="container-retention-policy"
2024-07-09T20:16:09.736454Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-2-2 package_name="container-retention-policy"
2024-07-09T20:16:09.736768Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-5 package_name="container-retention-policy"
2024-07-09T20:16:09.737525Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-5-1 package_name="container-retention-policy"
2024-07-09T20:16:09.737642Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-1 package_name="container-retention-policy"
2024-07-09T20:16:09.739264Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-3-3 package_name="container-retention-policy"
2024-07-09T20:16:09.740638Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-4-2 package_name="container-retention-policy"
2024-07-09T20:16:09.742139Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-4-1 package_name="container-retention-policy"
2024-07-09T20:16:09.742256Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-1-2 package_name="container-retention-policy"
2024-07-09T20:16:09.743704Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-1-3 package_name="container-retention-policy"
2024-07-09T20:16:09.743972Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-4 package_name="container-retention-policy"
2024-07-09T20:16:09.747995Z DEBUG container_retention_policy::core::select_package_versions: Found 0 associated digests for container-retention-policy:test-5-3 package_name="container-retention-policy"
2024-07-09T20:16:09.751936Z  INFO container_retention_policy::core::select_package_versions: Found 4 associated digests for container-retention-policy:v3.0.0 package_name="container-retention-policy"
2024-07-09T20:16:09.751983Z DEBUG container_retention_policy::core::select_package_versions: Skipping deletion of sha256:fd406d3355ff2ebb6c178200c9a37bba7bd697aee249ec06e6eab42751bddfcb because it's associated with container-retention-policy:v3.0.0
2024-07-09T20:16:09.751997Z DEBUG container_retention_policy::core::select_package_versions: Skipping deletion of sha256:5ae87259b90d005268e1fe36b057f57552a5f31f8087d0f15ab5adf125702d8f because it's associated with container-retention-policy:v3.0.0
2024-07-09T20:16:09.752009Z DEBUG container_retention_policy::core::select_package_versions: Skipping deletion of sha256:a1239d006e3e1ffb4fbb422ee2fea17faf39943878a8714fc2ef0934a41579a7 because it's associated with container-retention-policy:v3.0.0
2024-07-09T20:16:09.752021Z DEBUG container_retention_policy::core::select_package_versions: Skipping deletion of sha256:aebfc2df6e545abf2375102e302d195cea007c8ef7ec312a62259538050df7b7 because it's associated with container-retention-policy:v3.0.0
2024-07-09T20:16:09.752158Z  INFO container_retention_policy::core::select_package_versions: Kept 1 of the 1 package versions requested by the `keep-n-most-recent` setting remaining_tagged_image_count=5
2024-07-09T20:16:09.752172Z  INFO container_retention_policy::core::select_package_versions: Selected 5 tagged and 10 untagged package versions for deletion package_name="container-retention-policy"
2024-07-09T20:16:09.752324Z DEBUG deleting package versions: container_retention_policy::core::delete_package_versions: Trimmed the selection to 10 untagged package versions to delete for package "container-retention-policy"
2024-07-09T20:16:09.752699Z DEBUG deleting package versions: container_retention_policy::core::delete_package_versions: Selected 5 tagged package versions to delete for package "container-retention-policy"
2024-07-09T20:16:09.764770Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:test-4-3 package_version=241406801
2024-07-09T20:16:09.764775Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:test-3-3 package_version=241406759
2024-07-09T20:16:09.764831Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:<untagged> package_version=241362605
2024-07-09T20:16:09.764809Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:test-4-2 package_version=241406801
2024-07-09T20:16:09.764823Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:test-2-3 package_version=241406711
2024-07-09T20:16:09.764837Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:test-1-3 package_version=241406656
2024-07-09T20:16:09.764838Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:test-3-2 package_version=241406759
2024-07-09T20:16:09.764851Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:<untagged> package_version=241387187
2024-07-09T20:16:09.764785Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:<untagged> package_version=241362761
2024-07-09T20:16:09.764912Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:<untagged> package_version=241387239
2024-07-09T20:16:09.764918Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:v3.0.0 package_version=234860550
2024-07-09T20:16:09.764924Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:<untagged> package_version=241387219
2024-07-09T20:16:09.764932Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:<untagged> package_version=241387198
2024-07-09T20:16:09.764934Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:<untagged> package_version=241387256
2024-07-09T20:16:09.765004Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:<untagged> package_version=241362709
2024-07-09T20:16:09.765021Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:test-4-1 package_version=241406801
2024-07-09T20:16:09.765041Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:test-2-2 package_version=241406711
2024-07-09T20:16:09.765377Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:test-1-2 package_version=241406656
2024-07-09T20:16:09.765399Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:test-3-1 package_version=241406759
2024-07-09T20:16:09.769532Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:test-3 package_version=241406759
2024-07-09T20:16:09.765815Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:<untagged> package_version=241362735
2024-07-09T20:16:09.768675Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:test-4 package_version=241406801
2024-07-09T20:16:09.769069Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:test-2-1 package_version=241406711
2024-07-09T20:16:09.769090Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:test-1-1 package_version=241406656
2024-07-09T20:16:09.765789Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:<untagged> package_version=241362655
2024-07-09T20:16:09.769941Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:test-2 package_version=241406711
2024-07-09T20:16:09.770362Z  INFO container_retention_policy::client::client: dry-run: Would have deleted container-retention-policy:test-1 package_version=241406656

Debug logs show which tags have 0 associated digests, and which SHAs are kept because of found associations, while we use info logs to show the aggregate counts for the tags with associated digests. I feel like this should give a good macro view of what's going on for info-logging, while debug offers enough details to understand the rest.

Any suggestions for improvements?

rcdailey commented 1 month ago

Thank you for getting back with me so quickly. I have a couple of follow up notes/questions, just to make sure I understand the logs correctly.

  1. Regarding the log Found 0 associated digests: Does this mean that the specified docker tag does not identify a multi-platform image? Maybe my understanding of how manifests work is wrong, but isn't there some image present (with some digest)? The fact that there are 0 digests is what confuses me I guess.

  2. The Skipping deletion of output I find especially interesting for my case, because it addresses one of the main concerns: Will this action delete multi-platform images associated with tags matching a specific pattern? I'm not sure if you've added the ability to specify a tag pattern to assist in excluding deletion of images that are associated with those tags, but the log message at least seems helpful.

  3. I still see this: dry-run: Would have deleted container-retention-policy:<untagged> which again leaves me wondering what image it is talking about. And the fact that "untagged" just doesn't help me at all. As I pointed out earlier, all multi-platform images are untagged technically, so the bigger question is, is this image mentioned in any manifest? And if so, what tag does that manifest belong? I think the new log messages I'm seeing are doing a better job of identifying that, so I'm not sure why these "untagged" log messages are still there.

I hope that helps.

sondrelg commented 1 month ago

Thank you for getting back with me so quickly. I have a couple of follow up notes/questions, just to make sure I understand the logs correctly.

Great!

  1. Regarding the log Found 0 associated digests: Does this mean that the specified docker tag does not identify a multi-platform image? Maybe my understanding of how manifests work is wrong, but isn't there some image present (with some digest)? The fact that there are 0 digests is what confuses me I guess.

It's the difference between this:

❯ docker manifest inspect ghcr.io/snok/container-retention-policy:test-4
{
        "schemaVersion": 2,
        "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
        "config": {
                "mediaType": "application/vnd.docker.container.image.v1+json",
                "size": 433,
                "digest": "sha256:3477b6ac614923929fa5212d718afc5f574f0f4eed5adc012633d07be5ccaf6c"
        },
        "layers": [
                {
                        "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
                        "size": 117,
                        "digest": "sha256:d95bac653fe4f1d5656daf19aa3b9d6dd3959bd8047ba6b9f339813eec9aead7"
                }
        ]
}

and this:

❯ docker manifest inspect ghcr.io/snok/container-retention-policy:v3.0.0
{
   "schemaVersion": 2,
   "mediaType": "application/vnd.oci.image.index.v1+json",
   "manifests": [
      {
         "mediaType": "application/vnd.oci.image.manifest.v1+json",
         "size": 669,
         "digest": "sha256:aebfc2df6e545abf2375102e302d195cea007c8ef7ec312a62259538050df7b7",
         "platform": {
            "architecture": "arm64",
            "os": "linux"
         }
      },
      {
         "mediaType": "application/vnd.oci.image.manifest.v1+json",
         "size": 567,
         "digest": "sha256:a1239d006e3e1ffb4fbb422ee2fea17faf39943878a8714fc2ef0934a41579a7",
         "platform": {
            "architecture": "unknown",
            "os": "unknown"
         }
      },
      {
         "mediaType": "application/vnd.oci.image.manifest.v1+json",
         "size": 669,
         "digest": "sha256:5ae87259b90d005268e1fe36b057f57552a5f31f8087d0f15ab5adf125702d8f",
         "platform": {
            "architecture": "amd64",
            "os": "linux"
         }
      },
      {
         "mediaType": "application/vnd.oci.image.manifest.v1+json",
         "size": 567,
         "digest": "sha256:fd406d3355ff2ebb6c178200c9a37bba7bd697aee249ec06e6eab42751bddfcb",
         "platform": {
            "architecture": "unknown",
            "os": "unknown"
         }
      }
   ]
}

Multi-platform images seem to be of the media type application/vnd.oci.image.manifest.v1+json which have their associated manifests declared under the manifests key, while a "regular" application/vnd.docker.distribution.manifest.v2+json image does not. Maybe we could reword the logger to make this clearer?

  1. The Skipping deletion of output I find especially interesting for my case, because it addresses one of the main concerns: Will this action delete multi-platform images associated with tags matching a specific pattern? I'm not sure if you've added the ability to specify a tag pattern to assist in excluding deletion of images that are associated with those tags, but the log message at least seems helpful.

Since we do fetch every single package version available for selected packages, we could exhaustively fetch the manifest for every single package version with a tag, and then use the information there to filter out other package versions that are found to be associated with those manifests.

Then I guess it's important that we:

Does that seem right to you? I think we could then get away with just using filter-tags for the filtering logic.

  1. I still see this: dry-run: Would have deleted container-retention-policy:<untagged> which again leaves me wondering what image it is talking about. And the fact that "untagged" just doesn't help me at all. As I pointed out earlier, all multi-platform images are untagged technically, so the bigger question is, is this image mentioned in any manifest? And if so, what tag does that manifest belong? I think the new log messages I'm seeing are doing a better job of identifying that, so I'm not sure why these "untagged" log messages are still there.

If we do the cataloguing of all tags and associated images, then I think it would be possible to add more information about which image belongs to which multi-platform image to this output 👍

Maybe we could do something like this?

... dry-run: Would have deleted container-retention-policy:<untagged> package_version=241387187 media_type=oci-image-index associated_tag=foo platform=arm64-linux
... dry-run: Would have deleted container-retention-policy:<untagged> package_version=241362761 media_type=distribution-manifest
... dry-run: Would have deleted container-retention-policy:<untagged> package_version=241387239 media_type=oci-image-index associated_tag=foo platform=amd64-linux
rcdailey commented 1 month ago

Multi-platform images seem to be of the media type application/vnd.oci.image.manifest.v1+json which have their associated manifests declared under the manifests key, while a "regular" application/vnd.docker.distribution.manifest.v2+json image does not.

Your explanation helps a lot; thank you. This confirms that I had the right understanding. I've sadly been unware of how multi-platform images (and even "regular" images) are represented internally; so these conversations have helped a lot!

Then I guess it's important that we:

  • Don't delete any image that belongs to a multi-platform image for a tag that isn't selected in the filter-tags input
  • Only delete entire multi-platform image collections for tags that are selected from the filter-tags input, when selected

Does that seem right to you? I think we could then get away with just using filter-tags for the filtering logic.

I think this 100% represents what I am expecting. I think this also 100% addresses my concerns with unwanted behavior by using the retention action.

If we do the cataloguing of all tags and associated images, then I think it would be possible to add more information about which image belongs to which multi-platform image to this output 👍

Maybe we could do something like this?

... dry-run: Would have deleted container-retention-policy:<untagged> package_version=241387187 media_type=oci-image-index associated_tag=foo platform=arm64-linux
... dry-run: Would have deleted container-retention-policy:<untagged> package_version=241362761 media_type=distribution-manifest
... dry-run: Would have deleted container-retention-policy:<untagged> package_version=241387239 media_type=oci-image-index associated_tag=foo platform=amd64-linux

I think this looks much much better. I think even in the distribution-mamnifest case, we could have associated_tag=foo -- Because that image is part of a manifest associated with a tag. Assuming I am correctly understanding each of the media_type:

I think all roads should lead to a tag, except if that image is truly orphaned (not mentioned by any manifest you're able to obtain for any known tag). I think we probably need a media_type scenario for that; maybe media_type=orphaned? Because it technically wouldn't even have a media type if it doesn't get mentioned in any manifest JSON, right?