Skip to content

Commit 36cd3c8

Browse files
m1kolam1kola
authored
Part 2: Reduce number of variable sources. Bundle variables. (#498)
* Move files Signed-off-by: Mikalai Radchuk <[email protected]> * Move bundle variable code into a func Signed-off-by: Mikalai Radchuk <[email protected]> --------- Signed-off-by: Mikalai Radchuk <[email protected]>
1 parent 2c3176d commit 36cd3c8

File tree

2 files changed

+223
-15
lines changed

2 files changed

+223
-15
lines changed

‎internal/resolution/variablesources/bundles_and_dependencies.go renamed to ‎internal/resolution/variablesources/bundle.go

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,8 @@ func NewBundlesAndDepsVariableSource(allBundles []*catalogmetadata.Bundle, input
3030
}
3131

3232
func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]deppy.Variable, error) {
33-
var variables []deppy.Variable
33+
variables := []deppy.Variable{}
3434

35-
// extract required package variables
3635
for _, variableSource := range b.variableSources {
3736
inputVariables, err := variableSource.GetVariables(ctx)
3837
if err != nil {
@@ -41,18 +40,43 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]depp
4140
variables = append(variables, inputVariables...)
4241
}
4342

44-
// create bundle queue for dependency resolution
45-
var bundleQueue []*catalogmetadata.Bundle
43+
requiredPackages := []*olmvariables.RequiredPackageVariable{}
44+
installedPackages := []*olmvariables.InstalledPackageVariable{}
4645
for _, variable := range variables {
4746
switch v := variable.(type) {
4847
case *olmvariables.RequiredPackageVariable:
49-
bundleQueue = append(bundleQueue, v.Bundles()...)
48+
requiredPackages = append(requiredPackages, v)
5049
case *olmvariables.InstalledPackageVariable:
51-
bundleQueue = append(bundleQueue, v.Bundles()...)
50+
installedPackages = append(installedPackages, v)
5251
}
5352
}
5453

54+
bundles, err := MakeBundleVariables(b.allBundles, requiredPackages, installedPackages)
55+
if err != nil {
56+
return nil, err
57+
}
58+
59+
for _, v := range bundles {
60+
variables = append(variables, v)
61+
}
62+
return variables, nil
63+
}
64+
65+
func MakeBundleVariables(
66+
allBundles []*catalogmetadata.Bundle,
67+
requiredPackages []*olmvariables.RequiredPackageVariable,
68+
installedPackages []*olmvariables.InstalledPackageVariable,
69+
) ([]*olmvariables.BundleVariable, error) {
70+
var bundleQueue []*catalogmetadata.Bundle
71+
for _, variable := range requiredPackages {
72+
bundleQueue = append(bundleQueue, variable.Bundles()...)
73+
}
74+
for _, variable := range installedPackages {
75+
bundleQueue = append(bundleQueue, variable.Bundles()...)
76+
}
77+
5578
// build bundle and dependency variables
79+
var result []*olmvariables.BundleVariable
5680
visited := sets.Set[deppy.Identifier]{}
5781
for len(bundleQueue) > 0 {
5882
// pop head of queue
@@ -68,32 +92,34 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]depp
6892
visited.Insert(id)
6993

7094
// get bundle dependencies
71-
dependencies, err := b.filterBundleDependencies(b.allBundles, head)
95+
dependencies, err := filterBundleDependencies(allBundles, head)
7296
if err != nil {
73-
return nil, fmt.Errorf("could not determine dependencies for bundle with id '%s': %w", id, err)
97+
return nil, fmt.Errorf("could not determine dependencies for bundle with id %q: %w", id, err)
7498
}
7599

76100
// add bundle dependencies to queue for processing
77101
bundleQueue = append(bundleQueue, dependencies...)
78102

79103
// create variable
80-
variables = append(variables, olmvariables.NewBundleVariable(head, dependencies))
104+
result = append(result, olmvariables.NewBundleVariable(head, dependencies))
81105
}
82106

83-
return variables, nil
107+
return result, nil
84108
}
85109

86-
func (b *BundlesAndDepsVariableSource) filterBundleDependencies(allBundles []*catalogmetadata.Bundle, bundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) {
110+
func filterBundleDependencies(allBundles []*catalogmetadata.Bundle, bundle *catalogmetadata.Bundle) ([]*catalogmetadata.Bundle, error) {
87111
var dependencies []*catalogmetadata.Bundle
88112
added := sets.Set[deppy.Identifier]{}
89113

90114
// gather required package dependencies
91-
// todo(perdasilva): disambiguate between not found and actual errors
92115
requiredPackages, _ := bundle.RequiredPackages()
93116
for _, requiredPackage := range requiredPackages {
94-
packageDependencyBundles := catalogfilter.Filter(allBundles, catalogfilter.And(catalogfilter.WithPackageName(requiredPackage.PackageName), catalogfilter.InBlangSemverRange(requiredPackage.SemverRange)))
117+
packageDependencyBundles := catalogfilter.Filter(allBundles, catalogfilter.And(
118+
catalogfilter.WithPackageName(requiredPackage.PackageName),
119+
catalogfilter.InBlangSemverRange(requiredPackage.SemverRange),
120+
))
95121
if len(packageDependencyBundles) == 0 {
96-
return nil, fmt.Errorf("could not find package dependencies for bundle '%s'", bundle.Name)
122+
return nil, fmt.Errorf("could not find package dependencies for bundle %q", bundle.Name)
97123
}
98124
for i := 0; i < len(packageDependencyBundles); i++ {
99125
bundle := packageDependencyBundles[i]

‎internal/resolution/variablesources/bundles_and_dependencies_test.go renamed to ‎internal/resolution/variablesources/bundle_test.go

Lines changed: 183 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,200 @@ import (
44
"context"
55
"encoding/json"
66
"errors"
7+
"testing"
78

9+
"github.com/google/go-cmp/cmp"
10+
"github.com/google/go-cmp/cmp/cmpopts"
811
. "github.com/onsi/ginkgo/v2"
912
. "github.com/onsi/gomega"
1013
"github.com/operator-framework/deppy/pkg/deppy"
14+
"github.com/operator-framework/deppy/pkg/deppy/constraint"
15+
"github.com/operator-framework/deppy/pkg/deppy/input"
1116
"github.com/operator-framework/operator-registry/alpha/declcfg"
1217
"github.com/operator-framework/operator-registry/alpha/property"
18+
"github.com/stretchr/testify/assert"
19+
"github.com/stretchr/testify/require"
1320

1421
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
1522
olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
1623
"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
1724
)
1825

26+
func TestMakeBundleVariables_ValidDepedencies(t *testing.T) {
27+
const fakeCatalogName = "fake-catalog"
28+
fakeChannel := catalogmetadata.Channel{Channel: declcfg.Channel{Name: "stable"}}
29+
bundleSet := map[string]*catalogmetadata.Bundle{
30+
// Test package which we will be using as input into
31+
// the testable function
32+
"test-package.v1.0.0": {
33+
Bundle: declcfg.Bundle{
34+
Name: "test-package.v1.0.0",
35+
Package: "test-package",
36+
Properties: []property.Property{
37+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)},
38+
{Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "first-level-dependency", "versionRange": ">=1.0.0 <2.0.0"}`)},
39+
},
40+
},
41+
CatalogName: fakeCatalogName,
42+
InChannels: []*catalogmetadata.Channel{&fakeChannel},
43+
},
44+
45+
// First level dependency of test-package. Will be explicitly
46+
// provided into the testable function as part of variable.
47+
// This package must have at least one dependency with a version
48+
// range so we can test that result has correct ordering:
49+
// the testable function must give priority to newer versions.
50+
"first-level-dependency.v1.0.0": {
51+
Bundle: declcfg.Bundle{
52+
Name: "first-level-dependency.v1.0.0",
53+
Package: "first-level-dependency",
54+
Properties: []property.Property{
55+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "first-level-dependency", "version": "1.0.0"}`)},
56+
{Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "second-level-dependency", "versionRange": ">=1.0.0 <2.0.0"}`)},
57+
},
58+
},
59+
CatalogName: fakeCatalogName,
60+
InChannels: []*catalogmetadata.Channel{&fakeChannel},
61+
},
62+
63+
// Second level dependency that matches requirements of the first level dependency.
64+
"second-level-dependency.v1.0.0": {
65+
Bundle: declcfg.Bundle{
66+
Name: "second-level-dependency.v1.0.0",
67+
Package: "second-level-dependency",
68+
Properties: []property.Property{
69+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "second-level-dependency", "version": "1.0.0"}`)},
70+
},
71+
},
72+
CatalogName: fakeCatalogName,
73+
InChannels: []*catalogmetadata.Channel{&fakeChannel},
74+
},
75+
76+
// Second level dependency that matches requirements of the first level dependency.
77+
"second-level-dependency.v1.0.1": {
78+
Bundle: declcfg.Bundle{
79+
Name: "second-level-dependency.v1.0.1",
80+
Package: "second-level-dependency",
81+
Properties: []property.Property{
82+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "second-level-dependency", "version": "1.0.1"}`)},
83+
},
84+
},
85+
CatalogName: fakeCatalogName,
86+
InChannels: []*catalogmetadata.Channel{&fakeChannel},
87+
},
88+
89+
// Second level dependency that does not match requirements of the first level dependency.
90+
"second-level-dependency.v2.0.0": {
91+
Bundle: declcfg.Bundle{
92+
Name: "second-level-dependency.v2.0.0",
93+
Package: "second-level-dependency",
94+
Properties: []property.Property{
95+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "second-level-dependency", "version": "2.0.0"}`)},
96+
},
97+
},
98+
CatalogName: fakeCatalogName,
99+
InChannels: []*catalogmetadata.Channel{&fakeChannel},
100+
},
101+
102+
// Package that is in a our fake catalog, but is not involved
103+
// in this dependency chain. We need this to make sure that
104+
// the testable function filters out unrelated bundles.
105+
"uninvolved-package.v1.0.0": {
106+
Bundle: declcfg.Bundle{
107+
Name: "uninvolved-package.v1.0.0",
108+
Package: "uninvolved-package",
109+
Properties: []property.Property{
110+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "uninvolved-package", "version": "1.0.0"}`)},
111+
},
112+
},
113+
CatalogName: fakeCatalogName,
114+
InChannels: []*catalogmetadata.Channel{&fakeChannel},
115+
},
116+
}
117+
118+
allBundles := make([]*catalogmetadata.Bundle, 0, len(bundleSet))
119+
for _, bundle := range bundleSet {
120+
allBundles = append(allBundles, bundle)
121+
}
122+
requiredPackages := []*olmvariables.RequiredPackageVariable{
123+
olmvariables.NewRequiredPackageVariable("test-package", []*catalogmetadata.Bundle{
124+
bundleSet["first-level-dependency.v1.0.0"],
125+
}),
126+
}
127+
installedPackages := []*olmvariables.InstalledPackageVariable{
128+
olmvariables.NewInstalledPackageVariable("test-package", []*catalogmetadata.Bundle{
129+
bundleSet["first-level-dependency.v1.0.0"],
130+
}),
131+
}
132+
133+
bundles, err := variablesources.MakeBundleVariables(allBundles, requiredPackages, installedPackages)
134+
require.NoError(t, err)
135+
136+
// Each dependency must have a variable.
137+
// Dependencies from the same package must be sorted by version
138+
// with higher versions first.
139+
expectedVariables := []*olmvariables.BundleVariable{
140+
olmvariables.NewBundleVariable(
141+
bundleSet["first-level-dependency.v1.0.0"],
142+
[]*catalogmetadata.Bundle{
143+
bundleSet["second-level-dependency.v1.0.1"],
144+
bundleSet["second-level-dependency.v1.0.0"],
145+
},
146+
),
147+
olmvariables.NewBundleVariable(
148+
bundleSet["second-level-dependency.v1.0.1"],
149+
nil,
150+
),
151+
olmvariables.NewBundleVariable(
152+
bundleSet["second-level-dependency.v1.0.0"],
153+
nil,
154+
),
155+
}
156+
gocmpopts := []cmp.Option{
157+
cmpopts.IgnoreUnexported(catalogmetadata.Bundle{}),
158+
cmp.AllowUnexported(
159+
olmvariables.BundleVariable{},
160+
input.SimpleVariable{},
161+
constraint.DependencyConstraint{},
162+
),
163+
}
164+
require.Empty(t, cmp.Diff(bundles, expectedVariables, gocmpopts...))
165+
}
166+
167+
func TestMakeBundleVariables_NonExistentDepedencies(t *testing.T) {
168+
const fakeCatalogName = "fake-catalog"
169+
fakeChannel := catalogmetadata.Channel{Channel: declcfg.Channel{Name: "stable"}}
170+
bundleSet := map[string]*catalogmetadata.Bundle{
171+
"test-package.v1.0.0": {
172+
Bundle: declcfg.Bundle{
173+
Name: "test-package.v1.0.0",
174+
Package: "test-package",
175+
Properties: []property.Property{
176+
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "1.0.0"}`)},
177+
{Type: property.TypePackageRequired, Value: json.RawMessage(`{"packageName": "first-level-dependency", "versionRange": ">=1.0.0 <2.0.0"}`)},
178+
},
179+
},
180+
CatalogName: fakeCatalogName,
181+
InChannels: []*catalogmetadata.Channel{&fakeChannel},
182+
},
183+
}
184+
185+
allBundles := make([]*catalogmetadata.Bundle, 0, len(bundleSet))
186+
for _, bundle := range bundleSet {
187+
allBundles = append(allBundles, bundle)
188+
}
189+
requiredPackages := []*olmvariables.RequiredPackageVariable{
190+
olmvariables.NewRequiredPackageVariable("test-package", []*catalogmetadata.Bundle{
191+
bundleSet["test-package.v1.0.0"],
192+
}),
193+
}
194+
installedPackages := []*olmvariables.InstalledPackageVariable{}
195+
196+
bundles, err := variablesources.MakeBundleVariables(allBundles, requiredPackages, installedPackages)
197+
assert.ErrorContains(t, err, `could not determine dependencies for bundle with id "fake-catalog-test-package-test-package.v1.0.0"`)
198+
assert.Nil(t, bundles)
199+
}
200+
19201
var _ = Describe("BundlesAndDepsVariableSource", func() {
20202
var (
21203
bdvs *variablesources.BundlesAndDepsVariableSource
@@ -368,7 +550,7 @@ var _ = Describe("BundlesAndDepsVariableSource", func() {
368550
)
369551
_, err := bdvs.GetVariables(context.TODO())
370552
Expect(err).To(HaveOccurred())
371-
Expect(err.Error()).To(ContainSubstring("could not determine dependencies for bundle with id 'fake-catalog-test-package-bundle-2': could not find package dependencies for bundle 'bundle-2'"))
553+
Expect(err.Error()).To(ContainSubstring(`could not determine dependencies for bundle with id "fake-catalog-test-package-bundle-2": could not find package dependencies for bundle "bundle-2"`))
372554
})
373555

374556
It("should return error if an inner variable source returns an error", func() {

0 commit comments

Comments
 (0)