diff --git a/.chloggen/allow_gateway_without_jaeger_ui.yaml b/.chloggen/allow_gateway_without_jaeger_ui.yaml new file mode 100755 index 000000000..186670aec --- /dev/null +++ b/.chloggen/allow_gateway_without_jaeger_ui.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. operator, github action) +component: operator + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Make Tempo-Query forwarding on gateway optional + +# One or more tracking issues related to the change +issues: [628] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: diff --git a/apis/tempo/v1alpha1/tempostack_webhook.go b/apis/tempo/v1alpha1/tempostack_webhook.go index 6363d5793..1841654ef 100644 --- a/apis/tempo/v1alpha1/tempostack_webhook.go +++ b/apis/tempo/v1alpha1/tempostack_webhook.go @@ -258,13 +258,6 @@ func (v *validator) validateQueryFrontend(tempo TempoStack) field.ErrorList { func (v *validator) validateGateway(tempo TempoStack) field.ErrorList { path := field.NewPath("spec").Child("template").Child("gateway").Child("enabled") if tempo.Spec.Template.Gateway.Enabled { - if !tempo.Spec.Template.QueryFrontend.JaegerQuery.Enabled { - return field.ErrorList{ - field.Invalid(path, tempo.Spec.Template.Gateway.Enabled, - "to use the gateway, please enable jaegerQuery", - )} - } - if tempo.Spec.Template.QueryFrontend.JaegerQuery.Ingress.Type != IngressTypeNone { return field.ErrorList{ field.Invalid(path, tempo.Spec.Template.Gateway.Enabled, diff --git a/apis/tempo/v1alpha1/tempostack_webhook_test.go b/apis/tempo/v1alpha1/tempostack_webhook_test.go index 7a53432ff..7dbdb0676 100644 --- a/apis/tempo/v1alpha1/tempostack_webhook_test.go +++ b/apis/tempo/v1alpha1/tempostack_webhook_test.go @@ -780,32 +780,6 @@ func TestValidateGatewayAndJaegerQuery(t *testing.T) { }, expected: nil, }, - { - name: "invalid config disable jaegerQuery", - input: TempoStack{ - Spec: TempoStackSpec{ - ReplicationFactor: 3, - Template: TempoTemplateSpec{ - QueryFrontend: TempoQueryFrontendSpec{ - JaegerQuery: JaegerQuerySpec{ - Enabled: false, - Ingress: IngressSpec{ - Type: "ingress", - }, - }, - }, - Gateway: TempoGatewaySpec{ - Enabled: true, - }, - }, - }, - }, - expected: field.ErrorList{ - field.Invalid(path, true, - "to use the gateway, please enable jaegerQuery", - ), - }, - }, { name: "invalid configuration, ingress and gateway enabled", input: TempoStack{ diff --git a/internal/manifests/gateway/gateway.go b/internal/manifests/gateway/gateway.go index ea3868794..a2bd9c0d7 100644 --- a/internal/manifests/gateway/gateway.go +++ b/internal/manifests/gateway/gateway.go @@ -98,6 +98,11 @@ func BuildGateway(params manifestutils.Params) ([]client.Object, error) { objs = append(objs, routeObj) } + dep.Spec.Template, err = patchTraceReadEndpoint(params, dep.Spec.Template) + if err != nil { + return nil, err + } + dep.Spec.Template, err = patchTracing(params.Tempo, dep.Spec.Template) if err != nil { return nil, err @@ -114,6 +119,8 @@ func httpScheme(tls bool) string { return "http" } +const containerNameTempoGateway = "tempo-gateway" + func deployment(params manifestutils.Params, rbacCfgHash string, tenantsCfgHash string) *appsv1.Deployment { tempo := params.Tempo labels := manifestutils.ComponentLabels(manifestutils.GatewayComponentName, tempo.Name) @@ -164,7 +171,7 @@ func deployment(params manifestutils.Params, rbacCfgHash string, tenantsCfgHash Tolerations: cfg.Tolerations, Containers: []corev1.Container{ { - Name: "tempo-gateway", + Name: containerNameTempoGateway, Image: image, Env: proxy.ReadProxyVarsFromEnv(), Args: append([]string{ @@ -172,8 +179,6 @@ func deployment(params manifestutils.Params, rbacCfgHash string, tenantsCfgHash fmt.Sprintf("--web.listen=0.0.0.0:%d", portPublic), fmt.Sprintf("--web.internal.listen=0.0.0.0:%d", portInternal), fmt.Sprintf("--traces.write.endpoint=%s:%d", naming.ServiceFqdn(tempo.Namespace, tempo.Name, manifestutils.DistributorComponentName), manifestutils.PortOtlpGrpcServer), - fmt.Sprintf("--traces.read.endpoint=%s://%s:%d", httpScheme(params.CtrlConfig.Gates.HTTPEncryption), - naming.ServiceFqdn(tempo.Namespace, tempo.Name, manifestutils.QueryFrontendComponentName), manifestutils.PortJaegerQuery), fmt.Sprintf("--traces.tempo.endpoint=%s://%s:%d", httpScheme(params.CtrlConfig.Gates.HTTPEncryption), naming.ServiceFqdn(tempo.Namespace, tempo.Name, manifestutils.QueryFrontendComponentName), manifestutils.PortHTTPServer), fmt.Sprintf("--grpc.listen=0.0.0.0:%d", portGRPC), @@ -279,6 +284,30 @@ func deployment(params manifestutils.Params, rbacCfgHash string, tenantsCfgHash return dep } +func patchTraceReadEndpoint(params manifestutils.Params, pod corev1.PodTemplateSpec) (corev1.PodTemplateSpec, error) { + if !params.Tempo.Spec.Template.QueryFrontend.JaegerQuery.Enabled { + return pod, nil + } + + container := corev1.Container{ + Args: []string{ + fmt.Sprintf("--traces.read.endpoint=%s://%s:%d", httpScheme(params.CtrlConfig.Gates.HTTPEncryption), + naming.ServiceFqdn(params.Tempo.Namespace, params.Tempo.Name, manifestutils.QueryFrontendComponentName), manifestutils.PortJaegerQuery), + }, + } + + for i := range pod.Spec.Containers { + if pod.Spec.Containers[i].Name != containerNameTempoGateway { + continue + } + if err := mergo.Merge(&pod.Spec.Containers[i], container, mergo.WithAppendSlice); err != nil { + return corev1.PodTemplateSpec{}, err + } + } + + return pod, nil +} + func patchTracing(tempo v1alpha1.TempoStack, pod corev1.PodTemplateSpec) (corev1.PodTemplateSpec, error) { if tempo.Spec.Observability.Tracing.SamplingFraction == "" { return pod, nil diff --git a/internal/manifests/gateway/gateway_test.go b/internal/manifests/gateway/gateway_test.go index afbc53e12..d9146b6e0 100644 --- a/internal/manifests/gateway/gateway_test.go +++ b/internal/manifests/gateway/gateway_test.go @@ -374,6 +374,126 @@ func TestPatchTracing(t *testing.T) { } } +func TestPatchTraceReadEndpoint(t *testing.T) { + tt := []struct { + name string + inputParams manifestutils.Params + inputPod corev1.PodTemplateSpec + expectPod corev1.PodTemplateSpec + expectErr error + }{ + { + name: "with trace read endpoint", + inputParams: manifestutils.Params{ + Tempo: v1alpha1.TempoStack{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + Namespace: "default", + }, + Spec: v1alpha1.TempoStackSpec{ + Template: v1alpha1.TempoTemplateSpec{ + QueryFrontend: v1alpha1.TempoQueryFrontendSpec{ + JaegerQuery: v1alpha1.JaegerQuerySpec{ + Enabled: true, + }, + }, + }, + }, + }, + }, + inputPod: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: containerNameTempoGateway, + Args: []string{ + "--abc", + }, + }, + { + Name: "second", + Args: []string{ + "--xyz", + }, + }, + }, + }, + }, + expectPod: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: containerNameTempoGateway, + Args: []string{ + "--abc", + "--traces.read.endpoint=http://tempo-name-query-frontend.default.svc.cluster.local:16686", + }, + }, + { + Name: "second", + Args: []string{ + "--xyz", + }, + }, + }, + }, + }, + }, + { + name: "without trace read endpoint", + inputParams: manifestutils.Params{ + Tempo: v1alpha1.TempoStack{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + Namespace: "default", + }, + Spec: v1alpha1.TempoStackSpec{ + Template: v1alpha1.TempoTemplateSpec{ + QueryFrontend: v1alpha1.TempoQueryFrontendSpec{ + JaegerQuery: v1alpha1.JaegerQuerySpec{ + Enabled: false, + }, + }, + }, + }, + }, + }, + inputPod: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: containerNameTempoGateway, + Args: []string{ + "--abc", + }, + }, + }, + }, + }, + expectPod: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: containerNameTempoGateway, + Args: []string{ + "--abc", + }, + }, + }, + }, + }, + }, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + pod, err := patchTraceReadEndpoint(tc.inputParams, tc.inputPod) + require.Equal(t, tc.expectErr, err) + assert.Equal(t, tc.expectPod, pod) + }) + } +} + func TestTLSParameters(t *testing.T) { tempo := v1alpha1.TempoStack{ ObjectMeta: metav1.ObjectMeta{ @@ -410,6 +530,11 @@ func TestTLSParameters(t *testing.T) { }, }, Template: v1alpha1.TempoTemplateSpec{ + QueryFrontend: v1alpha1.TempoQueryFrontendSpec{ + JaegerQuery: v1alpha1.JaegerQuerySpec{ + Enabled: true, + }, + }, Gateway: v1alpha1.TempoGatewaySpec{ Enabled: true, }, @@ -444,8 +569,6 @@ func TestTLSParameters(t *testing.T) { assert.Contains(t, args, fmt.Sprintf("--traces.tls.key-file=%s/tls.key", manifestutils.TempoServerTLSDir())) assert.Contains(t, args, fmt.Sprintf("--traces.tls.cert-file=%s/tls.crt", manifestutils.TempoServerTLSDir())) assert.Contains(t, args, fmt.Sprintf("--traces.tls.ca-file=%s/service-ca.crt", manifestutils.CABundleDir)) - assert.Contains(t, args, fmt.Sprintf("--traces.read.endpoint=https://%s:16686", - naming.ServiceFqdn(tempo.Namespace, tempo.Name, manifestutils.QueryFrontendComponentName))) assert.Contains(t, args, fmt.Sprintf("--traces.tempo.endpoint=https://%s:%d", naming.ServiceFqdn(tempo.Namespace, tempo.Name, manifestutils.QueryFrontendComponentName), manifestutils.PortHTTPServer)) assert.Equal(t, corev1.URISchemeHTTPS, dep.Spec.Template.Spec.Containers[0].ReadinessProbe.HTTPGet.Scheme) diff --git a/tests/e2e/gateway/02-assert.yaml b/tests/e2e/gateway/02-assert.yaml index 69c88a0f3..e7122f244 100644 --- a/tests/e2e/gateway/02-assert.yaml +++ b/tests/e2e/gateway/02-assert.yaml @@ -139,3 +139,19 @@ spec: status: readyReplicas: 1 +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app.kubernetes.io/component: query-frontend + app.kubernetes.io/instance: foo + app.kubernetes.io/managed-by: tempo-operator + app.kubernetes.io/name: tempo + name: tempo-foo-query-frontend +spec: + template: + spec: + containers: + - name: tempo + - name: tempo-query diff --git a/tests/e2e/gateway/03-if-ui-exists-error.yaml b/tests/e2e/gateway/03-if-ui-exists-error.yaml new file mode 100644 index 000000000..fd312d3f7 --- /dev/null +++ b/tests/e2e/gateway/03-if-ui-exists-error.yaml @@ -0,0 +1,5 @@ +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: "while true; do container_names=$(kubectl get deployment/tempo-foo-query-frontend -n $NAMESPACE -o=jsonpath='{.spec.template.spec.containers[*].name}'); if [[ -n $(echo \"$container_names\" | grep \"tempo-query\") ]]; then echo \"tempo-query container still exists. Retrying...\"; else echo \"tempo-query container no longer exists. Test passed.\"; exit 0; fi; sleep 5; done" diff --git a/tests/e2e/gateway/03-install-disable-jaeger-query.yaml b/tests/e2e/gateway/03-install-disable-jaeger-query.yaml new file mode 100644 index 000000000..7e16eb4a7 --- /dev/null +++ b/tests/e2e/gateway/03-install-disable-jaeger-query.yaml @@ -0,0 +1,47 @@ +--- +apiVersion: tempo.grafana.com/v1alpha1 +kind: TempoStack +metadata: + name: foo +spec: + template: + gateway: + enabled: true + queryFrontend: + jaegerQuery: + enabled: false + storage: + secret: + type: s3 + name: minio-test + storageSize: 200M + tenants: + mode: static + authentication: + - tenantName: test-oidc + tenantId: test-oidc + oidc: + issuerURL: http://dex.svc:30556/dex + redirectURL: http://tempo-foo-gateway.svc:8080/oidc/test-oidc/callback + usernameClaim: email + secret: + name: oidc-test + authorization: + roleBindings: + - name: "test" + roles: + - read-write + subjects: + - kind: user + name: "admin@example.com" + roles: + - name: read-write + permissions: + - read + - write + resources: + - logs + - metrics + - traces + tenants: + - test-oidc