traefik_mesh: add container-based config discovery support - #24637
Closed
vitkyrka wants to merge 2 commits into
Closed
traefik_mesh: add container-based config discovery support#24637vitkyrka wants to merge 2 commits into
vitkyrka wants to merge 2 commits into
Conversation
Adds a `local:from_traefik_mesh_kube_daemon_set` discovery strategy that gates candidates on the container's `kube_daemon_set` tag instead of its image. The proxy container shares the stock upstream `traefik` image with any plain (non-mesh) Traefik deployment, but the chart hardcodes its DaemonSet name to `traefik-mesh-proxy`, so the Kubernetes-derived tag reliably distinguishes it (same class of fix as argocd's PR #24624, keyed on object identity instead of a label). Also fixes two pre-existing bugs found while getting the kind-based E2E discovery tests to pass, both independent of discovery: - tests/kind/traefik_mesh.yaml hardcoded the proxy's controller lookup to the `default` namespace instead of `traefik-mesh`, leaving the proxy pod permanently un-Ready. - spec.yaml's optional endpoint fields used a lowercase-scheme example (`http://<...>`), which defeated ddev's placeholder-vs-default codegen heuristic and made the generated Pydantic default the literal placeholder string instead of None, crashing with InvalidURL on any config that left them unset. Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Validation Report
Run Passed validations (19)
|
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 8adfcfc | Docs | Datadog PR Page | Give us feedback! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds container-based config discovery support to traefik_mesh.
Discovery strategy: Traefik Mesh's proxy shares the stock upstream
traefikimage with any plain (non-mesh) Traefik reverse-proxy deployment, so image/ad_identifiers-only matching can't tell them apart. This PR uses a local discovery strategy that reads the container'skube_daemon_setKubernetes tag instead — the chart hardcodes the proxy's DaemonSet name totraefik-mesh-proxy, so that tag reliably identifies the role even though the image itself is generic. This mirrors the approach in argocd's discovery PR (#24624), which solved the same shared-image problem with a different Kubernetes-derived tag (kube_app_name, since ArgoCD's chart setsapp.kubernetes.io/nameper role; Traefik Mesh's chart predates that labeling convention and doesn't).Base branch note: this branch is based on the not-yet-merged
vwhitchurch/velero-discoverybranch, which adds the shared Kubernetes-discovery E2E helpers (datadog_checks_dev/datadog_checks/dev/kube_discovery.py) this PR depends on, the same dependency argocd's discovery PR has. The diff againstmastertherefore also includes velero's own discovery changes; rebase ontomasteronce that branch merges.Two independent bug fixes, both needed for the new E2E discovery tests to pass, both unrelated to discovery itself:
tests/kind/traefik_mesh.yaml's vendored manifest hardcoded the proxy's--providers.http.endpointcontroller lookup to thedefaultnamespace (traefik-mesh-controller.default.svc.cluster.local) instead oftraefik-mesh. Confirmed against the live upstream chart template (traefik/mesh-helm-chart, which correctly uses{{ .Release.Namespace }}) that this is a rendering bug in the checked-in fixture. It silently left the proxy pod permanently un-Ready — undetected until now because the existingtest_e2e_openmetrics_v2test port-forwards directly and never checks pod readiness (the proxy's/metrics//api/versionendpoints, served on a different Traefik entrypoint, worked fine regardless).traefik_controller_api_endpoint/traefik_proxy_api_endpointare optional fields whosespec.yamlexample used a lowercase URL scheme (http://<...>). ddev's config-model codegen only treats an example as "documentation, not a real default" when the whole string is uppercase; the lowercase scheme broke that heuristic, so the generated Pydantic default for these unconfigured optional fields was the literal placeholder string, notNone. Any real customer leavingtraefik_controller_api_endpointunset would hitrequests.exceptions.InvalidURLon every check run. Existing unit tests never caught this because they mockself.http.getentirely, bypassing the URL-parsing crash. Fixed by uppercasing both examples.Motivation
https://datadoghq.atlassian.net/browse/DSCVR-484
Part of DSCVR-518.
Container-based discovery lets the Agent auto-configure this integration from Autodiscovery without hand-written instance config.
Validated with new unit tests for the discovery strategy, and a clean
ddev env test --dev --new-env traefik_mesh py3.13run: all 3 E2E tests pass, including the pre-existingtest_e2e_openmetrics_v2(no regression) and the two new discovery tests —test_e2e_discovery_all_candidates(generated config probes the real proxy pod cleanly) andtest_e2e_discovery(the real in-cluster Agent resolveskube_daemon_set:traefik-mesh-proxyvia its own Kubernetes tagger and runs the check through Autodiscovery,config.provider: ad-container-discovery, 76 metrics collected).Known gap: the controller's readiness/mesh-node-status metrics (
traefik_mesh.controller.ready,traefik_mesh.node.ready) require a second container's (traefik-mesh-controller) host/port that a single-container discovery candidate can't see — only the proxy's OpenMetrics endpoint is discovered; the controller fields are left unset (same category as aerospike's legacy-mode gap).Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged