Skip to content

Commit 62f0442

Browse files
committed
fix(resolver): Exclude all installed packages in dependency search
Currently, resolver only excludes one installed package at a time during dependency search. However, this is not enough as there is a corner case where there are two operators that are requiring the same CRD(s). As a result, resolver only excludes one package but not the other(s). Now, the list of installed packages in the same namespace will be added to list of packages that are not considered during dependency search. Signed-off-by: Vu Dinh <[email protected]>
1 parent e43db06 commit 62f0442

File tree

6 files changed

+30
-24
lines changed

6 files changed

+30
-24
lines changed

pkg/controller/registry/registry_client.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ type ChannelEntryStream interface {
2121
// ClientInterface that extends client.Interface
2222
type ClientInterface interface {
2323
client.Interface
24-
FindBundleThatProvides(ctx context.Context, group, version, kind, excludedPkgName string) (*registryapi.Bundle, error)
24+
FindBundleThatProvides(ctx context.Context, group, version, kind string, excludedPackages map[string]struct{}) (*registryapi.Bundle, error)
2525
GetLatestChannelEntriesThatProvide(ctx context.Context, group, version, kind string) (*ChannelEntryIterator, error)
2626
}
2727

@@ -81,14 +81,14 @@ func (rc *Client) GetLatestChannelEntriesThatProvide(ctx context.Context, group,
8181

8282
// FindBundleThatProvides returns a bundle that provides the request API and
8383
// doesn't belong to the provided package
84-
func (rc *Client) FindBundleThatProvides(ctx context.Context, group, version, kind, excludedPkgName string) (*registryapi.Bundle, error) {
84+
func (rc *Client) FindBundleThatProvides(ctx context.Context, group, version, kind string, excludedPackages map[string]struct{}) (*registryapi.Bundle, error) {
8585
it, err := rc.GetLatestChannelEntriesThatProvide(ctx, group, version, kind)
8686
if err != nil {
8787
return nil, err
8888
}
89-
entry := rc.filterChannelEntries(ctx, it, excludedPkgName)
89+
entry := rc.filterChannelEntries(ctx, it, excludedPackages)
9090
if entry == nil {
91-
return nil, fmt.Errorf("Unable to find a channel entry which doesn't belong to package %s's default channel", excludedPkgName)
91+
return nil, fmt.Errorf("Unable to find a channel entry that satisfies the requirements")
9292
}
9393
bundle, err := rc.Client.Registry.GetBundle(ctx, &registryapi.GetBundleRequest{PkgName: entry.PackageName, ChannelName: entry.ChannelName, CsvName: entry.BundleName})
9494
if err != nil {
@@ -100,12 +100,12 @@ func (rc *Client) FindBundleThatProvides(ctx context.Context, group, version, ki
100100
// FilterChannelEntries filters out channel entries that provide the requested
101101
// API and come from the same package with original operator and returns the
102102
// first entry on the list from the default channel of that package
103-
func (rc *Client) filterChannelEntries(ctx context.Context, it *ChannelEntryIterator, excludedPkgName string) *opregistry.ChannelEntry {
103+
func (rc *Client) filterChannelEntries(ctx context.Context, it *ChannelEntryIterator, excludedPackages map[string]struct{}) *opregistry.ChannelEntry {
104104
defChannels := make(map[string]string, 0)
105105

106106
var entries []*opregistry.ChannelEntry
107107
for e := it.Next(); e != nil; e = it.Next() {
108-
if e.PackageName != excludedPkgName {
108+
if _, ok := excludedPackages[e.PackageName]; !ok {
109109
// keep track of the default channel for each package
110110
if _, ok := defChannels[e.PackageName]; !ok {
111111
defChannel, err := rc.getDefaultPackageChannel(ctx, e.PackageName)

pkg/controller/registry/resolver/evolver.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,14 @@ func (e *NamespaceGenerationEvolver) queryForRequiredAPIs() error {
132132
initialSource = operator.SourceInfo()
133133
break
134134
}
135+
// Get the list of installed operators in the namespace
136+
opList := make(map[string]struct{})
137+
for _, operator := range e.gen.Operators() {
138+
opList[operator.SourceInfo().Package] = struct{}{}
139+
}
135140

136141
// attempt to find a bundle that provides that api
137-
if bundle, key, err := e.querier.FindProvider(*api, initialSource.Catalog, initialSource.Package); err == nil {
142+
if bundle, key, err := e.querier.FindProvider(*api, initialSource.Catalog, opList); err == nil {
138143
// add a bundle that provides the api to the generation
139144
o, err := NewOperatorFromBundle(bundle, "", *key)
140145
if err != nil {

pkg/controller/registry/resolver/fakes/fake_registry_client_interface.go

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/controller/registry/resolver/querier.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ type SourceRef struct {
2828
}
2929

3030
type SourceQuerier interface {
31-
FindProvider(api opregistry.APIKey, initialSource CatalogKey, excludedPkgName string) (*api.Bundle, *CatalogKey, error)
31+
FindProvider(api opregistry.APIKey, initialSource CatalogKey, excludedPackages map[string]struct{}) (*api.Bundle, *CatalogKey, error)
3232
FindBundle(pkgName, channelName, bundleName string, initialSource CatalogKey) (*api.Bundle, *CatalogKey, error)
3333
FindLatestBundle(pkgName, channelName string, initialSource CatalogKey) (*api.Bundle, *CatalogKey, error)
3434
FindReplacement(currentVersion *semver.Version, bundleName, pkgName, channelName string, initialSource CatalogKey) (*api.Bundle, *CatalogKey, error)
@@ -54,23 +54,23 @@ func (q *NamespaceSourceQuerier) Queryable() error {
5454
return nil
5555
}
5656

57-
func (q *NamespaceSourceQuerier) FindProvider(api opregistry.APIKey, initialSource CatalogKey, excludedPkgName string) (*registryapi.Bundle, *CatalogKey, error) {
57+
func (q *NamespaceSourceQuerier) FindProvider(api opregistry.APIKey, initialSource CatalogKey, excludedPackages map[string]struct{}) (*registryapi.Bundle, *CatalogKey, error) {
5858
if initialSource.Name != "" && initialSource.Namespace != "" {
5959
source, ok := q.sources[initialSource]
6060
if ok {
61-
if bundle, err := source.FindBundleThatProvides(context.TODO(), api.Group, api.Version, api.Kind, excludedPkgName); err == nil {
61+
if bundle, err := source.FindBundleThatProvides(context.TODO(), api.Group, api.Version, api.Kind, excludedPackages); err == nil {
6262
return bundle, &initialSource, nil
6363
}
64-
if bundle, err := source.FindBundleThatProvides(context.TODO(), api.Plural+"."+api.Group, api.Version, api.Kind, excludedPkgName); err == nil {
64+
if bundle, err := source.FindBundleThatProvides(context.TODO(), api.Plural+"."+api.Group, api.Version, api.Kind, excludedPackages); err == nil {
6565
return bundle, &initialSource, nil
6666
}
6767
}
6868
}
6969
for key, source := range q.sources {
70-
if bundle, err := source.FindBundleThatProvides(context.TODO(), api.Group, api.Version, api.Kind, excludedPkgName); err == nil {
70+
if bundle, err := source.FindBundleThatProvides(context.TODO(), api.Group, api.Version, api.Kind, excludedPackages); err == nil {
7171
return bundle, &key, nil
7272
}
73-
if bundle, err := source.FindBundleThatProvides(context.TODO(), api.Plural+"."+api.Group, api.Version, api.Kind, excludedPkgName); err == nil {
73+
if bundle, err := source.FindBundleThatProvides(context.TODO(), api.Plural+"."+api.Group, api.Version, api.Kind, excludedPackages); err == nil {
7474
return bundle, &key, nil
7575
}
7676
}

pkg/controller/registry/resolver/querier_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ func TestNamespaceSourceQuerier_FindProvider(t *testing.T) {
121121
CatalogKey{"test", "ns"}: &fakeSource,
122122
CatalogKey{"test2", "ns"}: &fakeSource2,
123123
}
124+
excludedPkgs := make(map[string]struct{})
124125
bundle := &api.Bundle{CsvName: "test", PackageName: "testPkg", ChannelName: "testChannel"}
125126
bundle2 := &api.Bundle{CsvName: "test2", PackageName: "testPkg2", ChannelName: "testChannel2"}
126127
fakeSource.GetBundleThatProvidesStub = func(ctx context.Context, group, version, kind string) (*api.Bundle, error) {
@@ -135,13 +136,13 @@ func TestNamespaceSourceQuerier_FindProvider(t *testing.T) {
135136
}
136137
return bundle2, nil
137138
}
138-
fakeSource.FindBundleThatProvidesStub = func(ctx context.Context, group, version, kind, pkgName string) (*api.Bundle, error) {
139+
fakeSource.FindBundleThatProvidesStub = func(ctx context.Context, group, version, kind string, excludedPkgs map[string]struct{}) (*api.Bundle, error) {
139140
if group != "group" || version != "version" || kind != "kind" {
140141
return nil, fmt.Errorf("Not Found")
141142
}
142143
return bundle, nil
143144
}
144-
fakeSource2.FindBundleThatProvidesStub = func(ctx context.Context, group, version, kind, pkgName string) (*api.Bundle, error) {
145+
fakeSource2.FindBundleThatProvidesStub = func(ctx context.Context, group, version, kind string, excludedPkgs map[string]struct{}) (*api.Bundle, error) {
145146
if group != "group2" || version != "version2" || kind != "kind2" {
146147
return nil, fmt.Errorf("Not Found")
147148
}
@@ -228,7 +229,7 @@ func TestNamespaceSourceQuerier_FindProvider(t *testing.T) {
228229
q := &NamespaceSourceQuerier{
229230
sources: tt.fields.sources,
230231
}
231-
bundle, key, err := q.FindProvider(tt.args.api, tt.args.catalogKey, "")
232+
bundle, key, err := q.FindProvider(tt.args.api, tt.args.catalogKey, excludedPkgs)
232233
require.Equal(t, tt.out.err, err)
233234
require.Equal(t, tt.out.bundle, bundle)
234235
require.Equal(t, tt.out.key, key)

pkg/controller/registry/resolver/util_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,15 +360,15 @@ func NewFakeSourceQuerier(bundlesByCatalog map[CatalogKey][]*api.Bundle) *Namesp
360360
return nil, fmt.Errorf("no bundle found")
361361
}
362362

363-
source.FindBundleThatProvidesStub = func(ctx context.Context, groupOrName, version, kind, pkgName string) (*api.Bundle, error) {
363+
source.FindBundleThatProvidesStub = func(ctx context.Context, groupOrName, version, kind string, excludedPkgs map[string]struct{}) (*api.Bundle, error) {
364364
bundles, ok := bundlesByCatalog[catKey]
365365
if !ok {
366366
return nil, fmt.Errorf("API (%s/%s/%s) not provided by a package in %s CatalogSource", groupOrName, version, kind, catKey)
367367
}
368368
sortedBundles := SortBundleInPackageChannel(bundles)
369369
for k, v := range sortedBundles {
370370
pkgname := getPkgName(k)
371-
if pkgname == pkgName {
371+
if _, ok := excludedPkgs[pkgname]; ok {
372372
continue
373373
}
374374

0 commit comments

Comments
 (0)