Skip to content

Comments

various cleanup#59

Merged
rgarcia merged 1 commit intomainfrom
small-fixes
Aug 25, 2025
Merged

various cleanup#59
rgarcia merged 1 commit intomainfrom
small-fixes

Conversation

@rgarcia
Copy link
Contributor

@rgarcia rgarcia commented Aug 21, 2025

  • consistent use of WITHDOCKER vs. WITH_DOCKER. set it in both Dockerfiles and then never worry about it again
  • no longer need to run chromium as root in the headful image! Not sure what changed but it seems to be working. Will retain the option to run as root for now
  • got rid of WITH_KERNEL_IMAGES_API since both now require it to run the 9222 cdp proxy so it's unlikely we'd ever turn it off completely

TL;DR

Standardized the WITH_DOCKER environment variable to WITHDOCKER for consistent usage across the codebase.

Why we made these changes

To eliminate naming inconsistencies for the Docker environment variable, simplifying its declaration and usage.

What changed?

  • images/chromium-headless/image/Dockerfile: Explicitly set WITHDOCKER=true; removed WITH_KERNEL_IMAGES_API.
  • images/chromium-headless/image/wrapper.sh: Updated conditional logic to use the WITHDOCKER variable.
  • server/e2e/e2e_chromium_test.go: Removed the now-obsolete WITH_DOCKER environment variable from test configurations.

Description generated by Mesa. Update settings

@rgarcia rgarcia requested a review from Sayan- August 21, 2025 14:07
Copy link

@mesa-dot-dev mesa-dot-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Performed full review of 7429755...fae0f3c

3 files reviewed | 0 comments | Review on Mesa | Edit Reviewer Settings

@rgarcia rgarcia changed the title consistent use of WITHDOCKER various cleanup Aug 22, 2025
cursor[bot]

This comment was marked as outdated.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Dockerfile Configuration Mismatch Causes Errors

The headful Dockerfile is missing ENV WITHDOCKER=true, unlike the headless image. This causes the wrapper.sh script to incorrectly attempt to mount /dev/shm and enable scale-to-zero functionality, which are operations intended for non-Docker environments. This can lead to /dev/shm mounting errors and unintended scale-to-zero behavior.

images/chromium-headful/Dockerfile#L169-L183

https://github.com/onkernel/kernel-images/blob/ef877babfa4ef95c95666cce9d41c800b22da3d5/images/chromium-headful/Dockerfile#L169-L183

images/chromium-headful/run-docker.sh#L30-L44

https://github.com/onkernel/kernel-images/blob/ef877babfa4ef95c95666cce9d41c800b22da3d5/images/chromium-headful/run-docker.sh#L30-L44

Fix in Cursor Fix in Web


@rgarcia
Copy link
Contributor Author

rgarcia commented Aug 22, 2025

Bug: Dockerfile Configuration Mismatch Causes Errors

The headful Dockerfile is missing ENV WITHDOCKER=true, unlike the headless image. This causes the wrapper.sh script to incorrectly attempt to mount /dev/shm and enable scale-to-zero functionality, which are operations intended for non-Docker environments. This can lead to /dev/shm mounting errors and unintended scale-to-zero behavior.

images/chromium-headful/Dockerfile#L169-L183

images/chromium-headful/run-docker.sh#L30-L44

Fix in Cursor Fix in Web

Look again lil cursor bro, it's there in the Dockerfile

@rgarcia rgarcia merged commit 59d3394 into main Aug 25, 2025
4 checks passed
@rgarcia rgarcia deleted the small-fixes branch August 25, 2025 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants