From 0cce6bac212314945b16f75a7b411cb5deae0c8b Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Fri, 6 Mar 2026 15:55:47 -0700 Subject: [PATCH 1/3] exporter: remove obsolete version checking for ceph v18 Rook no longer supports earlier than Ceph v18, so remove the obsolete version checking in the ceph exporter reconcile. Signed-off-by: Travis Nielsen (cherry picked from commit 733406b175084a3f01454550d738dfaec1b7782b) --- .../ceph/cluster/nodedaemon/exporter.go | 8 --- .../ceph/cluster/nodedaemon/reconcile.go | 67 ++++++------------- 2 files changed, 19 insertions(+), 56 deletions(-) diff --git a/pkg/operator/ceph/cluster/nodedaemon/exporter.go b/pkg/operator/ceph/cluster/nodedaemon/exporter.go index 90d9c99a7..bd4b944e8 100644 --- a/pkg/operator/ceph/cluster/nodedaemon/exporter.go +++ b/pkg/operator/ceph/cluster/nodedaemon/exporter.go @@ -51,16 +51,8 @@ const ( exporterKeyName = "rook-ceph-exporter-keyring" ) -var MinVersionForCephExporter = cephver.CephVersion{Major: 18, Minor: 0, Extra: 0} - // createOrUpdateCephExporter is a wrapper around controllerutil.CreateOrUpdate func (r *ReconcileNode) createOrUpdateCephExporter(node corev1.Node, tolerations []corev1.Toleration, cephCluster cephv1.CephCluster, cephVersion *cephver.CephVersion) (controllerutil.OperationResult, error) { - // CephVersion change is done temporarily, as some regression was detected in Ceph version 17.2.6 which is summarised here https://github.com/ceph/ceph/pull/50718#issuecomment-1505608312. - // Thus, disabling ceph-exporter for now until all the regression are fixed. - if !cephVersion.IsAtLeast(MinVersionForCephExporter) { - log.NamespacedInfo(cephCluster.Namespace, logger, "Skipping exporter reconcile on ceph version %q", cephVersion.String()) - return controllerutil.OperationResultNone, nil - } if cephCluster.Spec.Monitoring.MetricsDisabled { log.NamespacedInfo(cephCluster.Namespace, logger, "Skipping exporter reconcile since monitoring.metricsDisabled is true") return controllerutil.OperationResultNone, nil diff --git a/pkg/operator/ceph/cluster/nodedaemon/reconcile.go b/pkg/operator/ceph/cluster/nodedaemon/reconcile.go index 12ce022c2..963b41294 100644 --- a/pkg/operator/ceph/cluster/nodedaemon/reconcile.go +++ b/pkg/operator/ceph/cluster/nodedaemon/reconcile.go @@ -42,6 +42,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" @@ -83,18 +84,6 @@ func (r *ReconcileNode) Reconcile(context context.Context, request reconcile.Req return result, err } -func (r *ReconcileNode) cleanupExporterResources(ns string, nodeName string) (reconcile.Result, error) { - err := k8sutil.DeleteServiceMonitor(r.context, r.opManagerContext, ns, cephExporterAppName) - if err != nil { - logger.Debugf("failed to delete service monitor for ceph exporter in namespace %q on node %q. %v", ns, nodeName, err) - } - err = k8sutil.DeleteService(r.opManagerContext, r.context.Clientset, ns, cephExporterAppName) - if err != nil { - return reconcile.Result{}, errors.Wrapf(err, "failed to delete ceph exporter metrics service in namespace %q on node %q", ns, nodeName) - } - return reconcile.Result{}, nil -} - func (r *ReconcileNode) reconcile(request reconcile.Request) (reconcile.Result, error) { log.NamespacedDebug(request.Namespace, logger, "reconciling node: %q", request.Name) @@ -206,20 +195,6 @@ func (r *ReconcileNode) reconcile(request reconcile.Request) (reconcile.Result, return reconcile.Result{}, errors.Wrapf(err, "failed to list and delete deployments in namespace %q on node %q", namespace, request.Name) } } - // Cleanup exporter if the ceph version isn't supported - if !cephVersion.IsAtLeast(MinVersionForCephExporter) { - for _, cephPod := range cephPods { - if cephPod.Spec.NodeName == request.Name { - if err := r.listDeploymentAndDelete(cephExporterAppName, request.Name, namespace); err != nil { - return reconcile.Result{}, errors.Wrap(err, "failed to delete ceph-exporter") - } - result, err := r.cleanupExporterResources(namespace, request.Name) - if err != nil { - return result, errors.Wrapf(err, "failed to cleanup exporter resources in namespace %q on node %q", namespace, request.Name) - } - } - } - } if err := r.reconcileCrashPruner(namespace, cephCluster, tolerations); err != nil { return reconcile.Result{}, err @@ -242,35 +217,31 @@ func (r *ReconcileNode) createOrUpdateNodeDaemons(node corev1.Node, tolerations log.NamespacedDebug(cephCluster.Namespace, logger, "crash collector successfully reconciled for node %q. operation: %q", node.Name, op) } } - if cephVersion.IsAtLeast(MinVersionForCephExporter) && !cephCluster.Spec.Monitoring.MetricsDisabled { + if !cephCluster.Spec.Monitoring.MetricsDisabled { op, err := r.createOrUpdateCephExporter(node, tolerations, cephCluster, cephVersion) if err != nil { - if op == "unchanged" { - log.NamespacedDebug(cephCluster.Namespace, logger, "ceph exporter unchanged on node %q", node.Name) + if op == controllerutil.OperationResultNone { + log.NamespacedInfo(cephCluster.Namespace, logger, "ceph exporter unchanged on node %q", node.Name) } else { return errors.Wrapf(err, "ceph exporter reconcile failed on op %q", op) } - } else { - // CephVersion change is done temporarily, as some regression was detected in Ceph version 17.2.6 which is summarised here https://github.com/ceph/ceph/pull/50718#issuecomment-1505608312. - // Thus, disabling ceph-exporter for now until all the regression are fixed. - if cephVersion.IsAtLeast(MinVersionForCephExporter) { - log.NamespacedDebug(cephCluster.Namespace, logger, "ceph exporter successfully reconciled for node %q. operation: %q", node.Name, op) - // create the metrics service - service, err := MakeCephExporterMetricsService(cephCluster, exporterServiceMetricName, r.scheme) - if err != nil { - return err - } - if _, err := k8sutil.CreateOrUpdateService(r.opManagerContext, r.context.Clientset, cephCluster.Namespace, service); err != nil { - return errors.Wrap(err, "failed to create ceph-exporter metrics service") - } + } - if cephCluster.Spec.Monitoring.Enabled { - if err := EnableCephExporterServiceMonitor(r.context, cephCluster, r.scheme, r.opManagerContext, exporterServiceMetricName); err != nil { - return errors.Wrap(err, "failed to enable service monitor") - } - log.NamespacedDebug(cephCluster.Namespace, logger, "service monitor for ceph exporter was enabled successfully") - } + log.NamespacedDebug(cephCluster.Namespace, logger, "ceph exporter successfully reconciled for node %q. operation: %q", node.Name, op) + // create the metrics service + service, err := MakeCephExporterMetricsService(cephCluster, exporterServiceMetricName, r.scheme) + if err != nil { + return err + } + if _, err := k8sutil.CreateOrUpdateService(r.opManagerContext, r.context.Clientset, cephCluster.Namespace, service); err != nil { + return errors.Wrap(err, "failed to create ceph-exporter metrics service") + } + + if cephCluster.Spec.Monitoring.Enabled { + if err := EnableCephExporterServiceMonitor(r.context, cephCluster, r.scheme, r.opManagerContext, exporterServiceMetricName); err != nil { + return errors.Wrap(err, "failed to enable service monitor") } + log.NamespacedDebug(cephCluster.Namespace, logger, "service monitor for ceph exporter was enabled successfully") } } From 46d9c79f8ac6498ec90e88f87b038ed18251a8ea Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Fri, 6 Mar 2026 16:01:16 -0700 Subject: [PATCH 2/3] exporter: remove exporter resources as best effort Remove the exporter deployment when needed as best effort in case the removal fails, there is no need to block other reconcile. Signed-off-by: Travis Nielsen (cherry picked from commit 256a338c14c6872a0de0413b9ef3a0aeb4ea0127) --- .../ceph/cluster/nodedaemon/reconcile.go | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/pkg/operator/ceph/cluster/nodedaemon/reconcile.go b/pkg/operator/ceph/cluster/nodedaemon/reconcile.go index 963b41294..ef7c0ebba 100644 --- a/pkg/operator/ceph/cluster/nodedaemon/reconcile.go +++ b/pkg/operator/ceph/cluster/nodedaemon/reconcile.go @@ -93,10 +93,7 @@ func (r *ReconcileNode) reconcile(request reconcile.Request) (reconcile.Result, if err != nil { if kerrors.IsNotFound(err) { // if a node is not present, check if there are any node daemons to remove - err := r.listNodeDaemonsAndDelete(request.Name, "") - if err != nil { - log.NamespacedError(request.Namespace, logger, "failed to list and delete deployment on node %q; user should delete them manually. %v", request.Name, err) - } + r.listNodeDaemonsAndDelete(request.Name, "") } else { return reconcile.Result{}, errors.Wrapf(err, "could not get node %q", request.Name) } @@ -190,10 +187,7 @@ func (r *ReconcileNode) reconcile(request reconcile.Request) (reconcile.Result, } else { // If there are no Ceph pods, check that there are no crash collector or ceph-exporter pods in case Ceph pods moved to another node // Thus the crash collector and ceph-exporter must be removed from that node - err := r.listNodeDaemonsAndDelete(request.Name, namespace) - if err != nil { - return reconcile.Result{}, errors.Wrapf(err, "failed to list and delete deployments in namespace %q on node %q", namespace, request.Name) - } + r.listNodeDaemonsAndDelete(request.Name, namespace) } if err := r.reconcileCrashPruner(namespace, cephCluster, tolerations); err != nil { @@ -324,18 +318,17 @@ func (r *ReconcileNode) cephPodList() ([]corev1.Pod, error) { return cephPods, nil } -func (r *ReconcileNode) listNodeDaemonsAndDelete(nodeName, ns string) error { +// Delete the daemons and related resources as a best-effort +func (r *ReconcileNode) listNodeDaemonsAndDelete(nodeName, ns string) { // delete the crash daemons on the given node if err := r.listDeploymentAndDelete(CrashCollectorAppName, nodeName, ns); err != nil { - return errors.Wrap(err, "failed to delete crash collector") + log.NamespacedInfo(ns, logger, "failed to delete crash collector. %v", err) } // delete the ceph-exporter daemons on the given node if err := r.listDeploymentAndDelete(cephExporterAppName, nodeName, ns); err != nil { - return errors.Wrap(err, "failed to delete ceph-exporter") + log.NamespacedInfo(ns, logger, "failed to delete ceph-exporter. %v", err) } - - return nil } func (r *ReconcileNode) deleteDeployment(deployment appsv1.Deployment) error { From 9ca5560d221a3d3b38894f943cd4ce9ac47657bf Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Fri, 6 Mar 2026 16:14:30 -0700 Subject: [PATCH 3/3] exporter: continue reconciling all clusters when disabled in the first When reconciling the crash collectors and exporters, if one cluster has disabled them, they will be deleted, then the reconcile will complete, thus skipping reconcile of the crash collectors and exporters for the remaining clusters. Signed-off-by: Travis Nielsen (cherry picked from commit 71e1dbffa46a6aeaa252a67b8d2e9fc6194421b1) --- pkg/operator/ceph/cluster/nodedaemon/reconcile.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/operator/ceph/cluster/nodedaemon/reconcile.go b/pkg/operator/ceph/cluster/nodedaemon/reconcile.go index ef7c0ebba..5dbf0f343 100644 --- a/pkg/operator/ceph/cluster/nodedaemon/reconcile.go +++ b/pkg/operator/ceph/cluster/nodedaemon/reconcile.go @@ -92,8 +92,9 @@ func (r *ReconcileNode) reconcile(request reconcile.Request) (reconcile.Result, err := r.client.Get(r.opManagerContext, request.NamespacedName, node) if err != nil { if kerrors.IsNotFound(err) { - // if a node is not present, check if there are any node daemons to remove - r.listNodeDaemonsAndDelete(request.Name, "") + // if a node is not present, ignore the event + log.NamespacedDebug(request.Namespace, logger, "node %q not found. Ignoring since object must be deleted. %v", request.Name, err) + return reconcile.Result{}, nil } else { return reconcile.Result{}, errors.Wrapf(err, "could not get node %q", request.Name) } @@ -142,7 +143,7 @@ func (r *ReconcileNode) reconcile(request reconcile.Request) (reconcile.Result, allDisabled := r.removeDisabledCrashCollectorDaemons(cephCluster.Spec, namespace) && r.removeDisabledCephExporterDaemons(cephCluster.Spec, namespace) if allDisabled { - return reconcile.Result{}, nil + continue } // checking if secret "rook-ceph-crash-collector-keyring" is present which is required to create crashcollector pods