Skip to content

Commit 431a700

Browse files
ecordellecordell
committed
feat(registry): remove in-mem registry implementation
1 parent b9f59b6 commit 431a700

File tree

20 files changed

+208
-2386
lines changed

20 files changed

+208
-2386
lines changed

‎Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ setup-bare: clean e2e.namespace
7979
. ./scripts/install_bare.sh $(shell cat ./e2e.namespace) test/e2e/resources
8080

8181
e2e:
82-
go test -v -timeout 30m ./test/e2e/... -namespace=openshift-operators -kubeconfig=${KUBECONFIG} -olmNamespace=openshift-operator-lifecycle-manager
82+
go test -v -timeout 50m ./test/e2e/... -namespace=openshift-operators -kubeconfig=${KUBECONFIG} -olmNamespace=openshift-operator-lifecycle-manager
8383

8484
e2e-local:
8585
. ./scripts/build_local.sh

‎pkg/api/apis/operators/v1alpha1/catalogsource_types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package v1alpha1
22

33
import (
44
"fmt"
5-
5+
66
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
77
"k8s.io/apimachinery/pkg/types"
88

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

Lines changed: 76 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,9 @@ func NewOperator(kubeconfigPath string, logger *logrus.Logger, wakeupInterval ti
124124
catsrcQueue,
125125
catsrcSharedIndexInformers,
126126
op.syncCatalogSources,
127-
nil,
127+
&cache.ResourceEventHandlerFuncs{
128+
DeleteFunc: op.handleCatSrcDeletion,
129+
},
128130
"catsrc",
129131
metrics.NewMetricsCatalogSource(op.client),
130132
logger,
@@ -242,9 +244,16 @@ func (o *Operator) syncObject(obj interface{}) (syncError error) {
242244
})
243245

244246
if ownerutil.IsOwnedByKind(metaObj, v1alpha1.CatalogSourceKind) {
245-
logger.Debug("requeueing owner CatalogSource")
246247
owner := ownerutil.GetOwnerByKind(metaObj, v1alpha1.CatalogSourceKind)
247-
o.catSrcQueue.AddRateLimited(fmt.Sprintf("%s/%s", metaObj.GetNamespace(), owner.Name))
248+
sourceKey := resolver.CatalogKey{Name: owner.Name, Namespace: metaObj.GetNamespace()}
249+
func() {
250+
o.sourcesLock.RLock()
251+
defer o.sourcesLock.RUnlock()
252+
if _, ok := o.sources[sourceKey]; ok {
253+
logger.Debug("requeueing owner CatalogSource")
254+
o.catSrcQueue.AddRateLimited(fmt.Sprintf("%s/%s", metaObj.GetNamespace(), owner.Name))
255+
}
256+
}()
248257
}
249258

250259
return nil
@@ -271,6 +280,18 @@ func (o *Operator) handleDeletion(obj interface{}) {
271280
}
272281
}
273282

283+
func (o *Operator) handleCatSrcDeletion(obj interface{}) {
284+
if catsrc, ok := obj.(*v1alpha1.CatalogSource); ok {
285+
sourceKey := resolver.CatalogKey{Name: catsrc.GetName(), Namespace: catsrc.GetNamespace()}
286+
func() {
287+
o.sourcesLock.Lock()
288+
defer o.sourcesLock.Unlock()
289+
delete(o.sources, sourceKey)
290+
}()
291+
o.Log.WithField("source", sourceKey).Info("removed client for deleted catalogsource")
292+
}
293+
}
294+
274295
func (o *Operator) syncCatalogSources(obj interface{}) (syncError error) {
275296
catsrc, ok := obj.(*v1alpha1.CatalogSource)
276297
if !ok {
@@ -353,7 +374,7 @@ func (o *Operator) syncConfigMapSource(logger *logrus.Entry, catsrc *v1alpha1.Ca
353374
defer o.sourcesLock.Unlock()
354375
address := catsrc.Status.RegistryServiceStatus.Address()
355376
currentSource, ok := o.sources[sourceKey]
356-
if !ok || currentSource.Address != address || currentSource.LastConnect.Before(&o.sourcesLastUpdate) {
377+
if !ok || currentSource.Address != address {
357378
client, err := registryclient.NewClient(address)
358379
if err != nil {
359380
logger.WithError(err).Warn("couldn't connect to registry")
@@ -378,26 +399,34 @@ func (o *Operator) syncConfigMapSource(logger *logrus.Entry, catsrc *v1alpha1.Ca
378399
}
379400

380401
// Sync any dependent Subscriptions
402+
// TODO: this should go away, we should resync the namespace instead
403+
o.syncDependentSubscriptions(logger, out.GetName(), out.GetNamespace())
404+
405+
return nil
406+
}
407+
408+
func (o *Operator) syncDependentSubscriptions(logger *logrus.Entry, catalogSource, catalogSourceNamespace string) {
381409
subs, err := o.lister.OperatorsV1alpha1().SubscriptionLister().List(labels.Everything())
382410
if err != nil {
383411
logger.Warnf("could not list Subscriptions")
384-
return nil
412+
return
385413
}
386414

387415
for _, sub := range subs {
388-
logger = logger.WithFields(logrus.Fields{"subscriptionCatalogSource": sub.Spec.CatalogSource, "subscriptionCatalogNamespace": sub.Spec.CatalogSourceNamespace})
416+
logger = logger.WithFields(logrus.Fields{
417+
"subscriptionCatalogSource": sub.Spec.CatalogSource,
418+
"subscriptionCatalogNamespace": sub.Spec.CatalogSourceNamespace,
419+
"subscription": sub.GetName(),
420+
})
389421
catalogNamespace := sub.Spec.CatalogSourceNamespace
390422
if catalogNamespace == "" {
391423
catalogNamespace = o.namespace
392424
}
393-
logger.Debug("checking subscription")
394-
if sub.Spec.CatalogSource == out.GetName() && catalogNamespace == out.GetNamespace() {
425+
if sub.Spec.CatalogSource == catalogSource && catalogNamespace == catalogSourceNamespace {
395426
logger.Debug("requeueing subscription because catalog changed")
396427
o.requeueSubscription(sub.GetName(), sub.GetNamespace())
397428
}
398429
}
399-
400-
return nil
401430
}
402431

403432
func (o *Operator) syncSubscriptions(obj interface{}) error {
@@ -427,6 +456,38 @@ func (o *Operator) syncSubscriptions(obj interface{}) error {
427456
return nil
428457
}
429458

459+
// get the set of sources that should be used for resolution and best-effort get their connections working
460+
logger.Debugf("resolving sources for %s", namespace)
461+
resolverSources := o.ensureResolverSources(logger, namespace)
462+
463+
// resolve a set of steps to apply to a cluster, a set of subscriptions to create/update, and any errors
464+
steps, subs, err := o.resolver.ResolveSteps(namespace, resolver.NewNamespaceSourceQuerier(resolverSources))
465+
if err != nil {
466+
return err
467+
}
468+
469+
// any subscription in the namespace with manual approval will force generated installplans to be manual
470+
// TODO: this is an odd artifact of the older resolver, and will probably confuse users. approval mode could be on the operatorgroup?
471+
installPlanApproval := v1alpha1.ApprovalAutomatic
472+
for _, sub := range subs {
473+
if sub.Spec.InstallPlanApproval == v1alpha1.ApprovalManual {
474+
installPlanApproval = v1alpha1.ApprovalManual
475+
break
476+
}
477+
}
478+
479+
installplanReference, err := o.createInstallPlan(namespace, subs, installPlanApproval, steps)
480+
if err != nil {
481+
return err
482+
}
483+
484+
if err := o.ensureSubscriptionInstallPlanState(namespace, subs, installplanReference); err != nil {
485+
return err
486+
}
487+
return nil
488+
}
489+
490+
func (o *Operator) ensureResolverSources(logger *logrus.Entry, namespace string) map[resolver.CatalogKey]registryclient.Interface {
430491
// TODO: record connection status onto an object
431492
resolverSources := make(map[resolver.CatalogKey]registryclient.Interface, 0)
432493
func() {
@@ -449,42 +510,18 @@ func (o *Operator) syncSubscriptions(obj interface{}) error {
449510
logger = logger.WithField("resolverSource", k)
450511
logger.WithField("clientState", client.Conn.GetState()).Debug("source")
451512
if client.Conn.GetState() == connectivity.TransientFailure {
452-
logger.WithField("clientState", client.Conn.GetState()).Debug("resetting connection")
453-
client.Conn.ResetConnectBackoff()
513+
logger.WithField("clientState", client.Conn.GetState()).Debug("waiting for connection")
454514
ctx, _ := context.WithTimeout(context.TODO(), 5*time.Second)
455515
changed := client.Conn.WaitForStateChange(ctx, connectivity.TransientFailure)
456516
if !changed {
457517
logger.WithField("clientState", client.Conn.GetState()).Debug("source in transient failure and didn't recover")
518+
delete(resolverSources, k)
519+
} else {
520+
logger.WithField("clientState", client.Conn.GetState()).Debug("connection re-established")
458521
}
459-
logger.WithField("clientState", client.Conn.GetState()).Debug("connection successfully reset")
460522
}
461523
}
462-
463-
// resolve a set of steps to apply to a cluster, a set of subscriptions to create/update, and any errors
464-
steps, subs, err := o.resolver.ResolveSteps(sub.GetNamespace(), resolver.NewNamespaceSourceQuerier(resolverSources))
465-
if err != nil {
466-
return err
467-
}
468-
469-
// any subscription in the namespace with manual approval will force generated installplans to be manual
470-
// TODO: this is an odd artifact of the older resolver, and will probably confuse users. approval mode could be on the operatorgroup?
471-
installPlanApproval := v1alpha1.ApprovalAutomatic
472-
for _, sub := range subs {
473-
if sub.Spec.InstallPlanApproval == v1alpha1.ApprovalManual {
474-
installPlanApproval = v1alpha1.ApprovalManual
475-
break
476-
}
477-
}
478-
479-
installplanReference, err := o.createInstallPlan(namespace, subs, installPlanApproval, steps)
480-
if err != nil {
481-
return err
482-
}
483-
484-
if err := o.ensureSubscriptionInstallPlanState(namespace, subs, installplanReference); err != nil {
485-
return err
486-
}
487-
return nil
524+
return resolverSources
488525
}
489526

490527
func (o *Operator) nothingToUpdate(logger *logrus.Entry, sub *v1alpha1.Subscription) bool {

‎pkg/controller/operators/catalog/subscriptions_test.go

Lines changed: 50 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,11 @@ func TestSyncSubscriptions(t *testing.T) {
6666
Resource: v1alpha1.StepResource{
6767
CatalogSource: "src",
6868
CatalogSourceNamespace: testNamespace,
69-
Group: v1alpha1.GroupName,
70-
Version: v1alpha1.GroupVersion,
71-
Kind: v1alpha1.ClusterServiceVersionKind,
72-
Name: "csv.v.1",
73-
Manifest: "{}",
69+
Group: v1alpha1.GroupName,
70+
Version: v1alpha1.GroupVersion,
71+
Kind: v1alpha1.ClusterServiceVersionKind,
72+
Name: "csv.v.1",
73+
Manifest: "{}",
7474
},
7575
},
7676
},
@@ -150,11 +150,11 @@ func TestSyncSubscriptions(t *testing.T) {
150150
Resource: v1alpha1.StepResource{
151151
CatalogSource: "src",
152152
CatalogSourceNamespace: testNamespace,
153-
Group: v1alpha1.GroupName,
154-
Version: v1alpha1.GroupVersion,
155-
Kind: v1alpha1.ClusterServiceVersionKind,
156-
Name: "csv.v.1",
157-
Manifest: "{}",
153+
Group: v1alpha1.GroupName,
154+
Version: v1alpha1.GroupVersion,
155+
Kind: v1alpha1.ClusterServiceVersionKind,
156+
Name: "csv.v.1",
157+
Manifest: "{}",
158158
},
159159
},
160160
},
@@ -195,11 +195,11 @@ func TestSyncSubscriptions(t *testing.T) {
195195
Resource: v1alpha1.StepResource{
196196
CatalogSource: "src",
197197
CatalogSourceNamespace: testNamespace,
198-
Group: v1alpha1.GroupName,
199-
Version: v1alpha1.GroupVersion,
200-
Kind: v1alpha1.ClusterServiceVersionKind,
201-
Name: "csv.v.2",
202-
Manifest: "{}",
198+
Group: v1alpha1.GroupName,
199+
Version: v1alpha1.GroupVersion,
200+
Kind: v1alpha1.ClusterServiceVersionKind,
201+
Name: "csv.v.2",
202+
Manifest: "{}",
203203
},
204204
},
205205
},
@@ -279,11 +279,11 @@ func TestSyncSubscriptions(t *testing.T) {
279279
Resource: v1alpha1.StepResource{
280280
CatalogSource: "src",
281281
CatalogSourceNamespace: testNamespace,
282-
Group: v1alpha1.GroupName,
283-
Version: v1alpha1.GroupVersion,
284-
Kind: v1alpha1.ClusterServiceVersionKind,
285-
Name: "csv.v.2",
286-
Manifest: "{}",
282+
Group: v1alpha1.GroupName,
283+
Version: v1alpha1.GroupVersion,
284+
Kind: v1alpha1.ClusterServiceVersionKind,
285+
Name: "csv.v.2",
286+
Manifest: "{}",
287287
},
288288
},
289289
},
@@ -324,35 +324,35 @@ func TestSyncSubscriptions(t *testing.T) {
324324
Resource: v1alpha1.StepResource{
325325
CatalogSource: "src",
326326
CatalogSourceNamespace: testNamespace,
327-
Group: v1alpha1.GroupName,
328-
Version: v1alpha1.GroupVersion,
329-
Kind: v1alpha1.ClusterServiceVersionKind,
330-
Name: "csv.v.2",
331-
Manifest: "{}",
327+
Group: v1alpha1.GroupName,
328+
Version: v1alpha1.GroupVersion,
329+
Kind: v1alpha1.ClusterServiceVersionKind,
330+
Name: "csv.v.2",
331+
Manifest: "{}",
332332
},
333333
},
334334
{
335335
Resolving: "csv.v.2",
336336
Resource: v1alpha1.StepResource{
337337
CatalogSource: "src",
338338
CatalogSourceNamespace: testNamespace,
339-
Group: v1alpha1.GroupName,
340-
Version: v1alpha1.GroupVersion,
341-
Kind: v1alpha1.ClusterServiceVersionKind,
342-
Name: "dep.v.1",
343-
Manifest: "{}",
339+
Group: v1alpha1.GroupName,
340+
Version: v1alpha1.GroupVersion,
341+
Kind: v1alpha1.ClusterServiceVersionKind,
342+
Name: "dep.v.1",
343+
Manifest: "{}",
344344
},
345345
},
346346
{
347347
Resolving: "csv.v.2",
348348
Resource: v1alpha1.StepResource{
349349
CatalogSource: "src",
350350
CatalogSourceNamespace: testNamespace,
351-
Group: v1alpha1.GroupName,
352-
Version: v1alpha1.GroupVersion,
353-
Kind: v1alpha1.SubscriptionKind,
354-
Name: "sub-dep",
355-
Manifest: "{}",
351+
Group: v1alpha1.GroupName,
352+
Version: v1alpha1.GroupVersion,
353+
Kind: v1alpha1.SubscriptionKind,
354+
Name: "sub-dep",
355+
Manifest: "{}",
356356
},
357357
},
358358
},
@@ -433,35 +433,35 @@ func TestSyncSubscriptions(t *testing.T) {
433433
Resource: v1alpha1.StepResource{
434434
CatalogSource: "src",
435435
CatalogSourceNamespace: testNamespace,
436-
Group: v1alpha1.GroupName,
437-
Version: v1alpha1.GroupVersion,
438-
Kind: v1alpha1.ClusterServiceVersionKind,
439-
Name: "csv.v.2",
440-
Manifest: "{}",
436+
Group: v1alpha1.GroupName,
437+
Version: v1alpha1.GroupVersion,
438+
Kind: v1alpha1.ClusterServiceVersionKind,
439+
Name: "csv.v.2",
440+
Manifest: "{}",
441441
},
442442
},
443443
{
444444
Resolving: "csv.v.2",
445445
Resource: v1alpha1.StepResource{
446446
CatalogSource: "src",
447447
CatalogSourceNamespace: testNamespace,
448-
Group: v1alpha1.GroupName,
449-
Version: v1alpha1.GroupVersion,
450-
Kind: v1alpha1.ClusterServiceVersionKind,
451-
Name: "dep.v.1",
452-
Manifest: "{}",
448+
Group: v1alpha1.GroupName,
449+
Version: v1alpha1.GroupVersion,
450+
Kind: v1alpha1.ClusterServiceVersionKind,
451+
Name: "dep.v.1",
452+
Manifest: "{}",
453453
},
454454
},
455455
{
456456
Resolving: "csv.v.2",
457457
Resource: v1alpha1.StepResource{
458458
CatalogSource: "src",
459459
CatalogSourceNamespace: testNamespace,
460-
Group: v1alpha1.GroupName,
461-
Version: v1alpha1.GroupVersion,
462-
Kind: v1alpha1.SubscriptionKind,
463-
Name: "sub-dep",
464-
Manifest: "{}",
460+
Group: v1alpha1.GroupName,
461+
Version: v1alpha1.GroupVersion,
462+
Kind: v1alpha1.SubscriptionKind,
463+
Name: "sub-dep",
464+
Manifest: "{}",
465465
},
466466
},
467467
},

0 commit comments

Comments
 (0)