From e8c0f389e65c74f55e482ccb96127e31d475931d Mon Sep 17 00:00:00 2001 From: RealAnna <89971034+RealAnna@users.noreply.github.com> Date: Thu, 9 Mar 2023 10:43:03 +0100 Subject: [PATCH] fix: remove missing 404 bug (#1006) Signed-off-by: realanna --- metrics-operator/pkg/metrics/server.go | 30 +++-- metrics-operator/pkg/metrics/server_test.go | 118 ++++++++++++++------ 2 files changed, 104 insertions(+), 44 deletions(-) diff --git a/metrics-operator/pkg/metrics/server.go b/metrics-operator/pkg/metrics/server.go index 4ebb7af6c7..b91df11d86 100644 --- a/metrics-operator/pkg/metrics/server.go +++ b/metrics-operator/pkg/metrics/server.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "net/http" - "os" "strconv" "strings" "sync" @@ -16,8 +15,10 @@ import ( "github.com/gorilla/mux" metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1alpha2" "github.com/open-feature/go-sdk/pkg/openfeature" + "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" @@ -27,8 +28,6 @@ type Metrics struct { gauges map[string]prometheus.Gauge } -var metrics Metrics - var instance *serverManager var smOnce sync.Once @@ -38,13 +37,13 @@ type serverManager struct { ofClient *openfeature.Client exposeMetrics bool k8sClient client.Client + metrics Metrics } // StartServerManager starts a server manager to expose metrics and runs until // the context is cancelled (i.e. an env variable gets changes and pod is restarted) func StartServerManager(ctx context.Context, client client.Client, ofClient *openfeature.Client, exposeMetrics bool, interval time.Duration) { smOnce.Do(func() { - metrics.gauges = make(map[string]prometheus.Gauge) instance = &serverManager{ ticker: clock.New().Ticker(interval), ofClient: ofClient, @@ -108,6 +107,9 @@ func (m *serverManager) setup() error { klog.Infof("Keptn Metrics server enabled: %v", serverEnabled) if serverEnabled && m.server == nil { + + m.metrics.gauges = make(map[string]prometheus.Gauge) + klog.Infof("serving Prometheus metrics at localhost:9999/metrics") klog.Infof("serving KeptnMetrics at localhost:9999/api/v1/metrics/{namespace}/{metric}") @@ -142,10 +144,15 @@ func (m *serverManager) returnMetric(w http.ResponseWriter, r *http.Request) { namespace := vars["namespace"] metric := vars["metric"] - metricObj := metricsapi.KeptnMetric{} - err := m.k8sClient.Get(context.Background(), types.NamespacedName{Name: metric, Namespace: namespace}, &metricObj) + metricObj := &metricsapi.KeptnMetric{} + err := m.k8sClient.Get(context.Background(), types.NamespacedName{Name: metric, Namespace: namespace}, metricObj) if err != nil { fmt.Println("failed to list keptn-metrics: " + err.Error()) + //nolint:errorlint + if status, ok := err.(k8serrors.APIStatus); ok || errors.As(err, &status) { + w.WriteHeader(int(status.Status().Code)) + } + return } data := map[string]string{ @@ -157,8 +164,9 @@ func (m *serverManager) returnMetric(w http.ResponseWriter, r *http.Request) { err = json.NewEncoder(w).Encode(data) if err != nil { fmt.Println("failed to encode data") - os.Exit(1) + w.WriteHeader(http.StatusUnprocessableEntity) } + } func (m *serverManager) recordMetrics() { @@ -174,15 +182,15 @@ func (m *serverManager) recordMetrics() { } for _, metric := range list.Items { normName := normalizeMetricName(metric.Name) - if _, ok := metrics.gauges[normName]; !ok { - metrics.gauges[normName] = prometheus.NewGauge(prometheus.GaugeOpts{ + if _, ok := m.metrics.gauges[normName]; !ok { + m.metrics.gauges[normName] = prometheus.NewGauge(prometheus.GaugeOpts{ Name: normName, Help: metric.Name, }) - prometheus.MustRegister(metrics.gauges[normName]) + prometheus.MustRegister(m.metrics.gauges[normName]) } val, _ := strconv.ParseFloat(metric.Status.Value, 64) - metrics.gauges[normName].Set(val) + m.metrics.gauges[normName].Set(val) } <-time.After(10 * time.Second) } diff --git a/metrics-operator/pkg/metrics/server_test.go b/metrics-operator/pkg/metrics/server_test.go index b4877fdcdf..7764ca72bb 100644 --- a/metrics-operator/pkg/metrics/server_test.go +++ b/metrics-operator/pkg/metrics/server_test.go @@ -4,9 +4,11 @@ import ( "bytes" "context" "net/http" + "os" "testing" "time" + "github.com/benbjohnson/clock" metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1alpha2" "github.com/open-feature/go-sdk/pkg/openfeature" "github.com/stretchr/testify/require" @@ -15,8 +17,40 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) +func TestMain(m *testing.M) { + err := metricsapi.AddToScheme(scheme.Scheme) + if err != nil { + panic("BAD SCHEME!") + } + code := m.Run() + os.Exit(code) +} + +func TestMetricServer_disabledServer(t *testing.T) { + + tInstance := &serverManager{ + ticker: clock.New().Ticker(3 * time.Second), + ofClient: openfeature.NewClient("klt-test"), + exposeMetrics: false, + k8sClient: fake.NewClientBuilder().WithScheme(scheme.Scheme).Build(), + } + tInstance.start(context.Background()) + + var err error + require.Eventually(t, func() bool { + cli := &http.Client{} + req, _ := http.NewRequestWithContext(context.TODO(), http.MethodGet, "http://localhost:9999/metrics", nil) + _, err = cli.Do(req) + return err != nil + }, 30*time.Second, 3*time.Second) + + require.Contains(t, err.Error(), "connection refused") + +} + func TestMetricServer_happyPath(t *testing.T) { - metric := metricsapi.KeptnMetric{ + + var metric = metricsapi.KeptnMetric{ ObjectMeta: v1.ObjectMeta{ Name: "sample-metric", Namespace: "keptn-lifecycle-toolkit-system", @@ -28,25 +62,34 @@ func TestMetricServer_happyPath(t *testing.T) { Query: "query", FetchIntervalSeconds: 5, }, + Status: metricsapi.KeptnMetricStatus{ + Value: "12", + RawValue: nil, + LastUpdated: v1.Time{ + Time: time.Now(), + }, + }, } - - err := metricsapi.AddToScheme(scheme.Scheme) - require.Nil(t, err) k8sClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(&metric).Build() - ctx, cancel := context.WithCancel(context.Background()) - - StartServerManager(ctx, k8sClient, openfeature.NewClient("klt-test"), true, 3*time.Second) + tInstance := &serverManager{ + ticker: clock.New().Ticker(3 * time.Second), + ofClient: openfeature.NewClient("klt-test"), + exposeMetrics: true, + k8sClient: k8sClient, + } + tInstance.start(context.Background()) require.Eventually(t, func() bool { - return instance.server != nil - }, 10*time.Second, time.Second) + return tInstance.server != nil + }, 30*time.Second, time.Second) var resp *http.Response + var err error require.Eventually(t, func() bool { cli := &http.Client{} - req, err2 := http.NewRequestWithContext(ctx, http.MethodGet, "http://localhost:9999/metrics", nil) + req, err2 := http.NewRequestWithContext(context.TODO(), http.MethodGet, "http://localhost:9999/metrics", nil) require.Nil(t, err2) resp, err = cli.Do(req) return err == nil @@ -61,41 +104,50 @@ func TestMetricServer_happyPath(t *testing.T) { require.Contains(t, newStr, "# TYPE sample_metric gauge") - cancel() - require.Eventually(t, func() bool { - return instance.server == nil + cli := &http.Client{} + req, _ := http.NewRequestWithContext(context.TODO(), http.MethodGet, "http://localhost:9999/api/v1/metrics/keptn-lifecycle-toolkit-system/sample-metric", nil) + resp, err = cli.Do(req) + return err == nil }, 10*time.Second, time.Second) + + defer resp.Body.Close() + + buf = new(bytes.Buffer) + _, err = buf.ReadFrom(resp.Body) + require.Nil(t, err) + newStr = buf.String() + + require.Contains(t, newStr, "\"metric\":\"sample-metric\",\"namespace\":\"keptn-lifecycle-toolkit-system\",\"value\":\"12\"") } -func TestMetricServer_disabledServer(t *testing.T) { - err2 := metricsapi.AddToScheme(scheme.Scheme) - require.Nil(t, err2) - k8sClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() +func TestMetricServer_noMetric(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) + k8sClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() - StartServerManager(ctx, k8sClient, openfeature.NewClient("klt-test"), false, 3*time.Second) + tInstance := &serverManager{ + ticker: clock.New().Ticker(3 * time.Second), + ofClient: openfeature.NewClient("klt-test"), + exposeMetrics: true, + k8sClient: k8sClient, + } + tInstance.start(context.Background()) require.Eventually(t, func() bool { - return instance.server == nil - }, 30*time.Second, 3*time.Second) + return tInstance.server != nil + }, 30*time.Second, time.Second) + var resp *http.Response var err error - require.Eventually(t, func() bool { cli := &http.Client{} - req, err2 := http.NewRequestWithContext(ctx, http.MethodGet, "http://localhost:9999/metrics", nil) - require.Nil(t, err2) - _, err = cli.Do(req) - return err != nil - }, 30*time.Second, 3*time.Second) - - require.Contains(t, err.Error(), "connection refused") + req, _ := http.NewRequestWithContext(context.TODO(), http.MethodGet, "http://localhost:9999/api/v1/metrics/default/sample", nil) + resp, err = cli.Do(req) + return err == nil + }, 10*time.Second, time.Second) - cancel() + defer resp.Body.Close() + stat := resp.StatusCode + require.Equal(t, 404, stat) - require.Eventually(t, func() bool { - return instance.server == nil - }, 30*time.Second, 3*time.Second) }