Skip to content

Commit b414e6e

Browse files
perdasilvaperdasilva
authored
Add system constraint provider logic to resolver and add step resolver init hooks (#2523)
Signed-off-by: Per G. da Silva <[email protected]>
1 parent cabe200 commit b414e6e

File tree

6 files changed

+199
-9
lines changed

6 files changed

+199
-9
lines changed

‎pkg/controller/registry/resolver/hooks.go

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package resolver
2+
3+
// stepResolverInitHook provides a way for the downstream
4+
// to modify the step resolver at creation time.
5+
// This is a bit of a hack to enable system constraints downstream
6+
// without affecting the upstream. We may want to clean this up when
7+
// either we have a more pluggable architecture; or system constraints
8+
// come to the upstream
9+
type stepResolverInitHook func(*OperatorStepResolver) error

‎pkg/controller/registry/resolver/resolver.go

+28-8
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,10 @@ type OperatorResolver interface {
2626
}
2727

2828
type SatResolver struct {
29-
cache cache.OperatorCacheProvider
30-
log logrus.FieldLogger
31-
pc *predicateConverter
29+
cache cache.OperatorCacheProvider
30+
log logrus.FieldLogger
31+
pc *predicateConverter
32+
systemConstraintsProvider solver.ConstraintProvider
3233
}
3334

3435
func NewDefaultSatResolver(rcp cache.SourceProvider, catsrcLister v1alpha1listers.CatalogSourceLister, logger logrus.FieldLogger) *SatResolver {
@@ -177,6 +178,25 @@ func (r *SatResolver) SolveOperators(namespaces []string, csvs []*v1alpha1.Clust
177178
return operators, nil
178179
}
179180

181+
// newBundleInstallableFromEntry converts an entry into a bundle installable with
182+
// system constraints applied, if they are defined for the entry
183+
func (r *SatResolver) newBundleInstallableFromEntry(entry *cache.Entry) (*BundleInstallable, error) {
184+
bundleInstalleble, err := NewBundleInstallableFromOperator(entry)
185+
if err != nil {
186+
return nil, err
187+
}
188+
189+
// apply system constraints if necessary
190+
if r.systemConstraintsProvider != nil && !(entry.SourceInfo.Catalog.Virtual()) {
191+
systemConstraints, err := r.systemConstraintsProvider.Constraints(entry)
192+
if err != nil {
193+
return nil, err
194+
}
195+
bundleInstalleble.constraints = append(bundleInstalleble.constraints, systemConstraints...)
196+
}
197+
return &bundleInstalleble, nil
198+
}
199+
180200
func (r *SatResolver) getSubscriptionInstallables(sub *v1alpha1.Subscription, current *cache.Entry, namespacedCache cache.MultiCatalogOperatorFinder, visited map[*cache.Entry]*BundleInstallable) (map[solver.Identifier]solver.Installable, error) {
181201
var cachePredicates, channelPredicates []cache.Predicate
182202
installables := make(map[solver.Identifier]solver.Installable, 0)
@@ -334,13 +354,13 @@ func (r *SatResolver) getBundleInstallables(preferredNamespace string, bundleSta
334354
continue
335355
}
336356

337-
bundleInstallable, err := NewBundleInstallableFromOperator(bundle)
357+
bundleInstallable, err := r.newBundleInstallableFromEntry(bundle)
338358
if err != nil {
339359
errs = append(errs, err)
340360
continue
341361
}
342362

343-
visited[bundle] = &bundleInstallable
363+
visited[bundle] = bundleInstallable
344364

345365
dependencyPredicates, err := r.pc.convertDependencyProperties(bundle.Properties)
346366
if err != nil {
@@ -389,12 +409,12 @@ func (r *SatResolver) getBundleInstallables(preferredNamespace string, bundleSta
389409
// (after sorting) to remove all bundles that
390410
// don't satisfy the dependency.
391411
for _, b := range cache.Filter(sortedBundles, d) {
392-
i, err := NewBundleInstallableFromOperator(b)
412+
i, err := r.newBundleInstallableFromEntry(b)
393413
if err != nil {
394414
errs = append(errs, err)
395415
continue
396416
}
397-
installables[i.Identifier()] = &i
417+
installables[i.Identifier()] = i
398418
bundleDependencies = append(bundleDependencies, i.Identifier())
399419
bundleStack = append(bundleStack, b)
400420
}
@@ -404,7 +424,7 @@ func (r *SatResolver) getBundleInstallables(preferredNamespace string, bundleSta
404424
))
405425
}
406426

407-
installables[bundleInstallable.Identifier()] = &bundleInstallable
427+
installables[bundleInstallable.Identifier()] = bundleInstallable
408428
}
409429

410430
if len(errs) > 0 {

‎pkg/controller/registry/resolver/resolver_test.go

+103
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,109 @@ func TestSolveOperators(t *testing.T) {
6666
require.EqualValues(t, expected, operators)
6767
}
6868

69+
func TestSolveOperators_WithSystemConstraints(t *testing.T) {
70+
const namespace = "test-namespace"
71+
catalog := cache.SourceKey{Name: "test-catalog", Namespace: namespace}
72+
73+
packageASub := newSub(namespace, "packageA", "alpha", catalog)
74+
packageDSub := existingSub(namespace, "packageD.v1", "packageD", "alpha", catalog)
75+
76+
APISet := cache.APISet{opregistry.APIKey{Group: "g", Version: "v", Kind: "k", Plural: "ks"}: struct{}{}}
77+
78+
// packageA requires an API that can be provided by B or C
79+
packageA := genOperator("packageA.v1", "0.0.1", "", "packageA", "alpha", catalog.Name, catalog.Namespace, APISet, nil, nil, "", false)
80+
packageB := genOperator("packageB.v1", "1.0.0", "", "packageB", "alpha", catalog.Name, catalog.Namespace, nil, APISet, nil, "", false)
81+
packageC := genOperator("packageC.v1", "1.0.0", "", "packageC", "alpha", catalog.Name, catalog.Namespace, nil, APISet, nil, "", false)
82+
83+
// Existing operators
84+
packageD := genOperator("packageD.v1", "1.0.0", "", "packageD", "alpha", catalog.Name, catalog.Namespace, nil, nil, nil, "", false)
85+
existingPackageD := existingOperator(namespace, "packageD.v1", "packageD", "alpha", "", nil, nil, nil, nil)
86+
existingPackageD.Annotations = map[string]string{"operatorframework.io/properties": `{"properties":[{"type":"olm.package","value":{"packageName":"packageD","version":"1.0.0"}}]}`}
87+
88+
whiteListConstraintProvider := func(whiteList ...*cache.Entry) solver.ConstraintProviderFunc {
89+
return func(entry *cache.Entry) ([]solver.Constraint, error) {
90+
for _, whiteListedEntry := range whiteList {
91+
if whiteListedEntry.Package() == entry.Package() &&
92+
whiteListedEntry.Name == entry.Name &&
93+
whiteListedEntry.Version == entry.Version {
94+
return nil, nil
95+
}
96+
}
97+
return []solver.Constraint{PrettyConstraint(
98+
solver.Prohibited(),
99+
fmt.Sprintf("package: %s is not white listed", entry.Package()),
100+
)}, nil
101+
}
102+
}
103+
104+
testCases := []struct {
105+
title string
106+
systemConstraintsProvider solver.ConstraintProvider
107+
expectedOperators cache.OperatorSet
108+
csvs []*v1alpha1.ClusterServiceVersion
109+
subs []*v1alpha1.Subscription
110+
snapshotEntries []*cache.Entry
111+
err string
112+
}{
113+
{
114+
title: "No runtime constraints",
115+
snapshotEntries: []*cache.Entry{packageA, packageB, packageC, packageD},
116+
systemConstraintsProvider: nil,
117+
expectedOperators: cache.OperatorSet{"packageA.v1": packageA, "packageB.v1": packageB},
118+
csvs: nil,
119+
subs: []*v1alpha1.Subscription{packageASub},
120+
err: "",
121+
},
122+
{
123+
title: "Runtime constraints only accept packages A and C",
124+
snapshotEntries: []*cache.Entry{packageA, packageB, packageC, packageD},
125+
systemConstraintsProvider: whiteListConstraintProvider(packageA, packageC),
126+
expectedOperators: cache.OperatorSet{"packageA.v1": packageA, "packageC.v1": packageC},
127+
csvs: nil,
128+
subs: []*v1alpha1.Subscription{packageASub},
129+
err: "",
130+
},
131+
{
132+
title: "Existing packages are ignored",
133+
snapshotEntries: []*cache.Entry{packageA, packageB, packageC, packageD},
134+
systemConstraintsProvider: whiteListConstraintProvider(packageA, packageC),
135+
expectedOperators: cache.OperatorSet{"packageA.v1": packageA, "packageC.v1": packageC},
136+
csvs: []*v1alpha1.ClusterServiceVersion{existingPackageD},
137+
subs: []*v1alpha1.Subscription{packageASub, packageDSub},
138+
err: "",
139+
},
140+
{
141+
title: "Runtime constraints don't allow A",
142+
snapshotEntries: []*cache.Entry{packageA, packageB, packageC, packageD},
143+
systemConstraintsProvider: whiteListConstraintProvider(),
144+
expectedOperators: nil,
145+
csvs: nil,
146+
subs: []*v1alpha1.Subscription{packageASub},
147+
err: "packageA is not white listed",
148+
},
149+
}
150+
151+
for _, testCase := range testCases {
152+
satResolver := SatResolver{
153+
cache: cache.New(cache.StaticSourceProvider{
154+
catalog: &cache.Snapshot{
155+
Entries: testCase.snapshotEntries,
156+
},
157+
}),
158+
log: logrus.New(),
159+
systemConstraintsProvider: testCase.systemConstraintsProvider,
160+
}
161+
operators, err := satResolver.SolveOperators([]string{namespace}, testCase.csvs, testCase.subs)
162+
163+
if testCase.err != "" {
164+
require.Containsf(t, err.Error(), testCase.err, "Test %s failed", testCase.title)
165+
} else {
166+
require.NoErrorf(t, err, "Test %s failed", testCase.title)
167+
}
168+
require.EqualValuesf(t, testCase.expectedOperators, operators, "Test %s failed", testCase.title)
169+
}
170+
}
171+
69172
func TestDisjointChannelGraph(t *testing.T) {
70173
const namespace = "test-namespace"
71174
catalog := cache.SourceKey{Name: "test-catalog", Namespace: namespace}

‎pkg/controller/registry/resolver/solver/constraint_provider.go

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package solver
2+
3+
import "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/cache"
4+
5+
// ConstraintProvider knows how to provide solver constraints for a given cache entry.
6+
// For instance, it could be used to surface additional constraints against an entry given some
7+
// properties it may expose. E.g. olm.maxOpenShiftVersion could be checked against the cluster version
8+
// and prohibit any entry that doesn't meet the requirement
9+
type ConstraintProvider interface {
10+
// Constraints returns a set of solver constraints for a cache entry.
11+
Constraints(e *cache.Entry) ([]Constraint, error)
12+
}
13+
14+
// ConstraintProviderFunc is a simple implementation of ConstraintProvider
15+
type ConstraintProviderFunc func(e *cache.Entry) ([]Constraint, error)
16+
17+
func (c ConstraintProviderFunc) Constraints(e *cache.Entry) ([]Constraint, error) {
18+
return c(e)
19+
}

‎pkg/controller/registry/resolver/step_resolver.go

+13-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ const (
2626
BundleLookupConditionPacked v1alpha1.BundleLookupConditionType = "BundleLookupNotPersisted"
2727
)
2828

29+
// init hooks provides the downstream a way to modify the upstream behavior
30+
var initHooks []stepResolverInitHook
31+
2932
var timeNow = func() metav1.Time { return metav1.NewTime(time.Now().UTC()) }
3033

3134
type StepResolver interface {
@@ -48,7 +51,7 @@ var _ StepResolver = &OperatorStepResolver{}
4851

4952
func NewOperatorStepResolver(lister operatorlister.OperatorLister, client versioned.Interface, kubeclient kubernetes.Interface,
5053
globalCatalogNamespace string, provider RegistryClientProvider, log logrus.FieldLogger) *OperatorStepResolver {
51-
return &OperatorStepResolver{
54+
stepResolver := &OperatorStepResolver{
5255
subLister: lister.OperatorsV1alpha1().SubscriptionLister(),
5356
csvLister: lister.OperatorsV1alpha1().ClusterServiceVersionLister(),
5457
ipLister: lister.OperatorsV1alpha1().InstallPlanLister(),
@@ -58,6 +61,15 @@ func NewOperatorStepResolver(lister operatorlister.OperatorLister, client versio
5861
satResolver: NewDefaultSatResolver(SourceProviderFromRegistryClientProvider(provider, log), lister.OperatorsV1alpha1().CatalogSourceLister(), log),
5962
log: log,
6063
}
64+
65+
// init hooks can be added to the downstream to
66+
// modify resolver behaviour
67+
for _, initHook := range initHooks {
68+
if err := initHook(stepResolver); err != nil {
69+
panic(err)
70+
}
71+
}
72+
return stepResolver
6173
}
6274

6375
func (r *OperatorStepResolver) Expire(key cache.SourceKey) {

‎pkg/controller/registry/resolver/step_resolver_test.go

+27
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,33 @@ var (
4747
Requires4 = APISet4
4848
)
4949

50+
func TestInitHooks(t *testing.T) {
51+
clientFake := fake.NewSimpleClientset()
52+
lister := operatorlister.NewLister()
53+
kClientFake := k8sfake.NewSimpleClientset()
54+
log := logrus.New()
55+
56+
// no init hooks
57+
resolver := NewOperatorStepResolver(lister, clientFake, kClientFake, "", nil, log)
58+
require.NotNil(t, resolver.satResolver)
59+
60+
// with init hook
61+
var testHook stepResolverInitHook = func(resolver *OperatorStepResolver) error {
62+
resolver.satResolver = nil
63+
return nil
64+
}
65+
66+
// defined in step_resolver.go
67+
initHooks = append(initHooks, testHook)
68+
defer func() {
69+
// reset initHooks
70+
initHooks = nil
71+
}()
72+
73+
resolver = NewOperatorStepResolver(lister, clientFake, kClientFake, "", nil, log)
74+
require.Nil(t, resolver.satResolver)
75+
}
76+
5077
func TestResolver(t *testing.T) {
5178
const namespace = "catsrc-namespace"
5279
catalog := resolvercache.SourceKey{Name: "catsrc", Namespace: namespace}

0 commit comments

Comments
 (0)