diff --git a/cmd/olm/manager.go b/cmd/olm/manager.go index dbb6d0d222..7c5a0a8f5c 100644 --- a/cmd/olm/manager.go +++ b/cmd/olm/manager.go @@ -3,6 +3,7 @@ package main import ( "context" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -87,6 +88,12 @@ func Manager(ctx context.Context, debug bool) (ctrl.Manager, error) { &rbacv1.ClusterRoleBinding{}: { Label: labels.SelectorFromValidatedSet(map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}), }, + &admissionregistrationv1.MutatingWebhookConfiguration{}: { + Label: labels.SelectorFromValidatedSet(map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}), + }, + &admissionregistrationv1.ValidatingWebhookConfiguration{}: { + Label: labels.SelectorFromValidatedSet(map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}), + }, &operatorsv1alpha1.ClusterServiceVersion{}: { Label: copiedLabelDoesNotExist, }, diff --git a/pkg/controller/operators/olm/operator.go b/pkg/controller/operators/olm/operator.go index de994971b0..44d3645ac8 100644 --- a/pkg/controller/operators/olm/operator.go +++ b/pkg/controller/operators/olm/operator.go @@ -11,7 +11,6 @@ import ( "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/labeller" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm/plugins" "github.com/sirupsen/logrus" - admissionregistrationv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -50,7 +49,6 @@ import ( csvutility "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/csv" "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/event" index "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/index" - "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/kubestate" "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/labeler" "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient" "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorlister" @@ -86,7 +84,6 @@ type Operator struct { olmConfigQueue workqueue.RateLimitingInterface csvCopyQueueSet *queueinformer.ResourceQueueSet copiedCSVGCQueueSet *queueinformer.ResourceQueueSet - objGCQueueSet *queueinformer.ResourceQueueSet nsQueueSet workqueue.RateLimitingInterface apiServiceQueue workqueue.RateLimitingInterface csvIndexers map[string]cache.Indexer @@ -202,7 +199,6 @@ func newOperatorWithConfig(ctx context.Context, config *operatorConfig) (*Operat olmConfigQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "olmConfig"), csvCopyQueueSet: queueinformer.NewEmptyResourceQueueSet(), copiedCSVGCQueueSet: queueinformer.NewEmptyResourceQueueSet(), - objGCQueueSet: queueinformer.NewEmptyResourceQueueSet(), apiServiceQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "apiservice"), resolver: config.strategyResolver, apiReconciler: config.apiReconciler, @@ -480,21 +476,6 @@ func newOperatorWithConfig(ctx context.Context, config *operatorConfig) (*Operat if err := op.RegisterQueueInformer(serviceAccountQueueInformer); err != nil { return nil, err } - - objGCQueue := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), fmt.Sprintf("%s/obj-gc", namespace)) - op.objGCQueueSet.Set(namespace, objGCQueue) - objGCQueueInformer, err := queueinformer.NewQueue( - ctx, - queueinformer.WithLogger(op.logger), - queueinformer.WithQueue(objGCQueue), - queueinformer.WithSyncer(queueinformer.LegacySyncHandler(op.syncGCObject).ToSyncer()), - ) - if err != nil { - return nil, err - } - if err := op.RegisterQueueInformer(objGCQueueInformer); err != nil { - return nil, err - } } complete := map[schema.GroupVersionResource][]bool{} @@ -568,22 +549,6 @@ func newOperatorWithConfig(ctx context.Context, config *operatorConfig) (*Operat return nil, err } - // add queue for all namespaces as well - objGCQueue := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), fmt.Sprintf("%s/obj-gc", "")) - op.objGCQueueSet.Set("", objGCQueue) - objGCQueueInformer, err := queueinformer.NewQueue( - ctx, - queueinformer.WithLogger(op.logger), - queueinformer.WithQueue(objGCQueue), - queueinformer.WithSyncer(queueinformer.LegacySyncHandler(op.syncGCObject).ToSyncer()), - ) - if err != nil { - return nil, err - } - if err := op.RegisterQueueInformer(objGCQueueInformer); err != nil { - return nil, err - } - // Register QueueInformer for olmConfig olmConfigInformer := externalversions.NewSharedInformerFactoryWithOptions( op.client, @@ -948,98 +913,6 @@ func (a *Operator) EnsureCSVMetric() error { return nil } -func (a *Operator) syncGCObject(obj interface{}) (syncError error) { - metaObj, ok := obj.(metav1.Object) - if !ok { - a.logger.Warn("object sync: casting to metav1.Object failed") - return - } - logger := a.logger.WithFields(logrus.Fields{ - "name": metaObj.GetName(), - "namespace": metaObj.GetNamespace(), - "self": metaObj.GetSelfLink(), - }) - - switch metaObj.(type) { - case *rbacv1.ClusterRole: - if name, ns, ok := ownerutil.GetOwnerByKindLabel(metaObj, v1alpha1.ClusterServiceVersionKind); ok { - _, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(ns).Get(name) - if err == nil { - logger.Debugf("CSV still present, must wait until it is deleted (owners=%v/%v)", ns, name) - syncError = fmt.Errorf("cleanup must wait") - return - } else if !apierrors.IsNotFound(err) { - syncError = err - return - } - } - - if err := a.opClient.DeleteClusterRole(metaObj.GetName(), &metav1.DeleteOptions{}); err != nil { - logger.WithError(err).Warn("cannot delete cluster role") - break - } - logger.Debugf("Deleted cluster role %v due to no owning CSV", metaObj.GetName()) - case *rbacv1.ClusterRoleBinding: - if name, ns, ok := ownerutil.GetOwnerByKindLabel(metaObj, v1alpha1.ClusterServiceVersionKind); ok { - _, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(ns).Get(name) - if err == nil { - logger.Debugf("CSV still present, must wait until it is deleted (owners=%v)", name) - syncError = fmt.Errorf("cleanup must wait") - return - } else if !apierrors.IsNotFound(err) { - syncError = err - return - } - } - - if err := a.opClient.DeleteClusterRoleBinding(metaObj.GetName(), &metav1.DeleteOptions{}); err != nil { - logger.WithError(err).Warn("cannot delete cluster role binding") - break - } - logger.Debugf("Deleted cluster role binding %v due to no owning CSV", metaObj.GetName()) - case *admissionregistrationv1.MutatingWebhookConfiguration: - if name, ns, ok := ownerutil.GetOwnerByKindLabel(metaObj, v1alpha1.ClusterServiceVersionKind); ok { - _, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(ns).Get(name) - if err == nil { - logger.Debugf("CSV still present, must wait until it is deleted (owners=%v)", name) - syncError = fmt.Errorf("cleanup must wait") - return - } else if !apierrors.IsNotFound(err) { - logger.Infof("error CSV retrieval error") - syncError = err - return - } - } - - if err := a.opClient.KubernetesInterface().AdmissionregistrationV1().MutatingWebhookConfigurations().Delete(context.TODO(), metaObj.GetName(), metav1.DeleteOptions{}); err != nil { - logger.WithError(err).Warn("cannot delete MutatingWebhookConfiguration") - break - } - logger.Debugf("Deleted MutatingWebhookConfiguration %v due to no owning CSV", metaObj.GetName()) - case *admissionregistrationv1.ValidatingWebhookConfiguration: - if name, ns, ok := ownerutil.GetOwnerByKindLabel(metaObj, v1alpha1.ClusterServiceVersionKind); ok { - _, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(ns).Get(name) - if err == nil { - logger.Debugf("CSV still present, must wait until it is deleted (owners=%v)", name) - syncError = fmt.Errorf("cleanup must wait") - return - } else if !apierrors.IsNotFound(err) { - logger.Infof("Error CSV retrieval error") - syncError = err - return - } - } - - if err := a.opClient.KubernetesInterface().AdmissionregistrationV1().ValidatingWebhookConfigurations().Delete(context.TODO(), metaObj.GetName(), metav1.DeleteOptions{}); err != nil { - logger.WithError(err).Warn("cannot delete ValidatingWebhookConfiguration") - break - } - logger.Debugf("Deleted ValidatingWebhookConfiguration %v due to no owning CSV", metaObj.GetName()) - } - - return -} - func (a *Operator) syncObject(obj interface{}) (syncError error) { // Assert as metav1.Object metaObj, ok := obj.(metav1.Object) @@ -1054,30 +927,8 @@ func (a *Operator) syncObject(obj interface{}) (syncError error) { "self": metaObj.GetSelfLink(), }) - // Requeues objects that can't have ownerrefs (cluster -> namespace, cross-namespace) - if ownerutil.IsOwnedByKindLabel(metaObj, v1alpha1.ClusterServiceVersionKind) { - name, ns, ok := ownerutil.GetOwnerByKindLabel(metaObj, v1alpha1.ClusterServiceVersionKind) - if !ok { - logger.Error("unexpected owner label retrieval failure") - } - _, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(ns).Get(name) - if !apierrors.IsNotFound(err) { - logger.Debug("requeueing owner csvs from owner label") - a.requeueOwnerCSVs(metaObj) - } else { - switch metaObj.(type) { - case *rbacv1.ClusterRole, *rbacv1.ClusterRoleBinding, *admissionregistrationv1.MutatingWebhookConfiguration, *admissionregistrationv1.ValidatingWebhookConfiguration: - resourceEvent := kubestate.NewResourceEvent( - kubestate.ResourceUpdated, - metaObj, - ) - if syncError = a.objGCQueueSet.RequeueEvent("", resourceEvent); syncError != nil { - logger.WithError(syncError).Warnf("failed to requeue gc event: %v", resourceEvent) - } - return - } - } - } + // Objects that can't have ownerrefs (cluster -> namespace, cross-namespace) + // are handled by finalizer // Requeue all owner CSVs if ownerutil.IsOwnedByKind(metaObj, v1alpha1.ClusterServiceVersionKind) { @@ -1332,50 +1183,6 @@ func (a *Operator) handleClusterServiceVersionDeletion(obj interface{}) { } } - ownerSelector := ownerutil.CSVOwnerSelector(clusterServiceVersion) - crbs, err := a.lister.RbacV1().ClusterRoleBindingLister().List(ownerSelector) - if err != nil { - logger.WithError(err).Warn("cannot list cluster role bindings") - } - for _, crb := range crbs { - if err := a.objGCQueueSet.RequeueEvent("", kubestate.NewResourceEvent(kubestate.ResourceUpdated, crb)); err != nil { - logger.WithError(err).Warnf("failed to requeue gc event: %v", crb) - } - } - - crs, err := a.lister.RbacV1().ClusterRoleLister().List(ownerSelector) - if err != nil { - logger.WithError(err).Warn("cannot list cluster roles") - } - for _, cr := range crs { - if err := a.objGCQueueSet.RequeueEvent("", kubestate.NewResourceEvent(kubestate.ResourceUpdated, cr)); err != nil { - logger.WithError(err).Warnf("failed to requeue gc event: %v", cr) - } - } - - webhookSelector := labels.SelectorFromSet(ownerutil.OwnerLabel(clusterServiceVersion, v1alpha1.ClusterServiceVersionKind)).String() - mWebhooks, err := a.opClient.KubernetesInterface().AdmissionregistrationV1().MutatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: webhookSelector}) - if err != nil { - logger.WithError(err).Warn("cannot list MutatingWebhookConfigurations") - } - for _, webhook := range mWebhooks.Items { - w := webhook - if err := a.objGCQueueSet.RequeueEvent("", kubestate.NewResourceEvent(kubestate.ResourceUpdated, &w)); err != nil { - logger.WithError(err).Warnf("failed to requeue gc event: %v", webhook) - } - } - - vWebhooks, err := a.opClient.KubernetesInterface().AdmissionregistrationV1().ValidatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: webhookSelector}) - if err != nil { - logger.WithError(err).Warn("cannot list ValidatingWebhookConfigurations") - } - for _, webhook := range vWebhooks.Items { - w := webhook - if err := a.objGCQueueSet.RequeueEvent("", kubestate.NewResourceEvent(kubestate.ResourceUpdated, &w)); err != nil { - logger.WithError(err).Warnf("failed to requeue gc event: %v", webhook) - } - } - // Conversion webhooks are defined within a CRD. // In an effort to prevent customer dataloss, OLM does not delete CRDs associated with a CSV when it is deleted. // Deleting a CSV that introduced a conversion webhook removes the deployment that serviced the conversion webhook calls. diff --git a/pkg/controller/operators/operatorconditiongenerator_controller.go b/pkg/controller/operators/operatorconditiongenerator_controller.go index 75bd847b88..1d1963c888 100644 --- a/pkg/controller/operators/operatorconditiongenerator_controller.go +++ b/pkg/controller/operators/operatorconditiongenerator_controller.go @@ -2,15 +2,20 @@ package operators import ( "context" + "fmt" "reflect" "github.com/go-logr/logr" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + rbacv1 "k8s.io/api/rbac/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + utilerrors "k8s.io/apimachinery/pkg/util/errors" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -95,6 +100,10 @@ func (r *OperatorConditionGeneratorReconciler) Reconcile(ctx context.Context, re return ctrl.Result{}, client.IgnoreNotFound(err) } + if err, ok := r.processFinalizer(ctx, in); !ok { + return ctrl.Result{}, err + } + operatorCondition := &operatorsv2.OperatorCondition{ ObjectMeta: metav1.ObjectMeta{ // For now, only generate an OperatorCondition with the same name as the csv. @@ -170,3 +179,112 @@ func (r *OperatorConditionGeneratorReconciler) ensureOperatorCondition(operatorC existingOperatorCondition.Spec.ServiceAccounts = operatorCondition.Spec.ServiceAccounts return r.Client.Update(context.TODO(), existingOperatorCondition) } + +// Return values, err, ok; ok == true: continue Reconcile, ok == false: exit Reconcile +func (r *OperatorConditionGeneratorReconciler) processFinalizer(ctx context.Context, csv *operatorsv1alpha1.ClusterServiceVersion) (error, bool) { + myFinalizerName := "operators.coreos.com/csv-cleanup" + log := r.log.WithValues("name", csv.GetName()).WithValues("namespace", csv.GetNamespace()) + + if csv.ObjectMeta.DeletionTimestamp.IsZero() { + // CSV is not being deleted, add finalizer if not present + if !controllerutil.ContainsFinalizer(csv, myFinalizerName) { + patch := csv.DeepCopy() + controllerutil.AddFinalizer(patch, myFinalizerName) + if err := r.Client.Patch(ctx, patch, client.MergeFrom(csv)); err != nil { + log.Error(err, "Adding finalizer") + return err, false + } + } + return nil, true + } + + if !controllerutil.ContainsFinalizer(csv, myFinalizerName) { + // Finalizer has been removed; stop reconciliation as the CSV is being deleted + return nil, false + } + + // CSV is being deleted and the finalizer still present; do any clean up + ownerSelector := ownerutil.CSVOwnerSelector(csv) + listOptions := client.ListOptions{ + LabelSelector: ownerSelector, + } + deleteOptions := client.DeleteAllOfOptions{ + ListOptions: listOptions, + } + // Look for resources owned by this CSV, and delete them. + log.WithValues("selector", ownerSelector).Info("Cleaning up resources after CSV deletion") + var errs []error + + err := r.Client.DeleteAllOf(ctx, &rbacv1.ClusterRoleBinding{}, &deleteOptions) + if client.IgnoreNotFound(err) != nil { + log.Error(err, "Deleting ClusterRoleBindings on CSV delete") + errs = append(errs, err) + } + + err = r.Client.DeleteAllOf(ctx, &rbacv1.ClusterRole{}, &deleteOptions) + if client.IgnoreNotFound(err) != nil { + log.Error(err, "Deleting ClusterRoles on CSV delete") + errs = append(errs, err) + } + + err = r.Client.DeleteAllOf(ctx, &admissionregistrationv1.MutatingWebhookConfiguration{}, &deleteOptions) + if client.IgnoreNotFound(err) != nil { + log.Error(err, "Deleting MutatingWebhookConfigurations on CSV delete") + errs = append(errs, err) + } + + err = r.Client.DeleteAllOf(ctx, &admissionregistrationv1.ValidatingWebhookConfiguration{}, &deleteOptions) + if client.IgnoreNotFound(err) != nil { + log.Error(err, "Deleting ValidatingWebhookConfigurations on CSV delete") + errs = append(errs, err) + } + + // Make sure things are deleted + crbList := &rbacv1.ClusterRoleBindingList{} + err = r.Client.List(ctx, crbList, &listOptions) + if err != nil { + errs = append(errs, err) + } else if len(crbList.Items) != 0 { + errs = append(errs, fmt.Errorf("waiting for ClusterRoleBindings to delete")) + } + + crList := &rbacv1.ClusterRoleList{} + err = r.Client.List(ctx, crList, &listOptions) + if err != nil { + errs = append(errs, err) + } else if len(crList.Items) != 0 { + errs = append(errs, fmt.Errorf("waiting for ClusterRoles to delete")) + } + + mwcList := &admissionregistrationv1.MutatingWebhookConfigurationList{} + err = r.Client.List(ctx, mwcList, &listOptions) + if err != nil { + errs = append(errs, err) + } else if len(mwcList.Items) != 0 { + errs = append(errs, fmt.Errorf("waiting for MutatingWebhookConfigurations to delete")) + } + + vwcList := &admissionregistrationv1.ValidatingWebhookConfigurationList{} + err = r.Client.List(ctx, vwcList, &listOptions) + if err != nil { + errs = append(errs, err) + } else if len(vwcList.Items) != 0 { + errs = append(errs, fmt.Errorf("waiting for ValidatingWebhookConfigurations to delete")) + } + + // Return any errors + if err := utilerrors.NewAggregate(errs); err != nil { + return err, false + } + + // If no errors, remove our finalizer from the CSV and update + patch := csv.DeepCopy() + controllerutil.RemoveFinalizer(patch, myFinalizerName) + if err := r.Client.Patch(ctx, patch, client.MergeFrom(csv)); err != nil { + log.Error(err, "Removing finalizer") + return err, false + } + + // Stop reconciliation as the csv is being deleted + return nil, false +}