Skip to content

Commit 647537d

Browse files
stevekuznetsovstevekuznetsov
authoredJun 6, 2023
declcfg/load: improvements (operator-framework#1106)
* *: thread through context into FBC load Signed-off-by: Steve Kuznetsov <[email protected]> * declcfg/load: allow configuring the parallelism Signed-off-by: Steve Kuznetsov <[email protected]> --------- Signed-off-by: Steve Kuznetsov <[email protected]>
1 parent b51aaf0 commit 647537d

17 files changed

+79
-53
lines changed
 

‎alpha/action/render.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ func (r Render) renderReference(ctx context.Context, ref string) (*declcfg.Decla
122122
if !r.AllowedRefMask.Allowed(RefDCDir) {
123123
return nil, fmt.Errorf("cannot render declarative config directory: %w", ErrNotAllowed)
124124
}
125-
return declcfg.LoadFS(os.DirFS(ref))
125+
return declcfg.LoadFS(ctx, os.DirFS(ref))
126126
} else {
127127
// The only supported file type is an sqlite DB file,
128128
// since declarative configs will be in a directory.
@@ -169,7 +169,7 @@ func (r Render) imageToDeclcfg(ctx context.Context, imageRef string) (*declcfg.D
169169
if !r.AllowedRefMask.Allowed(RefDCImage) {
170170
return nil, fmt.Errorf("cannot render declarative config image: %w", ErrNotAllowed)
171171
}
172-
cfg, err = declcfg.LoadFS(os.DirFS(filepath.Join(tmpDir, configsDir)))
172+
cfg, err = declcfg.LoadFS(ctx, os.DirFS(filepath.Join(tmpDir, configsDir)))
173173
if err != nil {
174174
return nil, err
175175
}

‎alpha/declcfg/load.go

+23-6
Original file line numberDiff line numberDiff line change
@@ -109,29 +109,46 @@ func walkFiles(root fs.FS, fn func(root fs.FS, path string, err error) error) er
109109
})
110110
}
111111

112+
type LoadOptions struct {
113+
concurrency int
114+
}
115+
116+
type LoadOption func(*LoadOptions)
117+
118+
func WithConcurrency(concurrency int) LoadOption {
119+
return func(opts *LoadOptions) {
120+
opts.concurrency = concurrency
121+
}
122+
}
123+
112124
// LoadFS loads a declarative config from the provided root FS. LoadFS walks the
113125
// filesystem from root and uses a gitignore-style filename matcher to skip files
114126
// that match patterns found in .indexignore files found throughout the filesystem.
115127
// If LoadFS encounters an error loading or parsing any file, the error will be
116128
// immediately returned.
117-
func LoadFS(root fs.FS) (*DeclarativeConfig, error) {
129+
func LoadFS(ctx context.Context, root fs.FS, opts ...LoadOption) (*DeclarativeConfig, error) {
118130
if root == nil {
119131
return nil, fmt.Errorf("no declarative config filesystem provided")
120132
}
121133

122-
concurrency := runtime.NumCPU()
134+
options := LoadOptions{
135+
concurrency: runtime.NumCPU(),
136+
}
137+
for _, opt := range opts {
138+
opt(&options)
139+
}
123140

124141
var (
125142
fcfg = &DeclarativeConfig{}
126-
pathChan = make(chan string, concurrency)
127-
cfgChan = make(chan *DeclarativeConfig, concurrency)
143+
pathChan = make(chan string, options.concurrency)
144+
cfgChan = make(chan *DeclarativeConfig, options.concurrency)
128145
)
129146

130147
// Create an errgroup to manage goroutines. The context is closed when any
131148
// goroutine returns an error. Goroutines should check the context
132149
// to see if they should return early (in the case of another goroutine
133150
// returning an error).
134-
eg, ctx := errgroup.WithContext(context.Background())
151+
eg, ctx := errgroup.WithContext(ctx)
135152

136153
// Walk the FS and send paths to a channel for parsing.
137154
eg.Go(func() error {
@@ -141,7 +158,7 @@ func LoadFS(root fs.FS) (*DeclarativeConfig, error) {
141158
// Parse paths concurrently. The waitgroup ensures that all paths are parsed
142159
// before the cfgChan is closed.
143160
var wg sync.WaitGroup
144-
for i := 0; i < concurrency; i++ {
161+
for i := 0; i < options.concurrency; i++ {
145162
wg.Add(1)
146163
eg.Go(func() error {
147164
defer wg.Done()

‎alpha/declcfg/load_benchmark_test.go

+17-11
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
package declcfg_test
22

33
import (
4+
"context"
45
"encoding/base64"
56
"fmt"
67
"math/rand"
78
"os"
9+
"runtime"
810
"testing"
911

1012
"github.com/blang/semver/v4"
@@ -20,18 +22,22 @@ func BenchmarkLoadFS(b *testing.B) {
2022
fbc := generateFBC(b, 300, 450, 3000)
2123
b.ResetTimer()
2224

23-
for i := 0; i < b.N; i++ {
24-
b.StopTimer()
25-
tempDir := b.TempDir()
26-
if err := declcfg.WriteFS(*fbc, tempDir, declcfg.WriteJSON, ".json"); err != nil {
27-
b.Error(err)
28-
}
29-
b.StartTimer()
25+
for _, n := range []int{1, runtime.NumCPU(), 2 * runtime.NumCPU()} {
26+
b.Run(fmt.Sprintf("%d routines", n), func(b *testing.B) {
27+
for i := 0; i < b.N; i++ {
28+
b.StopTimer()
29+
tempDir := b.TempDir()
30+
if err := declcfg.WriteFS(*fbc, tempDir, declcfg.WriteJSON, ".json"); err != nil {
31+
b.Error(err)
32+
}
33+
b.StartTimer()
3034

31-
_, err := declcfg.LoadFS(os.DirFS(tempDir))
32-
if err != nil {
33-
b.Error(err)
34-
}
35+
_, err := declcfg.LoadFS(context.Background(), os.DirFS(tempDir), declcfg.WithConcurrency(n))
36+
if err != nil {
37+
b.Error(err)
38+
}
39+
}
40+
})
3541
}
3642
}
3743

‎alpha/declcfg/load_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package declcfg
22

33
import (
4+
"context"
45
"encoding/json"
56
"io/fs"
67
"os"
@@ -338,7 +339,7 @@ func TestLoadFS(t *testing.T) {
338339

339340
for _, s := range specs {
340341
t.Run(s.name, func(t *testing.T) {
341-
cfg, err := LoadFS(s.fsys)
342+
cfg, err := LoadFS(context.Background(), s.fsys)
342343
s.assertion(t, err)
343344
if err == nil {
344345
require.NotNil(t, cfg)

‎alpha/template/composite/builder.go

+11-11
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ type BuilderConfig struct {
3434

3535
type Builder interface {
3636
Build(ctx context.Context, reg image.Registry, dir string, td TemplateDefinition) error
37-
Validate(dir string) error
37+
Validate(ctx context.Context, dir string) error
3838
}
3939

4040
type BasicBuilder struct {
@@ -94,8 +94,8 @@ func (bb *BasicBuilder) Build(ctx context.Context, reg image.Registry, dir strin
9494
return build(dcfg, destPath, bb.builderCfg.OutputType)
9595
}
9696

97-
func (bb *BasicBuilder) Validate(dir string) error {
98-
return validate(bb.builderCfg, dir)
97+
func (bb *BasicBuilder) Validate(ctx context.Context, dir string) error {
98+
return validate(ctx, bb.builderCfg, dir)
9999
}
100100

101101
type SemverBuilder struct {
@@ -156,8 +156,8 @@ func (sb *SemverBuilder) Build(ctx context.Context, reg image.Registry, dir stri
156156
return build(dcfg, destPath, sb.builderCfg.OutputType)
157157
}
158158

159-
func (sb *SemverBuilder) Validate(dir string) error {
160-
return validate(sb.builderCfg, dir)
159+
func (sb *SemverBuilder) Validate(ctx context.Context, dir string) error {
160+
return validate(ctx, sb.builderCfg, dir)
161161
}
162162

163163
type RawBuilder struct {
@@ -216,8 +216,8 @@ func (rb *RawBuilder) Build(ctx context.Context, _ image.Registry, dir string, t
216216
return build(dcfg, destPath, rb.builderCfg.OutputType)
217217
}
218218

219-
func (rb *RawBuilder) Validate(dir string) error {
220-
return validate(rb.builderCfg, dir)
219+
func (rb *RawBuilder) Validate(ctx context.Context, dir string) error {
220+
return validate(ctx, rb.builderCfg, dir)
221221
}
222222

223223
type CustomBuilder struct {
@@ -285,8 +285,8 @@ func (cb *CustomBuilder) Build(ctx context.Context, reg image.Registry, dir stri
285285
return build(dcfg, destPath, cb.builderCfg.OutputType)
286286
}
287287

288-
func (cb *CustomBuilder) Validate(dir string) error {
289-
return validate(cb.builderCfg, dir)
288+
func (cb *CustomBuilder) Validate(ctx context.Context, dir string) error {
289+
return validate(ctx, cb.builderCfg, dir)
290290
}
291291

292292
func writeDeclCfg(dcfg declcfg.DeclarativeConfig, w io.Writer, output string) error {
@@ -300,7 +300,7 @@ func writeDeclCfg(dcfg declcfg.DeclarativeConfig, w io.Writer, output string) er
300300
}
301301
}
302302

303-
func validate(builderCfg BuilderConfig, dir string) error {
303+
func validate(ctx context.Context, builderCfg BuilderConfig, dir string) error {
304304

305305
path := path.Join(builderCfg.WorkingDir, dir)
306306
s, err := os.Stat(path)
@@ -311,7 +311,7 @@ func validate(builderCfg BuilderConfig, dir string) error {
311311
return fmt.Errorf("%q is not a directory", path)
312312
}
313313

314-
if err := config.Validate(os.DirFS(path)); err != nil {
314+
if err := config.Validate(ctx, os.DirFS(path)); err != nil {
315315
return fmt.Errorf("validation failure in path %q: %v", path, err)
316316
}
317317
return nil

‎alpha/template/composite/builder_test.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ func TestBasicBuilder(t *testing.T) {
242242
tc.buildAssertions(t, outPath, buildErr)
243243

244244
if tc.validate {
245-
validateErr := tc.basicBuilder.Validate(outDir)
245+
validateErr := tc.basicBuilder.Validate(context.Background(), outDir)
246246
tc.validateAssertions(t, validateErr)
247247
}
248248
})
@@ -706,7 +706,7 @@ func TestSemverBuilder(t *testing.T) {
706706
tc.buildAssertions(t, outPath, buildErr)
707707

708708
if tc.validate {
709-
validateErr := tc.semverBuilder.Validate(outDir)
709+
validateErr := tc.semverBuilder.Validate(context.Background(), outDir)
710710
tc.validateAssertions(t, validateErr)
711711
}
712712
})
@@ -1176,7 +1176,7 @@ func TestRawBuilder(t *testing.T) {
11761176
tc.buildAssertions(t, outPath, buildErr)
11771177

11781178
if tc.validate {
1179-
validateErr := tc.rawBuilder.Validate(outDir)
1179+
validateErr := tc.rawBuilder.Validate(context.Background(), outDir)
11801180
tc.validateAssertions(t, validateErr)
11811181
}
11821182
})
@@ -1575,7 +1575,7 @@ func TestCustomBuilder(t *testing.T) {
15751575
tc.buildAssertions(t, outPath, buildErr)
15761576

15771577
if tc.validate {
1578-
validateErr := tc.customBuilder.Validate(outDir)
1578+
validateErr := tc.customBuilder.Validate(context.Background(), outDir)
15791579
tc.validateAssertions(t, validateErr)
15801580
}
15811581
})
@@ -1759,7 +1759,7 @@ const customBuiltFbcJson = `{
17591759
`
17601760

17611761
func TestValidateFailure(t *testing.T) {
1762-
err := validate(BuilderConfig{}, "")
1762+
err := validate(context.Background(), BuilderConfig{}, "")
17631763
require.Error(t, err)
17641764
require.Contains(t, err.Error(), "no such file or directory")
17651765
}

‎alpha/template/composite/composite.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ func (t *Template) Render(ctx context.Context, validate bool) error {
134134

135135
if validate {
136136
// run the validation for the builder
137-
err = builder.Validate(component.Destination.Path)
137+
err = builder.Validate(ctx, component.Destination.Path)
138138
if err != nil {
139139
return fmt.Errorf("validating component %q: %w", component.Name, err)
140140
}

‎alpha/template/composite/composite_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func (tb *TestBuilder) Build(ctx context.Context, reg image.Registry, dir string
3030
return nil
3131
}
3232

33-
func (tb *TestBuilder) Validate(dir string) error {
33+
func (tb *TestBuilder) Validate(ctx context.Context, dir string) error {
3434
if tb.validateShouldError {
3535
return fmt.Errorf("validate error!")
3636
}

‎cmd/opm/serve/serve.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ func (s *serve) run(ctx context.Context) error {
136136
return err
137137
}
138138
} else {
139-
if err := cache.LoadOrRebuild(store, os.DirFS(s.configDir)); err != nil {
139+
if err := cache.LoadOrRebuild(ctx, store, os.DirFS(s.configDir)); err != nil {
140140
return err
141141
}
142142
}

‎cmd/opm/validate/validate.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ func NewCmd() *cobra.Command {
1717
Short: "Validate the declarative index config",
1818
Long: "Validate the declarative config JSON file(s) in a given directory",
1919
Args: cobra.ExactArgs(1),
20-
RunE: func(_ *cobra.Command, args []string) error {
20+
RunE: func(c *cobra.Command, args []string) error {
2121
directory := args[0]
2222
s, err := os.Stat(directory)
2323
if err != nil {
@@ -27,7 +27,7 @@ func NewCmd() *cobra.Command {
2727
return fmt.Errorf("%q is not a directory", directory)
2828
}
2929

30-
if err := config.Validate(os.DirFS(directory)); err != nil {
30+
if err := config.Validate(c.Context(), os.DirFS(directory)); err != nil {
3131
logger.Fatal(err)
3232
}
3333
return nil

‎pkg/cache/cache.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,13 @@ type Cache interface {
1818
registry.GRPCQuery
1919

2020
CheckIntegrity(fbc fs.FS) error
21-
Build(fbc fs.FS) error
21+
Build(ctx context.Context, fbc fs.FS) error
2222
Load() error
2323
}
2424

25-
func LoadOrRebuild(c Cache, fbc fs.FS) error {
25+
func LoadOrRebuild(ctx context.Context, c Cache, fbc fs.FS) error {
2626
if err := c.CheckIntegrity(fbc); err != nil {
27-
if err := c.Build(fbc); err != nil {
27+
if err := c.Build(ctx, fbc); err != nil {
2828
return err
2929
}
3030
}

‎pkg/cache/cache_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ func genTestCaches(t *testing.T, fbcFS fs.FS) []Cache {
205205
}
206206

207207
for _, c := range caches {
208-
err := c.Build(fbcFS)
208+
err := c.Build(context.Background(), fbcFS)
209209
require.NoError(t, err)
210210
err = c.Load()
211211
require.NoError(t, err)

‎pkg/cache/json.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ func (q *JSON) computeDigest(fbcFsys fs.FS) (string, error) {
178178
return fmt.Sprintf("%x", computedHasher.Sum(nil)), nil
179179
}
180180

181-
func (q *JSON) Build(fbcFsys fs.FS) error {
181+
func (q *JSON) Build(ctx context.Context, fbcFsys fs.FS) error {
182182
// ensure that generated cache is available to all future users
183183
oldUmask := umask(000)
184184
defer umask(oldUmask)
@@ -190,7 +190,7 @@ func (q *JSON) Build(fbcFsys fs.FS) error {
190190
return fmt.Errorf("ensure clean base directory: %v", err)
191191
}
192192

193-
fbc, err := declcfg.LoadFS(fbcFsys)
193+
fbc, err := declcfg.LoadFS(ctx, fbcFsys)
194194
if err != nil {
195195
return err
196196
}

‎pkg/cache/json_test.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package cache
22

33
import (
4+
"context"
45
"io/fs"
56
"os"
67
"path/filepath"
@@ -12,7 +13,7 @@ import (
1213
func TestJSON_StableDigest(t *testing.T) {
1314
cacheDir := t.TempDir()
1415
c := NewJSON(cacheDir)
15-
require.NoError(t, c.Build(validFS))
16+
require.NoError(t, c.Build(context.Background(), validFS))
1617

1718
actualDigest, err := c.existingDigest()
1819
require.NoError(t, err)
@@ -96,7 +97,7 @@ func TestJSON_CheckIntegrity(t *testing.T) {
9697
c := NewJSON(cacheDir)
9798

9899
if tc.build {
99-
require.NoError(t, c.Build(tc.fbcFS))
100+
require.NoError(t, c.Build(context.Background(), tc.fbcFS))
100101
}
101102
if tc.mod != nil {
102103
require.NoError(t, tc.mod(&tc, cacheDir))

‎pkg/lib/config/validate.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package config
22

33
import (
4+
"context"
45
"io/fs"
56

67
"github.com/operator-framework/operator-registry/alpha/declcfg"
@@ -13,9 +14,9 @@ import (
1314
// directory: a filesystem where declarative config file(s) exist
1415
// Outputs:
1516
// error: a wrapped error that contains a tree of error strings
16-
func Validate(root fs.FS) error {
17+
func Validate(ctx context.Context, root fs.FS) error {
1718
// Load config files and convert them to declcfg objects
18-
cfg, err := declcfg.LoadFS(root)
19+
cfg, err := declcfg.LoadFS(ctx, root)
1920
if err != nil {
2021
return err
2122
}

‎pkg/lib/registry/registry_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ func newCache(t *testing.T, bundles []*model.Bundle) cache.Cache {
102102
reg, err := cache.New(cacheDir)
103103
require.NoError(t, err)
104104

105-
require.NoError(t, reg.Build(os.DirFS(fbcDir)))
105+
require.NoError(t, reg.Build(context.Background(), os.DirFS(fbcDir)))
106106
require.NoError(t, reg.Load())
107107

108108
return reg

‎pkg/server/server_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func createDBStore(dbPath string) *sqlite.SQLQuerier {
6666

6767
func fbcJsonCache(catalogDir, cacheDir string) (cache2.Cache, error) {
6868
store := cache2.NewJSON(cacheDir)
69-
if err := store.Build(os.DirFS(catalogDir)); err != nil {
69+
if err := store.Build(context.Background(), os.DirFS(catalogDir)); err != nil {
7070
return nil, err
7171
}
7272
if err := store.Load(); err != nil {

0 commit comments

Comments
 (0)
Please sign in to comment.