Skip to content

Fix resolution error if inner entry doesn't provide a required API. #2104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions pkg/controller/registry/resolver/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,12 @@ func WithLabel(label string) OperatorPredicate {
})
}

func WithCatalog(key registry.CatalogKey) OperatorPredicate {
return OperatorPredicateFunc(func(o *Operator) bool {
return key.Equal(o.SourceInfo().Catalog)
})
}

func ProvidingAPI(api opregistry.APIKey) OperatorPredicate {
return OperatorPredicateFunc(func(o *Operator) bool {
for _, p := range o.Properties() {
Expand Down Expand Up @@ -611,6 +617,12 @@ func True() OperatorPredicate {
})
}

func False() OperatorPredicate {
return OperatorPredicateFunc(func(*Operator) bool {
return false
})
}

func CountingPredicate(p OperatorPredicate, n *int) OperatorPredicate {
return OperatorPredicateFunc(func(o *Operator) bool {
if p.Test(o) {
Expand Down
46 changes: 41 additions & 5 deletions pkg/controller/registry/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,15 +339,38 @@ func (r *SatResolver) getBundleInstallables(catalog registry.CatalogKey, predica
}

for _, d := range dependencyPredicates {
// errors ignored; this will build an empty/unsatisfiable dependency if no candidates are found
candidateBundles, _ := AtLeast(1, namespacedCache.FindPreferred(&bundle.sourceInfo.Catalog, d))
sortedBundles, err := r.sortBundles(candidateBundles)
sourcePredicate := False()
// Build a filter matching all (catalog,
// package, channel) combinations that contain
// at least one candidate bundle, even if only
// a subset of those bundles actually satisfy
// the dependency.
sources := map[OperatorSourceInfo]struct{}{}
for _, b := range namespacedCache.Find(d) {
si := b.SourceInfo()

if _, ok := sources[*si]; ok {
// Predicate already covers this source.
continue
}
sources[*si] = struct{}{}

sourcePredicate = Or(sourcePredicate, And(
WithPackage(si.Package),
WithChannel(si.Channel),
WithCatalog(si.Catalog),
))
}
sortedBundles, err := r.sortBundles(namespacedCache.FindPreferred(&bundle.sourceInfo.Catalog, sourcePredicate))
if err != nil {
errs = append(errs, err)
continue
}
bundleDependencies := make([]solver.Identifier, 0)
for _, b := range sortedBundles {
// The dependency predicate is applied here
// (after sorting) to remove all bundles that
// don't satisfy the dependency.
for _, b := range Filter(sortedBundles, d) {
src := b.SourceInfo()
if src == nil {
err := fmt.Errorf("unable to resolve the source of bundle %s, invalid cache", bundle.Identifier())
Expand All @@ -360,7 +383,6 @@ func (r *SatResolver) getBundleInstallables(catalog registry.CatalogKey, predica
bundleDependencies = append(bundleDependencies, i.Identifier())
bundleStack = append(bundleStack, b)
}

bundleInstallable.AddDependency(bundleDependencies)
}

Expand Down Expand Up @@ -469,6 +491,20 @@ func (r *SatResolver) newSnapshotForNamespace(namespace string, subs []*v1alpha1
op.sourceInfo = &OperatorSourceInfo{
Catalog: existingOperatorCatalog,
}
// Try to determine source package name from properties and add to SourceInfo.
for _, p := range op.properties {
if p.Type != opregistry.PackageType {
continue
}
var pp opregistry.PackageProperty
err := json.Unmarshal([]byte(p.Value), &pp)
if err != nil {
r.log.Warnf("failed to unmarshal package property of csv %q: %w", csv.Name, err)
continue
}
op.sourceInfo.Package = pp.PackageName
}

standaloneOperators = append(standaloneOperators, op)

// all standalone operators are mandatory
Expand Down
2 changes: 2 additions & 0 deletions pkg/controller/registry/resolver/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1319,6 +1319,8 @@ func genOperator(name, version, replaces, pkg, channel, catalogName, catalogName
Namespace: catalogNamespace,
},
DefaultChannel: defaultChannel != "" && channel == defaultChannel,
Package: pkg,
Channel: channel,
},
providedAPIs: providedAPIs,
requiredAPIs: requiredAPIs,
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/registry/resolver/step_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,7 @@ func TestResolver(t *testing.T) {
steps, lookups, subs, err := resolver.ResolveSteps(namespace, nil)
if tt.out.solverError == nil {
if tt.out.errAssert == nil {
assert.Nil(t, err)
assert.NoError(t, err)
} else {
tt.out.errAssert(t, err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/registry/resolver/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ func bundle(name, pkg, channel, replaces string, providedCRDs, requiredCRDs, pro
RequiredApis: apiSetToGVK(requiredCRDs, requiredAPIServices),
Replaces: replaces,
Dependencies: apiSetToDependencies(requiredCRDs, requiredAPIServices),
Properties: append(apiSetToProperties(providedCRDs, providedAPIServices, false),
Properties: append(apiSetToProperties(providedCRDs, providedAPIServices, false),
packageNameToProperty(pkg, "0.0.0"),
),
}
Expand Down
70 changes: 70 additions & 0 deletions test/e2e/subscription_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,76 @@ var _ = Describe("Subscription", func() {
TearDown(testNamespace)
})

When("an entry in the middle of a channel does not provide a required GVK", func() {
var (
teardown func()
)

BeforeEach(func() {
teardown = func() {}

packages := []registry.PackageManifest{
{
PackageName: "dependency",
Channels: []registry.PackageChannel{
{Name: "channel-dependency", CurrentCSVName: "csv-dependency-3"},
},
DefaultChannelName: "channel-dependency",
},
{
PackageName: "root",
Channels: []registry.PackageChannel{
{Name: "channel-root", CurrentCSVName: "csv-root"},
},
DefaultChannelName: "channel-root",
},
}

crds := []apiextensions.CustomResourceDefinition{newCRD(genName("crd-"))}
csvs := []operatorsv1alpha1.ClusterServiceVersion{
newCSV("csv-dependency-1", testNamespace, "", semver.MustParse("1.0.0"), crds, nil, nil),
newCSV("csv-dependency-2", testNamespace, "csv-dependency-1", semver.MustParse("2.0.0"), nil, nil, nil),
newCSV("csv-dependency-3", testNamespace, "csv-dependency-2", semver.MustParse("3.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", "channel-root", "", operatorsv1alpha1.ApprovalAutomatic)
})

AfterEach(func() {
teardown()
})

It("should create a Subscription for the latest entry providing the required GVK", 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,
StartingCSV: in.Spec.StartingCSV,
}
},
Equal(operatorsv1alpha1.SubscriptionSpec{
CatalogSource: "test-catalog",
CatalogSourceNamespace: testNamespace,
Package: "dependency",
Channel: "channel-dependency",
StartingCSV: "csv-dependency-3",
}),
)))
})
})

// I. Creating a new subscription
// A. If package is not installed, creating a subscription should install latest version
It("creation if not installed", func() {
Expand Down