Add with-browser Docker image #829

The-9880 · 2024-08-19T09:51:45Z

Issue: https://github.com/grafana/synthetic-monitoring/issues/71
Adds a 3rd stage to the Dockerfile to build the sm-agent image from an Alpine base, installing chromium-switshader.

The Drone CI pipeline is also updated to publish new image versions (tagged *-with-browser) corresponding to the new build target:

latest-with-browser
<semver-long>-with-browser

We should probably prevent this from publishing until we have some logic in the agent that can leverage it.

CLAassistant · 2024-08-19T09:53:18Z

All committers have signed the CLA.

nadiamoe

Left a few small comments, and a few bigger ones that we can leave for follow-up PRs if this one has been tested working.

scripts/configs/drone/main.jsonnet

nadiamoe · 2024-08-19T11:31:14Z

scripts/configs/drone/main.jsonnet

+    step(
+      'docker publish (with browser) tags',
+      [
+        '{ echo latest-with-browser,$(eval ./scripts/version)-with-browser ; } > .tags',  // use with-browser tags for docker plugin


I'd suggest to use a shorter -browser suffix instead of -with-browser, just for the sake of brevity. I think the short version does not leave much room for ambiguity. WDYT?

I agree! I noticed other Docker images like that for k6 were using -with-browser, but I don't mind shortening it for us

Well, normally I would want it to be consistent with other releases but because we build a longer tag name for our images, including the commit hash, I think I like keeping it shorter for this case

Hmm... that's a good point, consistency is good... I don't see a clear winner but I'm still leaning towards -browser for short. Maybe we can ask the rest of the team to see if anyone has a strong opinion?

Dockerfile

nadiamoe · 2024-08-19T11:38:37Z

Dockerfile

+ENV SM_CHROME_BIN=/usr/bin/chromium-browser
+ENV SM_CHROME_PATH=/usr/lib/chromium/


What are these two for?

I wondered if providing an env var might be useful instead of relying on PATH to have chromium/assuming the binary name, for when sm-agent leverages it.

Will we add xk6-browser to the k6 setup for this? We may want to add K6_BROWSER_HEADLESS and K6_BROWSER_ARGS for that

xk6-browser is now part of k6, so it's already there :D

I believe it should already be looking for the correct path, but we definitely want to tweak K6_BROWSER_ARGS. For instance, --disable-dev-shm-usage is required for some webpages to not crash on a container.

The K6 Dockerfile seems to do the following:

# no-sandbox chrome arg is required to run chrome browser in # alpine and avoids the usage of SYS_ADMIN Docker capability ENV K6_BROWSER_ARGS=no-sandbox

Should we be using no-sandbox as well? I'll add it tentatively to this PR, will re-request your review again before I merge anything.

nadiamoe · 2024-08-19T11:39:47Z

Dockerfile

+COPY --from=release /usr/local/lib/synthetic-monitoring-agent/pre-stop.sh /usr/local/lib/synthetic-monitoring-agent/pre-stop.sh
+COPY --from=release /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt
+
+ENTRYPOINT ["/usr/local/bin/synthetic-monitoring-agent"]


With browser added in, we'll have the agent start the k6 process, which will itself start a chromium process, which will spawn a gazillion processes itself. I think this calls for adding tini to make sure all those children are properly terminated.

nadiamoe · 2024-08-19T11:41:52Z

Dockerfile

+# third stage with alpine base for better access to chromium
+FROM alpine:3.18 as with-browser
+
+RUN apk --no-cache add chromium-swiftshader


At this point we will be running everything as root. I think this can be left for a follow-up as well, but running a browser as root makes me a bit scared so we might want to avoid that.

👍 I can add a user and operate the remainder of the target from there

Re-thinking this (sorry)... This might get us into problems where running the agent as non-root might cause some check types to fail. e.g. traceroute/ping checks require either elevated privileges or net_raw capabilities to be added to the binary.

It might be better for now to run everything as root, and open an issue to narrow-down the privileges without breaking agent checks.

nadiamoe

If this passes some happy path tests, it looks good to me as a first iteration to unblock early adopters and internal tests.

I'd suggest to open a few follow-up issues to act on before we make this more widely available:

Migrate base image to alpine to consolidate in one distro
Pin and auto update chromium versions (Copy from ci/renovate: update pinned alpine packages crocochrome#5 when it is merged)
Run both agent and chromium as non-root, granting required capabilities (i.e. net_raw) to the agent binary.

nadiamoe · 2024-08-20T12:43:59Z

Forgot to mention, making sure that --disable-dev-shm-usage is passed to chromium is probably something we should figure out in this PR.

nadiamoe

Looking good!

nadiamoe · 2024-08-27T09:27:55Z

scripts/configs/drone/main.jsonnet

+    step(
+      'docker publish (with browser) tags',
+      [
+        '{ echo latest-browser,$(eval ./scripts/version)-browser ; } > .tags',  // use with-browser tags for docker plugin


As a small nit, is there a reason for this command using eval and curly braces?
On my PC, this seems to work:

echo "latest-browser,$(./scripts/version)-browser"

That should work - updating!

Updates the Dockerfile into a multi-stage build. The new final stage is based off of Alpine to enable simple access to chromium-swiftshader as the choice of headless browser. The Drone CI config is updated with additional stages to tag and push alternate Docker images corresponding to this stage.

The-9880 · 2024-08-27T18:10:11Z

Merging without re-approval as the only change was to sort out the .drone.yml conflict with main.

* k6runner: always log error code and string to user's logger * Update module golang.org/x/sync to v0.8.0 (#812) * Update module golang.org/x/net to v0.28.0 (#813) * Build(deps): Bump the prometheus-go group across 1 directory with 2 updates (#827) * Update ghcr.io/grafana/grafana-build-tools Docker tag to v0.22.0 (#817) * Update module go.k6.io/k6 to v0.53.0 (#823) * Update dependency grafana/k6 to v0.53.0 * Update module github.com/miekg/dns to v1.1.62 * Update github.com/grafana/loki/pkg/push digest to 9315b3d * Update golang.org/x/exp digest to 9b4947d * Update github.com/securego/gosec/v2 digest to ab3f6c1 * Update github.com/grafana/loki/pkg/push digest to 246a1df * Update ghcr.io/grafana/grafana-build-tools Docker tag to v0.23.0 * Build(deps): Bump github.com/prometheus/client_golang * Update module github.com/securego/gosec to v2 * renovate: fix grafana-build-tools dependency regex * Update ghcr.io/grafana/grafana-build-tools Docker tag to v0.23.0 * drone: regenerate pipelines * Add with-browser Docker image (#829) * Update module github.com/prometheus/prometheus to v0.54.1 (#843) * Update module github.com/prometheus/common to v0.56.0 (#849) * Build(deps): Bump google.golang.org/grpc from 1.65.0 to 1.66.0 * Build(deps): Bump github.com/prometheus/common * renovate: group prometheus-go updates * renovate: enable default managers * renovate: add `dependencies` label to PRs * dependabot: remove * Update renovatebot/github-action action to v40.2.7 (#859) * go: upgrade to 1.23 (#838) * Update module github.com/golangci/golangci-lint to v1.60.0 (#825) * Update alpine Docker tag to v3.20 (#858) * Update actions/checkout action to v4.1.7 (#857) * Dockerfile: pin hash of debian:stable-slim image (#828) * Update module github.com/mccutchen/go-httpbin/v2 to v2.14.1 (#860) * Update module github.com/securego/gosec to v2 (#861) * feat: Validate browser capability (#809) Signed-off-by: ka3de <[email protected]>

The-9880 requested a review from a team as a code owner August 19, 2024 09:51

The-9880 force-pushed the build-browser-image branch from d5101df to dc88a47 Compare August 19, 2024 09:53

The-9880 requested a review from nadiamoe August 19, 2024 10:01

nadiamoe reviewed Aug 19, 2024

View reviewed changes

nadiamoe approved these changes Aug 20, 2024

View reviewed changes

The-9880 requested a review from nadiamoe August 20, 2024 19:35

nadiamoe approved these changes Aug 27, 2024

View reviewed changes

The-9880 added 4 commits August 27, 2024 10:39

Code review.

7dc5b2b

Remove user and add --disable-dev-shm-usage to chromium args.

c091617

Roll back GBT to v0.22.0

4adbab7

The-9880 force-pushed the build-browser-image branch from 637fb3f to 4adbab7 Compare August 27, 2024 14:50

The-9880 added 3 commits August 27, 2024 11:02

Remove eval from shell command.

dae6082

Return to v0.23.0.

32449cd

Remove curly braces.

dd0bd54

The-9880 merged commit dd89415 into main Aug 27, 2024
2 checks passed

The-9880 deleted the build-browser-image branch August 27, 2024 18:10

ka3de mentioned this pull request Sep 2, 2024

Release v0.26.0 #863

Merged

nadiamoe mentioned this pull request Sep 30, 2024

Specifying --vus and --iterations prevents execution of browser checks #915

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add with-browser Docker image #829

Add with-browser Docker image #829

The-9880 commented Aug 19, 2024 •

edited

Loading

CLAassistant commented Aug 19, 2024 •

edited

Loading

nadiamoe left a comment

nadiamoe Aug 19, 2024

The-9880 Aug 19, 2024

The-9880 Aug 19, 2024

nadiamoe Aug 20, 2024

nadiamoe Aug 19, 2024

The-9880 Aug 19, 2024

The-9880 Aug 19, 2024

nadiamoe Aug 20, 2024

The-9880 Aug 20, 2024

nadiamoe Aug 19, 2024

The-9880 Aug 20, 2024

nadiamoe Aug 19, 2024

The-9880 Aug 19, 2024

nadiamoe Aug 20, 2024

nadiamoe left a comment

nadiamoe commented Aug 20, 2024

nadiamoe left a comment

nadiamoe Aug 27, 2024

The-9880 Aug 27, 2024

The-9880 commented Aug 27, 2024

		ENV SM_CHROME_BIN=/usr/bin/chromium-browser
		ENV SM_CHROME_PATH=/usr/lib/chromium/

Add with-browser Docker image #829

Add with-browser Docker image #829

Conversation

The-9880 commented Aug 19, 2024 • edited Loading

CLAassistant commented Aug 19, 2024 • edited Loading

nadiamoe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nadiamoe left a comment

Choose a reason for hiding this comment

nadiamoe commented Aug 20, 2024

nadiamoe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

The-9880 commented Aug 27, 2024

The-9880 commented Aug 19, 2024 •

edited

Loading

CLAassistant commented Aug 19, 2024 •

edited

Loading