From f749c5a317a917e6e91625fef41e621437155040 Mon Sep 17 00:00:00 2001 From: Ben Luddy Date: Tue, 30 Mar 2021 12:36:24 -0400 Subject: [PATCH] Fix inconsistent dependency candidate order. After bundle dependency candidates are sorted, a second query is made to the catalog cache. This query filters by CSV name alone, so it's ambiguous when the same CSV name appears in multiple channels (or packages). The query appears to be redundant, so it has been removed. Signed-off-by: Ben Luddy --- pkg/controller/registry/resolver/resolver.go | 10 +-- test/e2e/subscription_e2e_test.go | 67 ++++++++++++++++++++ 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/pkg/controller/registry/resolver/resolver.go b/pkg/controller/registry/resolver/resolver.go index 30d05ed45c..0a82f6a979 100644 --- a/pkg/controller/registry/resolver/resolver.go +++ b/pkg/controller/registry/resolver/resolver.go @@ -324,15 +324,7 @@ func (r *SatResolver) getBundleInstallables(catalog registry.CatalogKey, predica continue } bundleDependencies := make([]solver.Identifier, 0) - for _, dep := range sortedBundles { - found := namespacedCache.Catalog(dep.SourceInfo().Catalog).Find(WithCSVName(dep.Identifier())) - if len(found) == 0 { - err := fmt.Errorf("couldn't find %s in %s", bundle.Identifier(), dep.sourceInfo.Catalog) - errs = append(errs, err) - r.log.Warnf("cache consistency error: %s not found in %s", bundle.Identifier(), dep.sourceInfo.Catalog) - continue - } - b := found[0] + for _, b := range sortedBundles { src := b.SourceInfo() if src == nil { err := fmt.Errorf("unable to resolve the source of bundle %s, invalid cache", bundle.Identifier()) diff --git a/test/e2e/subscription_e2e_test.go b/test/e2e/subscription_e2e_test.go index 6ee245c777..24582436be 100644 --- a/test/e2e/subscription_e2e_test.go +++ b/test/e2e/subscription_e2e_test.go @@ -2063,6 +2063,73 @@ var _ = Describe("Subscription", func() { }).ShouldNot(Succeed()) }) }) + + When("there exists a Subscription to an operator having dependency candidates in both default and nondefault channels", func() { + var ( + teardown func() + ) + + BeforeEach(func() { + teardown = func() {} + + packages := []registry.PackageManifest{ + { + PackageName: "dependency", + Channels: []registry.PackageChannel{ + {Name: "default", CurrentCSVName: "csv-dependency"}, + {Name: "nondefault", CurrentCSVName: "csv-dependency"}, + }, + DefaultChannelName: "default", + }, + { + PackageName: "root", + Channels: []registry.PackageChannel{ + {Name: "unimportant", CurrentCSVName: "csv-root"}, + }, + DefaultChannelName: "unimportant", + }, + } + + crds := []apiextensions.CustomResourceDefinition{newCRD(genName("crd-"))} + csvs := []operatorsv1alpha1.ClusterServiceVersion{ + newCSV("csv-dependency", testNamespace, "", semver.MustParse("1.0.0"), crds, nil, nil), + newCSV("csv-root", testNamespace, "", semver.MustParse("1.0.0"), nil, crds, nil), + } + + _, teardown = createInternalCatalogSource(ctx.Ctx().KubeClient(), ctx.Ctx().OperatorClient(), "test-catalog", testNamespace, packages, crds, csvs) + + createSubscriptionForCatalog(ctx.Ctx().OperatorClient(), testNamespace, "test-subscription", "test-catalog", "root", "unimportant", "", operatorsv1alpha1.ApprovalAutomatic) + }) + + AfterEach(func() { + teardown() + }) + + It("should create a Subscription using the candidate's default channel", func() { + Eventually(func() ([]operatorsv1alpha1.Subscription, error) { + var list operatorsv1alpha1.SubscriptionList + if err := ctx.Ctx().Client().List(context.TODO(), &list); err != nil { + return nil, err + } + return list.Items, nil + }).Should(ContainElement(WithTransform( + func(in operatorsv1alpha1.Subscription) operatorsv1alpha1.SubscriptionSpec { + return operatorsv1alpha1.SubscriptionSpec{ + CatalogSource: in.Spec.CatalogSource, + CatalogSourceNamespace: in.Spec.CatalogSourceNamespace, + Package: in.Spec.Package, + Channel: in.Spec.Channel, + } + }, + Equal(operatorsv1alpha1.SubscriptionSpec{ + CatalogSource: "test-catalog", + CatalogSourceNamespace: testNamespace, + Package: "dependency", + Channel: "default", + }), + ))) + }) + }) }) const (