Skip to content

Commit fd9577f

Browse files
committed
Add system constraint provider logic to resolver and add step resolver init hooks
Signed-off-by: Per G. da Silva <[email protected]>
1 parent 7a9e1af commit fd9577f

File tree

6 files changed

+193
-9
lines changed

6 files changed

+193
-9
lines changed
Lines changed: 9 additions & 0 deletions
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

Lines changed: 26 additions & 8 deletions
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 {
@@ -175,6 +176,23 @@ func (r *SatResolver) SolveOperators(namespaces []string, csvs []*v1alpha1.Clust
175176
return operators, nil
176177
}
177178

179+
func (r *SatResolver) toBundleInstallable(entry *cache.Entry) (*BundleInstallable, error) {
180+
bundleInstalleble, err := NewBundleInstallableFromOperator(entry)
181+
if err != nil {
182+
return nil, err
183+
}
184+
185+
// apply system constraints if necessary
186+
if r.systemConstraintsProvider != nil && !(entry.SourceInfo.Catalog.Virtual()) {
187+
systemConstraints, err := r.systemConstraintsProvider.Constraints(entry)
188+
if err != nil {
189+
return nil, err
190+
}
191+
bundleInstalleble.constraints = append(bundleInstalleble.constraints, systemConstraints...)
192+
}
193+
return &bundleInstalleble, nil
194+
}
195+
178196
func (r *SatResolver) getSubscriptionInstallables(sub *v1alpha1.Subscription, current *cache.Entry, namespacedCache cache.MultiCatalogOperatorFinder, visited map[*cache.Entry]*BundleInstallable) (map[solver.Identifier]solver.Installable, error) {
179197
var cachePredicates, channelPredicates []cache.Predicate
180198
installables := make(map[solver.Identifier]solver.Installable, 0)
@@ -332,13 +350,13 @@ func (r *SatResolver) getBundleInstallables(preferredNamespace string, bundleSta
332350
continue
333351
}
334352

335-
bundleInstallable, err := NewBundleInstallableFromOperator(bundle)
353+
bundleInstallable, err := r.toBundleInstallable(bundle)
336354
if err != nil {
337355
errs = append(errs, err)
338356
continue
339357
}
340358

341-
visited[bundle] = &bundleInstallable
359+
visited[bundle] = bundleInstallable
342360

343361
dependencyPredicates, err := r.pc.convertDependencyProperties(bundle.Properties)
344362
if err != nil {
@@ -387,12 +405,12 @@ func (r *SatResolver) getBundleInstallables(preferredNamespace string, bundleSta
387405
// (after sorting) to remove all bundles that
388406
// don't satisfy the dependency.
389407
for _, b := range cache.Filter(sortedBundles, d) {
390-
i, err := NewBundleInstallableFromOperator(b)
408+
i, err := r.toBundleInstallable(b)
391409
if err != nil {
392410
errs = append(errs, err)
393411
continue
394412
}
395-
installables[i.Identifier()] = &i
413+
installables[i.Identifier()] = i
396414
bundleDependencies = append(bundleDependencies, i.Identifier())
397415
bundleStack = append(bundleStack, b)
398416
}
@@ -402,7 +420,7 @@ func (r *SatResolver) getBundleInstallables(preferredNamespace string, bundleSta
402420
))
403421
}
404422

405-
installables[bundleInstallable.Identifier()] = &bundleInstallable
423+
installables[bundleInstallable.Identifier()] = bundleInstallable
406424
}
407425

408426
if len(errs) > 0 {

pkg/controller/registry/resolver/resolver_test.go

Lines changed: 103 additions & 0 deletions
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}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
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+
type ConstraintProvider interface {
7+
// Constraints returns a set of solver constraints for a cache entry.
8+
Constraints(e *cache.Entry) ([]Constraint, error)
9+
}
10+
11+
type ConstraintProviderFunc func(e *cache.Entry) ([]Constraint, error)
12+
13+
func (c ConstraintProviderFunc) Constraints(e *cache.Entry) ([]Constraint, error) {
14+
return c(e)
15+
}

pkg/controller/registry/resolver/step_resolver.go

Lines changed: 13 additions & 1 deletion
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

Lines changed: 27 additions & 0 deletions
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)