pkg/metrics: Allow multi port metrics Service creation

CHANGELOG.md

@@ -16,12 +16,14 @@
 - Remove TypeMeta declaration from the implementation of the objects [#1462](https://github.com/operator-framework/operator-sdk/pull/1462/)
 - Relaxed API version format check when parsing `pkg/apis` in code generators. API dir structures can now be of the format `pkg/apis/<group>/<anything>`, where `<anything>` was previously required to be in the Kubernetes version format, ex. `v1alpha1`. ([#1525](https://github.com/operator-framework/operator-sdk/pull/1525))
 - The SDK and operator projects will work outside of `$GOPATH/src` when using [Go modules](https://github.com/golang/go/wiki/Modules). ([#1475](https://github.com/operator-framework/operator-sdk/pull/1475))
+-  `CreateMetricsService()` function from the metrics package accepts an array of ServicePort objects ([]v1.ServicePort) as input to create Service metrics. `CRPortName` constant is added to describe the string of custom resource port name. ([#1560](https://github.com/operator-framework/operator-sdk/pull/1560))
 
 ### Deprecated
 
 ### Removed
 
 - The SDK no longer depends on a `vendor/` directory to manage dependencies *only if* using [Go modules](https://github.com/golang/go/wiki/Modules). The SDK and operator projects will only use vendoring if using `dep`, or modules and a `vendor/` dir is present. ([#1519](https://github.com/operator-framework/operator-sdk/pull/1519))
+- **Breaking change:** `ExposeMetricsPort` is removed and replaced with `CreateMetricsService()` function. `PrometheusPortName` constant is replaced with `OperatorPortName`. ([#1560](https://github.com/operator-framework/operator-sdk/pull/1560))
 
 ### Bug Fixes
 

doc/user/metrics/README.md

@@ -6,16 +6,20 @@
 
 ### General metrics
 
-The `ExposeMetricsPort(ctx context.Context, port int32) (*v1.Service, error)` function exposes general metrics about the running program. These metrics are inherited from controller-runtime. To understand which metrics are exposed, read the metrics package doc of [controller-runtime][controller-metrics]. The `ExposeMetricsPort` function creates a [Service][service] object with the metrics port exposed, which can then be accessed by Prometheus. The Service object is [garbage collected][gc] when the leader pod's root owner is deleted.
+The `"CreateMetricsService(ctx context.Context, servicePorts []v1.ServicePort) (*v1.Service, error)` function exposes general metrics about the running program. These metrics are inherited from controller-runtime. To understand which metrics are exposed, read the metrics package doc of [controller-runtime][controller-metrics]. The `ExposeMetricsPort` function creates a [Service][service] object with the metrics port exposed, which can then be accessed by Prometheus. The Service object is [garbage collected][gc] when the leader pod's root owner is deleted.
 
 By default, the metrics are served on `0.0.0.0:8383/metrics`. To modify the port the metrics are exposed on, change the `var metricsPort int32 = 8383` variable in the `cmd/manager/main.go` file of the generated operator.
 
 #### Usage:
 
 ```go
     import(
+        "context"
+
         "github.com/operator-framework/operator-sdk/pkg/metrics"
         "sigs.k8s.io/controller-runtime/pkg/manager"
+        "k8s.io/api/core/v1"
+        "k8s.io/apimachinery/pkg/util/intstr"
     )
 
     func main() {
@@ -34,14 +38,19 @@ By default, the metrics are served on `0.0.0.0:8383/metrics`. To modify the port
 
         ...
 
+        // Add to the below struct any other metrics ports you want to expose.
+	    servicePorts := []v1.ServicePort{
+		    {Port: metricsPort, Name: metrics.OperatorPortName, Protocol: v1.ProtocolTCP, TargetPort: intstr.IntOrString{Type: intstr.Int, IntVal: metricsPort}},
+	    }
+
         // Create Service object to expose the metrics port.
-        _, err = metrics.ExposeMetricsPort(ctx, metricsPort)
+        _, err = metrics.CreateMetricsService(context.TODO(), servicePorts)
         if err != nil {
             // handle error
-            log.Info(err.Error())
         }
 
         ...
+
     }
 ```
 

hack/tests/e2e-helm.sh

@@ -34,15 +34,17 @@ test_operator() {
     fi
 
     # verify that metrics service was created
-    if ! timeout 20s bash -c -- "until kubectl get service/nginx-operator > /dev/null 2>&1; do sleep 1; done";
+    if ! timeout 20s bash -c -- "until kubectl get service/nginx-operator-metrics > /dev/null 2>&1; do sleep 1; done";
     then
+        echo "Failed to get metrics service"
         kubectl logs deployment/nginx-operator
         exit 1
     fi
 
     # verify that the metrics endpoint exists
-    if ! timeout 1m bash -c -- "until kubectl run -it --rm --restart=Never test-metrics --image=registry.access.redhat.com/ubi7/ubi-minimal:latest -- curl -sfo /dev/null http://nginx-operator:8383/metrics; do sleep 1; done";
+    if ! timeout 1m bash -c -- "until kubectl run -it --rm --restart=Never test-metrics --image=registry.access.redhat.com/ubi7/ubi-minimal:latest -- curl -sfo /dev/null http://nginx-operator-metrics:8383/metrics; do sleep 1; done";
     then
+        echo "Failed to verify that metrics endpoint exists"
         kubectl logs deployment/nginx-operator
         exit 1
     fi

internal/pkg/scaffold/cmd.go

@@ -58,6 +58,8 @@ import (
 	"github.com/operator-framework/operator-sdk/pkg/restmapper"
 	sdkVersion "github.com/operator-framework/operator-sdk/version"
 	"github.com/spf13/pflag"
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
@@ -150,9 +152,14 @@ func main() {
 	if err = serveCRMetrics(cfg); err != nil {
 		log.Info("Could not generate and serve custom resource metrics: ", err.Error())
 	}
-
-	// Create Service object to expose the metrics port.
-	_, err = metrics.ExposeMetricsPort(ctx, metricsPort)
+
+	// Add to the below struct any other metrics ports you want to expose.
+	servicePorts := []v1.ServicePort{
+		{Port: metricsPort, Name: metrics.OperatorPortName, Protocol: v1.ProtocolTCP, TargetPort: intstr.IntOrString{Type: intstr.Int, IntVal: metricsPort}},
+		{Port: operatorMetricsPort, Name: metrics.CRPortName, Protocol: v1.ProtocolTCP, TargetPort: intstr.IntOrString{Type: intstr.Int, IntVal: operatorMetricsPort}},
+	}
+	// Create Service object to expose the metrics port(s).
+	_, err = metrics.CreateMetricsService(ctx, servicePorts)
 	if err != nil {
 		log.Info(err.Error())
 	}

internal/pkg/scaffold/cmd_test.go

@@ -57,6 +57,8 @@ import (
 	"github.com/operator-framework/operator-sdk/pkg/restmapper"
 	sdkVersion "github.com/operator-framework/operator-sdk/version"
 	"github.com/spf13/pflag"
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
@@ -150,8 +152,13 @@ func main() {
 		log.Info("Could not generate and serve custom resource metrics: ", err.Error())
 	}
 
-	// Create Service object to expose the metrics port.
-	_, err = metrics.ExposeMetricsPort(ctx, metricsPort)
+	// Add to the below struct any other metrics ports you want to expose.
+	servicePorts := []v1.ServicePort{
+		{Port: metricsPort, Name: metrics.OperatorPortName, Protocol: v1.ProtocolTCP, TargetPort: intstr.IntOrString{Type: intstr.Int, IntVal: metricsPort}},
+		{Port: operatorMetricsPort, Name: metrics.CRPortName, Protocol: v1.ProtocolTCP, TargetPort: intstr.IntOrString{Type: intstr.Int, IntVal: operatorMetricsPort}},
+	}
+	// Create Service object to expose the metrics port(s).
+	_, err = metrics.CreateMetricsService(ctx, servicePorts)
 	if err != nil {
 		log.Info(err.Error())
 	}

pkg/ansible/run.go

@@ -28,14 +28,19 @@ import (
 	"github.com/operator-framework/operator-sdk/pkg/leader"
 	"github.com/operator-framework/operator-sdk/pkg/metrics"
 	sdkVersion "github.com/operator-framework/operator-sdk/version"
+	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
 )
 
-var log = logf.Log.WithName("cmd")
+var (
+	log               = logf.Log.WithName("cmd")
+	metricsPort int32 = 8383
+)
 
 func printVersion() {
 	log.Info(fmt.Sprintf("Go Version: %s", runtime.Version()))
@@ -66,7 +71,7 @@ func Run(flags *aoflags.AnsibleOperatorFlags) error {
 	// TODO: probably should expose the host & port as an environment variables
 	mgr, err := manager.New(cfg, manager.Options{
 		Namespace:          namespace,
-		MetricsBindAddress: "0.0.0.0:8383",
+		MetricsBindAddress: fmt.Sprintf("0.0.0.0:%d", metricsPort),
 	})
 	if err != nil {
 		log.Error(err, "Failed to create a new manager.")
@@ -85,8 +90,13 @@ func Run(flags *aoflags.AnsibleOperatorFlags) error {
 		return err
 	}
 
+	// Add to the below struct any other metrics ports you want to expose.
+	servicePorts := []v1.ServicePort{
+		{Port: metricsPort, Name: metrics.OperatorPortName, Protocol: v1.ProtocolTCP, TargetPort: intstr.IntOrString{Type: intstr.Int, IntVal: metricsPort}},
+	}
+	// Create Service object to expose the metrics port(s).
 	// TODO: probably should expose the port as an environment variable
-	_, err = metrics.ExposeMetricsPort(context.TODO(), 8383)
+	_, err = metrics.CreateMetricsService(context.TODO(), servicePorts)
 	if err != nil {
 		log.Error(err, "Exposing metrics port failed.")
 		return err

pkg/helm/run.go

@@ -29,7 +29,9 @@ import (
 	"github.com/operator-framework/operator-sdk/pkg/metrics"
 	sdkVersion "github.com/operator-framework/operator-sdk/version"
 
+	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
@@ -117,8 +119,11 @@ func Run(flags *hoflags.HelmOperatorFlags) error {
 		return err
 	}
 
-	// Create Service object to expose the metrics port.
-	_, err = metrics.ExposeMetricsPort(ctx, metricsPort)
+	servicePorts := []v1.ServicePort{
+		{Port: metricsPort, Name: metrics.OperatorPortName, Protocol: v1.ProtocolTCP, TargetPort: intstr.IntOrString{Type: intstr.Int, IntVal: metricsPort}},
+	}
+	// Create Service object to expose the metrics port(s).
+	_, err = metrics.CreateMetricsService(ctx, servicePorts)
 	if err != nil {
 		log.Info(err.Error())
 	}

pkg/metrics/metrics.go

@@ -25,7 +25,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/apimachinery/pkg/util/intstr"
 	crclient "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
@@ -36,19 +35,23 @@ var log = logf.Log.WithName("metrics")
 var trueVar = true
 
 const (
-	// PrometheusPortName defines the port name used in the metrics Service.
-	PrometheusPortName = "metrics"
+	// OperatorPortName defines the default operator metrics port name used in the metrics Service.
+	OperatorPortName = "http-metrics"
+	// CRPortName defines the custom resource specific metrics' port name used in the metrics Service.
+	CRPortName = "cr-metrics"
 )
 
-// ExposeMetricsPort creates a Kubernetes Service to expose the passed metrics port.
-func ExposeMetricsPort(ctx context.Context, port int32) (*v1.Service, error) {
+// CreateMetricsService creates a Kubernetes Service to expose the passed metrics
+// port(s) with the given name(s).
+func CreateMetricsService(ctx context.Context, servicePorts []v1.ServicePort) (*v1.Service, error) {
+	if len(servicePorts) < 1 {
+		return nil, fmt.Errorf("failed to create metrics Serice; service ports were empty")
+	}
 	client, err := createClient()
 	if err != nil {
 		return nil, fmt.Errorf("failed to create new client: %v", err)
 	}
-	// We do not need to check the validity of the port, as controller-runtime
-	// would error out and we would never get to this stage.
-	s, err := initOperatorService(ctx, client, port, PrometheusPortName)
+	s, err := initOperatorService(ctx, client, servicePorts)
 	if err != nil {
 		if err == k8sutil.ErrNoNamespace {
 			log.Info("Skipping metrics Service creation; not running in a cluster.")
@@ -93,8 +96,8 @@ func createOrUpdateService(ctx context.Context, client crclient.Client, s *v1.Se
 	return s, nil
 }
 
-// initOperatorService returns the static service which exposes specified port.
-func initOperatorService(ctx context.Context, client crclient.Client, port int32, portName string) (*v1.Service, error) {
+// initOperatorService returns the static service which exposes specified port(s).
+func initOperatorService(ctx context.Context, client crclient.Client, sp []v1.ServicePort) (*v1.Service, error) {
 	operatorName, err := k8sutil.GetOperatorName()
 	if err != nil {
 		return nil, err
@@ -103,27 +106,16 @@ func initOperatorService(ctx context.Context, client crclient.Client, port int32
 	if err != nil {
 		return nil, err
 	}
-
 	label := map[string]string{"name": operatorName}
 
 	service := &v1.Service{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      operatorName,
+			Name:      fmt.Sprintf("%s-metrics", operatorName),
 			Namespace: namespace,
 			Labels:    label,
 		},
 		Spec: v1.ServiceSpec{
-			Ports: []v1.ServicePort{
-				{
-					Port:     port,
-					Protocol: v1.ProtocolTCP,
-					TargetPort: intstr.IntOrString{
-						Type:   intstr.Int,
-						IntVal: port,
-					},
-					Name: portName,
-				},
-			},
+			Ports:    sp,
 			Selector: label,
 		},
 	}

test/e2e/memcached_test.go

@@ -633,7 +633,7 @@ func memcachedMetricsTest(t *testing.T, f *framework.Framework, ctx *framework.T
 
 	// Make sure metrics Service exists
 	s := v1.Service{}
-	err = f.Client.Get(context.TODO(), types.NamespacedName{Name: operatorName, Namespace: namespace}, &s)
+	err = f.Client.Get(context.TODO(), types.NamespacedName{Name: fmt.Sprintf("%s-metrics", operatorName), Namespace: namespace}, &s)
 	if err != nil {
 		return fmt.Errorf("could not get metrics Service: (%v)", err)
 	}