Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions applicationsets/addons-tempo.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,19 @@ spec:
# association — no static keys. Without the annotation the block renders empty and the
# base values' local backend stands. `index` yields "" for the missing key under
# missingkey=error.
#
# `endpoint` is REQUIRED and is not the same story as loki's. Tempo reaches S3 through
# the minio-go client, which validates the endpoint before it ever looks at the region
# and refuses an empty one:
#
# failed to create minio client: Endpoint: does not follow ip address or
# domain name standards.
#
# Tempo then exits non-zero at startup, so the StatefulSet crashloops and the
# Application sits Progressing forever — it never reaches Healthy, and on a fresh
# install that is the one thing holding up convergence. loki does not need it because
# its AWS client derives the endpoint from the region, which is why the two blocks
# look asymmetric and have to stay that way.
values: |
{{- if index .metadata.annotations "observability/tempo-bucket" }}
tempo:
Expand All @@ -60,6 +73,7 @@ spec:
s3:
bucket: {{ index .metadata.annotations "observability/tempo-bucket" }}
region: {{ index .metadata.labels "region" }}
endpoint: s3.{{ index .metadata.labels "region" }}.amazonaws.com
{{- end }}
valueFiles:
- $values/addons/observability/tempo/values.yaml
Expand Down
26 changes: 24 additions & 2 deletions applicationsets/rendertest/loki_tempo_render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,9 @@ func TestTempoS3Storage(t *testing.T) {
Trace struct {
Backend string `yaml:"backend"`
S3 struct {
Bucket string `yaml:"bucket"`
Region string `yaml:"region"`
Bucket string `yaml:"bucket"`
Region string `yaml:"region"`
Endpoint string `yaml:"endpoint"`
} `yaml:"s3"`
} `yaml:"trace"`
} `yaml:"storage"`
Expand All @@ -101,6 +102,27 @@ func TestTempoS3Storage(t *testing.T) {
if want := labels["region"]; v.Tempo.Storage.Trace.S3.Region != want {
t.Fatalf("tempo.storage.trace.s3.region = %q, want %q\n%s", v.Tempo.Storage.Trace.S3.Region, want, out)
}
// The assertion this test was missing, and it is not symmetric with loki's.
//
// Tempo reaches S3 through the minio-go client, which validates the endpoint before
// it looks at anything else and refuses an empty one:
//
// failed to create minio client: Endpoint: does not follow ip address or
// domain name standards.
//
// Tempo then exits at startup, the StatefulSet crashloops, and the Application sits
// Progressing forever — on a fresh install that is the single thing that holds up
// catalog convergence, for thirty minutes, until the installer gives up. A bucket
// and a region rendered correctly the whole time, which is why asserting those was
// not enough: the manifest was well-formed and the config was incomplete.
//
// loki needs no endpoint because its AWS client derives one from the region. The two
// blocks are asymmetric on purpose.
if want := "s3." + labels["region"] + ".amazonaws.com"; v.Tempo.Storage.Trace.S3.Endpoint != want {
t.Fatalf("tempo.storage.trace.s3.endpoint = %q, want %q — tempo's minio client "+
"rejects an empty endpoint and the pod crashloops\n%s",
v.Tempo.Storage.Trace.S3.Endpoint, want, out)
}
})

t.Run("cluster without the annotation keeps the local backend", func(t *testing.T) {
Expand Down