Skip to content

Commit d71a9da

Browse files
vitkyrkaclaude
andauthored
autodiscovery: dedupe the fake krakend test server, drop ticket references
The two docker-compose e2e fixtures (fake-krakend, fake-krakend-conflict) embedded byte-for-byte identical copies of a fake krakend metrics HTTP server. Extract it into its own file (fake-krakend-server.py) and splice it into both compose files via text/template -- the same pattern this package already uses for Helm values (k8s_test.go's helmValuesTemplate) -- rather than a docker-compose-native extends:/include: mechanism, which isn't used anywhere else in this repo's e2e fixtures and would be an unverified first use of that specific compose feature in this harness. Verified both templates still render byte-for-byte identical (base fixture) or with only the intended comment changes (conflict fixture) to the pre-refactor files, and that the rendered YAML parses correctly. Also drops the remaining Jira ticket references from this test file and the conflict fixture's comments, and fixes a stale reference to a since-renamed function (GenericIntegrationNamespaceRoots/NamespaceRoot). Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 129ac58 commit d71a9da

6 files changed

Lines changed: 174 additions & 171 deletions

test/new-e2e/tests/discovery/config_discovery_linux_test.go

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@
66
package discovery
77

88
import (
9+
"bytes"
910
_ "embed"
1011
"encoding/json"
1112
"strings"
1213
"testing"
14+
"text/template"
1315
"time"
1416

1517
"github.com/DataDog/datadog-agent/test/e2e-framework/components/datadog/dockeragentparams"
@@ -22,11 +24,49 @@ import (
2224
awsdocker "github.com/DataDog/datadog-agent/test/e2e-framework/testing/provisioners/aws/docker"
2325
)
2426

25-
//go:embed testdata/compose/docker-compose.fake-krakend.yaml
26-
var fakeKrakendComposeStr string
27+
// fakeKrakendServerScript is the fake krakend metrics HTTP server, shared
28+
// between the two compose fixtures below via text/template rather than
29+
// copy-pasted, since it's byte-for-byte identical in both.
30+
//
31+
//go:embed testdata/compose/fake-krakend-server.py
32+
var fakeKrakendServerScript string
33+
34+
//go:embed testdata/compose/docker-compose.fake-krakend.yaml.tmpl
35+
var fakeKrakendComposeTmpl string
36+
37+
//go:embed testdata/compose/docker-compose.fake-krakend-conflict.yaml.tmpl
38+
var fakeKrakendConflictComposeTmpl string
2739

28-
//go:embed testdata/compose/docker-compose.fake-krakend-conflict.yaml
29-
var fakeKrakendConflictComposeStr string
40+
var fakeKrakendComposeStr = mustRenderFakeKrakendCompose(fakeKrakendComposeTmpl)
41+
42+
var fakeKrakendConflictComposeStr = mustRenderFakeKrakendCompose(fakeKrakendConflictComposeTmpl)
43+
44+
// mustRenderFakeKrakendCompose splices fakeKrakendServerScript into a compose
45+
// template's `{{.Script}}` placeholder, indented to match the surrounding
46+
// YAML block scalar. YAML literal block scalars (`|`) only require non-blank
47+
// lines to be indented at least as much as the block itself, so blank lines
48+
// are left alone.
49+
func mustRenderFakeKrakendCompose(tmplSrc string) string {
50+
tmpl := template.Must(template.New("fake-krakend-compose").Parse(tmplSrc))
51+
var buf bytes.Buffer
52+
data := struct{ Script string }{Script: indentLines(fakeKrakendServerScript, 8)}
53+
if err := tmpl.Execute(&buf, data); err != nil {
54+
panic(err)
55+
}
56+
return buf.String()
57+
}
58+
59+
func indentLines(s string, n int) string {
60+
prefix := strings.Repeat(" ", n)
61+
lines := strings.Split(strings.TrimRight(s, "\n"), "\n")
62+
for i, line := range lines {
63+
if line == "" {
64+
continue
65+
}
66+
lines[i] = prefix + line
67+
}
68+
return strings.Join(lines, "\n")
69+
}
3070

3171
type configDiscoverySuite struct {
3272
e2e.BaseSuite[environments.DockerHost]
@@ -158,15 +198,14 @@ func (s *configDiscoverySuite) verifyKrakendCheckProvider(c *assert.CollectT) {
158198
}
159199

160200
// TestKrakendConfigDiscoverySuppressedByConflictingGenericIntegration verifies
161-
// the DSCVR-626 fix: configuration discovery must not schedule a second,
162-
// duplicate check on a container that already has a manually-configured
163-
// generic openmetrics/prometheus check claiming the same (or a rooted-in)
164-
// metric namespace as the dedicated integration would use. Without the fix,
165-
// both krakend (via discovery) and the manual openmetrics config would run
201+
// that configuration discovery must not schedule a second, duplicate check on
202+
// a container that already has a manually-configured generic
203+
// openmetrics/prometheus check claiming the same (or a rooted-in) metric
204+
// namespace as the dedicated integration would use. Without this, both
205+
// krakend (via discovery) and the manual openmetrics config would run
166206
// concurrently against the fake-krakend-conflict container, double-collecting
167207
// (and, for counter metrics, doubling the reported value of) the same
168-
// metrics — see https://datadoghq.atlassian.net/browse/DSCVR-626 for the
169-
// underlying bug and its real-world impact.
208+
// metrics.
170209
//
171210
// fake-krakend-conflict is a second, independent container from
172211
// fake-krakend (used by TestKrakendConfigDiscovery): it's also image-matched

test/new-e2e/tests/discovery/testdata/compose/docker-compose.fake-krakend-conflict.yaml

Lines changed: 0 additions & 93 deletions
This file was deleted.
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
services:
2+
fake-krakend-conflict:
3+
container_name: fake-krakend-conflict
4+
# Built and tagged as image "krakend" (rather than using the
5+
# com.datadoghq.ad.check.id override label like docker-compose.fake-krakend.yaml)
6+
# so that this container's own entity ID *also* remains among its AD
7+
# identifiers -- needed so the manual com.datadoghq.ad.checks openmetrics
8+
# config below (matched by entity ID) is a sibling of krakend's
9+
# image-matched discovery template on the exact same service, reproducing
10+
# the scenario where a manual openmetrics config and a later-discovered
11+
# dedicated integration both target the same container.
12+
image: krakend:e2e-conflict-test
13+
build:
14+
context: .
15+
args:
16+
DD_REGISTRY: ${DD_REGISTRY:-docker.io}
17+
# see docker-compose.fake-process.yaml for why this needs double-escaping
18+
dockerfile_inline: |
19+
ARG DD_REGISTRY=docker.io
20+
FROM $${DD_REGISTRY}/library/python:3.12-slim
21+
command:
22+
- sh
23+
- -c
24+
- |
25+
python3 << 'EOF'
26+
{{.Script}}
27+
EOF
28+
labels:
29+
# A manual openmetrics config, as a user might configure for a service
30+
# that doesn't (yet) have a dedicated Datadog integration. Its namespace
31+
# ("krakend.api") is rooted in krakend's own metric namespace
32+
# (__NAMESPACE__ = "krakend.api" in datadog_checks/krakend/check.py), so
33+
# once krakend's configuration-discovery template matches this same
34+
# container, discovery must be suppressed (see
35+
# comp/core/autodiscovery/listeners/common_filter.go's
36+
# GenericIntegrationNamespaceRoots/NamespaceRoot).
37+
com.datadoghq.ad.checks: |
38+
{
39+
"openmetrics": {
40+
"init_config": {},
41+
"instances": [
42+
{
43+
"openmetrics_endpoint": "http://%%host%%:9091/metrics",
44+
"namespace": "krakend.api"
45+
}
46+
]
47+
}
48+
}
49+
expose:
50+
- "9090"
51+
- "9091"

test/new-e2e/tests/discovery/testdata/compose/docker-compose.fake-krakend.yaml

Lines changed: 0 additions & 67 deletions
This file was deleted.
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
services:
2+
fake-krakend:
3+
container_name: fake-krakend
4+
# python:3.12-slim is already pulled by the mutatedbyadmissioncontroller
5+
# k8s e2e test (test/e2e-framework/components/datadog/apps/mutatedbyadmissioncontroller/k8s.go)
6+
# through the same ${DD_REGISTRY}/dockerhub/library/python ECR pull-through
7+
# cache path, so reusing it here avoids warming the cache for a new tag.
8+
image: ${DD_REGISTRY:-docker.io}/library/python:3.12-slim
9+
command:
10+
- sh
11+
- -c
12+
- |
13+
python3 << 'EOF'
14+
{{.Script}}
15+
EOF
16+
labels:
17+
com.datadoghq.ad.check.id: krakend
18+
expose:
19+
- "9090"
20+
- "9091"
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import http.server
2+
import socketserver
3+
import threading
4+
5+
6+
class MetricsHandler(http.server.BaseHTTPRequestHandler):
7+
# The real krakend metrics endpoint. Deliberately NOT on 9090 (see
8+
# below) so that the test proves discover_config actually probes
9+
# and picks the right port among several exposed ones, rather
10+
# than trivially succeeding because there's only one candidate.
11+
BODY = (
12+
b"# HELP krakend_requests_total Total requests.\n"
13+
b"# TYPE krakend_requests_total counter\n"
14+
b"krakend_requests_total 42\n"
15+
b"# HELP go_goroutines Number of goroutines.\n"
16+
b"# TYPE go_goroutines gauge\n"
17+
b"go_goroutines 17\n"
18+
)
19+
20+
def do_GET(self):
21+
self.send_response(200)
22+
self.send_header("Content-Type", "text/plain; version=0.0.4; charset=utf-8")
23+
self.send_header("Content-Length", str(len(self.BODY)))
24+
self.end_headers()
25+
self.wfile.write(self.BODY)
26+
27+
def log_message(self, *args):
28+
pass
29+
30+
31+
class DummyHandler(http.server.BaseHTTPRequestHandler):
32+
# A decoy endpoint on krakend's default metrics port (9090) that
33+
# is NOT valid krakend/openmetrics output. If config discovery
34+
# picked a port blindly instead of probing and validating each
35+
# candidate, it would pick this one and the test would fail.
36+
BODY = b"not krakend metrics\n"
37+
38+
def do_GET(self):
39+
self.send_response(200)
40+
self.send_header("Content-Type", "text/plain; charset=utf-8")
41+
self.send_header("Content-Length", str(len(self.BODY)))
42+
self.end_headers()
43+
self.wfile.write(self.BODY)
44+
45+
def log_message(self, *args):
46+
pass
47+
48+
49+
threading.Thread(
50+
target=lambda: socketserver.TCPServer(("", 9090), DummyHandler).serve_forever(),
51+
daemon=True,
52+
).start()
53+
socketserver.TCPServer(("", 9091), MetricsHandler).serve_forever()

0 commit comments

Comments
 (0)