Skip to content

Commit 6988cbe

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #860 from tkashem/refactor
(refactor) Move csv set and replace to a package
2 parents f370461 + 6fa553d commit 6988cbe

File tree

4 files changed

+194
-58
lines changed

4 files changed

+194
-58
lines changed

‎pkg/controller/operators/olm/operator.go

Lines changed: 31 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil"
3737
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer"
3838
"github.com/operator-framework/operator-lifecycle-manager/pkg/metrics"
39+
csvutility "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/csv"
3940
)
4041

4142
var (
@@ -64,6 +65,8 @@ type Operator struct {
6465
gcQueueIndexer *queueinformer.QueueIndexer
6566
apiLabeler labeler.Labeler
6667
csvIndexers map[string]cache.Indexer
68+
csvSetGenerator csvutility.SetGenerator
69+
csvReplaceFinder csvutility.ReplaceFinder
6770
}
6871

6972
func NewOperator(logger *logrus.Logger, crClient versioned.Interface, opClient operatorclient.ClientInterface, strategyResolver install.StrategyResolverInterface, wakeupInterval time.Duration, namespaces []string) (*Operator, error) {
@@ -83,17 +86,24 @@ func NewOperator(logger *logrus.Logger, crClient versioned.Interface, opClient o
8386
return nil, err
8487
}
8588

89+
lister := operatorlister.NewLister()
90+
csvSetGenerator := csvutility.NewSetGenerator(logger, lister)
91+
92+
csvReplaceFinder := csvutility.NewReplaceFinder(logger, crClient)
93+
8694
op := &Operator{
87-
Operator: queueOperator,
88-
csvQueueSet: queueinformer.NewEmptyResourceQueueSet(),
89-
ogQueueSet: queueinformer.NewEmptyResourceQueueSet(),
90-
client: crClient,
91-
resolver: strategyResolver,
92-
apiReconciler: resolver.APIIntersectionReconcileFunc(resolver.ReconcileAPIIntersection),
93-
lister: operatorlister.NewLister(),
94-
recorder: eventRecorder,
95-
apiLabeler: labeler.Func(resolver.LabelSetsFor),
96-
csvIndexers: map[string]cache.Indexer{},
95+
Operator: queueOperator,
96+
csvQueueSet: queueinformer.NewEmptyResourceQueueSet(),
97+
ogQueueSet: queueinformer.NewEmptyResourceQueueSet(),
98+
client: crClient,
99+
resolver: strategyResolver,
100+
apiReconciler: resolver.APIIntersectionReconcileFunc(resolver.ReconcileAPIIntersection),
101+
lister: lister,
102+
recorder: eventRecorder,
103+
apiLabeler: labeler.Func(resolver.LabelSetsFor),
104+
csvIndexers: map[string]cache.Indexer{},
105+
csvSetGenerator: csvSetGenerator,
106+
csvReplaceFinder: csvReplaceFinder,
97107
}
98108

99109
// Set up RBAC informers
@@ -352,6 +362,14 @@ func (a *Operator) syncAPIService(obj interface{}) (syncError error) {
352362
return nil
353363
}
354364

365+
func (a *Operator) GetCSVSetGenerator() csvutility.SetGenerator {
366+
return a.csvSetGenerator
367+
}
368+
369+
func (a *Operator) GetReplaceFinder() csvutility.ReplaceFinder {
370+
return a.csvReplaceFinder
371+
}
372+
355373
func (a *Operator) syncObject(obj interface{}) (syncError error) {
356374
// Assert as metav1.Object
357375
metaObj, ok := obj.(metav1.Object)
@@ -1171,21 +1189,7 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v
11711189

11721190
// csvSet gathers all CSVs in the given namespace into a map keyed by CSV name; if metav1.NamespaceAll gets the set across all namespaces
11731191
func (a *Operator) csvSet(namespace string, phase v1alpha1.ClusterServiceVersionPhase) map[string]*v1alpha1.ClusterServiceVersion {
1174-
csvsInNamespace, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(namespace).List(labels.Everything())
1175-
1176-
if err != nil {
1177-
a.Log.Warnf("could not list CSVs while constructing CSV set")
1178-
return nil
1179-
}
1180-
1181-
csvs := make(map[string]*v1alpha1.ClusterServiceVersion, len(csvsInNamespace))
1182-
for _, csv := range csvsInNamespace {
1183-
if phase != v1alpha1.CSVPhaseAny && csv.Status.Phase != phase {
1184-
continue
1185-
}
1186-
csvs[csv.Name] = csv.DeepCopy()
1187-
}
1188-
return csvs
1192+
return a.csvSetGenerator.WithNamespace(namespace, phase)
11891193
}
11901194

11911195
// checkReplacementsAndUpdateStatus returns an error if we can find a newer CSV and sets the status if so
@@ -1370,30 +1374,11 @@ func (a *Operator) apiServiceOwnerConflicts(csv *v1alpha1.ClusterServiceVersion)
13701374
}
13711375

13721376
func (a *Operator) isBeingReplaced(in *v1alpha1.ClusterServiceVersion, csvsInNamespace map[string]*v1alpha1.ClusterServiceVersion) (replacedBy *v1alpha1.ClusterServiceVersion) {
1373-
for _, csv := range csvsInNamespace {
1374-
a.Log.Infof("checking %s", csv.GetName())
1375-
if csv.Spec.Replaces == in.GetName() {
1376-
a.Log.Infof("%s replaced by %s", in.GetName(), csv.GetName())
1377-
replacedBy = csv.DeepCopy()
1378-
return
1379-
}
1380-
}
1381-
return
1377+
return a.csvReplaceFinder.IsBeingReplaced(in, csvsInNamespace)
13821378
}
13831379

13841380
func (a *Operator) isReplacing(in *v1alpha1.ClusterServiceVersion) *v1alpha1.ClusterServiceVersion {
1385-
a.Log.Debugf("checking if csv is replacing an older version")
1386-
if in.Spec.Replaces == "" {
1387-
return nil
1388-
}
1389-
1390-
// using the client instead of a lister; missing an object because of a cache sync can cause upgrades to fail
1391-
previous, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(in.GetNamespace()).Get(in.Spec.Replaces, metav1.GetOptions{})
1392-
if err != nil {
1393-
a.Log.WithField("replacing", in.Spec.Replaces).WithError(err).Debugf("unable to get previous csv")
1394-
return nil
1395-
}
1396-
return previous
1381+
return a.csvReplaceFinder.IsReplacing(in)
13971382
}
13981383

13991384
func (a *Operator) handleDeletion(obj interface{}) {

‎pkg/controller/operators/olm/operator_test.go

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ import (
6161
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil"
6262
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer"
6363
"github.com/operator-framework/operator-lifecycle-manager/pkg/metrics"
64+
csvutility "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/csv"
6465
)
6566

6667
// Fakes
@@ -163,16 +164,24 @@ func NewFakeOperator(clientObjs []runtime.Object, k8sObjs []runtime.Object, extO
163164
}
164165

165166
// Create the new operator
166-
queueOperator, err := queueinformer.NewOperatorFromClient(opClientFake, logrus.StandardLogger())
167+
logger := logrus.StandardLogger()
168+
queueOperator, err := queueinformer.NewOperatorFromClient(opClientFake, logger)
169+
170+
lister := operatorlister.NewLister()
171+
csvSetGenerator := csvutility.NewSetGenerator(logger, lister)
172+
csvReplaceFinder := csvutility.NewReplaceFinder(logger, clientFake)
173+
167174
op := &Operator{
168-
Operator: queueOperator,
169-
client: clientFake,
170-
resolver: strategyResolver,
171-
apiReconciler: apiReconciler,
172-
lister: operatorlister.NewLister(),
173-
csvQueueSet: queueinformer.NewEmptyResourceQueueSet(),
174-
recorder: eventRecorder,
175-
apiLabeler: apiLabeler,
175+
Operator: queueOperator,
176+
client: clientFake,
177+
resolver: strategyResolver,
178+
apiReconciler: apiReconciler,
179+
lister: lister,
180+
csvQueueSet: queueinformer.NewEmptyResourceQueueSet(),
181+
recorder: eventRecorder,
182+
apiLabeler: apiLabeler,
183+
csvSetGenerator: csvSetGenerator,
184+
csvReplaceFinder: csvReplaceFinder,
176185
}
177186

178187
wakeupInterval := 5 * time.Minute
@@ -216,7 +225,7 @@ func NewFakeOperator(clientObjs []runtime.Object, k8sObjs []runtime.Object, extO
216225
}
217226
// Register separate queue for copying csvs
218227
csvCopyQueue := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "csvCopy")
219-
csvQueueIndexer := queueinformer.NewQueueIndexer(csvCopyQueue, csvIndexes, op.syncCopyCSV, "csvCopy", logrus.StandardLogger(), metrics.NewMetricsNil())
228+
csvQueueIndexer := queueinformer.NewQueueIndexer(csvCopyQueue, csvIndexes, op.syncCopyCSV, "csvCopy", logger, metrics.NewMetricsNil())
220229
op.RegisterQueueIndexer(csvQueueIndexer)
221230
op.copyQueueIndexer = csvQueueIndexer
222231

@@ -260,6 +269,23 @@ func buildFakeAPIIntersectionReconcilerThatReturns(result resolver.APIReconcilia
260269
return reconciler
261270
}
262271

272+
func NewFakeOperatorDefault() *Operator {
273+
logger := logrus.StandardLogger()
274+
275+
clientFake := fake.NewSimpleClientset()
276+
lister := operatorlister.NewLister()
277+
csvSetGenerator := csvutility.NewSetGenerator(logger, lister)
278+
csvReplaceFinder := csvutility.NewReplaceFinder(logger, clientFake)
279+
280+
return &Operator{
281+
Operator: &queueinformer.Operator{
282+
Log: logrus.New(),
283+
},
284+
csvSetGenerator: csvSetGenerator,
285+
csvReplaceFinder: csvReplaceFinder,
286+
}
287+
}
288+
263289
// Tests
264290

265291
func deployment(deploymentName, namespace, serviceAccountName string, templateAnnotations map[string]string) *appsv1.Deployment {
@@ -4403,7 +4429,7 @@ func TestIsBeingReplaced(t *testing.T) {
44034429
for _, tt := range tests {
44044430
t.Run(tt.name, func(t *testing.T) {
44054431
// configure cluster state
4406-
op := &Operator{Operator: &queueinformer.Operator{Log: logrus.New()}}
4432+
op := NewFakeOperatorDefault()
44074433

44084434
require.Equal(t, tt.expected, op.isBeingReplaced(tt.in, tt.initial.csvs))
44094435
})
@@ -4451,7 +4477,7 @@ func TestCheckReplacement(t *testing.T) {
44514477
for _, tt := range tests {
44524478
t.Run(tt.name, func(t *testing.T) {
44534479
// configure cluster state
4454-
op := &Operator{Operator: &queueinformer.Operator{Log: logrus.New()}}
4480+
op := NewFakeOperatorDefault()
44554481

44564482
require.Equal(t, tt.expected, op.isBeingReplaced(tt.in, tt.initial.csvs))
44574483
})

‎pkg/lib/csv/csvset.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
package csv
2+
3+
import (
4+
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1"
5+
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorlister"
6+
"github.com/sirupsen/logrus"
7+
"k8s.io/apimachinery/pkg/labels"
8+
)
9+
10+
// NewSetGenerator returns a new instance of SetGenerator.
11+
func NewSetGenerator(logger *logrus.Logger, lister operatorlister.OperatorLister) SetGenerator {
12+
return &csvSet{
13+
logger: logger,
14+
lister: lister,
15+
}
16+
}
17+
18+
// SetGenerator is an interface that returns a map of ClusterServiceVersion
19+
// objects that match a certain set of criteria.
20+
//
21+
// SetGenerator gathers all CSV(s) in the given namespace into a map keyed by
22+
// CSV name; if metav1.NamespaceAll gets the set across all namespaces
23+
type SetGenerator interface {
24+
WithNamespace(namespace string, phase v1alpha1.ClusterServiceVersionPhase) map[string]*v1alpha1.ClusterServiceVersion
25+
}
26+
27+
type csvSet struct {
28+
lister operatorlister.OperatorLister
29+
logger *logrus.Logger
30+
}
31+
32+
// WithNamespace returns all ClusterServiceVersion resource(s) that matches the
33+
// specified phase from a given namespace.
34+
func (s *csvSet) WithNamespace(namespace string, phase v1alpha1.ClusterServiceVersionPhase) map[string]*v1alpha1.ClusterServiceVersion {
35+
return s.with(namespace, phase, labels.Everything())
36+
}
37+
38+
func (s *csvSet) with(namespace string, phase v1alpha1.ClusterServiceVersionPhase, selector labels.Selector) map[string]*v1alpha1.ClusterServiceVersion {
39+
csvsInNamespace, err := s.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(namespace).List(selector)
40+
41+
if err != nil {
42+
s.logger.Warnf("could not list CSVs while constructing CSV set")
43+
return nil
44+
}
45+
46+
csvs := make(map[string]*v1alpha1.ClusterServiceVersion, len(csvsInNamespace))
47+
for _, csv := range csvsInNamespace {
48+
if phase != v1alpha1.CSVPhaseAny && csv.Status.Phase != phase {
49+
continue
50+
}
51+
csvs[csv.Name] = csv.DeepCopy()
52+
}
53+
54+
return csvs
55+
}

‎pkg/lib/csv/replace_finder.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
package csv
2+
3+
import (
4+
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1"
5+
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned"
6+
"github.com/sirupsen/logrus"
7+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
8+
)
9+
10+
// NewReplaceFinder returns an instance of ReplaceFinder
11+
func NewReplaceFinder(logger *logrus.Logger, client versioned.Interface) ReplaceFinder {
12+
return &replace{
13+
logger: logger,
14+
client: client,
15+
}
16+
}
17+
18+
// ReplaceFinder is an interface that finds the next or previous
19+
// ClusterServiceVersion object in the upgrade path for a given CSV.
20+
type ReplaceFinder interface {
21+
IsBeingReplaced(in *v1alpha1.ClusterServiceVersion, csvsInNamespace map[string]*v1alpha1.ClusterServiceVersion) (replacedBy *v1alpha1.ClusterServiceVersion)
22+
IsReplacing(in *v1alpha1.ClusterServiceVersion) *v1alpha1.ClusterServiceVersion
23+
}
24+
25+
type replace struct {
26+
logger *logrus.Logger
27+
client versioned.Interface
28+
}
29+
30+
// IsBeingReplaced returns the corresponding ClusterServiceVersion object that
31+
// is replacing the given CSV specified.
32+
//
33+
// If the corresponding ClusterServiceVersion is not found nil is returned.
34+
func (r *replace) IsBeingReplaced(in *v1alpha1.ClusterServiceVersion, csvsInNamespace map[string]*v1alpha1.ClusterServiceVersion) (replacedBy *v1alpha1.ClusterServiceVersion) {
35+
for _, csv := range csvsInNamespace {
36+
if csv.IsCopied() {
37+
continue
38+
}
39+
40+
r.logger.Infof("checking %s", csv.GetName())
41+
42+
if csv.Spec.Replaces == in.GetName() {
43+
r.logger.Infof("%s replaced by %s", in.GetName(), csv.GetName())
44+
replacedBy = csv
45+
return
46+
}
47+
}
48+
49+
return
50+
}
51+
52+
// IsReplacing returns the corresponding ClusterServiceVersion object that the
53+
// given CSV specified replaces.
54+
//
55+
// If the corresponding ClusterServiceVersion is not found nil is returned.
56+
func (r *replace) IsReplacing(in *v1alpha1.ClusterServiceVersion) *v1alpha1.ClusterServiceVersion {
57+
r.logger.Debugf("checking if csv is replacing an older version")
58+
if in.Spec.Replaces == "" {
59+
return nil
60+
}
61+
62+
// using the client instead of a lister; missing an object because of a cache sync can cause upgrades to fail
63+
previous, err := r.client.OperatorsV1alpha1().ClusterServiceVersions(in.GetNamespace()).Get(in.Spec.Replaces, metav1.GetOptions{})
64+
if err != nil {
65+
r.logger.WithField("replacing", in.Spec.Replaces).WithError(err).Debugf("unable to get previous csv")
66+
return nil
67+
}
68+
69+
return previous
70+
}

0 commit comments

Comments
 (0)