Skip to content

Commit c24fdbc

Browse files
m1kolam1kola
authored
Reduce variable count (#453)
After this commit we will be creating one bundle per package. Previously we we creating one variable per bundle occurance in a channel which is currently unnecessary. Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
1 parent b896365 commit c24fdbc

File tree

8 files changed

+61
-74
lines changed

8 files changed

+61
-74
lines changed

‎internal/resolution/variables/bundle.go

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,17 @@ func (b *BundleVariable) Dependencies() []*catalogmetadata.Bundle {
2626
return b.dependencies
2727
}
2828

29-
func NewBundleVariable(id deppy.Identifier, bundle *catalogmetadata.Bundle, dependencies []*catalogmetadata.Bundle) *BundleVariable {
30-
var dependencyIDs []deppy.Identifier
29+
func NewBundleVariable(bundle *catalogmetadata.Bundle, dependencies []*catalogmetadata.Bundle) *BundleVariable {
30+
dependencyIDs := make([]deppy.Identifier, 0, len(dependencies))
3131
for _, bundle := range dependencies {
32-
dependencyIDs = append(dependencyIDs, BundleToBundleVariableIDs(bundle)...)
32+
dependencyIDs = append(dependencyIDs, BundleVariableID(bundle))
3333
}
3434
var constraints []deppy.Constraint
3535
if len(dependencyIDs) > 0 {
3636
constraints = append(constraints, constraint.Dependency(dependencyIDs...))
3737
}
3838
return &BundleVariable{
39-
SimpleVariable: input.NewSimpleVariable(id, constraints...),
39+
SimpleVariable: input.NewSimpleVariable(BundleVariableID(bundle), constraints...),
4040
bundle: bundle,
4141
dependencies: dependencies,
4242
}
@@ -59,16 +59,9 @@ func NewBundleUniquenessVariable(id deppy.Identifier, atMostIDs ...deppy.Identif
5959
}
6060
}
6161

62-
// BundleToBundleVariableIDs returns a list of all possible IDs for a bundle.
63-
// A bundle can be present in multiple channels and we need a separate variable
64-
// with a unique ID for each occurrence.
65-
// Result must be deterministic.
66-
func BundleToBundleVariableIDs(bundle *catalogmetadata.Bundle) []deppy.Identifier {
67-
out := make([]deppy.Identifier, 0, len(bundle.InChannels))
68-
for _, ch := range bundle.InChannels {
69-
out = append(out, deppy.Identifier(
70-
fmt.Sprintf("%s-%s-%s-%s", bundle.CatalogName, bundle.Package, ch.Name, bundle.Name),
71-
))
72-
}
73-
return out
62+
// BundleVariableID returns an ID for a given bundle.
63+
func BundleVariableID(bundle *catalogmetadata.Bundle) deppy.Identifier {
64+
return deppy.Identifier(
65+
fmt.Sprintf("%s-%s-%s", bundle.CatalogName, bundle.Package, bundle.Name),
66+
)
7467
}

‎internal/resolution/variables/bundle_test.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,7 @@ func TestBundleVariable(t *testing.T) {
4040
}},
4141
},
4242
}
43-
ids := olmvariables.BundleToBundleVariableIDs(bundle)
44-
if len(ids) != len(bundle.InChannels) {
45-
t.Fatalf("bundle should produce one variable ID per channel; received: %d", len(bundle.InChannels))
46-
}
47-
bv := olmvariables.NewBundleVariable(ids[0], bundle, dependencies)
43+
bv := olmvariables.NewBundleVariable(bundle, dependencies)
4844

4945
if bv.Bundle() != bundle {
5046
t.Errorf("bundle '%v' does not match expected '%v'", bv.Bundle(), bundle)

‎internal/resolution/variables/installed_package.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ func (r *InstalledPackageVariable) Bundles() []*catalogmetadata.Bundle {
2323

2424
func NewInstalledPackageVariable(packageName string, bundles []*catalogmetadata.Bundle) *InstalledPackageVariable {
2525
id := deppy.IdentifierFromString(fmt.Sprintf("installed package %s", packageName))
26-
var variableIDs []deppy.Identifier
26+
variableIDs := make([]deppy.Identifier, 0, len(bundles))
2727
for _, bundle := range bundles {
28-
variableIDs = append(variableIDs, BundleToBundleVariableIDs(bundle)...)
28+
variableIDs = append(variableIDs, BundleVariableID(bundle))
2929
}
3030
return &InstalledPackageVariable{
3131
SimpleVariable: input.NewSimpleVariable(id, constraint.Mandatory(), constraint.Dependency(variableIDs...)),

‎internal/resolution/variables/required_package.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ func (r *RequiredPackageVariable) Bundles() []*catalogmetadata.Bundle {
2323

2424
func NewRequiredPackageVariable(packageName string, bundles []*catalogmetadata.Bundle) *RequiredPackageVariable {
2525
id := deppy.IdentifierFromString(fmt.Sprintf("required package %s", packageName))
26-
var variableIDs []deppy.Identifier
26+
variableIDs := make([]deppy.Identifier, 0, len(bundles))
2727
for _, bundle := range bundles {
28-
variableIDs = append(variableIDs, BundleToBundleVariableIDs(bundle)...)
28+
variableIDs = append(variableIDs, BundleVariableID(bundle))
2929
}
3030
return &RequiredPackageVariable{
3131
SimpleVariable: input.NewSimpleVariable(id, constraint.Mandatory(), constraint.Dependency(variableIDs...)),

‎internal/resolution/variablesources/bundles_and_dependencies.go

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -63,25 +63,25 @@ func (b *BundlesAndDepsVariableSource) GetVariables(ctx context.Context) ([]depp
6363
var head *catalogmetadata.Bundle
6464
head, bundleQueue = bundleQueue[0], bundleQueue[1:]
6565

66-
for _, id := range olmvariables.BundleToBundleVariableIDs(head) {
67-
// ignore bundles that have already been processed
68-
if _, ok := visited[id]; ok {
69-
continue
70-
}
71-
visited[id] = struct{}{}
72-
73-
// get bundle dependencies
74-
dependencies, err := b.filterBundleDependencies(allBundles, head)
75-
if err != nil {
76-
return nil, fmt.Errorf("could not determine dependencies for bundle with id '%s': %w", id, err)
77-
}
66+
id := olmvariables.BundleVariableID(head)
7867

79-
// add bundle dependencies to queue for processing
80-
bundleQueue = append(bundleQueue, dependencies...)
68+
// ignore bundles that have already been processed
69+
if _, ok := visited[id]; ok {
70+
continue
71+
}
72+
visited[id] = struct{}{}
8173

82-
// create variable
83-
variables = append(variables, olmvariables.NewBundleVariable(id, head, dependencies))
74+
// get bundle dependencies
75+
dependencies, err := b.filterBundleDependencies(allBundles, head)
76+
if err != nil {
77+
return nil, fmt.Errorf("could not determine dependencies for bundle with id '%s': %w", id, err)
8478
}
79+
80+
// add bundle dependencies to queue for processing
81+
bundleQueue = append(bundleQueue, dependencies...)
82+
83+
// create variable
84+
variables = append(variables, olmvariables.NewBundleVariable(head, dependencies))
8585
}
8686

8787
return variables, nil
@@ -101,11 +101,10 @@ func (b *BundlesAndDepsVariableSource) filterBundleDependencies(allBundles []*ca
101101
}
102102
for i := 0; i < len(packageDependencyBundles); i++ {
103103
bundle := packageDependencyBundles[i]
104-
for _, id := range olmvariables.BundleToBundleVariableIDs(bundle) {
105-
if _, ok := added[id]; !ok {
106-
dependencies = append(dependencies, bundle)
107-
added[id] = struct{}{}
108-
}
104+
id := olmvariables.BundleVariableID(bundle)
105+
if _, ok := added[id]; !ok {
106+
dependencies = append(dependencies, bundle)
107+
added[id] = struct{}{}
109108
}
110109
}
111110
}

‎internal/resolution/variablesources/bundles_and_dependencies_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -316,13 +316,13 @@ var _ = Describe("BundlesAndDepsVariableSource", func() {
316316
// dependencies (bundles 7, 8, 9, 10, 11) to appear here due to their GVKs being required by
317317
// some of the packages.
318318
Expect(bundleVariables).To(WithTransform(CollectBundleVariableIDs, Equal([]string{
319-
"fake-catalog-test-package-stable-bundle-2",
320-
"fake-catalog-test-package-stable-bundle-1",
321-
"fake-catalog-test-package-2-stable-bundle-15",
322-
"fake-catalog-test-package-2-stable-bundle-16",
323-
"fake-catalog-test-package-2-stable-bundle-17",
324-
"fake-catalog-some-package-stable-bundle-5",
325-
"fake-catalog-some-package-stable-bundle-4",
319+
"fake-catalog-test-package-bundle-2",
320+
"fake-catalog-test-package-bundle-1",
321+
"fake-catalog-test-package-2-bundle-15",
322+
"fake-catalog-test-package-2-bundle-16",
323+
"fake-catalog-test-package-2-bundle-17",
324+
"fake-catalog-some-package-bundle-5",
325+
"fake-catalog-some-package-bundle-4",
326326
})))
327327

328328
// check dependencies for one of the bundles
@@ -374,7 +374,7 @@ var _ = Describe("BundlesAndDepsVariableSource", func() {
374374
)
375375
_, err := bdvs.GetVariables(context.TODO())
376376
Expect(err).To(HaveOccurred())
377-
Expect(err.Error()).To(ContainSubstring("could not determine dependencies for bundle with id 'fake-catalog-test-package-stable-bundle-2': could not find package dependencies for bundle 'bundle-2'"))
377+
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'"))
378378
})
379379

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

‎internal/resolution/variablesources/crd_constraints.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,14 @@ func (g *CRDUniquenessConstraintsVariableSource) GetVariables(ctx context.Contex
5050
bundles := []*catalogmetadata.Bundle{v.Bundle()}
5151
bundles = append(bundles, v.Dependencies()...)
5252
for _, bundle := range bundles {
53-
for _, id := range olmvariables.BundleToBundleVariableIDs(bundle) {
54-
// get bundleID package and update map
55-
packageName := bundle.Package
53+
id := olmvariables.BundleVariableID(bundle)
54+
// get bundleID package and update map
55+
packageName := bundle.Package
5656

57-
if _, ok := pkgToBundleMap[packageName]; !ok {
58-
pkgToBundleMap[packageName] = map[deppy.Identifier]struct{}{}
59-
}
60-
pkgToBundleMap[packageName][id] = struct{}{}
57+
if _, ok := pkgToBundleMap[packageName]; !ok {
58+
pkgToBundleMap[packageName] = map[deppy.Identifier]struct{}{}
6159
}
60+
pkgToBundleMap[packageName][id] = struct{}{}
6261
}
6362
}
6463
}

‎internal/resolution/variablesources/crd_constraints_test.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ var _ = Describe("CRDUniquenessConstraintsVariableSource", func() {
203203
bundleSet["bundle-15"],
204204
bundleSet["bundle-16"],
205205
}),
206-
olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-2"])[0],
206+
olmvariables.NewBundleVariable(
207207
bundleSet["bundle-2"],
208208
[]*catalogmetadata.Bundle{
209209
bundleSet["bundle-3"],
@@ -213,59 +213,59 @@ var _ = Describe("CRDUniquenessConstraintsVariableSource", func() {
213213
bundleSet["bundle-7"],
214214
},
215215
),
216-
olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-1"])[0],
216+
olmvariables.NewBundleVariable(
217217
bundleSet["bundle-1"],
218218
[]*catalogmetadata.Bundle{
219219
bundleSet["bundle-6"],
220220
bundleSet["bundle-7"],
221221
bundleSet["bundle-8"],
222222
},
223223
),
224-
olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-3"])[0],
224+
olmvariables.NewBundleVariable(
225225
bundleSet["bundle-3"],
226226
[]*catalogmetadata.Bundle{},
227227
),
228-
olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-4"])[0],
228+
olmvariables.NewBundleVariable(
229229
bundleSet["bundle-4"],
230230
[]*catalogmetadata.Bundle{},
231231
),
232-
olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-5"])[0],
232+
olmvariables.NewBundleVariable(
233233
bundleSet["bundle-5"],
234234
[]*catalogmetadata.Bundle{},
235235
),
236-
olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-6"])[0],
236+
olmvariables.NewBundleVariable(
237237
bundleSet["bundle-6"],
238238
[]*catalogmetadata.Bundle{},
239239
),
240-
olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-7"])[0],
240+
olmvariables.NewBundleVariable(
241241
bundleSet["bundle-7"],
242242
[]*catalogmetadata.Bundle{
243243
bundleSet["bundle-8"],
244244
bundleSet["bundle-9"],
245245
bundleSet["bundle-10"],
246246
},
247247
),
248-
olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-8"])[0],
248+
olmvariables.NewBundleVariable(
249249
bundleSet["bundle-8"],
250250
[]*catalogmetadata.Bundle{},
251251
),
252-
olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-9"])[0],
252+
olmvariables.NewBundleVariable(
253253
bundleSet["bundle-9"],
254254
[]*catalogmetadata.Bundle{},
255255
),
256-
olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-10"])[0],
256+
olmvariables.NewBundleVariable(
257257
bundleSet["bundle-10"],
258258
[]*catalogmetadata.Bundle{},
259259
),
260-
olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-14"])[0],
260+
olmvariables.NewBundleVariable(
261261
bundleSet["bundle-14"],
262262
[]*catalogmetadata.Bundle{},
263263
),
264-
olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-15"])[0],
264+
olmvariables.NewBundleVariable(
265265
bundleSet["bundle-15"],
266266
[]*catalogmetadata.Bundle{},
267267
),
268-
olmvariables.NewBundleVariable(olmvariables.BundleToBundleVariableIDs(bundleSet["bundle-16"])[0],
268+
olmvariables.NewBundleVariable(
269269
bundleSet["bundle-16"],
270270
[]*catalogmetadata.Bundle{},
271271
),

0 commit comments

Comments
 (0)