Skip to content

CSV Finalizer #3148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 11, 2024
Merged

CSV Finalizer #3148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions cmd/olm/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
},
Expand Down
197 changes: 2 additions & 195 deletions pkg/controller/operators/olm/operator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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{}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -948,98 +913,6 @@ func (a *Operator) EnsureCSVMetric() error {
return nil
}

func (a *Operator) syncGCObject(obj interface{}) (syncError error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to self: events on these objects by definition cannot be triggers for cleanup, so losing the cross-queueing should in theory not make any difference

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)
Expand All @@ -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) {
Expand Down Expand Up @@ -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.
Expand Down
Loading