*: address comments

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
operator-framework · Oct 3, 2023 · 30f2150 · 30f2150
1 parent 3aa08f8
commit 30f2150
Show file tree

Hide file tree

Showing 6 changed files with 19 additions and 49 deletions.
diff --git a/cmd/olm/manager.go b/cmd/olm/manager.go
@@ -3,15 +3,10 @@ package main
 import (
 	"context"
 
-	appsv1 "k8s.io/api/apps/v1"
-	corev1 "k8s.io/api/core/v1"
-	rbacv1 "k8s.io/api/rbac/v1"
-	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/selection"
-	apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -56,40 +51,8 @@ func Manager(ctx context.Context, debug bool) (ctrl.Manager, error) {
 		Scheme:             scheme,
 		MetricsBindAddress: "0", // TODO(njhale): Enable metrics on non-conflicting port (not 8080)
 		Cache: cache.Options{
+			DefaultLabelSelector: labels.SelectorFromValidatedSet(map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}),
 			ByObject: map[client.Object]cache.ByObject{
-				&appsv1.Deployment{}: {
-					Label: labels.SelectorFromValidatedSet(map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}),
-				},
-				&corev1.Service{}: {
-					Label: labels.SelectorFromValidatedSet(map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}),
-				},
-				&apiextensionsv1.CustomResourceDefinition{}: {
-					Label: labels.SelectorFromValidatedSet(map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}),
-				},
-				&apiregistrationv1.APIService{}: {
-					Label: labels.SelectorFromValidatedSet(map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}),
-				},
-				&corev1.ConfigMap{}: {
-					Label: labels.SelectorFromValidatedSet(map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}),
-				},
-				&corev1.ServiceAccount{}: {
-					Label: labels.SelectorFromValidatedSet(map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}),
-				},
-				&rbacv1.Role{}: {
-					Label: labels.SelectorFromValidatedSet(map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}),
-				},
-				&rbacv1.RoleBinding{}: {
-					Label: labels.SelectorFromValidatedSet(map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}),
-				},
-				&rbacv1.ClusterRole{}: {
-					Label: labels.SelectorFromValidatedSet(map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}),
-				},
-				&rbacv1.ClusterRoleBinding{}: {
-					Label: labels.SelectorFromValidatedSet(map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}),
-				},
-				&corev1.Secret{}: {
-					Label: labels.SelectorFromValidatedSet(map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}),
-				},
 				&operatorsv1alpha1.ClusterServiceVersion{}: {
 					Label: copiedLabelDoesNotExist,
 				},

diff --git a/pkg/controller/operators/labeller/labels.go b/pkg/controller/operators/labeller/labels.go
@@ -123,7 +123,8 @@ func ObjectPatchLabeler(
 				if gvrFullyLabelled {
 					allObjectsLabelled := done()
 					if allObjectsLabelled {
-						logrus.Fatal("detected that every object is labelled, exiting...")
+						logrus.Info("detected that every object is labelled, exiting to re-start the process...")
+						os.Exit(0)
 					}
 				}
 				return nil

diff --git a/pkg/controller/operators/labeller/rbac.go b/pkg/controller/operators/labeller/rbac.go
@@ -3,6 +3,7 @@ package labeller
 import (
 	"context"
 	"fmt"
+	"os"
 
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver"
@@ -52,7 +53,8 @@ func ContentHashLabeler[T metav1.Object, A ApplyConfig[A]](
 				if gvrFullyLabelled {
 					allObjectsLabelled := done()
 					if allObjectsLabelled {
-						logrus.Fatal("detected that every object is labelled, exiting...")
+						logrus.Info("detected that every object is labelled, exiting to re-start the process...")
+						os.Exit(0)
 					}
 				}
 				return nil

diff --git a/pkg/controller/operators/olm/operator.go b/pkg/controller/operators/olm/operator.go
@@ -136,13 +136,13 @@ func (a *Operator) getRuleChecker() func(*v1alpha1.ClusterServiceVersion) *insta
 	clusterRolesLister := sif.Rbac().V1().ClusterRoles().Lister()
 	clusterRoleBindingsLister := sif.Rbac().V1().ClusterRoleBindings().Lister()
 
-	done := make(chan struct{})
-	go func() {
-		<-a.ctx.Done()
-		done <- struct{}{}
-	}()
-	sif.Start(done)
-	sif.WaitForCacheSync(done)
+	sif.Start(a.ctx.Done())
+	sif.WaitForCacheSync(a.ctx.Done())
+
+	if a.ctx.Err() != nil {
+		a.ruleChecker = nil
+		return nil
+	}
 
 	a.ruleChecker = func(csv *v1alpha1.ClusterServiceVersion) *install.CSVRuleChecker {
 		return install.NewCSVRuleChecker(

diff --git a/pkg/controller/operators/olm/requirements.go b/pkg/controller/operators/olm/requirements.go
@@ -3,6 +3,7 @@ package olm
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"strings"
 
@@ -349,6 +350,9 @@ func (a *Operator) permissionStatus(strategyDetailsDeployment *v1alpha1.Strategy
 			// if we have not filtered our informers or if we were unable to detect the correct permissions, we have
 			// no choice but to page in the world and see if the user pre-created permissions for this CSV
 			ruleChecker := a.getRuleChecker()(csv)
+			if ruleChecker == nil {
+				return false, errors.New("could not create a rule checker (are we shutting down?)")
+			}
 			for _, rule := range perm.Rules {
 				dependent := v1alpha1.DependentStatus{
 					Group:   "rbac.authorization.k8s.io",

diff --git a/pkg/lib/scoped/syncer.go b/pkg/lib/scoped/syncer.go
@@ -100,8 +100,8 @@ func (s *UserDefinedServiceAccountSyncer) SyncOperatorGroup(in *v1.OperatorGroup
 		return
 	}
 
-	// A service account has been specified, but likely does not have the labels we expect it to have so it will
-	// show up in our listers, so let's add that and queue again later
+	// A service account has been specified, but likely does not have the labels we require it to have to ensure it
+	// shows up in our listers, so let's add that and queue again later
 	config := corev1applyconfigurations.ServiceAccount(serviceAccountName, namespace)
 	config.Labels = map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}
 	if _, err := s.client.KubernetesInterface().CoreV1().ServiceAccounts(namespace).Apply(context.TODO(), config, metav1.ApplyOptions{FieldManager: "operator-lifecycle-manager"}); err != nil {