From 30f2150cd5598ff1d9e5a6692a70cc56116dab29 Mon Sep 17 00:00:00 2001 From: Steve Kuznetsov Date: Thu, 28 Sep 2023 10:13:55 -0600 Subject: [PATCH] *: address comments Signed-off-by: Steve Kuznetsov --- cmd/olm/manager.go | 39 +------------------- pkg/controller/operators/labeller/labels.go | 3 +- pkg/controller/operators/labeller/rbac.go | 4 +- pkg/controller/operators/olm/operator.go | 14 +++---- pkg/controller/operators/olm/requirements.go | 4 ++ pkg/lib/scoped/syncer.go | 4 +- 6 files changed, 19 insertions(+), 49 deletions(-) diff --git a/cmd/olm/manager.go b/cmd/olm/manager.go index 66f29d991f..953b2ad306 100644 --- 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 index dabf6dd258..f131ed1053 100644 --- 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 index 395e19592a..939ea82429 100644 --- 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 index c22d53ef64..d30005b678 100644 --- 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 index 0a20fe8f3c..890fb06cbf 100644 --- 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 index f69505beff..15208fd247 100644 --- 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 {