Merge pull request #809 from jpeeler/wip-test-mux

add logging and separate muxer for metrics

Author: openshift-merge-robot

cmd/catalog/main.go

@@ -99,22 +99,35 @@ func main() {
 	if *tlsCertPath != "" && *tlsKeyPath == "" || *tlsCertPath == "" && *tlsKeyPath != "" {
 		logger.Warn("both --tls-key and --tls-crt must be provided for TLS to be enabled, falling back to non-https")
 	} else if *tlsCertPath == "" && *tlsKeyPath == "" {
-		logger.Info("TLS keys not set, using non-https")
+		logger.Info("TLS keys not set, using non-https for metrics")
 	} else {
+		logger.Info("TLS keys set, using https for metrics")
 		useTLS = true
 	}
 
 	// Serve a health check.
-	http.HandleFunc("/healthz", func(w http.ResponseWriter, r *http.Request) {
+	healthMux := http.NewServeMux()
+	healthMux.HandleFunc("/healthz", func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
 	})
-	go http.ListenAndServe(":8080", nil)
+	go http.ListenAndServe(":8080", healthMux)
 
-	http.Handle("/metrics", promhttp.Handler())
+	metricsMux := http.NewServeMux()
+	metricsMux.Handle("/metrics", promhttp.Handler())
 	if useTLS {
-		go http.ListenAndServeTLS(":8081", *tlsCertPath, *tlsKeyPath, nil)
+		go func() {
+			err := http.ListenAndServeTLS(":8081", *tlsCertPath, *tlsKeyPath, metricsMux)
+			if err != nil {
+				logger.Errorf("Metrics (https) serving failed: %v", err)
+			}
+		}()
 	} else {
-		go http.ListenAndServe(":8081", nil)
+		go func() {
+			err := http.ListenAndServe(":8081", metricsMux)
+			if err != nil {
+				logger.Errorf("Metrics (http) serving failed: %v", err)
+			}
+		}()
 	}
 
 	// create a config client for operator status

cmd/olm/main.go

@@ -123,22 +123,40 @@ func main() {
 	if *tlsCertPath != "" && *tlsKeyPath == "" || *tlsCertPath == "" && *tlsKeyPath != "" {
 		logger.Warn("both --tls-key and --tls-crt must be provided for TLS to be enabled, falling back to non-https")
 	} else if *tlsCertPath == "" && *tlsKeyPath == "" {
-		logger.Info("TLS keys not set, using non-https")
+		logger.Info("TLS keys not set, using non-https for metrics")
 	} else {
+		logger.Info("TLS keys set, using https for metrics")
 		useTLS = true
 	}
 
 	// Serve a health check.
-	http.HandleFunc("/healthz", func(w http.ResponseWriter, r *http.Request) {
+	healthMux := http.NewServeMux()
+	healthMux.HandleFunc("/healthz", func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
 	})
-	go http.ListenAndServe(":8080", nil)
+	go func() {
+		err := http.ListenAndServe(":8080", healthMux)
+		if err != nil {
+			logger.Errorf("Health serving failed: %v", err)
+		}
+	}()
 
-	http.Handle("/metrics", promhttp.Handler())
+	metricsMux := http.NewServeMux()
+	metricsMux.Handle("/metrics", promhttp.Handler())
 	if useTLS {
-		go http.ListenAndServeTLS(":8081", *tlsCertPath, *tlsKeyPath, nil)
+		go func() {
+			err := http.ListenAndServeTLS(":8081", *tlsCertPath, *tlsKeyPath, metricsMux)
+			if err != nil {
+				logger.Errorf("Metrics (https) serving failed: %v", err)
+			}
+		}()
 	} else {
-		go http.ListenAndServe(":8081", nil)
+		go func() {
+			err := http.ListenAndServe(":8081", metricsMux)
+			if err != nil {
+				logger.Errorf("Metrics (http) serving failed: %v", err)
+			}
+		}()
 	}
 
 	ready, done, sync := operator.Run(stopCh)

deploy/chart/templates/0000_50_olm_07-olm-operator.deployment.yaml

@@ -88,7 +88,6 @@ spec:
       - name: serving-cert
         secret:
           secretName: olm-operator-serving-cert
-          optional: true
       {{ end }}
     {{- if .Values.olm.nodeSelector }}
       nodeSelector:

deploy/chart/templates/0000_50_olm_08-catalog-operator.deployment.yaml

@@ -84,7 +84,6 @@ spec:
       - name: serving-cert
         secret:
           secretName: catalog-operator-serving-cert
-          optional: true
       {{ end }}
     {{- if .Values.catalog.nodeSelector }}
       nodeSelector:

manifests/0000_50_olm_07-olm-operator.deployment.yaml

@@ -66,7 +66,6 @@ spec:
       - name: serving-cert
         secret:
           secretName: olm-operator-serving-cert
-          optional: true
 
       nodeSelector:
         beta.kubernetes.io/os: linux

manifests/0000_50_olm_08-catalog-operator.deployment.yaml

@@ -63,7 +63,6 @@ spec:
       - name: serving-cert
         secret:
           secretName: catalog-operator-serving-cert
-          optional: true
 
       nodeSelector:
         beta.kubernetes.io/os: linux

test/e2e/metrics_e2e_test.go

@@ -3,12 +3,12 @@
 package e2e
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
 	log "github.com/sirupsen/logrus"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/net"
 )
 
 // TestMetrics tests the metrics endpoint of the OLM pod.
@@ -26,21 +26,49 @@ func TestMetricsEndpoint(t *testing.T) {
 	}
 
 	podName := podList.Items[0].GetName()
+	log.Infof("Looking at pod %v in namespace %v", podName, operatorNamespace)
 
-	rawOutput, err := getMetricsFromPod(t, c, podName, operatorNamespace, 8080)
+	rawOutput, err := getMetricsFromPod(t, c, podName, operatorNamespace, "8081")
 	if err != nil {
 		t.Fatalf("Metrics test failed: %v\n", err)
 	}
 
 	log.Debugf("Metrics:\n%v", rawOutput)
 }
 
-func getMetricsFromPod(t *testing.T, client operatorclient.ClientInterface, podName string, namespace string, port int) (string, error) {
+func getMetricsFromPod(t *testing.T, client operatorclient.ClientInterface, podName string, namespace string, port string) (string, error) {
+	olmPod, err := client.KubernetesInterface().CoreV1().Pods(namespace).Get(podName, metav1.GetOptions{})
+	if err != nil {
+		return "", err
+	}
+	if len(olmPod.Spec.Containers) != 1 {
+		t.Fatalf("Expected only 1 container in olm-operator pod, got %v", len(olmPod.Spec.Containers))
+	}
+
+	var foundCert bool
+	var foundKey bool
+	// assuming -tls-cert and -tls-key aren't used anywhere else as a parameter value
+	for _, param := range olmPod.Spec.Containers[0].Args {
+		if param == "-tls-cert" {
+			foundCert = true
+		} else if param == "-tls-key" {
+			foundKey = true
+		}
+	}
+
+	var scheme string
+	if foundCert && foundKey {
+		scheme = "https"
+	} else {
+		scheme = "http"
+	}
+	log.Infof("Retrieving metrics using scheme %v\n", scheme)
+
 	rawOutput, err := client.KubernetesInterface().CoreV1().RESTClient().Get().
 		Namespace(namespace).
 		Resource("pods").
 		SubResource("proxy").
-		Name(fmt.Sprintf("%v:%v", podName, port)).
+		Name(net.JoinSchemeNamePort(scheme, podName, port)).
 		Suffix("metrics").
 		Do().Raw()
 	if err != nil {