Skip to content

Commit

Permalink
added check against tagResolver
Browse files Browse the repository at this point in the history 
Signed-off-by: Xiaoxuan Wang <[email protected]>
  • Loading branch information
@wangxiaoxuan273
wangxiaoxuan273 committed Dec 13, 2023
1 parent eecb490 commit 3a117b6
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 193 deletions.
8 changes: 4 additions & 4 deletions content/oci/oci.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func (s *Store) Delete(ctx context.Context, target ocispec.Descriptor) error {

// delete the dangling nodes caused by the delete
if s.AutoGC {
return s.autoGCInDelete(ctx, danglings)
return s.cleanGraphs(ctx, danglings)
}
return nil
}
Expand Down Expand Up @@ -193,14 +193,14 @@ func (s *Store) delete(ctx context.Context, target ocispec.Descriptor) ([]ocispe
return danglings, nil
}

func (s *Store) autoGCInDelete(ctx context.Context, danglings []ocispec.Descriptor) error {
func (s *Store) cleanGraphs(ctx context.Context, danglings []ocispec.Descriptor) error {
// for each item in dangling, if it exists and it is dangling, remove it and
// add new dangling nodes into dangling

for len(danglings) > 0 {
node := danglings[0]
danglings = danglings[1:]
if s.graph.IsDanglingNode(node) {
// don't delete the node if it exists in index.json
if _, err := s.tagResolver.Resolve(ctx, string(node.Digest)); err != nil {
descs, err := s.delete(ctx, node)
if err != nil {
return err
Expand Down
83 changes: 49 additions & 34 deletions content/oci/oci_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2283,9 +2283,9 @@ func TestStore_DeleteWithGarbageCollection(t *testing.T) {
appendBlob(ocispec.MediaTypeImageLayer, []byte("foo")) // Blob 1
appendBlob(ocispec.MediaTypeImageLayer, []byte("bar")) // Blob 2
appendBlob(ocispec.MediaTypeImageLayer, []byte("hello")) // Blob 3
generateManifest(descs[0], descs[1:3]...) // Blob 4
generateManifest(descs[0], descs[3]) // Blob 5
generateManifest(descs[0], descs[1:4]...) // Blob 6
generateManifest(descs[0], descs[1]) // Blob 4
generateManifest(descs[0], descs[2]) // Blob 5
generateManifest(descs[0], descs[3]) // Blob 6
generateIndex(descs[4:6]...) // Blob 7
generateIndex(descs[6]) // Blob 8

Expand All @@ -2305,67 +2305,82 @@ func TestStore_DeleteWithGarbageCollection(t *testing.T) {
t.Fatal(err)
}

// delete blob 7 and verify the result
if err := s.Delete(egCtx, descs[7]); err != nil {
// delete blob 4 and verify the result
if err := s.Delete(egCtx, descs[4]); err != nil {
t.Fatal(err)
}

// node 7, 4 and 5 are now deleted, and other nodes are still present
notPresent := []ocispec.Descriptor{descs[4], descs[5], descs[7]}
// blob 1 and 4 are now deleted, and other blobs are still present
notPresent := []ocispec.Descriptor{descs[1], descs[4]}
for _, node := range notPresent {
if exists, _ := s.Exists(egCtx, node); exists {
t.Errorf("%v should not exist in store", node)
}
}
stillPresent := []ocispec.Descriptor{descs[0], descs[1], descs[2], descs[3], descs[6], descs[8]}
stillPresent := []ocispec.Descriptor{descs[0], descs[2], descs[3], descs[5], descs[6], descs[7], descs[8]}
for _, node := range stillPresent {
if exists, _ := s.Exists(egCtx, node); !exists {
t.Errorf("%v should exist in store", node)
}
}

// verify predecessors information
wants := [][]ocispec.Descriptor{
{descs[6]}, // Blob 0
{descs[6]}, // Blob 1
{descs[6]}, // Blob 2
{descs[6]}, // Blob 3
nil, // Blob 4
nil, // Blob 5
{descs[8]}, // Blob 6
nil, // Blob 7
nil, // Blob 8
// delete blob 8 and verify the result
if err := s.Delete(egCtx, descs[8]); err != nil {
t.Fatal(err)
}
for i, want := range wants {
predecessors, err := s.Predecessors(ctx, descs[i])
if err != nil {
t.Errorf("Store.Predecessors(%d) error = %v", i, err)

// blob 1, 4 and 8 are now deleted, and other blobs are still present
notPresent = []ocispec.Descriptor{descs[1], descs[4], descs[8]}
for _, node := range notPresent {
if exists, _ := s.Exists(egCtx, node); exists {
t.Errorf("%v should not exist in store", node)
}
if !equalDescriptorSet(predecessors, want) {
t.Errorf("Store.Predecessors(%d) = %v, want %v", i, predecessors, want)
}
stillPresent = []ocispec.Descriptor{descs[0], descs[2], descs[3], descs[5], descs[6], descs[7]}
for _, node := range stillPresent {
if exists, _ := s.Exists(egCtx, node); !exists {
t.Errorf("%v should exist in store", node)
}
}

// delete blob 8 and verify the result
if err := s.Delete(egCtx, descs[8]); err != nil {
// delete blob 6 and verify the result
if err := s.Delete(egCtx, descs[6]); err != nil {
t.Fatal(err)
}

// all nodes should have been deleted
for i := 0; i <= 8; i++ {
if exists, _ := s.Exists(egCtx, descs[i]); exists {
t.Errorf("%v should not exist in store", descs[i])
// blob 1, 3, 4, 6 and 8 are now deleted, and other blobs are still present
notPresent = []ocispec.Descriptor{descs[1], descs[3], descs[4], descs[6], descs[8]}
for _, node := range notPresent {
if exists, _ := s.Exists(egCtx, node); exists {
t.Errorf("%v should not exist in store", node)
}
}
stillPresent = []ocispec.Descriptor{descs[0], descs[2], descs[5], descs[7]}
for _, node := range stillPresent {
if exists, _ := s.Exists(egCtx, node); !exists {
t.Errorf("%v should exist in store", node)
}
}

// verify predecessors information
for i := 0; i <= 8; i++ {
wants := [][]ocispec.Descriptor{
{descs[5]}, // Blob 0
nil, // Blob 1
{descs[5]}, // Blob 2
nil, // Blob 3
{descs[7]}, // Blob 4's predecessor is descs[7], even though blob 4 no longer exist
{descs[7]}, // Blob 5
nil, // Blob 6
nil, // Blob 7
nil, // Blob 8
}
for i, want := range wants {
predecessors, err := s.Predecessors(ctx, descs[i])
if err != nil {
t.Errorf("Store.Predecessors(%d) error = %v", i, err)
}
if predecessors != nil {
t.Errorf("Store.Predecessors(%d) = %v, want %v", i, predecessors, nil)
if !equalDescriptorSet(predecessors, want) {
t.Errorf("Store.Predecessors(%d) = %v, want %v", i, predecessors, want)
}
}
}
Expand Down
9 changes: 0 additions & 9 deletions internal/graph/memory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,3 @@ func (m *Memory) index(ctx context.Context, fetcher content.Fetcher, node ocispe
}
return successors, nil
}

// IsDanglingNode decides whether a node is dangling. A node is considered
// dangling if it is present and has no predecessors.
func (m *Memory) IsDanglingNode(desc ocispec.Descriptor) bool {
key := descriptor.FromOCI(desc)
_, existsInMemory := m.nodes[key]
_, existsInPredecessors := m.predecessors[key]
return existsInMemory && !existsInPredecessors
}
146 changes: 0 additions & 146 deletions internal/graph/memory_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"encoding/json"
"io"
"reflect"
"sort"
"testing"

"github.com/opencontainers/go-digest"
Expand Down Expand Up @@ -611,148 +610,3 @@ func TestMemory_IndexAllAndPredecessors(t *testing.T) {
t.Errorf("incorrect predecessor result")
}
}

// This test is for intermediate testing and may be removed when the pull request
// is finalized.
// +-----------------------------------------------+
// | |
// | +-----------+ |
// | |A(manifest)| |
// | +-----+-----+ |
// | | |
// | +------------+------------+ |
// | | | | |
// | | | | |
// | v v v |
// | +---------+ +--------+ +--------+ |
// | |B(config)| |C(layer)| |D(layer)| |
// | +---------+ +--------+ +--------+ |
// | |
// | |
// +-----------------------------------------------+
func TestMemory_trackDanglingNodes(t *testing.T) {
testFetcher := cas.NewMemory()
testMemory := NewMemory()
ctx := context.Background()

// generate test content
var blobs [][]byte
var descs []ocispec.Descriptor
appendBlob := func(mediaType string, blob []byte) ocispec.Descriptor {
blobs = append(blobs, blob)
descs = append(descs, ocispec.Descriptor{
MediaType: mediaType,
Digest: digest.FromBytes(blob),
Size: int64(len(blob)),
})
return descs[len(descs)-1]
}
descB := appendBlob("layer node B", []byte("Node B is a config")) // blobs[0], layer "B"
descC := appendBlob("layer node C", []byte("Node C is a layer")) // blobs[1], layer "C"
descD := appendBlob("layer node D", []byte("Node D is a layer")) // blobs[2], layer "D"

generateManifest := func(layers ...ocispec.Descriptor) ocispec.Descriptor {
manifest := ocispec.Manifest{
Config: descB,
Layers: layers,
}
manifestJSON, err := json.Marshal(manifest)
if err != nil {
t.Fatal(err)
}
return appendBlob(ocispec.MediaTypeImageManifest, manifestJSON)
}
descA := generateManifest(descs[1:3]...) // blobs[3], manifest "A"

// prepare the content in the fetcher, so that it can be used to test Index
testContents := []ocispec.Descriptor{descB, descC, descD, descA}
for i := 0; i < len(blobs); i++ {
testFetcher.Push(ctx, testContents[i], bytes.NewReader(blobs[i]))
}

// make sure that testFetcher works
rc, err := testFetcher.Fetch(ctx, descA)
if err != nil {
t.Errorf("testFetcher.Fetch() error = %v", err)
}
got, err := io.ReadAll(rc)
if err != nil {
t.Errorf("testFetcher.Fetch().Read() error = %v", err)
}
err = rc.Close()
if err != nil {
t.Errorf("testFetcher.Fetch().Close() error = %v", err)
}
if !bytes.Equal(got, blobs[3]) {
t.Errorf("testFetcher.Fetch() = %v, want %v", got, blobs[4])
}

nodeKeyA := descriptor.FromOCI(descA)
nodeKeyB := descriptor.FromOCI(descB)
nodeKeyC := descriptor.FromOCI(descC)
nodeKeyD := descriptor.FromOCI(descD)
nodeKeys := []descriptor.Descriptor{nodeKeyA, nodeKeyB, nodeKeyC, nodeKeyD}

// index the nodes and verify the information
testMemory.IndexAll(ctx, testFetcher, descA)

// check that all nodes exist in the memory
for i := 0; i < len(nodeKeys); i++ {
if _, exists := testMemory.nodes[nodeKeys[i]]; !exists {
t.Errorf("nodes entry of %v should exist", nodeKeys[i])
}
}

// check that predecessor of B,C,D is A
for i := 1; i < len(nodeKeys); i++ {
if len(testMemory.predecessors[nodeKeys[i]]) != 1 || !testMemory.predecessors[nodeKeys[i]].Contains(nodeKeyA) {
t.Errorf("the predecessor of %v should be A", nodeKeys[i])
}
}

// remove node A and verify the information
danglings := testMemory.Remove(descA)

// check that A is no longer in the memory, while B,C,D still exist in memory
if _, exists := testMemory.nodes[nodeKeyA]; exists {
t.Errorf("nodes entry of %s should not exist", "A")
}
for i := 1; i < len(nodeKeys); i++ {
if _, exists := testMemory.nodes[nodeKeys[i]]; !exists {
t.Errorf("nodes entry of %v should exist", nodeKeys[i])
}
}

// check that B,C,D have no predecessors and are dangling nodes
if len(testMemory.predecessors) != 0 {
t.Errorf("testMemory.predecessors should be empty")
}

// check that B,C,D are all garbage
for i := 1; i < len(nodeKeys); i++ {
if isGarbage := testMemory.IsDanglingNode(testMemory.nodes[nodeKeys[i]]); !isGarbage {
t.Errorf("%v should be considered a dangling node", nodeKeys[i])
}
}

// check that danglings has the correct content
expectedDanglings := []ocispec.Descriptor{}
for i := 1; i < len(nodeKeys); i++ {
expectedDanglings = append(expectedDanglings, testMemory.nodes[nodeKeys[i]])
}

sort.SliceStable(expectedDanglings, func(i, j int) bool { return expectedDanglings[i].Digest <= expectedDanglings[j].Digest })
sort.SliceStable(danglings, func(i, j int) bool { return danglings[i].Digest <= danglings[j].Digest })

if len(danglings) != len(expectedDanglings) {
t.Errorf("danglings has length = %d, want length %d", len(danglings), len(expectedDanglings))
}
for i := 0; i < len(expectedDanglings); i++ {
if !reflect.DeepEqual(danglings, expectedDanglings) {
t.Errorf("danglings[i] = %v, want %v", danglings[i], expectedDanglings[i])
}
if isGarbage := testMemory.IsDanglingNode(danglings[i]); !isGarbage {
t.Errorf("%v should be considered a dangling node", danglings[i])
}
}
}

0 comments on commit 3a117b6

Please sign in to comment.