Remove dependency on operator-sdk metrics package

This commit removes the dependency on operator-sdk
metrics package by adding service and service monitors
through package manifests and validating their creation
on operator startup. Since WMCO is no longer creating a
service with selector, no Endpoint object is created.
This commit also ensures we create an Endpoints object
before accessing it. We cannot create Endpoints resources
through manifests as they are not currently OLM supported
operator-framework/operator-lifecycle-manager#1996

Author: VaishnaviHire

cmd/manager/main.go

@@ -22,7 +22,7 @@ import (
 	"github.com/openshift/windows-machine-config-operator/apis"
 	"github.com/openshift/windows-machine-config-operator/controllers"
 	"github.com/openshift/windows-machine-config-operator/pkg/cluster"
-	winmetrics "github.com/openshift/windows-machine-config-operator/pkg/metrics"
+	"github.com/openshift/windows-machine-config-operator/pkg/metrics"
 	"github.com/openshift/windows-machine-config-operator/pkg/nodeconfig/payload"
 	"github.com/openshift/windows-machine-config-operator/version"
 )
@@ -131,7 +131,7 @@ func main() {
 	// Create a new Cmd to provide shared dependencies and start components
 	mgr, err := manager.New(cfg, manager.Options{
 		NewCache:           cache.MultiNamespacedCacheBuilder(namespaces),
-		MetricsBindAddress: fmt.Sprintf("%s:%d", winmetrics.Host, winmetrics.Port),
+		MetricsBindAddress: fmt.Sprintf("%s:%d", metrics.Host, metrics.Port),
 	})
 	if err != nil {
 		log.Error(err, "failed to create a new Manager")
@@ -152,9 +152,16 @@ func main() {
 		os.Exit(1)
 	}
 
-	// Add the Metrics Service and Service Monitor
-	if err := winmetrics.Add(ctx, cfg, namespace); err != nil {
-		log.Error(err, "failed to add Metrics Service and Service Monitor")
+	metricsConfig, err := metrics.NewConfig(mgr, cfg, namespace)
+	if err != nil {
+		log.Error(err, "failed to create MetricsConfig object")
+		os.Exit(1)
+	}
+
+	// Configure the metric resources
+	if err := metricsConfig.Configure(ctx); err != nil {
+		log.Error(err, "error setting up metrics")
+		os.Exit(1)
 	}
 
 	log.Info("starting the Cmd.")

deploy/olm-catalog/windows-machine-config-operator/manifests/service-monitor.yaml

@@ -0,0 +1,23 @@
+# Prometheus Monitor Service (Metrics)
+apiVersion: monitoring.coreos.com/v1
+kind: ServiceMonitor
+metadata:
+  labels:
+    name: windows-machine-config-operator
+  name: windows-machine-config-operator-metrics
+spec:
+  endpoints:
+    - path: /metrics
+      port: metrics
+      interval: 30s
+      honorLabels: true
+      relabelings:
+        - action: replace
+          regex: (.*)
+          replacement: $1
+          sourceLabels:
+            - __meta_kubernetes_endpoint_address_target_name
+          targetLabel: instance
+  selector:
+    matchLabels:
+      name: windows-machine-config-operator

deploy/olm-catalog/windows-machine-config-operator/manifests/service.yaml

@@ -0,0 +1,12 @@
+kind: Service
+apiVersion: v1
+metadata:
+  name: windows-machine-config-operator-metrics
+  labels:
+    name: windows-machine-config-operator
+spec:
+  ports:
+    - name: metrics
+      protocol: TCP
+      port: 9182
+      targetPort: 9182

deploy/olm-catalog/windows-machine-config-operator/manifests/windows-machine-config-operator.clusterserviceversion.yaml

@@ -284,8 +284,6 @@ spec:
           - create
           - delete
           - get
-          - update
-          - patch
         - apiGroups:
           - ""
           resources:
@@ -304,16 +302,20 @@ spec:
           - create
           - get
           - update
+        - apiGroups:
+          - ""
+          resources:
+          - namespaces
+          verbs:
+          - get
         - apiGroups:
           - monitoring.coreos.com
           resources:
           - servicemonitors
           verbs:
           - get
           - create
-          - update
           - list
-          - patch
           - delete
         - apiGroups:
           - apps

deploy/role.yaml

@@ -23,8 +23,6 @@ rules:
   - create
   - delete
   - get
-  - update
-  - patch
 - apiGroups:
     - ""
   resources:
@@ -43,16 +41,20 @@ rules:
   - create
   - get
   - update
+- apiGroups:
+  - ""
+  resources:
+  - namespaces
+  verbs:
+  - get
 - apiGroups:
   - monitoring.coreos.com
   resources:
   - servicemonitors
   verbs:
   - get
   - create
-  - update
   - list
-  - patch
   - delete
 # deployment/finalizers permissions needed for the metrics server
 - apiGroups:

pkg/metrics/metrics.go

@@ -3,21 +3,20 @@ package metrics
 import (
 	"context"
 	"encoding/json"
-	"fmt"
 
 	"github.com/operator-framework/operator-sdk/pkg/k8sutil"
-	"github.com/operator-framework/operator-sdk/pkg/metrics"
 	"github.com/pkg/errors"
 	monclient "github.com/prometheus-operator/prometheus-operator/pkg/client/versioned/typed/monitoring/v1"
 	"k8s.io/api/core/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	core "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/kubernetes"
 	k8sclient "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
+	"k8s.io/client-go/tools/record"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
 
 	"github.com/openshift/windows-machine-config-operator/pkg/nodeconfig"
 )
@@ -26,8 +25,6 @@ var (
 	log = logf.Log.WithName("metrics")
 	// metricsEnabled specifies if metrics are enabled in the current cluster
 	metricsEnabled = true
-	// windowsMetricsResource is the name of an object created for Windows metrics
-	windowsMetricsResource = ""
 )
 
 const (
@@ -37,6 +34,9 @@ const (
 	Host = "0.0.0.0"
 	// Port is the port number on which windows-exporter is exposed.
 	Port int32 = 9182
+	// WindowsMetricsResource is the name for objects created for Prometheus monitoring
+	// by current operator version. Its name is defined through the bundle manifests
+	WindowsMetricsResource = "windows-machine-config-operator-metrics"
 )
 
 // PrometheusNodeConfig holds the information required to configure Prometheus, so that it can scrape metrics from the
@@ -48,6 +48,18 @@ type PrometheusNodeConfig struct {
 	namespace string
 }
 
+// Config holds the information required to interact with metrics objects
+type Config struct {
+	// a handle that allows us to interact with the Kubernetes API.
+	*kubernetes.Clientset
+	// a handle that allows us to interact with the Monitoring API.
+	*monclient.MonitoringV1Client
+	// namespace is the namespace in which metrics objects are created
+	namespace string
+	// recorder to generate events
+	Recorder record.EventRecorder
+}
+
 // patchEndpoint contains information regarding patching metrics Endpoint
 type patchEndpoint struct {
 	// op defines patch operation to be performed on the Endpoints object
@@ -71,74 +83,21 @@ func NewPrometheusNodeConfig(clientset *kubernetes.Clientset) (*PrometheusNodeCo
 	}, err
 }
 
-// Add will create the Services and Service Monitors that allows the operator to export the metrics by using
-// the Prometheus operator
-func Add(ctx context.Context, cfg *rest.Config, namespace string) error {
-	// Add to the below struct any other metrics ports you want to expose.
-	servicePorts := []v1.ServicePort{
-		{Port: Port, Name: PortName, Protocol: v1.ProtocolTCP, TargetPort: intstr.IntOrString{Type: intstr.Int, IntVal: Port}},
-	}
-
-	// Create Service object to expose the metrics port(s).
-	service, err := metrics.CreateMetricsService(ctx, cfg, servicePorts)
-	if err != nil {
-		return errors.Wrap(err, "could not create metrics Service")
+// NewConfig creates a new instance for Config  to be used by the caller.
+func NewConfig(mgr manager.Manager, cfg *rest.Config, namespace string) (*Config, error) {
+	if cfg == nil {
+		return nil, errors.New("config should not be nil")
 	}
-
-	// the name for the metrics resources is set during creation of metrics service and is equivalent to the service name
-	windowsMetricsResource = service.GetName()
-
-	// Create a monitoring client to interact with the ServiceMonitor object
-	mclient, err := monclient.NewForConfig(cfg)
-	if err != nil {
-		return errors.Wrap(err, "could not create monitoring client")
-	}
-
-	// In the case of an operator restart, a previous SM object will be deleted and a new one will
-	// be created. We are deleting to ensure that the SM always exists with the correct spec. Otherwise,
-	// metrics may exhibit unexpected behavior if created by a previous version of WMCO.
-	err = mclient.ServiceMonitors(namespace).Delete(context.TODO(), windowsMetricsResource, metav1.DeleteOptions{})
-	if err != nil && !apierrors.IsNotFound(err) {
-		return errors.Wrap(err, "could not delete existing ServiceMonitor object")
-	}
-
-	// CreateServiceMonitors will automatically create the prometheus-operator ServiceMonitor resources
-	// necessary to configure Prometheus to scrape metrics from this operator.
-	services := []*v1.Service{service}
-	_, err = metrics.CreateServiceMonitors(cfg, namespace, services)
-	if err != nil {
-		log.Error(err, "could not create ServiceMonitor object")
-		// If this operator is deployed to a cluster without the prometheus-operator running, it will return
-		// ErrServiceMonitorNotPresent, which can be used to safely skip ServiceMonitor creation.
-		if err == metrics.ErrServiceMonitorNotPresent {
-			metricsEnabled = false
-			return errors.Wrap(err, "install prometheus-operator in your cluster to create ServiceMonitor objects")
-
-		}
-	}
-
-	// The ServiceMonitor created by the operator-sdk metrics package doesn't have fields required to display
-	// node graphs for Windows. Update the Service monitor with the required fields.
-	err = updateServiceMonitors(cfg, namespace)
-	if err != nil {
-		return errors.Wrap(err, "error updating service monitor")
-	}
-
 	oclient, err := k8sclient.NewForConfig(cfg)
 	if err != nil {
-		return errors.Wrap(err, "could not create config clientset")
+		return nil, errors.Wrap(err, "error creating config client")
 	}
-	// When a selector is present in a headless service i.e. spec.ClusterIP=None, Kubernetes manages the
-	// list of endpoints reverting all the changes made by the operator. Remove selector from Metrics Service to avoid
-	// reverting changes in the Endpoints object.
-	patchData := fmt.Sprintf(`{"spec":{"selector": null }}`)
-	service, err = oclient.CoreV1().Services(namespace).Patch(ctx, service.Name, types.MergePatchType,
-		[]byte(patchData), metav1.PatchOptions{})
+	mclient, err := monclient.NewForConfig(cfg)
 	if err != nil {
-		return errors.Wrap(err, "could not remove selector from metrics service")
+		return nil, errors.Wrap(err, "error creating monitoring client")
 	}
-
-	return nil
+	return &Config{oclient, mclient, namespace,
+		mgr.GetEventRecorderFor("metrics")}, nil
 }
 
 // syncMetricsEndpoint updates the endpoint object with the new list of IP addresses from the Windows nodes and the
@@ -171,7 +130,7 @@ func (pc *PrometheusNodeConfig) syncMetricsEndpoint(nodeEndpointAdressess []v1.E
 	}
 
 	_, err = pc.k8sclientset.CoreV1().Endpoints(pc.namespace).
-		Patch(context.TODO(), windowsMetricsResource, types.JSONPatchType, patchDataBytes, metav1.PatchOptions{})
+		Patch(context.TODO(), WindowsMetricsResource, types.JSONPatchType, patchDataBytes, metav1.PatchOptions{})
 	return errors.Wrap(err, "unable to sync metrics endpoints")
 }
 
@@ -191,9 +150,9 @@ func (pc *PrometheusNodeConfig) Configure() error {
 
 	// get Metrics Endpoints object
 	endpoints, err := pc.k8sclientset.CoreV1().Endpoints(pc.namespace).Get(context.TODO(),
-		windowsMetricsResource, metav1.GetOptions{})
+		WindowsMetricsResource, metav1.GetOptions{})
 	if err != nil {
-		return errors.Wrapf(err, "could not get metrics endpoints %v", windowsMetricsResource)
+		return errors.Wrapf(err, "could not get metrics endpoints %v", WindowsMetricsResource)
 	}
 
 	if !isEndpointsValid(nodes, endpoints) {
@@ -204,7 +163,7 @@ func (pc *PrometheusNodeConfig) Configure() error {
 			return errors.Wrap(err, "error updating endpoints object with list of endpoint addresses")
 		}
 	}
-	log.Info("Prometheus configured", "endpoints", windowsMetricsResource, "port", Port, "name", PortName)
+	log.Info("Prometheus configured", "endpoints", WindowsMetricsResource, "port", Port, "name", PortName)
 	return nil
 }
 
@@ -256,25 +215,60 @@ func isEndpointsValid(nodes *v1.NodeList, endpoints *v1.Endpoints) bool {
 	return true
 }
 
-// updateServiceMonitors patches the metrics Service Monitor to include required fields to display node graphs on the
-// OpenShift console. Console graph queries require metrics endpoint target name to be node name, however
-// windows_exporter returns node IP. We replace the target name by adding `replace` action field to the ServiceMonitor
-// object that replaces node IP to node name as the metrics endpoint target.
-func updateServiceMonitors(cfg *rest.Config, namespace string) error {
-
-	patchData := fmt.Sprintf("[{\"op\": \"replace\", \"path\": \"/spec/endpoints/0\", "+
-		"\"value\":{\"path\": \"/%s\",\"port\": \"%s\",\"relabelings\": [{\"action\": \"replace\", \"regex\": \"(.*)\", "+
-		"\"replacement\": \"$1\", \"sourceLabels\": [\"__meta_kubernetes_endpoint_address_target_name\"],"+
-		"\"targetLabel\": \"instance\"}]}}]", PortName, PortName)
+// Configure takes care of all the required configuration steps
+// for Prometheus monitoring like validating monitoring label
+// and creating metrics Endpoints object.
+func (c *Config) Configure(ctx context.Context) error {
+	// validate if cluster monitoring is enabled in the operator namespace
+	if err := c.validate(ctx); err != nil {
+		log.Error(err, "error validating cluster monitoring label")
+		return nil
+	}
+	// Create Metrics Endpoint object in the operator namespace
+	if err := c.createEndpoint(); err != nil {
+		return errors.Wrap(err, "error creating metrics Endpoint")
+	}
+	return nil
+}
 
-	mclient, err := monclient.NewForConfig(cfg)
+// validate will verify if cluster monitoring is enabled in the operator namespace.
+// If the label is not present, it will log and send warning events to the user.
+func (c *Config) validate(ctx context.Context) error {
+	// validate if metrics label is added to namespace
+	wmcoNamespace, err := c.CoreV1().Namespaces().Get(ctx, c.namespace, metav1.GetOptions{})
 	if err != nil {
-		return errors.Wrap(err, "error creating monitoring client")
+		return errors.Wrap(err, "error getting operator namespace")
+	}
+	if wmcoNamespace.Labels["openshift.io/cluster-monitoring"] != "true" {
+		metricsEnabled = false
+		c.Recorder.Eventf(wmcoNamespace, core.EventTypeWarning, "labelValidationFailed",
+			"Cluster monitoring openshift.io/cluster-monitoring label is not enabled in %s namespace", c.namespace)
+		return errors.Errorf("monitoring not enabled in %s namespace", c.namespace)
+	}
+	return nil
+}
+
+// createEndpoint creates an endpoint object in the operator namespace.
+// WMCO is no longer creating a service with a selector therefore no Endpoint
+// object is created and WMCO needs to create the Endpoint object.
+// We cannot create endpoints as a part of manifests deployment as
+// Endpoints resources are not currently OLM-supported for bundle creation.
+func (c *Config) createEndpoint() error {
+	// create new Endpoint
+	newEndpoint := &v1.Endpoints{
+		TypeMeta: metav1.TypeMeta{
+			Kind: "Endpoints",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      WindowsMetricsResource,
+			Namespace: c.namespace,
+			Labels:    map[string]string{"name": WindowsMetricsResource},
+		},
+		Subsets: nil,
 	}
-	_, err = mclient.ServiceMonitors(namespace).Patch(context.TODO(), windowsMetricsResource, types.JSONPatchType, []byte(patchData),
-		metav1.PatchOptions{})
+	_, err := c.CoreV1().Endpoints(c.namespace).Create(context.TODO(), newEndpoint, metav1.CreateOptions{})
 	if err != nil {
-		return errors.Wrap(err, "unable to patch service monitor")
+		return errors.Wrap(err, "error creating metrics Endpoint")
 	}
 	return nil
 }