Skip to content

Commit

Permalink
Merge pull request #139 from Ajpantuso/apantuso/extractor_store
Browse files Browse the repository at this point in the history
refactor: extractor cache cleanup
  • Loading branch information
Ajpantuso authored Sep 23, 2022
2 parents 361ebd9 + 124fc51 commit fd790b4
Show file tree
Hide file tree
Showing 15 changed files with 414 additions and 248 deletions.
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ require (
github.com/blang/semver/v4 v4.0.0
github.com/fatih/color v1.13.0
github.com/go-logr/logr v1.2.3
github.com/golang/snappy v0.0.4
github.com/magefile/mage v1.13.0
github.com/mt-sre/client v0.1.3
github.com/mt-sre/go-ci v0.3.3
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -538,8 +538,6 @@ github.com/golang/snappy v0.0.0-20170215233205-553a64147049/go.mod h1:/XxbfmMg8l
github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
github.com/golang/snappy v0.0.1/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
github.com/golang/snappy v0.0.3/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
github.com/golang/snappy v0.0.4 h1:yAGX7huGHXlcLOEtBnF4w7FQwA26wojNCwOYAEhLjQM=
github.com/golang/snappy v0.0.4/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
github.com/golangplus/testing v0.0.0-20180327235837-af21d9c3145e/go.mod h1:0AA//k/eakGydO4jKRoRL2j92ZKSzTgj9tclaCrvXHk=
github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
Expand Down
23 changes: 18 additions & 5 deletions pkg/extractor/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,18 @@ import (
k8syaml "k8s.io/apimachinery/pkg/util/yaml"
)

// BundleCache provides a cache of OPM bundles which are referenced by
// bundleImage name.
type BundleCache interface {
// GetBundle returns a bundle for the given image. An error is
// returned if the bundle cannot be retrieved or the data is
// corrupted.
GetBundle(img string) (*registry.Bundle, error)
// SetBundle caches a bundle for the given image. An error
// is returned if the bundle cannot be cached.
SetBundle(img string, bundle registry.Bundle) error
}

type DefaultBundleExtractor struct {
Log logrus.FieldLogger
Cache BundleCache
Expand All @@ -41,7 +53,7 @@ func NewBundleExtractor(opts ...BundleExtractorOpt) *DefaultBundleExtractor {
}

if extractor.Cache == nil {
extractor.Cache = NewBundleMemoryCache(NewJSONSnappyEncoder())
extractor.Cache = NewBundleCacheImpl()
}

extractor.Log = extractor.Log.WithField("source", "bundleExtractor")
Expand Down Expand Up @@ -69,10 +81,11 @@ func WithBundleTimeout(timeout time.Duration) BundleExtractorOpt {
}

func (e *DefaultBundleExtractor) Extract(ctx context.Context, bundleImage string) (*registry.Bundle, error) {
bundle, err := e.Cache.Get(bundleImage)
bundle, err := e.Cache.GetBundle(bundleImage)
if err != nil {
return nil, fmt.Errorf("cache error: %w", err)
e.Log.Warnf("retrieving bundle %q from cache: %w", bundleImage, err)
}

if bundle != nil {
e.Log.Debugf("cache hit for '%s'", bundleImage)
return bundle, nil
Expand All @@ -99,8 +112,8 @@ func (e *DefaultBundleExtractor) Extract(ctx context.Context, bundleImage string
}
bundle.BundleImage = bundleImage // not set by OPM

if err := e.Cache.Set(bundleImage, bundle); err != nil {
return nil, fmt.Errorf("unable to cache bundle: %w", err)
if err := e.Cache.SetBundle(bundleImage, *bundle); err != nil {
e.Log.Warnf("caching bundle %q: %w", bundleImage, err)
}

return bundle, nil
Expand Down
88 changes: 42 additions & 46 deletions pkg/extractor/bundle_cache.go
Original file line number Diff line number Diff line change
@@ -1,74 +1,70 @@
package extractor

import (
"sync"
"errors"
"fmt"

"github.com/operator-framework/operator-registry/pkg/registry"
)

type bundleMemoryCache struct {
encoder BundleEncoder
// NewBundleCacheImpl returns an initialized BundleCacheImpl. A
// variadic slice of options can be used to alter the behavior
// of the cache.
func NewBundleCacheImpl(opts ...BundleCacheImplOption) *BundleCacheImpl {
var cfg BundleCacheImplConfig

lock sync.RWMutex
store map[string][]byte
}
cfg.Option(opts...)
cfg.Default()

// NewBundleMemoryCache - in-memory cache for bundle, using an encoder
func NewBundleMemoryCache(encoder BundleEncoder) BundleCache {
return &bundleMemoryCache{
encoder: encoder,
store: make(map[string][]byte),
return &BundleCacheImpl{
cfg: cfg,
}
}

func (c *bundleMemoryCache) Get(bundleImage string) (*registry.Bundle, error) {
c.lock.RLock()
defer c.lock.RUnlock()
type BundleCacheImpl struct {
cfg BundleCacheImplConfig
}

var ErrInvalidBundleData = errors.New("invalid bundle data")

if data, ok := c.store[bundleImage]; ok {
bundle, err := c.encoder.Decode(data)
if err != nil {
return nil, err
}
func (c *BundleCacheImpl) GetBundle(img string) (*registry.Bundle, error) {
data, ok := c.cfg.Store.Read(img)
if !ok {
return nil, nil
}

return hackSetCacheStaleToTrue(bundle), nil
bundle, ok := data.(registry.Bundle)
if !ok {
return nil, ErrInvalidBundleData
}

return nil, nil
return &bundle, nil
}

// hack to set b.cacheStale to true otherwise we can't access the csv of the
// underlying bundle. This is a bug on OPM's side, which does not support
// serialization/deserialization of their bundles.
// https://github.com/operator-framework/operator-registry/blob/master/pkg/registry/bundle.go#L103
func hackSetCacheStaleToTrue(b *registry.Bundle) *registry.Bundle {
if len(b.Objects) == 0 {
return b
func (c *BundleCacheImpl) SetBundle(img string, bundle registry.Bundle) error {
if err := c.cfg.Store.Write(img, bundle); err != nil {
return fmt.Errorf("writing bundle data: %w", err)
}

obj := b.Objects[0]
b.Objects = b.Objects[1:]
b.Add(obj)

return b
return nil
}

func (c *bundleMemoryCache) Set(bundleImage string, bundle *registry.Bundle) error {
c.lock.Lock()
defer c.lock.Unlock()
type BundleCacheImplConfig struct {
Store Store
}

// bundles are supposed to be immutable, so we can save a few CPU cycles by
// avoiding re-encoding bundles and unnecessarily overwriting the cache
if _, ok := c.store[bundleImage]; ok {
return nil
func (c *BundleCacheImplConfig) Option(opts ...BundleCacheImplOption) {
for _, opt := range opts {
opt.ConfigureBundleCacheImpl(c)
}
}

data, err := c.encoder.Encode(bundle)
if err != nil {
return err
func (c *BundleCacheImplConfig) Default() {
if c.Store == nil {
c.Store = NewThreadSafeStore()
}
}

c.store[bundleImage] = data

return nil
type BundleCacheImplOption interface {
ConfigureBundleCacheImpl(*BundleCacheImplConfig)
}
13 changes: 13 additions & 0 deletions pkg/extractor/bundle_cache_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package extractor

import (
"testing"

"github.com/stretchr/testify/require"
)

func TestBundleCacheImplInterfaces(t *testing.T) {
t.Parallel()

require.Implements(t, new(BundleCache), new(BundleCacheImpl))
}
35 changes: 0 additions & 35 deletions pkg/extractor/bundle_encoder.go

This file was deleted.

93 changes: 48 additions & 45 deletions pkg/extractor/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,80 +6,83 @@ import (

"github.com/operator-framework/operator-registry/pkg/registry"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

type testCase struct {
bundleImage string
expectedPackageName string
expectedCSVName string
expectedCSVVersion string
}

func TestDefaultBundleExtractorImplements(t *testing.T) {
t.Parallel()

require.Implements(t, new(BundleExtractor), &DefaultBundleExtractor{})
}

func TestExtractorInMemoryCacheJSONSnappyEncoder(t *testing.T) {
cases := []testCase{
// reference-addon:0.1.6
{
bundleImage: "quay.io/osd-addons/reference-addon-bundle@sha256:a62fd3f3b55aa58c587f0b7630f5e70b123d036a1a04a1bd5a866b5c576a04f4",
expectedPackageName: "reference-addon",
expectedCSVName: "reference-addon.v0.1.6",
expectedCSVVersion: "0.1.6",
},
// reference-addon:0.1.5
{
bundleImage: "quay.io/osd-addons/reference-addon-bundle@sha256:29879d193bd8da42e7b6500252b4d21bef733666bd893de2a3f9b250e591658e",
expectedPackageName: "reference-addon",
expectedCSVName: "reference-addon.v0.1.5",
expectedCSVVersion: "0.1.5",
},
}
encoder := NewJSONSnappyEncoder()
cache := NewBundleMemoryCache(encoder)
func TestDefaultBundleExtractor(t *testing.T) {
t.Parallel()

cache := NewBundleCacheImpl()

// adding extra logging for easier test debugging
log := logrus.New()
log.SetLevel(logrus.DebugLevel)

extractor := NewBundleExtractor(WithBundleCache(cache), WithBundleLog(log))

for _, tc := range cases {
for name, tc := range map[string]testCase{
"reference-addon:0.1.6": {
BundleImage: "quay.io/osd-addons/reference-addon-bundle@sha256:a62fd3f3b55aa58c587f0b7630f5e70b123d036a1a04a1bd5a866b5c576a04f4",
ExpectedPackageName: "reference-addon",
ExpectedCSVName: "reference-addon.v0.1.6",
ExpectedCSVVersion: "0.1.6",
},
"reference-addon:0.1.5": {
BundleImage: "quay.io/osd-addons/reference-addon-bundle@sha256:29879d193bd8da42e7b6500252b4d21bef733666bd893de2a3f9b250e591658e",
ExpectedPackageName: "reference-addon",
ExpectedCSVName: "reference-addon.v0.1.5",
ExpectedCSVVersion: "0.1.5",
},
} {
tc := tc // pin
t.Run(tc.bundleImage, func(t *testing.T) {

t.Run(name, func(t *testing.T) {
t.Parallel()
bundle, err := extractor.Extract(context.Background(), tc.bundleImage)

bundle, err := extractor.Extract(context.Background(), tc.BundleImage)
require.NoError(t, err)

require.NotNil(t, bundle)
testBundleFields(t, bundle, tc)
tc.AssertExpectations(t, bundle)

cachedBundle, err := cache.Get(tc.bundleImage)
cachedBundle, err := cache.GetBundle(tc.BundleImage)
require.NoError(t, err)
testBundleFields(t, cachedBundle, tc)

// make sure we encode to same length as an equality check
n1, err := encoder.Encode(bundle)
require.NoError(t, err)
n2, err := encoder.Encode(cachedBundle)
require.NoError(t, err)
require.Equal(t, len(n1), len(n2))
tc.AssertExpectations(t, cachedBundle)
})
}
}

func testBundleFields(t *testing.T, b *registry.Bundle, tc testCase) {
require.Equal(t, b.Name, tc.expectedPackageName)
require.Equal(t, b.BundleImage, tc.bundleImage)
require.NotNil(t, b.Annotations)
require.Equal(t, b.Annotations.PackageName, tc.expectedPackageName)
type testCase struct {
BundleImage string
ExpectedPackageName string
ExpectedCSVName string
ExpectedCSVVersion string
}

func (tc testCase) AssertExpectations(t *testing.T, b *registry.Bundle) {
t.Helper()

assert.Equal(t, b.Name, tc.ExpectedPackageName)
assert.Equal(t, b.BundleImage, tc.BundleImage)
assert.NotNil(t, b.Annotations)
assert.Equal(t, b.Annotations.PackageName, tc.ExpectedPackageName)

csv, err := b.ClusterServiceVersion()
require.NoError(t, err)
require.NotNil(t, csv)
require.Equal(t, csv.Name, tc.expectedCSVName)

assert.Equal(t, csv.Name, tc.ExpectedCSVName)

version, err := csv.GetVersion()
require.NoError(t, err)
require.Equal(t, version, tc.expectedCSVVersion)

assert.Equal(t, version, tc.ExpectedCSVVersion)
}
Loading

0 comments on commit fd790b4

Please sign in to comment.