From feb0a80c7422a56a75cef584fd31a633551e2b8f Mon Sep 17 00:00:00 2001 From: travisn Date: Mon, 26 Jun 2023 12:26:09 -0600 Subject: [PATCH 1/2] exporter: ignore failed deletion of service monitor If the ceph version does not suppport the exporter, the operator will attempt to delete the service monitor related to the exporter to ensure it does not exist for a node. If the expected rbac does not exist, this will cause unnecessary errors since there would anyway be no service monitor to delete. So we ignore the error of deleting the service monitor for the exporter. The context is also passed to the exporter so a new kubeconfig does not need to be initiated and cause unnecessary logging about invalid options for the config. Signed-off-by: travisn (cherry picked from commit 417f05311fa911e43c116b08ecc6c7c3fb07ef6b) --- pkg/operator/ceph/cluster/mgr/mgr.go | 2 +- .../ceph/cluster/nodedaemon/exporter.go | 5 ++-- .../ceph/cluster/nodedaemon/reconcile.go | 6 ++-- pkg/operator/k8sutil/prometheus.go | 28 +++++++++++-------- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/pkg/operator/ceph/cluster/mgr/mgr.go b/pkg/operator/ceph/cluster/mgr/mgr.go index 5cd51151a..1af927658 100644 --- a/pkg/operator/ceph/cluster/mgr/mgr.go +++ b/pkg/operator/ceph/cluster/mgr/mgr.go @@ -515,7 +515,7 @@ func (c *Cluster) EnableServiceMonitor() error { applyMonitoringLabels(c, serviceMonitor) - if _, err = k8sutil.CreateOrUpdateServiceMonitor(c.clusterInfo.Context, serviceMonitor); err != nil { + if _, err = k8sutil.CreateOrUpdateServiceMonitor(c.context, c.clusterInfo.Context, serviceMonitor); err != nil { return errors.Wrap(err, "service monitor could not be enabled") } return nil diff --git a/pkg/operator/ceph/cluster/nodedaemon/exporter.go b/pkg/operator/ceph/cluster/nodedaemon/exporter.go index 2e289535b..e8d8573db 100644 --- a/pkg/operator/ceph/cluster/nodedaemon/exporter.go +++ b/pkg/operator/ceph/cluster/nodedaemon/exporter.go @@ -24,6 +24,7 @@ import ( "github.com/pkg/errors" monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" + "github.com/rook/rook/pkg/clusterd" "github.com/rook/rook/pkg/operator/ceph/config" "github.com/rook/rook/pkg/operator/ceph/config/keyring" "github.com/rook/rook/pkg/operator/ceph/controller" @@ -219,7 +220,7 @@ func MakeCephExporterMetricsService(cephCluster cephv1.CephCluster, servicePortM } // EnableCephExporterServiceMonitor add a servicemonitor that allows prometheus to scrape from the monitoring endpoint of the exporter -func EnableCephExporterServiceMonitor(cephCluster cephv1.CephCluster, scheme *runtime.Scheme, opManagerContext context.Context) error { +func EnableCephExporterServiceMonitor(context *clusterd.Context, cephCluster cephv1.CephCluster, scheme *runtime.Scheme, opManagerContext context.Context) error { serviceMonitor := k8sutil.GetServiceMonitor(cephExporterAppName, cephCluster.Namespace) cephv1.GetCephExporterLabels(cephCluster.Spec.Labels).OverwriteApplyToObjectMeta(&serviceMonitor.ObjectMeta) @@ -230,7 +231,7 @@ func EnableCephExporterServiceMonitor(cephCluster cephv1.CephCluster, scheme *ru serviceMonitor.Spec.Selector.MatchLabels = controller.AppLabels(cephExporterAppName, cephCluster.Namespace) applyCephExporterLabels(cephCluster, serviceMonitor) - if _, err = k8sutil.CreateOrUpdateServiceMonitor(opManagerContext, serviceMonitor); err != nil { + if _, err = k8sutil.CreateOrUpdateServiceMonitor(context, opManagerContext, serviceMonitor); err != nil { return errors.Wrap(err, "service monitor could not be enabled") } return nil diff --git a/pkg/operator/ceph/cluster/nodedaemon/reconcile.go b/pkg/operator/ceph/cluster/nodedaemon/reconcile.go index 7a2ff3a26..d9c30c9a0 100644 --- a/pkg/operator/ceph/cluster/nodedaemon/reconcile.go +++ b/pkg/operator/ceph/cluster/nodedaemon/reconcile.go @@ -86,9 +86,9 @@ func (r *ReconcileNode) Reconcile(context context.Context, request reconcile.Req } func (r *ReconcileNode) cleanupExporterResources(clusterNamespace string, ns string, nodeName string) (reconcile.Result, error) { - err := k8sutil.DeleteServiceMonitor(r.opManagerContext, ns, cephExporterAppName) + 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", ns, nodeName) + 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, r.opConfig.OperatorNamespace, cephExporterAppName) if err != nil { @@ -271,7 +271,7 @@ func (r *ReconcileNode) createOrUpdateNodeDaemons(node corev1.Node, tolerations } if cephCluster.Spec.Monitoring.Enabled { - if err := EnableCephExporterServiceMonitor(cephCluster, r.scheme, r.opManagerContext); err != nil { + if err := EnableCephExporterServiceMonitor(r.context, cephCluster, r.scheme, r.opManagerContext); err != nil { return errors.Wrap(err, "failed to enable service monitor") } logger.Debug("service monitor for ceph exporter was enabled successfully") diff --git a/pkg/operator/k8sutil/prometheus.go b/pkg/operator/k8sutil/prometheus.go index ae745b287..e06ada871 100644 --- a/pkg/operator/k8sutil/prometheus.go +++ b/pkg/operator/k8sutil/prometheus.go @@ -21,20 +21,17 @@ import ( "context" "fmt" + "github.com/pkg/errors" monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" monitoringclient "github.com/prometheus-operator/prometheus-operator/pkg/client/versioned" + "github.com/rook/rook/pkg/clusterd" kerror "k8s.io/apimachinery/pkg/api/errors" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/tools/clientcmd" ) -func getMonitoringClient() (*monitoringclient.Clientset, error) { - cfg, err := clientcmd.BuildConfigFromFlags("", "") - if err != nil { - return nil, fmt.Errorf("failed to build config. %v", err) - } - client, err := monitoringclient.NewForConfig(cfg) +func getMonitoringClient(context *clusterd.Context) (*monitoringclient.Clientset, error) { + client, err := monitoringclient.NewForConfig(context.KubeConfig) if err != nil { return nil, fmt.Errorf("failed to get monitoring client. %v", err) } @@ -75,11 +72,11 @@ func GetServiceMonitor(name string, namespace string) *monitoringv1.ServiceMonit } // CreateOrUpdateServiceMonitor creates serviceMonitor object or an error -func CreateOrUpdateServiceMonitor(ctx context.Context, serviceMonitorDefinition *monitoringv1.ServiceMonitor) (*monitoringv1.ServiceMonitor, error) { +func CreateOrUpdateServiceMonitor(context *clusterd.Context, ctx context.Context, serviceMonitorDefinition *monitoringv1.ServiceMonitor) (*monitoringv1.ServiceMonitor, error) { name := serviceMonitorDefinition.GetName() namespace := serviceMonitorDefinition.GetNamespace() logger.Debugf("creating servicemonitor %s", name) - client, err := getMonitoringClient() + client, err := getMonitoringClient(context) if err != nil { return nil, fmt.Errorf("failed to get monitoring client. %v", err) } @@ -104,16 +101,23 @@ func CreateOrUpdateServiceMonitor(ctx context.Context, serviceMonitorDefinition } // DeleteServiceMonitor deletes a ServiceMonitor and returns the error if any -func DeleteServiceMonitor(ctx context.Context, ns string, name string) error { - client, err := getMonitoringClient() +func DeleteServiceMonitor(context *clusterd.Context, ctx context.Context, ns string, name string) error { + client, err := getMonitoringClient(context) if err != nil { return fmt.Errorf("failed to get monitoring client. %v", err) } + _, err = client.MonitoringV1().ServiceMonitors(ns).Get(ctx, name, metav1.GetOptions{}) + if err != nil { + // Either the service monitor does not exist or there are not privileges to detect it + // so we ignore any errors + return nil + } err = client.MonitoringV1().ServiceMonitors(ns).Delete(ctx, name, metav1.DeleteOptions{}) if err != nil { if kerror.IsNotFound(err) { return nil } + return errors.Wrapf(err, "failed to delete service monitor %q", name) } - return err + return nil } From 2f96fcd504744e9a77e30d27c4eba81fd91c404d Mon Sep 17 00:00:00 2001 From: travisn Date: Mon, 26 Jun 2023 12:26:39 -0600 Subject: [PATCH 2/2] ci: add exporter to commit prefixes Add the exporter to the list of commit prefixes Signed-off-by: travisn (cherry picked from commit 96ff80f8b02880dfc3b00120e024ea65f8a0855a) --- .commitlintrc.json | 1 + pkg/operator/k8sutil/prometheus.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.commitlintrc.json b/.commitlintrc.json index 05b430a34..427dcd7ea 100644 --- a/.commitlintrc.json +++ b/.commitlintrc.json @@ -15,6 +15,7 @@ "core", "csi", "docs", + "exporter", "external", "file", "helm", diff --git a/pkg/operator/k8sutil/prometheus.go b/pkg/operator/k8sutil/prometheus.go index e06ada871..fe9bd664e 100644 --- a/pkg/operator/k8sutil/prometheus.go +++ b/pkg/operator/k8sutil/prometheus.go @@ -108,7 +108,7 @@ func DeleteServiceMonitor(context *clusterd.Context, ctx context.Context, ns str } _, err = client.MonitoringV1().ServiceMonitors(ns).Get(ctx, name, metav1.GetOptions{}) if err != nil { - // Either the service monitor does not exist or there are not privileges to detect it + // Either the service monitor does not exist or there are no privileges to detect it // so we ignore any errors return nil }