oras-project / oras-go

ORAS Go library
https://oras.land
Apache License 2.0
172 stars 94 forks source link

feat: mark untagged manifests as garbage during GC and delete #680

Closed wangxiaoxuan273 closed 7 months ago

wangxiaoxuan273 commented 7 months ago

Related to #664

codecov[bot] commented 7 months ago

Codecov Report

Attention: 24 lines in your changes are missing coverage. Please review.

Comparison is base (82cc505) 75.78% compared to head (423f3ed) 75.85%.

Files Patch % Lines
content/oci/oci.go 66.03% 12 Missing and 6 partials :warning:
internal/manifestutil/parser.go 81.25% 2 Missing and 1 partial :warning:
internal/resolver/memory.go 93.87% 2 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #680 +/- ## ========================================== + Coverage 75.78% 75.85% +0.07% ========================================== Files 59 59 Lines 5769 5873 +104 ========================================== + Hits 4372 4455 +83 - Misses 1026 1040 +14 - Partials 371 378 +7 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

wangxiaoxuan273 commented 7 months ago

Hi!

Thank you for this PR! As advised in #664, I tested this PR in a branch:

francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ sudo -E ./ig image build -t opens -f gadgets/trace_open/gadget.yaml gadgets/trace_open
INFO[0000] Experimental features enabled                
Successfully built ghcr.io/inspektor-gadget/gadget/opens:latest@sha256:a5de3655d6c7640eb6d43f7d9d7182b233ac86aedddfe6c132cba6b876264d97
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ sudo cat /var/lib/ig/oci-store/index.json | jq             francis/rmi-upstream % u=
{
  "schemaVersion": 2,
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.index.v1+json",
      "digest": "sha256:a5de3655d6c7640eb6d43f7d9d7182b233ac86aedddfe6c132cba6b876264d97",
      "size": 491,
      "annotations": {
        "org.opencontainers.image.ref.name": "ghcr.io/inspektor-gadget/gadget/opens:latest"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:6fc908439cd210687651e36693d4fa96e59259403826c7cf929ac5382c8d6a33",
      "size": 582,
      "platform": {
        "architecture": "amd64",
        "os": "linux"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:b4c03573365a929cbde0bf6e92ed91e067647501851a6d84a6f2a1a072bf22ce",
      "size": 582,
      "platform": {
        "architecture": "arm64",
        "os": "linux"
      }
    }
  ]
}
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ sudo -E ./ig image remove opens                            francis/rmi-upstream % u=
INFO[0000] Experimental features enabled                
Successfully removed opens
francis@pwmachine:~/Codes/kinvolk/inspektor-gadget$ sudo cat /var/lib/ig/oci-store/index.json | jq             francis/rmi-upstream % u=
{
  "schemaVersion": 2,
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.index.v1+json",
      "digest": "sha256:a5de3655d6c7640eb6d43f7d9d7182b233ac86aedddfe6c132cba6b876264d97",
      "size": 491
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:6fc908439cd210687651e36693d4fa96e59259403826c7cf929ac5382c8d6a33",
      "size": 582,
      "platform": {
        "architecture": "amd64",
        "os": "linux"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:b4c03573365a929cbde0bf6e92ed91e067647501851a6d84a6f2a1a072bf22ce",
      "size": 582,
      "platform": {
        "architecture": "arm64",
        "os": "linux"
      }
    }
  ]
}

As a result, the tag was just removed from the index but nothing no blobs were actually removed. Am I missing something?

With regard to the code itself, I found two nits.

Best regards.

Hi @eiffel-fl,

If I understand your scenario correctly, when you delete an image index, you also want the manifests in the index to be automatically removed, given that the manifests are untagged and do not have other tagged predecessors.

I have added a simple unit test to cover the scenario of deleting an image index. It looks like it's working as expected. In this unit test, untagged manifests are all removed and the tagged one remains.

I see that in your branch, you replaced deleteTree (which deletes all successors of root if they have no other predecessors?) with Delete and GC. Logically it should work. Do the manifests have no other tagged predecessors, other than the image index?