Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dockerfile and co updates #2986

Closed
wants to merge 26 commits into from
Closed

Conversation

ruslandoga
Copy link
Contributor

@ruslandoga ruslandoga commented May 30, 2023

Changes

This PR explores possible changes to the current docker build setup:

  • adds .dockerignore that stops docker build from copying ~300MB into the build context (this speeds up builds on mac a bit (~10s) since macs use a vm for docker host and it seems like copying these node_modules and co into the vm is slow)
  • cuts down the image size from 172MB to 92.7MB (details in the comments)
  • keeps root as /app owner and allows plausible to read and execute stuff in /app and write to /tmp or user-set locations (like volumes, etc.) via env vars (fixes Hardcoded docker user causes problems #1404)
SOME MANUAL CHECKS HERE
$ whoami
plausibleuser
$ id -u
1000
$ id -g
1000
$ ls -l
total 20
drwxr-xr-x    2 root     root            23 May 30 08:24 bin
-rwxr-xr-x    1 root     root           129 May 30 08:24 createdb.sh
drwxr-xr-x    3 root     root            17 May 30 08:24 erts-13.1.5
-rwxr-xr-x    1 root     root           151 May 30 08:24 init-admin.sh
drwxr-xr-x    1 root     root            25 May 30 08:34 lib
-rwxr-xr-x    1 root     root           120 May 30 08:24 migrate.sh
drwxr-xr-x    3 root     root            55 May 30 08:24 releases
-rwxr-xr-x    1 root     root            95 May 30 08:24 rollback.sh
-rwxr-xr-x    1 root     root            91 May 30 08:24 seed.sh
$ ls /tmp
plausible_tzdata_data
$ stat /tmp/plausible_tzdata_data/
  File: /tmp/plausible_tzdata_data/
  Size: 76        	Blocks: 0          IO Block: 4096   directory
Device: 28h/40d	Inode: 4918127     Links: 4
Access: (0755/drwxr-xr-x)  Uid: ( 1000/plausibleuser)   Gid: ( 1000/plausibleuser)
Access: 2023-05-30 08:34:31.443933171 +0000
Modify: 2023-05-30 08:34:36.623984606 +0000
Change: 2023-05-30 08:34:36.623984606 +0000
iex(plausible@08f1d6cd7be0)3> :locus.get_info(:geolocation)
{:ok,
 %{
   metadata: %{
     binary_format_version: {2, 0},
     build_epoch: 1685109884,
     database_type: "GeoLite2-Country",
     description: %{"en" => "GeoLite2 Country database"},
     ip_version: 6,
     languages: ["de", "en", "es", "fr", "ja", "pt-BR", "ru", "zh-CN"],
     node_count: 965719,
     record_size: 24
   },
   source: {:remote, {:maxmind, :"GeoLite2-Country"}},
   version: {{2023, 5, 26}, {14, 4, 44}}
 }}

And after adding user: 1010:1010 to plausible in docker-compose.yml:

$ id
uid=1010 gid=1010
$ whoami
whoami: unknown uid 1010
$ ls -l
total 20
drwxr-xr-x    2 root     root            23 May 30 08:24 bin
-rwxr-xr-x    1 root     root           129 May 30 08:24 createdb.sh
drwxr-xr-x    3 root     root            17 May 30 08:24 erts-13.1.5
-rwxr-xr-x    1 root     root           151 May 30 08:24 init-admin.sh
drwxr-xr-x    1 root     root            25 May 30 10:22 lib
-rwxr-xr-x    1 root     root           120 May 30 08:24 migrate.sh
drwxr-xr-x    3 root     root            55 May 30 08:24 releases
-rwxr-xr-x    1 root     root            95 May 30 08:24 rollback.sh
-rwxr-xr-x    1 root     root            91 May 30 08:24 seed.sh
$ stat /tmp/plausible_tzdata_data/
  File: /tmp/plausible_tzdata_data/
  Size: 76        	Blocks: 0          IO Block: 4096   directory
Device: 4ch/76d	Inode: 1693681     Links: 4
Access: (0755/drwxr-xr-x)  Uid: ( 1010/ UNKNOWN)   Gid: ( 1010/ UNKNOWN)
Access: 2023-05-30 10:23:04.844287792 +0000
Modify: 2023-05-30 10:23:10.236341029 +0000
Change: 2023-05-30 10:23:10.236341029 +0000

Tests

Changelog

  • Entry has been added to changelog

Documentation

  • Docs have been updated

Dark mode

  • This PR does not change the UI

@bundlemon
Copy link

bundlemon bot commented May 30, 2023

BundleMon

Unchanged files (7)
Status Path Size Limits
static/js/dashboard.js
318.9KB -
static/js/app.js
46.68KB -
static/css/app.css
15.04KB -
static/js/embed.host.js
5.59KB -
static/js/embed.content.js
5.06KB -
tracker/js/plausible.js
740B -
static/js/applyTheme.js
313B -

No change in files bundle size

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@ruslandoga ruslandoga requested review from cnkk and a team May 30, 2023 03:40
Dockerfile Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@@ -7,6 +7,7 @@ if config_env() in [:dev, :test] do
end

config_dir = System.get_env("CONFIG_DIR", "/run/secrets")
storage_dir = get_var_from_path_or_env(config_dir, "STORAGE_DIR", System.tmp_dir!())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To make the above work we need to move all writing outside of /app, for example to /tmp.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me.
After implementing this, we could use tmpfs mounts when running the container to prevent writing data into the container's writable layer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The two dependencies that write to /app/lib/.../<dependency>/priv do it for caching reasons:

  • tzdata caches timezone info
  • locus caches geolocation databases

So I think it might make sense to actually persist this data across restarts.

config/runtime.exs Outdated Show resolved Hide resolved
config/runtime.exs Outdated Show resolved Hide resolved
@ruslandoga ruslandoga marked this pull request as ready for review May 30, 2023 10:28
@ruslandoga ruslandoga requested a review from ukutaht May 30, 2023 10:37
Dockerfile Outdated Show resolved Hide resolved
@cnkk
Copy link
Member

cnkk commented May 31, 2023

Thanks! Looks good in general.

For deployment, we should start with deploying to staging and test there first.

Dockerfile Outdated Show resolved Hide resolved
@cnkk
Copy link
Member

cnkk commented Feb 13, 2024

Thanks! Looks good in general.

For deployment, we should start with deploying to staging and test there first.

@ruslandoga could you update your fork, so we can test this on staging?

config/runtime.exs Outdated Show resolved Hide resolved
@cnkk cnkk added deploy-to-staging Special label that triggers a deploy to a staging environment and removed deploy-to-staging Special label that triggers a deploy to a staging environment labels Feb 16, 2024
@ruslandoga
Copy link
Contributor Author

ruslandoga commented Feb 16, 2024

I'm going to run it for a few days on my instance and report back if anything bad happens.

ruslandoga#116

@@ -48,7 +48,7 @@ jobs:
build-args: |
MIX_ENV=small
BUILD_METADATA=${{ steps.meta.outputs.json }}
ERL_FLAGS=+JPperf true
ERL_FLAGS=+JMsingle true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it should be ready now erlang/otp#6340

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems to work, I built an arm64 image and it boots successfully on arm64 mac.

@ruslandoga
Copy link
Contributor Author

I'm going to run it for a few days on my instance and report back if anything bad happens.

Everything seems OK!

@@ -229,7 +229,7 @@ ip_geolocation_db = get_var_from_path_or_env(config_dir, "IP_GEOLOCATION_DB", ge
geonames_source_file = get_var_from_path_or_env(config_dir, "GEONAMES_SOURCE_FILE")
maxmind_license_key = get_var_from_path_or_env(config_dir, "MAXMIND_LICENSE_KEY")
maxmind_edition = get_var_from_path_or_env(config_dir, "MAXMIND_EDITION", "GeoLite2-City")
maxmind_cache_dir = get_var_from_path_or_env(config_dir, "PERSISTENT_CACHE_DIR")
persistent_cache_dir = get_var_from_path_or_env(config_dir, "PERSISTENT_CACHE_DIR")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe this could fallback to /tmp?

persistent_cache_dir = get_var_from_path_or_env(config_dir, "PERSISTENT_CACHE_DIR") || System.tmp_dir!()

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good

config :tzdata,
:data_dir,
get_var_from_path_or_env(config_dir, "STORAGE_DIR", Application.app_dir(:tzdata, "priv"))
config :tzdata, :data_dir, persistent_cache_dir || Path.join(System.tmp_dir!(), "tzdata_data")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should it be scoped to tzdata_data dir in both cases?

config :tzdata, :data_dir, Path.join(persistent_cache_dir || System.tmp_dir!(), "tzdata_data")

@ruslandoga
Copy link
Contributor Author

Closing in favor of #3811

@ruslandoga ruslandoga closed this Feb 26, 2024
@ruslandoga ruslandoga deleted the dockerfile-wip branch June 10, 2024 16:33
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.

Hardcoded docker user causes problems
2 participants