Skip to content

Commit 67745af

Browse files
perdasilvaperdasilva
committed
Refactor SatResolver constructor
Signed-off-by: Per G. da Silva <[email protected]>
1 parent df687f4 commit 67745af

File tree

4 files changed

+108
-85
lines changed

4 files changed

+108
-85
lines changed

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

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,28 @@ type SatResolver struct {
3131
runtimeConstraintsProvider *runtime_constraints.RuntimeConstraintsProvider
3232
}
3333

34-
func NewDefaultSatResolver(rcp cache.SourceProvider, catsrcLister v1alpha1listers.CatalogSourceLister, logger logrus.FieldLogger) *SatResolver {
35-
runtimeConstraintProvider, err := runtime_constraints.NewFromEnv()
36-
if err != nil {
37-
logger.Errorf("Error creating runtime constraints from file: %s", err)
38-
panic(err)
39-
} else {
40-
logger.Info("Cluster runtime constraints are in effect")
34+
type SatSolverOption func(resolver *SatResolver)
35+
36+
func WithRuntimeConstraintsProvider(provider *runtime_constraints.RuntimeConstraintsProvider) SatSolverOption {
37+
return func(satSolver *SatResolver) {
38+
if satSolver != nil {
39+
satSolver.runtimeConstraintsProvider = provider
40+
}
41+
}
42+
}
43+
44+
func NewDefaultSatResolver(rcp cache.SourceProvider, catsrcLister v1alpha1listers.CatalogSourceLister, logger logrus.FieldLogger, opts ...SatSolverOption) *SatResolver {
45+
satSolver := &SatResolver{
46+
cache: cache.New(rcp, cache.WithLogger(logger), cache.WithCatalogSourceLister(catsrcLister)),
47+
log: logger,
4148
}
4249

43-
return &SatResolver{
44-
cache: cache.New(rcp, cache.WithLogger(logger), cache.WithCatalogSourceLister(catsrcLister)),
45-
log: logger,
46-
runtimeConstraintsProvider: runtimeConstraintProvider,
50+
// apply options
51+
for _, opt := range opts {
52+
opt(satSolver)
4753
}
54+
55+
return satSolver
4856
}
4957

5058
type debugWriter struct {

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

Lines changed: 0 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package resolver
33
import (
44
"errors"
55
"fmt"
6-
"os"
76
"testing"
87

98
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/runtime_constraints"
@@ -43,78 +42,6 @@ func (l *fakeCatalogSourceLister) CatalogSources(namespace string) listersv1alph
4342
return nil
4443
}
4544

46-
func TestNewDefaultSatResolver_NoClusterRuntimeConstraints(t *testing.T) {
47-
// Ensure no runtime constraints are loaded if the runtime constraints env
48-
// var is not set
49-
sourceProvider := &fakeSourceProvider{}
50-
catSrcLister := &fakeCatalogSourceLister{}
51-
logger := logrus.New()
52-
53-
// Unset the runtime constraints file path environment variable
54-
// signaling that no runtime constraints should be considered by the resolver
55-
require.Nil(t, os.Unsetenv(runtime_constraints.RuntimeConstraintEnvVarName))
56-
resolver := NewDefaultSatResolver(sourceProvider, catSrcLister, logger)
57-
require.Nil(t, resolver.runtimeConstraintsProvider)
58-
}
59-
60-
func TestNewDefaultSatResolver_BadClusterRuntimeConstraintsEnvVar(t *testing.T) {
61-
// Ensure TestNewDefaultSatResolver panics if the runtime constraints
62-
// environment variable does not point to an existing file or valid path
63-
sourceProvider := &fakeSourceProvider{}
64-
catSrcLister := &fakeCatalogSourceLister{}
65-
logger := logrus.New()
66-
t.Cleanup(func() { _ = os.Unsetenv(runtime_constraints.RuntimeConstraintEnvVarName) })
67-
68-
// This test expects a panic to happen
69-
defer func() {
70-
if r := recover(); r == nil {
71-
t.Errorf("The code did not panic")
72-
}
73-
}()
74-
75-
// Set the runtime constraints env var to something that isn't a valid filesystem path
76-
require.Nil(t, os.Setenv(runtime_constraints.RuntimeConstraintEnvVarName, "%#$%#$ %$#%#$%"))
77-
_ = NewDefaultSatResolver(sourceProvider, catSrcLister, logger)
78-
}
79-
80-
func TestNewDefaultSatResolver_BadClusterRuntimeConstraintsFile(t *testing.T) {
81-
// Ensure TestNewDefaultSatResolver panics if the runtime constraints
82-
// environment variable points to a poorly formatted runtime constraints file
83-
sourceProvider := &fakeSourceProvider{}
84-
catSrcLister := &fakeCatalogSourceLister{}
85-
logger := logrus.New()
86-
t.Cleanup(func() { _ = os.Unsetenv(runtime_constraints.RuntimeConstraintEnvVarName) })
87-
88-
// This test expects a panic to happen
89-
defer func() {
90-
if r := recover(); r == nil {
91-
t.Errorf("The code did not panic")
92-
}
93-
}()
94-
95-
runtimeConstraintsFilePath := "runtime_constraints/testdata/bad_runtime_constraints.json"
96-
// set the runtime constraints env var to something that isn't a valid filesystem path
97-
require.Nil(t, os.Setenv(runtime_constraints.RuntimeConstraintEnvVarName, runtimeConstraintsFilePath))
98-
_ = NewDefaultSatResolver(sourceProvider, catSrcLister, logger)
99-
}
100-
101-
func TestNewDefaultSatResolver_GoodClusterRuntimeConstraintsFile(t *testing.T) {
102-
// Ensure TestNewDefaultSatResolver loads the runtime constraints
103-
// defined in a well formatted file point to by the runtime constraints env var
104-
sourceProvider := &fakeSourceProvider{}
105-
catSrcLister := &fakeCatalogSourceLister{}
106-
logger := logrus.New()
107-
t.Cleanup(func() { _ = os.Unsetenv(runtime_constraints.RuntimeConstraintEnvVarName) })
108-
109-
runtimeConstraintsFilePath := "runtime_constraints/testdata/runtime_constraints.json"
110-
// set the runtime constraints env var to something that isn't a valid filesystem path
111-
require.Nil(t, os.Setenv(runtime_constraints.RuntimeConstraintEnvVarName, runtimeConstraintsFilePath))
112-
resolver := NewDefaultSatResolver(sourceProvider, catSrcLister, logger)
113-
runtimeConstraints := resolver.runtimeConstraintsProvider.Constraints()
114-
require.Len(t, runtimeConstraints, 1)
115-
require.Equal(t, "with package: etcd", runtimeConstraints[0].String())
116-
}
117-
11845
func TestSolveOperators(t *testing.T) {
11946
APISet := cache.APISet{opregistry.APIKey{Group: "g", Version: "v", Kind: "k", Plural: "ks"}: struct{}{}}
12047
Provides := APISet

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"fmt"
77
"time"
88

9+
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/runtime_constraints"
10+
911
"github.com/sirupsen/logrus"
1012
corev1 "k8s.io/api/core/v1"
1113
"k8s.io/apimachinery/pkg/api/errors"
@@ -48,14 +50,29 @@ var _ StepResolver = &OperatorStepResolver{}
4850

4951
func NewOperatorStepResolver(lister operatorlister.OperatorLister, client versioned.Interface, kubeclient kubernetes.Interface,
5052
globalCatalogNamespace string, provider RegistryClientProvider, log logrus.FieldLogger) *OperatorStepResolver {
53+
54+
runtimeConstraintProvider, err := runtime_constraints.NewFromEnv()
55+
if err != nil {
56+
log.Errorf("Error creating runtime constraints from file: %s", err)
57+
panic(err)
58+
} else {
59+
log.Info("Cluster runtime constraints are in effect")
60+
}
61+
62+
satResolver := NewDefaultSatResolver(
63+
SourceProviderFromRegistryClientProvider(provider, log),
64+
lister.OperatorsV1alpha1().CatalogSourceLister(),
65+
log,
66+
WithRuntimeConstraintsProvider(runtimeConstraintProvider))
67+
5168
return &OperatorStepResolver{
5269
subLister: lister.OperatorsV1alpha1().SubscriptionLister(),
5370
csvLister: lister.OperatorsV1alpha1().ClusterServiceVersionLister(),
5471
ipLister: lister.OperatorsV1alpha1().InstallPlanLister(),
5572
client: client,
5673
kubeclient: kubeclient,
5774
globalCatalogNamespace: globalCatalogNamespace,
58-
satResolver: NewDefaultSatResolver(SourceProviderFromRegistryClientProvider(provider, log), lister.OperatorsV1alpha1().CatalogSourceLister(), log),
75+
satResolver: satResolver,
5976
log: log,
6077
}
6178
}

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@ package resolver
22

33
import (
44
"fmt"
5+
"os"
56
"strings"
67
"testing"
78
"time"
89

10+
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/runtime_constraints"
11+
912
"github.com/sirupsen/logrus"
1013
"github.com/stretchr/testify/assert"
1114
"github.com/stretchr/testify/require"
@@ -47,6 +50,74 @@ var (
4750
Requires4 = APISet4
4851
)
4952

53+
func TestNewOperatorStepResolver_NoClusterRuntimeConstraints(t *testing.T) {
54+
// Ensure no runtime constraints are loaded if the runtime constraints env
55+
// var is not set
56+
logger := logrus.New()
57+
lister := operatorlister.NewLister()
58+
59+
// Unset the runtime constraints file path environment variable
60+
// signaling that no runtime constraints should be considered by the resolver
61+
require.Nil(t, os.Unsetenv(runtime_constraints.RuntimeConstraintEnvVarName))
62+
resolver := NewOperatorStepResolver(lister, nil, nil, "", nil, logger)
63+
require.Nil(t, resolver.satResolver.runtimeConstraintsProvider)
64+
}
65+
66+
func TestNewOperatorStepResolver_BadClusterRuntimeConstraintsEnvVar(t *testing.T) {
67+
// Ensure TestNewDefaultSatResolver panics if the runtime constraints
68+
// environment variable does not point to an existing file or valid path
69+
lister := operatorlister.NewLister()
70+
logger := logrus.New()
71+
t.Cleanup(func() { _ = os.Unsetenv(runtime_constraints.RuntimeConstraintEnvVarName) })
72+
73+
// This test expects a panic to happen
74+
defer func() {
75+
if r := recover(); r == nil {
76+
t.Errorf("The code did not panic")
77+
}
78+
}()
79+
80+
// Set the runtime constraints env var to something that isn't a valid filesystem path
81+
require.Nil(t, os.Setenv(runtime_constraints.RuntimeConstraintEnvVarName, "%#$%#$ %$#%#$%"))
82+
_ = NewOperatorStepResolver(lister, nil, nil, "", nil, logger)
83+
}
84+
85+
func TestNewOperatorStepResolver_BadClusterRuntimeConstraintsFile(t *testing.T) {
86+
// Ensure TestNewDefaultSatResolver panics if the runtime constraints
87+
// environment variable points to a poorly formatted runtime constraints file
88+
lister := operatorlister.NewLister()
89+
logger := logrus.New()
90+
t.Cleanup(func() { _ = os.Unsetenv(runtime_constraints.RuntimeConstraintEnvVarName) })
91+
92+
// This test expects a panic to happen
93+
defer func() {
94+
if r := recover(); r == nil {
95+
t.Errorf("The code did not panic")
96+
}
97+
}()
98+
99+
runtimeConstraintsFilePath := "runtime_constraints/testdata/bad_runtime_constraints.json"
100+
// set the runtime constraints env var to something that isn't a valid filesystem path
101+
require.Nil(t, os.Setenv(runtime_constraints.RuntimeConstraintEnvVarName, runtimeConstraintsFilePath))
102+
_ = NewOperatorStepResolver(lister, nil, nil, "", nil, logger)
103+
}
104+
105+
func TestNewOperatorStepResolver_GoodClusterRuntimeConstraintsFile(t *testing.T) {
106+
// Ensure TestNewDefaultSatResolver loads the runtime constraints
107+
// defined in a well formatted file point to by the runtime constraints env var
108+
lister := operatorlister.NewLister()
109+
logger := logrus.New()
110+
t.Cleanup(func() { _ = os.Unsetenv(runtime_constraints.RuntimeConstraintEnvVarName) })
111+
112+
runtimeConstraintsFilePath := "runtime_constraints/testdata/runtime_constraints.json"
113+
// set the runtime constraints env var to something that isn't a valid filesystem path
114+
require.Nil(t, os.Setenv(runtime_constraints.RuntimeConstraintEnvVarName, runtimeConstraintsFilePath))
115+
resolver := NewOperatorStepResolver(lister, nil, nil, "", nil, logger)
116+
runtimeConstraints := resolver.satResolver.runtimeConstraintsProvider.Constraints()
117+
require.Len(t, runtimeConstraints, 1)
118+
require.Equal(t, "with package: etcd", runtimeConstraints[0].String())
119+
}
120+
50121
func TestResolver(t *testing.T) {
51122
const namespace = "catsrc-namespace"
52123
catalog := resolvercache.SourceKey{Name: "catsrc", Namespace: namespace}

0 commit comments

Comments
 (0)