From 031f4d666fd97ec22813e0a5c3db07ffd1733f30 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Thu, 22 Feb 2024 19:10:46 +0530 Subject: [PATCH 1/8] fixes #2207 by adding server-url to JaegerMetricsStorageSpec as ServerUrl for providing external prometheus service url. Added test for verifying env variable PROMETHEUS_SERVER_URL is not empty when specifying metric storage type as prometheus for both AllInOne and Query. Signed-off-by: Gaurav Singh --- apis/v1/jaeger_types.go | 3 +++ config/crd/bases/jaegertracing.io_jaegers.yaml | 4 ++++ pkg/deployment/all_in_one.go | 8 ++++++++ pkg/deployment/all_in_one_test.go | 12 ++++++++++++ pkg/deployment/query.go | 8 ++++++++ pkg/deployment/query_test.go | 13 +++++++++++++ 6 files changed, 48 insertions(+) diff --git a/apis/v1/jaeger_types.go b/apis/v1/jaeger_types.go index 870fd0ee0..b8091619a 100644 --- a/apis/v1/jaeger_types.go +++ b/apis/v1/jaeger_types.go @@ -573,6 +573,9 @@ type JaegerStorageSpec struct { type JaegerMetricsStorageSpec struct { // +optional Type JaegerStorageType `json:"type,omitempty"` + + // +optional + ServerUrl string `json:"server-url,omitempty"` } // ElasticsearchSpec represents the ES configuration options that we pass down to the OpenShift Elasticsearch operator. diff --git a/config/crd/bases/jaegertracing.io_jaegers.yaml b/config/crd/bases/jaegertracing.io_jaegers.yaml index e1a5a4510..490372241 100644 --- a/config/crd/bases/jaegertracing.io_jaegers.yaml +++ b/config/crd/bases/jaegertracing.io_jaegers.yaml @@ -2561,6 +2561,8 @@ spec: type: object metricsStorage: properties: + server-url: + type: string type: type: string type: object @@ -8847,6 +8849,8 @@ spec: type: object metricsStorage: properties: + server-url: + type: string type: type: string type: object diff --git a/pkg/deployment/all_in_one.go b/pkg/deployment/all_in_one.go index a74c473ad..fabb173a2 100644 --- a/pkg/deployment/all_in_one.go +++ b/pkg/deployment/all_in_one.go @@ -152,6 +152,14 @@ func (a *AllInOne) Get() *appsv1.Deployment { Value: strconv.FormatBool(jaegerDisabled), }, } + + if a.jaeger.Spec.AllInOne.MetricsStorage.Type == "prometheus" { + envVars = append(envVars, corev1.EnvVar{ + Name: "PROMETHEUS_SERVER_URL", + Value: a.jaeger.Spec.AllInOne.MetricsStorage.ServerUrl, + }) + } + envVars = append(envVars, proxy.ReadProxyVarsFromEnv()...) ports := []corev1.ContainerPort{ diff --git a/pkg/deployment/all_in_one_test.go b/pkg/deployment/all_in_one_test.go index c251edb7f..397e9f7b6 100644 --- a/pkg/deployment/all_in_one_test.go +++ b/pkg/deployment/all_in_one_test.go @@ -608,6 +608,18 @@ func TestAllInOnePriorityClassName(t *testing.T) { assert.Equal(t, priorityClassName, dep.Spec.Template.Spec.PriorityClassName) } +func TestAllInOnePrometheusMetricStorage(t *testing.T) { + jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestAllInOnePrometheusMetricStorage"}) + + jaeger.Spec.AllInOne.MetricsStorage.Type = "prometheus" + jaeger.Spec.AllInOne.MetricsStorage.ServerUrl = "http://prometheus:9090" + + d := NewAllInOne(jaeger).Get() + + assert.Equal(t, "prometheus", getEnvVarByName(d.Spec.Template.Spec.Containers[0].Env, "METRICS_STORAGE_TYPE").Value) + assert.NotEmpty(t, getEnvVarByName(d.Spec.Template.Spec.Containers[0].Env, "PROMETHEUS_SERVER_URL").Value) +} + func getEnvVarByName(vars []corev1.EnvVar, name string) corev1.EnvVar { envVar := corev1.EnvVar{} for _, v := range vars { diff --git a/pkg/deployment/query.go b/pkg/deployment/query.go index 17765536b..7103625fa 100644 --- a/pkg/deployment/query.go +++ b/pkg/deployment/query.go @@ -135,6 +135,14 @@ func (q *Query) Get() *appsv1.Deployment { Value: strconv.FormatBool(jaegerDisabled), }, } + + if q.jaeger.Spec.Query.MetricsStorage.Type == "prometheus" { + envVars = append(envVars, corev1.EnvVar{ + Name: "PROMETHEUS_SERVER_URL", + Value: q.jaeger.Spec.Query.MetricsStorage.ServerUrl, + }) + } + envVars = append(envVars, proxy.ReadProxyVarsFromEnv()...) deployment := &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ diff --git a/pkg/deployment/query_test.go b/pkg/deployment/query_test.go index dcd632eef..d5cf7ee20 100644 --- a/pkg/deployment/query_test.go +++ b/pkg/deployment/query_test.go @@ -510,3 +510,16 @@ func TestQueryNodeSelector(t *testing.T) { assert.Equal(t, nodeSelector, dep.Spec.Template.Spec.NodeSelector) } + +func TestQueryPrometheusMetricStorage(t *testing.T) { + + jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestQueryPrometheusMetricStorage"}) + + jaeger.Spec.Query.MetricsStorage.Type = "prometheus" + jaeger.Spec.Query.MetricsStorage.ServerUrl = "http://prometheus:9090" + + d := NewQuery(jaeger).Get() + + assert.Equal(t, "prometheus", getEnvVarByName(d.Spec.Template.Spec.Containers[0].Env, "METRICS_STORAGE_TYPE").Value) + assert.NotEmpty(t, getEnvVarByName(d.Spec.Template.Spec.Containers[0].Env, "PROMETHEUS_SERVER_URL").Value) +} From 47e38202e7acf199a51ac553bbccf42009e01763 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Tue, 5 Mar 2024 18:49:28 +0530 Subject: [PATCH 2/8] added empty check for ServerUrl while setting env for external prometheus instance Signed-off-by: Gaurav Singh --- pkg/deployment/all_in_one.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/deployment/all_in_one.go b/pkg/deployment/all_in_one.go index fabb173a2..061f1a3c7 100644 --- a/pkg/deployment/all_in_one.go +++ b/pkg/deployment/all_in_one.go @@ -153,7 +153,7 @@ func (a *AllInOne) Get() *appsv1.Deployment { }, } - if a.jaeger.Spec.AllInOne.MetricsStorage.Type == "prometheus" { + if a.jaeger.Spec.AllInOne.MetricsStorage.Type == "prometheus" && a.jaeger.Spec.AllInOne.MetricsStorage.ServerUrl != "" { envVars = append(envVars, corev1.EnvVar{ Name: "PROMETHEUS_SERVER_URL", Value: a.jaeger.Spec.AllInOne.MetricsStorage.ServerUrl, From b26abfe35b21b66bb604f009b4071dd1f82a3be4 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Tue, 5 Mar 2024 18:54:26 +0530 Subject: [PATCH 3/8] updated manifests by running make bundle Signed-off-by: Gaurav Singh --- bundle/manifests/jaegertracing.io_jaegers.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bundle/manifests/jaegertracing.io_jaegers.yaml b/bundle/manifests/jaegertracing.io_jaegers.yaml index f095a709d..e11919bdc 100644 --- a/bundle/manifests/jaegertracing.io_jaegers.yaml +++ b/bundle/manifests/jaegertracing.io_jaegers.yaml @@ -2564,6 +2564,8 @@ spec: type: object metricsStorage: properties: + server-url: + type: string type: type: string type: object @@ -8850,6 +8852,8 @@ spec: type: object metricsStorage: properties: + server-url: + type: string type: type: string type: object From 1fca5a59e8611f3abe170042d931cd31b6a932e1 Mon Sep 17 00:00:00 2001 From: amerlin Date: Thu, 14 Mar 2024 10:28:54 +0530 Subject: [PATCH 4/8] updated api.md by running make api-docs Signed-off-by: amerlin --- docs/api.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/api.md b/docs/api.md index ba9781d93..349e734d7 100644 --- a/docs/api.md +++ b/docs/api.md @@ -9556,6 +9556,13 @@ Resource Types: + server-url + string + +
+ + false + type string @@ -33321,6 +33328,13 @@ Resource Types: + server-url + string + +
+ + false + type string From 984ae6327371ac481ac2649df24358d588d148be Mon Sep 17 00:00:00 2001 From: amerlin Date: Thu, 14 Mar 2024 10:40:33 +0530 Subject: [PATCH 5/8] fix format of pkg/ingress/query_test.go Signed-off-by: amerlin --- pkg/ingress/query_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/ingress/query_test.go b/pkg/ingress/query_test.go index 4449fb079..976850ee4 100644 --- a/pkg/ingress/query_test.go +++ b/pkg/ingress/query_test.go @@ -293,7 +293,6 @@ func TestQueryIngressClass(t *testing.T) { assert.NotNil(t, dep.Spec.IngressClassName) assert.Equal(t, "nginx", *dep.Spec.IngressClassName) assert.Nil(t, ingressNoClass.Get().Spec.IngressClassName) - } func TestForDefaultIngressClass(t *testing.T) { From 4f0c3fef64f3de3dec1a1242ff564ec66ffca68c Mon Sep 17 00:00:00 2001 From: amerlin Date: Thu, 14 Mar 2024 10:45:40 +0530 Subject: [PATCH 6/8] fix format of pkg/deployment/query_test.go Signed-off-by: amerlin --- pkg/deployment/query_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/deployment/query_test.go b/pkg/deployment/query_test.go index d5cf7ee20..087a51ff0 100644 --- a/pkg/deployment/query_test.go +++ b/pkg/deployment/query_test.go @@ -512,7 +512,6 @@ func TestQueryNodeSelector(t *testing.T) { } func TestQueryPrometheusMetricStorage(t *testing.T) { - jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestQueryPrometheusMetricStorage"}) jaeger.Spec.Query.MetricsStorage.Type = "prometheus" From 786b4deac72011c2a03af003f4788403de03be46 Mon Sep 17 00:00:00 2001 From: amerlin Date: Thu, 14 Mar 2024 15:00:12 +0530 Subject: [PATCH 7/8] updated test TestQueryIngressClass from pkg/ingress/query_test.go by remove empty check for ingress as it will be taken care by default ingressclass func Signed-off-by: amerlin --- config/rbac/secret.yaml | 10 ++++++++++ pkg/ingress/query_test.go | 7 ++----- 2 files changed, 12 insertions(+), 5 deletions(-) create mode 100644 config/rbac/secret.yaml diff --git a/config/rbac/secret.yaml b/config/rbac/secret.yaml new file mode 100644 index 000000000..b5555ffe2 --- /dev/null +++ b/config/rbac/secret.yaml @@ -0,0 +1,10 @@ +apiVersion: v1 +kind: Secret +metadata: + name: registry.devhawkstudio.duckdns.org +data: + .dockercfg: >- + eyJodHRwczovL3JlZ2lzdHJ5LmRldmhhd2tzdHVkaW8uZHVja2Rucy5vcmc6NTAwMC92MS8iOiB7ImF1dGgiOiAiWVdSdGFXNDZZbUZrYldsdSJ9fQ== +type: kubernetes.io/dockercfg + + \ No newline at end of file diff --git a/pkg/ingress/query_test.go b/pkg/ingress/query_test.go index 976850ee4..07daa2874 100644 --- a/pkg/ingress/query_test.go +++ b/pkg/ingress/query_test.go @@ -4,12 +4,12 @@ import ( "context" "testing" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes" networkingv1 "k8s.io/api/networking/v1" @@ -280,19 +280,16 @@ func TestQueryIngressTLSSecret(t *testing.T) { func TestQueryIngressClass(t *testing.T) { jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestQueryIngressClass"}) - jaegerNoIngressNoClass := v1.NewJaeger(types.NamespacedName{Name: "TestQueryIngressNoClass"}) inressClassName := "nginx" jaeger.Spec.Ingress.IngressClassName = &inressClassName ingress := NewQueryIngress(jaeger) - ingressNoClass := NewQueryIngress(jaegerNoIngressNoClass) dep := ingress.Get() assert.NotNil(t, dep.Spec.IngressClassName) assert.Equal(t, "nginx", *dep.Spec.IngressClassName) - assert.Nil(t, ingressNoClass.Get().Spec.IngressClassName) } func TestForDefaultIngressClass(t *testing.T) { From dc70a28a5b81bd60e25532bc17313e56c40eb3aa Mon Sep 17 00:00:00 2001 From: amerlin Date: Thu, 14 Mar 2024 15:12:45 +0530 Subject: [PATCH 8/8] fixed format Signed-off-by: amerlin --- config/rbac/secret.yaml | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 config/rbac/secret.yaml diff --git a/config/rbac/secret.yaml b/config/rbac/secret.yaml deleted file mode 100644 index b5555ffe2..000000000 --- a/config/rbac/secret.yaml +++ /dev/null @@ -1,10 +0,0 @@ -apiVersion: v1 -kind: Secret -metadata: - name: registry.devhawkstudio.duckdns.org -data: - .dockercfg: >- - eyJodHRwczovL3JlZ2lzdHJ5LmRldmhhd2tzdHVkaW8uZHVja2Rucy5vcmc6NTAwMC92MS8iOiB7ImF1dGgiOiAiWVdSdGFXNDZZbUZrYldsdSJ9fQ== -type: kubernetes.io/dockercfg - - \ No newline at end of file