Skip to content

Commit 42eb608

Browse files
committed
Improve the run-e2e skill
Also fill in the flag help for `new-e2e-tests.run`, where nine options had none, so the skill flag reference can point at `--help` rather than carry its own copy that would drift.
1 parent ae05ce6 commit 42eb608

8 files changed

Lines changed: 1215 additions & 69 deletions

File tree

.agents/skills/run-e2e/SKILL.md

Lines changed: 214 additions & 66 deletions
Large diffs are not rendered by default.
Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
# Running E2E tests inside a dev env
2+
3+
Why the dev-env path needs a bootstrap script at all, and what each of its steps works around.
4+
Read this when `devenv_e2e.py` exits non-zero, or when something behaves differently inside the
5+
container than it does on the host.
6+
7+
## What the container gets for free, and what it does not
8+
9+
`dda env dev start` already shares the parts of the host that matter most for E2E:
10+
11+
- The host's `~/.aws` is bind-mounted read-write at `/home/dd/.aws`, so the container resolves the same
12+
profile definitions. Note that this shares configuration, not authorization — see below.
13+
- `AWS_PROFILE`, `AWS_REGION` and `AWS_DEFAULT_REGION` are forwarded. A stray `AWS_PROFILE` on the
14+
host therefore follows you into the container.
15+
- `pulumi` is on `PATH` at `/usr/local/bin/pulumi`, inherited from the builder base image.
16+
17+
What it does not give you:
18+
19+
- **No E2E config.** `~/.test_infra_config.yaml` does not exist in a fresh container, and
20+
`dda inv -- new-e2e-tests.run` refuses to start without it.
21+
- **No Pulumi state directory.** Only `/var/lib/dd` and `/var/cache/dd` are persistent volumes.
22+
`$HOME` is not, so `~/.pulumi` — the backend selection, the plugins, and the local stack state —
23+
is recreated for each container and lost when it is removed.
24+
- **No keypair.** The config's `privateKeyPath` and `publicKeyPath` are host-absolute paths that do
25+
not resolve inside the container.
26+
- **No AWS authorization, despite the shared `~/.aws`.** The framework authenticates as a profile
27+
whose `credential_process` runs `aws-vault`, and aws-vault keeps its tokens in its own keyring, not
28+
in `~/.aws/sso/cache`. That keyring is local to the container and there is no way to pre-authorize it
29+
from the host, so authenticating in a new container means completing an SSO flow inside it, however
30+
healthy the host's session is. The env's browser proxy opens that flow on your desktop, so it is
31+
workable — but it needs a person, which is why the bootstrap warns before the step that triggers it
32+
and gives up rather than waiting indefinitely. The bootstrap probes rather than predicts, because
33+
the host's session state carries no information about the container's.
34+
35+
## How `devenv_e2e.py up` closes the gaps
36+
37+
**The keypair is mounted read-only at `/.e2e/`, not under `$HOME`.** On first start the image
38+
entrypoint runs `chown -R dd: /home/dd` under `set -e`. A read-only mount anywhere inside `$HOME`
39+
makes that `chown` fail, which aborts the entrypoint, so the container never reaches
40+
`Server listening on :: port 22` and `dda env dev start` times out waiting for readiness. The error
41+
surfaces as `Container ... is not ready`, with `chown: ... Read-only file system` further up the
42+
container log. Mounting outside `$HOME` avoids it entirely.
43+
44+
**A copy of the config is mounted read-only next to the keys, then copied again to `$HOME` inside the
45+
container.** It has to live at `$HOME/.test_infra_config.yaml`: `getConfigFilePath` in
46+
`/test/e2e-framework/testing/runner/local_profile.go` and `get_full_profile_path` in
47+
`/tasks/e2e_framework/config.py` both hardcode that location and neither accepts an override. Since
48+
`$HOME` cannot hold a read-only mount, a copy is the way in — and it has the nicer property that the
49+
container works on its own copy, so nothing it does can reach the host's file. The script re-copies
50+
on every `up`, so host edits propagate, and chmods it to `600` because it carries the Pulumi
51+
passphrase and the Datadog API key.
52+
53+
`dda env dev fs import` would be the obvious tool for this and is deliberately not used. It builds
54+
its internal `mv` with paths wrapped in literal double quotes, which the nu shell passes through as
55+
part of the filename, so it fails with `nu::shell::io::not_found` against a path like
56+
`'"/home/dd/.test_infra_config.yaml"'` whenever the env's shell is nu — which it is by default under
57+
`env.dev.universal-shell`. A plain `cp` of arguments the script controls sidesteps it. Expect
58+
`fs export` to have the same problem, since it shares that quoting.
59+
60+
**The key paths are rewritten in that copy, not overridden per run.** The copy the container gets is
61+
generated on the host with `privateKeyPath` and `publicKeyPath` pointing at `/.e2e/`; everything else,
62+
including the keypair name and both passphrases, is carried over untouched. That copy lives at
63+
`<temp>/dda-e2e-<id>/.test_infra_config.yaml` in a `0700` directory with mode `0600`, since it holds
64+
the same secrets as the original, and `devenv_e2e.py down` deletes it along with the env.
65+
66+
The alternative — exporting `E2E_AWS_PRIVATE_KEY_PATH` and `E2E_AWS_PUBLIC_KEY_PATH` on the test
67+
command line, which would work because the runner builds
68+
`parameters.NewCascadingStore(envValueStore, configFileValueStore)` and so prefers environment
69+
variables — is avoided on purpose. **On a Windows host, MSYS path conversion rewrites absolute
70+
POSIX-looking arguments when a command goes through Git Bash**, so `/.e2e/key.pem` arrives inside the
71+
container as `C:/Users/.../git/.e2e/key.pem` and the run fails when it tries to reach the VM. Keeping
72+
container paths out of the command line removes the exposure. `E2E_STACK_NAME_SUFFIX` is still passed
73+
that way because it contains no slashes.
74+
75+
If you do need to type a container path into a command yourself on Windows, prefix it with
76+
`MSYS_NO_PATHCONV=1`. The full list of overridable values is in `references/flags.md`, sourced from
77+
`/test/e2e-framework/testing/runner/parameters/store_env.go`.
78+
79+
**`E2E_STACK_NAME_SUFFIX` is set to the host username.** `localProfile.NamePrefix()` derives the
80+
stack prefix from the OS username, which is `dd` for every developer's container. Without a
81+
per-developer suffix, two people running the same suite would collide on stack names and on the
82+
resource names derived from them, in a shared cloud account. Pass `--stack-name-suffix S` to `up` and
83+
it becomes `<hostuser>-S`, keeping that property.
84+
85+
This is a workaround for a gap in the framework rather than the right fix. `localProfile.NamePrefix()`
86+
already rejects usernames that do not identify a person — it maps `""` and `root` to `nouser` — and
87+
`dd` is one it does not know about, while `ciProfile` takes its prefix as injected data rather than
88+
guessing. The proper fix is a name-prefix runner parameter that `dda inv -- e2e.setup` writes once, after
89+
which this injection and this paragraph both go away. Until then, two consequences are worth knowing:
90+
the suffix has to be re-supplied on every command, and the `username` tag on provisioned resources is
91+
derived separately inside the Pulumi program, so it reads `dd` regardless — which is why hunting for
92+
orphaned resources by owner tag does not work from the dev-env path.
93+
94+
**Pulumi's backend is established with `dda inv -- e2e.setup --no-interactive`.** The binary is already
95+
present, but the plugins and the local-backend selection live in the ephemeral home. The
96+
`--no-interactive` flag is what makes this safe to run inside a container: it does the Pulumi work
97+
and skips `setup_aws_config` entirely. Never run the interactive form here — see
98+
`references/setup.md` for what it would do to your AWS keypair. `PULUMI_SKIP_UPDATE_CHECK=true` is
99+
set alongside it because `pulumi_version()` in `/tasks/e2e_framework/setup/pulumi.py` treats "a newer
100+
Pulumi exists upstream" as not-installed and re-downloads it into the ephemeral home, where the
101+
image's copy shadows it anyway.
102+
103+
## The bootstrap only handles AWS targets
104+
105+
It copies in and rewrites the paths for `configParams.aws` alone, and its access check probes the AWS
106+
profile. A target provisioning into Azure or GCP would reach the container with `configParams.azure` or
107+
`configParams.gcp` still holding host-absolute key paths, which do not resolve there, and its
108+
credentials unverified. Run those with `--host` until this handles them; the shape of the fix is the
109+
same three steps applied per configured provider.
110+
111+
Targets that use the framework's local provisioners need none of this — no keypair, no cloud
112+
credentials — but they still need the config file and the Pulumi backend, so the bootstrap is still the
113+
way in. Give it `--no-aws-check` for those: the access check costs an interactive SSO acceptance, and
114+
gating a local, cost-free run behind that is a toll for nothing. The bootstrap cannot work this out for
115+
itself, because it never sees which target you are about to run.
116+
117+
## Dev env states
118+
119+
The states and their transitions are documented in
120+
`/docs/public/tutorials/dev/env.md`. What matters here is
121+
the preconditions: `dda env dev start` accepts only `nonexistent` and `stopped`, `remove` only `error`
122+
and `stopped`, `stop` only `started`, and `start` refuses new mount options on a `stopped` env because
123+
a stopped env keeps its saved configuration. The script handles this:
124+
125+
| State | What it does |
126+
|---|---|
127+
| `started` | Nothing |
128+
| `stopped` | Resumes with `dda env dev start` and no options. Never removes it — its Pulumi state may still be needed |
129+
| `error` | Refuses. The state means the container exited non-zero, which covers both "never started" and "ran tests, then died" — and the second holds the only copy of those stacks' state, which cannot be inspected while it is down. Recreating is the user's call |
130+
| `nonexistent` | Starts with the mounts |
131+
132+
To change the mounts on an existing env, remove it first — but only once you are sure it has no live
133+
stacks.
134+
135+
The script starts envs with `--no-pull`, so a container is created from whatever image is already
136+
local. That keeps a routine `up` from re-downloading a 12 GB image, at the cost of not picking up
137+
image fixes on its own. To move to a current image, remove the env and pull explicitly:
138+
139+
```bash
140+
docker pull datadog/agent-dev-env-linux
141+
```
142+
143+
## `env.dev.clone-repos` has to be off
144+
145+
With that setting enabled, `dda env dev start` fetches datadog-agent from GitHub instead of
146+
bind-mounting your checkout, so the container tests the default branch and not your changes. There is
147+
no `--no-clone` flag to override it for a single command, so `devenv_e2e.py` detects it and asks you
148+
to turn it off:
149+
150+
```bash
151+
dda config set env.dev.clone-repos false
152+
```
153+
154+
It only affects envs created afterwards, so remove and recreate the env for it to take effect.
155+
156+
Working around it by pre-mounting the checkout and letting the clone step skip a non-empty directory
157+
does not work either: `git dd-clone` exits 1 in that case, `dda env dev start` reports failure, and
158+
because `save_config()` runs only after a successful start, every later `dda env dev run` falls back
159+
to the default shell and sends `&&` to nu, which fails. The revision check below is the backstop that
160+
catches an env built this way regardless.
161+
162+
## The checkout has to be a plain clone named `datadog-agent`
163+
164+
`dda env dev start` derives the repo bind mount from the current directory's parent: it mounts
165+
`<cwd>/../datadog-agent` at `/repos/datadog-agent` and aborts with
166+
`Local repository not found: datadog-agent` if that does not exist.
167+
168+
Two consequences:
169+
170+
- The checkout directory must be named `datadog-agent`.
171+
- **A git worktree does not work.** Its `.git` is a file pointing into the main clone's
172+
`.git/worktrees/`, which is outside every mount, so git inside the container fails. That is fatal
173+
rather than cosmetic, because `new-e2e-tests.run` reads the commit SHA unconditionally to populate
174+
`E2E_COMMIT_SHA`, so the run aborts before its own preflight.
175+
176+
`/.agents/skills/follow-pr/create_devenv.sh` sidesteps the naming rule with
177+
`--repo "$(git rev-parse --show-toplevel)"`, which makes the mount destination
178+
`/repos/<absolute path>`. That is a POSIX-only trick: a Windows path contains a drive colon, which
179+
makes the resulting `-v` spec unparseable. Do not copy it here.
180+
181+
From a worktree, either run from the main clone or use `--host`.
182+
183+
`devenv_e2e.py up` verifies this rather than trusting it: it compares `git rev-parse HEAD` on the host
184+
with the same command in the container and refuses to continue if they differ. That catches every way
185+
the container can end up holding the wrong code — a clone instead of a mount, or an env created before
186+
you switched branches in a way that did not propagate — because testing the wrong revision silently is
187+
worse than failing.
188+
189+
## Windows hosts
190+
191+
Pass `-t linux-container` to every `dda env dev` command, for the reason given in
192+
`/docs/public/tutorials/dev/env.md`. The script always
193+
passes it explicitly, since it is a no-op on other platforms.
194+
195+
## Getting files out
196+
197+
Test output is written to `$HOME/e2e-output/<suite>/<timestamp>` with a `latest` symlink, resolved
198+
from the home directory of whoever ran the test — so in the dev-env case, the container's. Use
199+
`docker cp`, because `dda env dev fs export` shares the quoting bug described above:
200+
201+
```bash
202+
docker cp dda-linux-container-e2e-run:/home/dd/e2e-output ./e2e-output
203+
```
204+
205+
For anything more interactive, get a shell in the env:
206+
207+
```bash
208+
dda env dev shell -t linux-container --id e2e-run
209+
```
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# Flags and runner variables
2+
3+
Read this when the request needs more than a target and a test name — a locally built package or
4+
image, dev mode, config-map overrides, retries — or when a single value from
5+
`~/.test_infra_config.yaml` needs overriding for one run.
6+
7+
## Test command
8+
9+
`dda inv -- new-e2e-tests.run --help` describes every flag and is the reference. This file only groups
10+
them by purpose and records the judgement calls the help text cannot make for you.
11+
12+
`--tags`, `--targets`, `--configparams`, `--run` and `--skip` are repeatable; the rest take one value.
13+
14+
- Choosing what runs: `--targets`, `--run`, `--skip`, `--tags`, `--osdescriptors`, `--no-recursive`
15+
- Choosing what gets tested: `--agent-image`, `--cluster-agent-image`, `--local-package`,
16+
`--pipeline-id`, `--flavor`, `--configparams`
17+
- Infrastructure lifecycle: `--keep-stack`, `--stack-name-suffix`, `--max-retries`, `--timeout`
18+
- Output: `--verbose`, `--cache`, `--logs-folder`, `--result-json`, `--junit-tar`, `--extra-flags`
19+
20+
### Judgement calls
21+
22+
- `--targets` resolves against `/test/new-e2e/`, so repeating that prefix inside the target is wrong.
23+
It repeats as a flag, but prefer one target per run: each provisions its own stack.
24+
- Anchor `--run` at both ends. `TestFlare` also selects `TestFlareOpts`.
25+
- On the dev-env path, give `--stack-name-suffix` to `devenv_e2e.py up` rather than to the test. It sets
26+
the same variable the bootstrap uses to keep your stacks distinct from other developers', so passing
27+
it here replaces that instead of adding to it.
28+
- `--keep-stack` in a dev env means keeping the env too, because the stack's state lives inside it.
29+
- `dda build docker` is the supported way to produce and push an image for `--agent-image`; it prints
30+
the matching command when it finishes.
31+
- Never `--profile ci` on a developer machine. It skips the local-config preflight that exists to fail
32+
early with a clear message, so the run fails later and less legibly instead.
33+
34+
## Overriding config values for one run
35+
36+
The runner resolves each parameter through
37+
`parameters.NewCascadingStore(envValueStore, configFileValueStore)`
38+
(`/test/e2e-framework/testing/runner/local_profile.go`), so an environment variable wins over
39+
`~/.test_infra_config.yaml`. This is what lets the dev-env path keep the host's config file while
40+
redirecting the key paths at the container's copies. The full mapping is
41+
`/test/e2e-framework/testing/runner/parameters/store_env.go`; the ones that come up:
42+
43+
| Variable | Overrides |
44+
|---|---|
45+
| `E2E_KEY_PAIR_NAME` | `configParams.aws.keyPairName` |
46+
| `E2E_AWS_PRIVATE_KEY_PATH` | `configParams.aws.privateKeyPath` |
47+
| `E2E_AWS_PUBLIC_KEY_PATH` | `configParams.aws.publicKeyPath` |
48+
| `E2E_AWS_PRIVATE_KEY_PASSWORD` | `configParams.aws.privateKeyPassword` |
49+
| `E2E_PULUMI_PASSWORD` | `configParams.pulumi.passphrase` |
50+
| `E2E_API_KEY`, `E2E_APP_KEY` | `configParams.agent.apiKey` / `.appKey` |
51+
| `E2E_STACK_NAME_SUFFIX` | Same as `--stack-name-suffix` |
52+
| `E2E_DEV_MODE` | Same as `--keep-stack` |
53+
| `E2E_EXTRA_RESOURCES_TAGS` | Extra tags on provisioned resources |
54+
| `E2E_OUTPUT_DIR` | Where test output and diagnostics are written |
55+
| `E2E_FAKEINTAKE_IMAGE_OVERRIDE` | The fakeintake image, instead of the pinned tag |
56+
57+
Note `DD_API_KEY` is not part of this — the E2E path uses `E2E_API_KEY` and the `configParams.agent`
58+
values, which are length-checked (32 and 40 characters).
59+
60+
Pass these through `dda env dev run` with an `env VAR=value ...` prefix, for the reason given in the
61+
skill's run step.
62+
63+
## Passing raw flags to `go test`
64+
65+
`--extra-flags` is appended verbatim after `-args`, which covers suite-specific flags the task does not
66+
model. Reach for that rather than calling `go test` yourself: the invoke task is what computes the build
67+
tags, runs the local-config preflight and exports `PULUMI_CONFIG_PASSPHRASE`, and a run that skips those
68+
can fail for reasons that have nothing to do with the code under test.
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# One-time E2E setup
2+
3+
What `dda inv -- e2e.setup` does, and why it belongs on the host. Read this before offering to run it.
4+
5+
Read `/docs/public/how-to/test/e2e.md` for what `dda inv -- e2e.setup` does, the AWS access it assumes,
6+
and the Azure and GCP variants — it is the authoritative guide and this file does not restate it. What follows is only what that guide does not
7+
cover: which machine to run setup on, and what breaks if you get that wrong.
8+
9+
Two facts from it are worth having in mind here. Setup does not arrange AWS authentication itself, so
10+
an expired session shows up as an aws-vault error partway through; `aws-vault login
11+
sso-agent-sandbox-account-admin-8h` **on the host** fixes that, and does nothing for a container's own
12+
authorization. And re-running is idempotent, so suggesting it is cheap even when you are not sure it is
13+
needed.
14+
15+
## Always run it on the host
16+
17+
Two reasons, and they both bite hard.
18+
19+
**The keypair name is derived from the OS username.** It is `e2e-<account>-<username>`, and inside a
20+
dev env the user is always `dd`. So an interactive setup in a container creates a *second* AWS
21+
keypair, `e2e-agent-sandbox-dd`, in the shared sandbox account rather than reusing yours.
22+
23+
**Worse, it only works once.** `_ensure_aws_keypair` in `/tasks/e2e_framework/setup/aws.py` has four
24+
branches: keypair present both in AWS and on disk is a no-op; local files only get imported to AWS;
25+
neither means create both; and **AWS-only is a hard failure**, deliberately, so it never silently
26+
overwrites a keypair others might be using. A container's home directory is not persistent, so the
27+
second fresh container hits exactly that branch — AWS has `e2e-agent-sandbox-dd`, the new container
28+
has no key files — and fails with no way forward except deleting the keypair by hand.
29+
30+
This is why the bootstrap copies the host's config and keypair into the container instead, and why the
31+
only form of the task that is safe to run inside one is the `--no-interactive` one, which does the
32+
Pulumi work and skips the AWS and config-file work entirely. `references/devenv.md` has the exact
33+
command and the reason for the environment variable that accompanies it.
34+
35+
## What the runner reads
36+
37+
The config file is looked up at `$HOME/.test_infra_config.yaml` by both the Go runner and the invoke
38+
preflight, and neither accepts an override — which is why the dev-env path has to place a copy there
39+
rather than point at one. Individual values inside it can still be overridden per run through
40+
environment variables; see `references/flags.md`.
41+
42+
One side effect worth knowing: the preflight keeps `~/.aws/config` in sync by appending the SSO
43+
profile when it is absent, for example after a role rename. `~/.aws` is bind-mounted read-write into a
44+
dev env, so a run inside a container can write to the host's file. It is idempotent and skips when the
45+
profile is already there.

0 commit comments

Comments
 (0)