From 08fcd2e18b81fb1c9f55d3c5c401f5b3285d8d69 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Wed, 17 Jan 2024 05:41:36 +0000 Subject: [PATCH] unit test Signed-off-by: Xiaoxuan Wang --- content/oci/oci.go | 54 +++++++++++++++++++--------- content/oci/oci_test.go | 3 ++ internal/resolver/memory.go | 70 ++++++++++++++++++++++++++++++------- 3 files changed, 97 insertions(+), 30 deletions(-) diff --git a/content/oci/oci.go b/content/oci/oci.go index ccefc0d9..286052cc 100644 --- a/content/oci/oci.go +++ b/content/oci/oci.go @@ -190,9 +190,8 @@ func (s *Store) Delete(ctx context.Context, target ocispec.Descriptor) error { } if s.AutoGC { for _, d := range danglings { - // do not delete existing manifests in tagResolver - _, err = s.tagResolver.Resolve(ctx, string(d.Digest)) - if errors.Is(err, errdef.ErrNotFound) { + // do not delete existing tagged manifests + if !s.isTagged(d) { deleteQueue = append(deleteQueue, d) } } @@ -455,19 +454,6 @@ func (s *Store) writeIndexFile() error { return os.WriteFile(s.indexPath, indexJSON, 0666) } -// reloadIndex reloads the index and updates metadata by creating a new store. -func (s *Store) reloadIndex(ctx context.Context) error { - newStore, err := NewWithContext(ctx, s.root) - if err != nil { - return err - } - s.index = newStore.index - s.storage = newStore.storage - s.tagResolver = newStore.tagResolver - s.graph = newStore.graph - return nil -} - // GC removes garbage from Store. Unsaved index will be lost. To prevent unexpected // loss, call SaveIndex() before GC or set AutoSaveIndex to true. // The garbage to be cleaned are: @@ -478,7 +464,7 @@ func (s *Store) GC(ctx context.Context) error { defer s.sync.Unlock() // get reachable nodes by reloading the index - err := s.reloadIndex(ctx) + err := s.reloadIndexforGC(ctx) if err != nil { return fmt.Errorf("unable to reload index: %w", err) } @@ -526,6 +512,40 @@ func (s *Store) GC(ctx context.Context) error { return nil } +// reloadIndexforGC reloads the index and updates metadata by creating a new +// store. +func (s *Store) reloadIndexforGC(ctx context.Context) error { + refMap := s.tagResolver.Map() + + s.tagResolver = resolver.NewMemory() + s.graph = graph.NewMemory() + for ref, desc := range refMap { + if ref == desc.Digest.String() { + continue + } + if err := s.tagResolver.Tag(ctx, deleteAnnotationRefName(desc), desc.Digest.String()); err != nil { + return err + } + if err := s.tagResolver.Tag(ctx, desc, ref); err != nil { + return err + } + plain := descriptor.Plain(desc) + if err := s.graph.IndexAll(ctx, s.storage, plain); err != nil { + return err + } + } + + return nil +} + +func (s *Store) isTagged(desc ocispec.Descriptor) bool { + tagSet := s.tagResolver.ReverseSearch(desc) + if tagSet.Contains(string(desc.Digest)) { + return len(tagSet) > 1 + } + return len(tagSet) > 0 +} + // unsafeStore is used to bypass lock restrictions in Delete. type unsafeStore struct { *Store diff --git a/content/oci/oci_test.go b/content/oci/oci_test.go index 42702a6d..8d06297f 100644 --- a/content/oci/oci_test.go +++ b/content/oci/oci_test.go @@ -2948,6 +2948,9 @@ func TestStore_GC(t *testing.T) { } } + // tag manifest blob 3 + s.Tag(ctx, descs[3], "blob 3 is tagged") + // perform GC if err = s.GC(ctx); err != nil { t.Fatal(err) diff --git a/internal/resolver/memory.go b/internal/resolver/memory.go index 2111d504..2717286b 100644 --- a/internal/resolver/memory.go +++ b/internal/resolver/memory.go @@ -19,48 +19,92 @@ import ( "context" "sync" + "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras-go/v2/errdef" + "oras.land/oras-go/v2/internal/container/set" ) // Memory is a memory based resolver. type Memory struct { - index sync.Map // map[string]ocispec.Descriptor + lock sync.RWMutex + index map[string]ocispec.Descriptor + reverse map[digest.Digest]set.Set[string] } // NewMemory creates a new Memory resolver. func NewMemory() *Memory { - return &Memory{} + return &Memory{ + index: make(map[string]ocispec.Descriptor), + reverse: make(map[digest.Digest]set.Set[string]), + } } // Resolve resolves a reference to a descriptor. func (m *Memory) Resolve(_ context.Context, reference string) (ocispec.Descriptor, error) { - desc, ok := m.index.Load(reference) + m.lock.RLock() + defer m.lock.RUnlock() + + desc, ok := m.index[reference] if !ok { return ocispec.Descriptor{}, errdef.ErrNotFound } - return desc.(ocispec.Descriptor), nil + return desc, nil } // Tag tags a descriptor with a reference string. func (m *Memory) Tag(_ context.Context, desc ocispec.Descriptor, reference string) error { - m.index.Store(reference, desc) + m.lock.Lock() + defer m.lock.Unlock() + + m.index[reference] = desc + tagSet, ok := m.reverse[desc.Digest] + if !ok { + tagSet = set.New[string]() + m.reverse[desc.Digest] = tagSet + } + tagSet.Add(reference) return nil } // Untag removes a reference from index map. func (m *Memory) Untag(reference string) { - m.index.Delete(reference) + m.lock.Lock() + defer m.lock.Unlock() + + desc, ok := m.index[reference] + if !ok { + return + } + delete(m.index, reference) + tagSet := m.reverse[desc.Digest] + tagSet.Delete(reference) + if len(tagSet) == 0 { + delete(m.reverse, desc.Digest) + } } // Map dumps the memory into a built-in map structure. -// Like other operations, calling Map() is go-routine safe. However, it does not -// necessarily correspond to any consistent snapshot of the storage contents. +// Like other operations, calling Map() is go-routine safe. func (m *Memory) Map() map[string]ocispec.Descriptor { - res := make(map[string]ocispec.Descriptor) - m.index.Range(func(key, value interface{}) bool { - res[key.(string)] = value.(ocispec.Descriptor) - return true - }) + m.lock.RLock() + defer m.lock.RUnlock() + + res := make(map[string]ocispec.Descriptor, len(m.index)) + for key, value := range m.index { + res[key] = value + } + return res +} + +func (m *Memory) ReverseSearch(desc ocispec.Descriptor) set.Set[string] { + m.lock.RLock() + defer m.lock.RUnlock() + + tagSet := m.reverse[desc.Digest] + res := make(set.Set[string], len(tagSet)) + for tag := range tagSet { + res.Add(tag) + } return res }