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

Improve PostgreSQL and minio container builds #4696

Merged
merged 16 commits into from
Oct 2, 2023

Conversation

samdoran
Copy link
Contributor

Description

Make the UID and GID of the postgres user in the container match those of the current user. This makes the PostgreSQL data files easy to manage from the container host. Build a PostgreSQL image with the correct UID, GID, and our entry point script baked in. The fewer bind mounts the better.

Use environment variables to configure the UID and GID of the minio container. There is an entry point script that sets up the user account, so we don't need to maintain a separate container file.

Move container files into dev/containers . This keeps the repo root clean and makes builds easier.

Testing

  1. Checkout Branch
  2. Delete containers and local data
make docker-down
make delete-db
make delete-trino
make delete-trino-data
rm -rf pg_data/*
rm -rf .trino/parquet_data/.minio.sys
  1. Start it all back up: make docker-up-min
  2. Check that things came up ok make docker-logs
  3. Load some data make load-test-customer-data

Notes

I would like to move all the dev container related files and data to dev/containers.

docker-compose.yml Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

Merging #4696 (6ab5646) into main (370e590) will increase coverage by 0.0%.
The diff coverage is n/a.

❗ Current head 6ab5646 differs from pull request most recent head f3ac753. Consider uploading reports for the commit f3ac753 to get more accurate results

@@          Coverage Diff          @@
##            main   #4696   +/-   ##
=====================================
  Coverage   93.8%   93.8%           
=====================================
  Files        367     367           
  Lines      30275   30229   -46     
  Branches    3580    3575    -5     
=====================================
- Hits       28383   28351   -32     
+ Misses      1226    1219    -7     
+ Partials     666     659    -7     

lcouzens
lcouzens previously approved these changes Sep 27, 2023
samdoran and others added 14 commits September 28, 2023 15:58
- Leverage entrypoint script in minio image to set UID and GID
- Change UID and GID of postgres account to match current user
- Move container files into directories to unclutter the project root
- Both USER_ID and GROUP_ID are required environment variables
- Update the .env.example file with new required variables
Trino runs as root inside the container, so it is not possible to
recursively change owenership of the trino data files. Leave that
alone for now.

Only set ownership and mode of the directories that are bind mounts
and not their contents.

Remove a minio build in the make target setting permissions. That
seems to have been added to that target by mistake.
- Use mc anonymous command since mc policy is deprecated
- Remove koku- prefix from service name
Use the correct environment variables used be the entry point script
to create the user account
Docker only allows variable substitution in .env files.
This ensures containers are named consistently. This can be overridden by
setting COMPOSE_PROJECT_NAME.
@samdoran samdoran marked this pull request as ready for review September 29, 2023 15:18
@samdoran
Copy link
Contributor Author

One other thing I would like to do is unify the service and container names. All containers would have a koku- prefix and use dashes instead of underscores. Service names would not have the koku- prefix. This is a bit disruptive because it requires updating any values in the .env file locally.

Here is what the containers look like currently:

trino
hive-metastore
koku-koku-worker-1
koku_server
masu_server
koku-koku-base-1
koku-koku-create-parquet-bucket-1
kokuminio
koku_redis
koku-unleash
koku_db

This is what I would like to change them to:

koku-trino
koku-hive-metastore
koku-worker-1
koku-masu-server
koku-server
koku-create-parquet-bucket
koku-base
koku-minio
koku-redis
koku-unleash
koku-db

I can do that in a separate PR if it's less disruptive.

@sonarcloud
Copy link

sonarcloud bot commented Oct 2, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@samdoran samdoran merged commit 8e9c1cf into main Oct 2, 2023
11 checks passed
@samdoran samdoran deleted the dev/more-docker-mount-fixes branch October 2, 2023 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants