From d5fe6fedc91ffbd4930b3414a0bed2847d95b7b2 Mon Sep 17 00:00:00 2001 From: "Benjamin A. Petersen" Date: Wed, 30 Oct 2019 16:58:53 -0400 Subject: [PATCH 1/3] Delay registry of console_url metric until starter.go --- pkg/console/metrics/metrics.go | 33 +++++++++++++++++++++++++++++++ pkg/console/operator/operator.go | 8 ++++++++ pkg/console/operator/sync_v400.go | 20 ++----------------- pkg/console/starter/starter.go | 6 ++++++ 4 files changed, 49 insertions(+), 18 deletions(-) create mode 100644 pkg/console/metrics/metrics.go diff --git a/pkg/console/metrics/metrics.go b/pkg/console/metrics/metrics.go new file mode 100644 index 0000000000..8100678b34 --- /dev/null +++ b/pkg/console/metrics/metrics.go @@ -0,0 +1,33 @@ +package metrics + +import ( + "sync" + + "github.com/prometheus/client_golang/prometheus" +) + +type ConsolePrometheusMetrics struct { + ConsoleURL *prometheus.GaugeVec +} + +var singleton *ConsolePrometheusMetrics +var once sync.Once + +// using an init() func for registering metrics is error +// prone because the prometheus client registry may not be ready. +func Register() *ConsolePrometheusMetrics { + // thread safe + once.Do(func() { + singleton = &ConsolePrometheusMetrics{} + + // metric: console_url{url="https://"} 1 + singleton.ConsoleURL = prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Name: "console_url", + Help: "URL of the console exposed on the cluster", + // one label + }, []string{"url"}) + + prometheus.MustRegister(singleton.ConsoleURL) + }) + return singleton +} diff --git a/pkg/console/operator/operator.go b/pkg/console/operator/operator.go index 285e5aad01..9108bcd271 100644 --- a/pkg/console/operator/operator.go +++ b/pkg/console/operator/operator.go @@ -6,6 +6,8 @@ import ( "reflect" "time" + "github.com/openshift/console-operator/pkg/console/metrics" + // kube "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -70,6 +72,8 @@ type consoleOperator struct { routeClient routeclientv1.RoutesGetter oauthClient oauthclientv1.OAuthClientsGetter versionGetter status.VersionGetter + // metrics + consoleMetrics *metrics.ConsolePrometheusMetrics // recorder recorder events.Recorder resourceSyncer resourcesynccontroller.ResourceSyncer @@ -96,6 +100,8 @@ func NewConsoleOperator( oauthClients oauthinformersv1.OAuthClientInformer, // openshift managed managedCoreV1 corev1.Interface, + // metrics + consoleMetrics *metrics.ConsolePrometheusMetrics, // event handling versionGetter status.VersionGetter, recorder events.Recorder, @@ -117,6 +123,8 @@ func NewConsoleOperator( routeClient: routev1Client, oauthClient: oauthv1Client, versionGetter: versionGetter, + // metrics + consoleMetrics: consoleMetrics, // recorder recorder: recorder, resourceSyncer: resourceSyncer, diff --git a/pkg/console/operator/sync_v400.go b/pkg/console/operator/sync_v400.go index 94bdd72d6e..84766e2a6c 100644 --- a/pkg/console/operator/sync_v400.go +++ b/pkg/console/operator/sync_v400.go @@ -5,8 +5,6 @@ import ( "fmt" "os" - "github.com/prometheus/client_golang/prometheus" - // kube oauthv1 "github.com/openshift/api/oauth/v1" routev1 "github.com/openshift/api/route/v1" @@ -37,19 +35,6 @@ import ( secretsub "github.com/openshift/console-operator/pkg/console/subresource/secret" ) -var ( - // metric: console_url{url="https://"} 1 - consoleURLMetric = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Name: "console_url", - Help: "URL of the console exposed on the cluster", - // one label - }, []string{"url"}) -) - -func init() { - prometheus.MustRegister(consoleURLMetric) -} - // The sync loop starts from zero and works its way through the requirements for a running console. // If at any point something is missing, it creates/updates that piece and immediately dies. // The next loop will pick up where they previous left off and move the process forward one step. @@ -207,15 +192,14 @@ func (co *consoleOperator) sync_v400(updatedOperatorConfig *operatorv1.Console, func (co *consoleOperator) SyncConsoleConfig(consoleConfig *configv1.Console, consoleURL string) (*configv1.Console, error) { updated := consoleConfig.DeepCopy() - // track the URL state in prometheus before we update it if consoleConfig.Status.ConsoleURL != consoleURL { // not using this URL anymore - consoleURLMetric.WithLabelValues(consoleConfig.Status.ConsoleURL).Set(0) + co.consoleMetrics.ConsoleURL.WithLabelValues(consoleConfig.Status.ConsoleURL).Set(0) } if len(consoleURL) != 0 { // only update to new if we have a url - consoleURLMetric.WithLabelValues(consoleURL).Set(1) + co.consoleMetrics.ConsoleURL.WithLabelValues(consoleURL).Set(1) } if updated.Status.ConsoleURL != consoleURL { diff --git a/pkg/console/starter/starter.go b/pkg/console/starter/starter.go index 43ca60fb68..f882a5ad2a 100644 --- a/pkg/console/starter/starter.go +++ b/pkg/console/starter/starter.go @@ -5,6 +5,8 @@ import ( "os" "time" + "github.com/openshift/console-operator/pkg/console/metrics" + // kube corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -150,6 +152,7 @@ func RunOperator(ctx *controllercmd.ControllerContext) error { if err != nil { return err } + consoleMetrics := metrics.Register() // TODO: rearrange these into informer,client pairs, NOT separated. consoleOperator := operator.NewConsoleOperator( @@ -174,6 +177,9 @@ func RunOperator(ctx *controllercmd.ControllerContext) error { oauthInformers.Oauth().V1().OAuthClients(), // OAuth clients // openshift managed kubeInformersManagedNamespaced.Core().V1(), // Managed ConfigMaps + // metrics + // TODO: when we get to testing, we may want an interface for this + consoleMetrics, // event handling versionGetter, recorder, From 0f81a1220ea959cf7dad176d0fab447014e5edfb Mon Sep 17 00:00:00 2001 From: "Benjamin A. Petersen" Date: Wed, 6 Nov 2019 21:36:44 -0500 Subject: [PATCH 2/3] Use k8s.io legacyregistry for metrics --- pkg/console/metrics/metrics.go | 19 +++++++++---------- pkg/console/operator/operator.go | 7 +++---- pkg/console/starter/starter.go | 4 +--- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/pkg/console/metrics/metrics.go b/pkg/console/metrics/metrics.go index 8100678b34..eeab9c6d47 100644 --- a/pkg/console/metrics/metrics.go +++ b/pkg/console/metrics/metrics.go @@ -3,31 +3,30 @@ package metrics import ( "sync" - "github.com/prometheus/client_golang/prometheus" + k8smetrics "k8s.io/component-base/metrics" + "k8s.io/component-base/metrics/legacyregistry" ) -type ConsolePrometheusMetrics struct { - ConsoleURL *prometheus.GaugeVec +type ConsoleMetrics struct { + ConsoleURL *k8smetrics.GaugeVec } -var singleton *ConsolePrometheusMetrics +var singleton *ConsoleMetrics var once sync.Once -// using an init() func for registering metrics is error -// prone because the prometheus client registry may not be ready. -func Register() *ConsolePrometheusMetrics { +func Register() *ConsoleMetrics { // thread safe once.Do(func() { - singleton = &ConsolePrometheusMetrics{} + singleton = &ConsoleMetrics{} // metric: console_url{url="https://"} 1 - singleton.ConsoleURL = prometheus.NewGaugeVec(prometheus.GaugeOpts{ + singleton.ConsoleURL = k8smetrics.NewGaugeVec(&k8smetrics.GaugeOpts{ Name: "console_url", Help: "URL of the console exposed on the cluster", // one label }, []string{"url"}) - prometheus.MustRegister(singleton.ConsoleURL) + legacyregistry.MustRegister(singleton.ConsoleURL) }) return singleton } diff --git a/pkg/console/operator/operator.go b/pkg/console/operator/operator.go index 9108bcd271..d95e957e3d 100644 --- a/pkg/console/operator/operator.go +++ b/pkg/console/operator/operator.go @@ -6,8 +6,6 @@ import ( "reflect" "time" - "github.com/openshift/console-operator/pkg/console/metrics" - // kube "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -44,6 +42,7 @@ import ( operatorclientv1 "github.com/openshift/client-go/operator/clientset/versioned/typed/operator/v1" // operator + "github.com/openshift/console-operator/pkg/console/metrics" statushelpers "github.com/openshift/console-operator/pkg/console/status" "github.com/openshift/console-operator/pkg/console/subresource/configmap" "github.com/openshift/console-operator/pkg/console/subresource/deployment" @@ -73,7 +72,7 @@ type consoleOperator struct { oauthClient oauthclientv1.OAuthClientsGetter versionGetter status.VersionGetter // metrics - consoleMetrics *metrics.ConsolePrometheusMetrics + consoleMetrics *metrics.ConsoleMetrics // recorder recorder events.Recorder resourceSyncer resourcesynccontroller.ResourceSyncer @@ -101,7 +100,7 @@ func NewConsoleOperator( // openshift managed managedCoreV1 corev1.Interface, // metrics - consoleMetrics *metrics.ConsolePrometheusMetrics, + consoleMetrics *metrics.ConsoleMetrics, // event handling versionGetter status.VersionGetter, recorder events.Recorder, diff --git a/pkg/console/starter/starter.go b/pkg/console/starter/starter.go index f882a5ad2a..8ad5ca6f91 100644 --- a/pkg/console/starter/starter.go +++ b/pkg/console/starter/starter.go @@ -5,8 +5,6 @@ import ( "os" "time" - "github.com/openshift/console-operator/pkg/console/metrics" - // kube corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -49,6 +47,7 @@ import ( "github.com/openshift/console-operator/pkg/console/clientwrapper" "github.com/openshift/console-operator/pkg/console/controllers/service" + "github.com/openshift/console-operator/pkg/console/metrics" "github.com/openshift/console-operator/pkg/console/operator" "github.com/openshift/library-go/pkg/operator/loglevel" ) @@ -154,7 +153,6 @@ func RunOperator(ctx *controllercmd.ControllerContext) error { } consoleMetrics := metrics.Register() - // TODO: rearrange these into informer,client pairs, NOT separated. consoleOperator := operator.NewConsoleOperator( // top level config configClient.ConfigV1(), From 656a8b0913b09c490b8428a120f8f4e4be644d4d Mon Sep 17 00:00:00 2001 From: "Benjamin A. Petersen" Date: Sat, 21 Sep 2019 15:29:27 -0400 Subject: [PATCH 3/3] Add e2e test for /metrics endpoint - Use cert based auth for client in metrics test --- test/e2e/framework/config.go | 23 ++++ test/e2e/framework/framework.go | 2 + test/e2e/metrics_test.go | 219 ++++++++++++++++++++++++++++++++ 3 files changed, 244 insertions(+) create mode 100644 test/e2e/metrics_test.go diff --git a/test/e2e/framework/config.go b/test/e2e/framework/config.go index 6cd81fbf48..4c5546dab6 100644 --- a/test/e2e/framework/config.go +++ b/test/e2e/framework/config.go @@ -8,6 +8,7 @@ import ( "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" + "k8s.io/client-go/tools/clientcmd/api" ) // GetConfig creates a *rest.Config for talking to a Kubernetes apiserver. @@ -20,6 +21,14 @@ import ( // * In-cluster config if running in cluster // // * $HOME/.kube/config if exists +// +// Note that if you have done `oc login...` the `current-context` will be used. +// this means that the test will run as whatever user is currently logged in. +// kube:admin has a token, but no certs. +// - kube:admin is an oauth user, this is why it has tokens. +// system:admin has certs, but no token. +// - system:admin is a cert based user only. +// other users, your mileage may vary. func GetConfig() (*rest.Config, error) { // If an env variable is specified with the config location, use that if len(os.Getenv("KUBECONFIG")) > 0 { @@ -39,3 +48,17 @@ func GetConfig() (*rest.Config, error) { return nil, fmt.Errorf("could not locate a kubeconfig") } + +// authn operator approach to getting config. +// choose func as needed. +// https://github.com/openshift/cluster-authentication-operator/blob/master/test/library/client.go +// NewClientConfigForTest returns a config configured to connect to the api server +func NewClientConfigForTest() (*rest.Config, error) { + loader := clientcmd.NewDefaultClientConfigLoadingRules() + clientConfig := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loader, &clientcmd.ConfigOverrides{ClusterInfo: api.Cluster{InsecureSkipTLSVerify: true}}) + config, err := clientConfig.ClientConfig() + if err == nil { + fmt.Printf("Found configuration for host %v.\n", config.Host) + } + return config, err +} diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index 89aae332d5..e94be83bf8 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -77,6 +77,7 @@ func DeleteAll(t *testing.T, client *ClientSet) { func GetResource(client *ClientSet, resource TestingResource) (runtime.Object, error) { var res runtime.Object var err error + switch resource.kind { case "ConfigMap": res, err = client.Core.ConfigMaps(resource.namespace).Get(resource.name, metav1.GetOptions{}) @@ -234,6 +235,7 @@ func IsResourceUnavailable(errChan chan error, client *ClientSet, resource Testi counter := 0 maxCount := 15 err := wait.Poll(1*time.Second, AsyncOperationTimeout, func() (stop bool, err error) { + obtainedResource, err := GetResource(client, resource) if err == nil { return true, fmt.Errorf("deleted console %s %s was recreated: %#v", resource.kind, resource.name, obtainedResource) diff --git a/test/e2e/metrics_test.go b/test/e2e/metrics_test.go new file mode 100644 index 0000000000..61721f9084 --- /dev/null +++ b/test/e2e/metrics_test.go @@ -0,0 +1,219 @@ +package e2e + +import ( + "bufio" + "crypto/tls" + "crypto/x509" + "fmt" + "io/ioutil" + "net/http" + "strings" + "testing" + "time" + + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/wait" + + operatorsv1 "github.com/openshift/api/operator/v1" + routev1 "github.com/openshift/api/route/v1" + "github.com/openshift/console-operator/pkg/api" + "github.com/openshift/console-operator/test/e2e/framework" +) + +const ( + consoleURLMetric = "console_url" +) + +func setupMetricsEndpointTestCase(t *testing.T) (*framework.ClientSet, *operatorsv1.Console) { + client, _ := framework.StandardSetup(t) + routeForTest := tempRouteForTesting() + _, err := client.Routes.Routes(api.OpenShiftConsoleOperatorNamespace).Create(routeForTest) + if err != nil && !errors.IsAlreadyExists(err) { + t.Fatalf("error: %s", err) + } + return client, nil +} + +func cleanUpMetricsEndpointTestCase(t *testing.T, client *framework.ClientSet) { + routeForTest := tempRouteForTesting() + err := client.Routes.Routes(api.OpenShiftConsoleOperatorNamespace).Delete(routeForTest.Name, &metav1.DeleteOptions{}) + if err != nil { + t.Fatalf("error: %s", err) + } + framework.StandardCleanup(t, client) +} + +// This test essentially does the following to verify the `console_url` is in the metrics endpoint: +// curl --insecure -H "Authorization: Bearer $(oc whoami --show-token)" $(oc get route metrics --template 'https://{{.spec.host}}/metrics') | grep console_url +// alternatively: +// oc exec -it console-operator- /bin/bash +// curl --insecure -H "Authorization: Bearer https://localhost:8443/metrics | grep console_url +func TestMetricsEndpoint(t *testing.T) { + client, _ := setupMetricsEndpointTestCase(t) + defer cleanUpMetricsEndpointTestCase(t, client) + + metricsURL := getMetricsURL(t, client) + t.Logf("fetching metrics url.... %s\n", metricsURL) + + t.Logf("fetching /metrics data...\n") + respString := metricsRequest(t, metricsURL) + + t.Logf("searching for %s in metrics data...\n", consoleURLMetric) + found := findLineInResponse(t, respString, consoleURLMetric) + if !found { + t.Fatalf("did not find %s", consoleURLMetric) + } +} + +func findLineInResponse(t *testing.T, haystack, needle string) (found bool) { + scanner := bufio.NewScanner(strings.NewReader(haystack)) + for scanner.Scan() { + text := scanner.Text() + // skip comments + if strings.HasPrefix(text, "#") { + continue + } + found := strings.Contains(text, needle) + if found { + t.Logf("found %s\n", scanner.Text()) + return true + } + } + return false +} + +func metricsRequest(t *testing.T, routeForMetrics string) string { + req := getRequest(t, routeForMetrics) + httpClient := getClientWithCertAuth(t) + + resp, err := httpClient.Do(req) + if err != nil { + t.Fatalf("http error: %s", err) + } + + if !httpOK(resp) { + t.Fatalf("http error: %d %s", resp.StatusCode, http.StatusText(resp.StatusCode)) + } + + defer resp.Body.Close() + + bytes, err := ioutil.ReadAll(resp.Body) + if err != nil { + t.Fatalf("read error: %s", err) + } + + return string(bytes) +} + +func httpOK(resp *http.Response) bool { + if resp.StatusCode >= 200 && resp.StatusCode <= 299 { + return true + } + return false +} + +func getRequest(t *testing.T, metricsURL string) *http.Request { + req, err := http.NewRequest("GET", metricsURL, nil) + if err != nil { + fmt.Printf("%s\n", err) + } + return req +} + +// kubeadmin is an oauth account. it will not work. to run tests instead do: +// oc login -u system:admin +// so that the config read gives correct data back. +// this only works if test are run as a cert based user (example: system:admin) +func getClientWithCertAuth(t *testing.T) *http.Client { + config, err := framework.GetConfig() + if err != nil { + t.Fatalf("error, can't get kube config: %s", err) + } + + // load from memory, not file + tlsCert, err := tls.X509KeyPair(config.CertData, config.KeyData) + // load from file, not memory + // tlsCert, err := tls.LoadX509KeyPair(config.CertFile, config.KeyFile) + + if err != nil { + t.Fatalf("error, cannot get key pair, are you logged in as system:admin (kube:admin will not work)? %s", err) + } + + rootCAs, err := x509.SystemCertPool() + if rootCAs == nil { + rootCAs = x509.NewCertPool() + } + + if err != nil { + // not sure if we should panic/die here + fmt.Printf("x509 certs err: %v", err) + } + + // need to add the CAData, the kubeconfig has router certs + // that are missing from the system trust roots + rootCAs.AppendCertsFromPEM(config.CAData) + + return &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + Certificates: []tls.Certificate{tlsCert}, + RootCAs: rootCAs, + // x509 error, certificate is valid for service, not route, so must + // use an insecure client. This is not really ideal after going through + // the trouble of wiring up the certs :/ + InsecureSkipVerify: true, + }, + }, + } +} + +// polls for the metrics route host, and returns https:///metrics +func getMetricsURL(t *testing.T, client *framework.ClientSet) string { + tempRoute := tempRouteForTesting() + routeForMetrics := "" + err := wait.Poll(1*time.Second, 30*time.Second, func() (stop bool, err error) { + tempRoute, err := client.Routes.Routes(api.OpenShiftConsoleOperatorNamespace).Get(tempRoute.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("error: %s", err) + } + if len(tempRoute.Spec.Host) == 0 { + return false, err + } + routeForMetrics = "https://" + tempRoute.Spec.Host + "/metrics" + t.Logf("route for metrics: %s", routeForMetrics) + return true, nil + }) + if err != nil { + t.Fatalf("error: %s", err) + } + t.Logf("route to /metrics: (%s)", routeForMetrics) + return routeForMetrics +} + +// in production the operator does not have a route. +func tempRouteForTesting() *routev1.Route { + return &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "metrics", + Namespace: api.OpenShiftConsoleOperatorNamespace, + }, + Spec: routev1.RouteSpec{ + To: routev1.RouteTargetReference{ + Kind: "Service", + Name: "metrics", + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.FromString("https"), + }, + TLS: &routev1.TLSConfig{ + // Termination: routev1.TLSTerminationReencrypt, + // NOTE: has to be passthrough for cert auth? + Termination: routev1.TLSTerminationPassthrough, + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect, + }, + WildcardPolicy: routev1.WildcardPolicyNone, + }, + } +}