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.

Sign up for GitHub

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

feat: docker-compose to work off repo Dockerfile #27434

Merged
merged 4 commits into from
Mar 11, 2024
Merged

feat: docker-compose to work off repo Dockerfile #27434

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
39a2b21
feat: docker-compose to work off repo Dockerfile
mistercrunch Feb 27, 2024
ccdc409
more tweaks + docs
mistercrunch Mar 9, 2024
5cf9f23
more tweaks + docs
mistercrunch Mar 9, 2024
08b7f98
removing env-non-dev
mistercrunch Mar 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions docker-compose-non-dev.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
x-superset-image: &superset-image apachesuperset.docker.scarf.sh/apache/superset:${TAG:-latest}
Copy link
Member

Choose a reason for hiding this comment

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

I personally think it's kind of cool to have non-dev point to a pre built image TAG, also this docker-compose does not mount current code into the container like docker-compose.yaml does, so non deterministic cases probably do not apply on this case.

Copy link
Member Author

Choose a reason for hiding this comment

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

About this, I think both use cases are valid. To me if I'm in a repo and on a specific ref (a branch, a release tag, or my own little branch with a feature), and I run some docker-related thing (whether it's docker build or a docker-compose related thing) I'm assuming that what I'm building is the particular ref I'm into right now.

I think the 2 options I want to provide here are really just "interactive" where we mount the code, and "non-interactive" where it's just immutable set of dockers that get me a fully working testable cluster that is lined up with the branch.

Now maybe we should ADD a new way to do docker-compose-any-image.yml that would work along with a TAG env var.

Copy link
Member

@dpgaspar dpgaspar Mar 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

fine by me! makes sense

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, I'm making a bunch of changes here and re-writing the docs too...

x-superset-depends-on: &superset-depends-on
- db
- redis
Expand All @@ -23,6 +22,12 @@ x-superset-volumes:
- ./docker:/app/docker
- superset_home:/app/superset_home

x-common-build: &common-build
context: .
target: dev
cache_from:
- apache/superset-cache:3.9-slim-bookworm
Copy link
Member

Choose a reason for hiding this comment

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

oh I was not aware of this, what process pushes to it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Anything that uses scripts/build_docker.py (a cli that wraps the docker build CLI) will use the cache-from, and cache-to, but can only push if it's logged in (push or pull_request against the main repo). Currently I think all the GitHub actions that build images (pull_request , push on master and releases) will use this thing and hopefully use the cache.

Docker-compose can piggy backing on this cache here that should really speed up the builds since in most case most layers can be re-used from the master builds

Copy link
Member

Choose a reason for hiding this comment

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

got it:
here: https://github.com/apache/superset/blob/master/scripts/build_docker.py#L27
and here: https://github.com/apache/superset/blob/master/scripts/build_docker.py#L193

pull_request will only push to cache when not in forks

Copy link
Member Author

@mistercrunch mistercrunch Mar 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

side note - one thing I noticed is cache doesn't always seem to hit when I think it should, I'm guessing that we have some limits / intelligent cache pruning that's preventing cache hit from always working .... cache hit rate is still pretty decent, and build times not awful either when missing the cache


version: "3.7"
services:
redis:
Expand All @@ -43,7 +48,8 @@ services:

superset:
env_file: docker/.env-non-dev
image: *superset-image
build:
<<: *common-build
container_name: superset_app
command: ["/app/docker/docker-bootstrap.sh", "app-gunicorn"]
user: "root"
Expand All @@ -54,8 +60,9 @@ services:
volumes: *superset-volumes

superset-init:
image: *superset-image
container_name: superset_init
build:
<<: *common-build
command: ["/app/docker/docker-init.sh"]
env_file: docker/.env-non-dev
depends_on: *superset-depends-on
Expand All @@ -65,7 +72,8 @@ services:
disable: true

superset-worker:
image: *superset-image
build:
<<: *common-build
container_name: superset_worker
command: ["/app/docker/docker-bootstrap.sh", "worker"]
env_file: docker/.env-non-dev
Expand All @@ -81,7 +89,8 @@ services:
]

superset-worker-beat:
image: *superset-image
build:
<<: *common-build
container_name: superset_worker_beat
command: ["/app/docker/docker-bootstrap.sh", "beat"]
env_file: docker/.env-non-dev
Expand Down
22 changes: 16 additions & 6 deletions docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
x-superset-image: &superset-image apachesuperset.docker.scarf.sh/apache/superset:${TAG:-master-dev}
x-superset-user: &superset-user root
x-superset-depends-on: &superset-depends-on
- db
Expand All @@ -27,6 +26,12 @@ x-superset-volumes: &superset-volumes
- superset_home:/app/superset_home
- ./tests:/app/tests

x-common-build: &common-build
context: .
target: dev
cache_from:
- apache/superset-cache:3.9-slim-bookworm

version: "3.7"
services:
nginx:
Expand Down Expand Up @@ -61,7 +66,8 @@ services:

superset:
env_file: docker/.env
image: *superset-image
build:
<<: *common-build
container_name: superset_app
command: ["/app/docker/docker-bootstrap.sh", "app"]
restart: unless-stopped
Expand Down Expand Up @@ -106,7 +112,8 @@ services:
- REDIS_SSL=false

superset-init:
image: *superset-image
build:
<<: *common-build
container_name: superset_init
command: ["/app/docker/docker-init.sh"]
env_file: docker/.env
Expand All @@ -129,7 +136,8 @@ services:
volumes: *superset-volumes

superset-worker:
image: *superset-image
build:
<<: *common-build
container_name: superset_worker
command: ["/app/docker/docker-bootstrap.sh", "worker"]
env_file: docker/.env
Expand All @@ -146,7 +154,8 @@ services:
# mem_reservation: 128M

superset-worker-beat:
image: *superset-image
build:
<<: *common-build
container_name: superset_worker_beat
command: ["/app/docker/docker-bootstrap.sh", "beat"]
env_file: docker/.env
Expand All @@ -158,7 +167,8 @@ services:
disable: true

superset-tests-worker:
image: *superset-image
build:
<<: *common-build
container_name: superset_tests_worker
command: ["/app/docker/docker-bootstrap.sh", "worker"]
env_file: docker/.env
Expand Down
Loading