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
+289 −132
Merged

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

Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions UPDATING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ assists people when migrating to a new version.

- [26450](https://github.com/apache/superset/pull/26450): Deprecates the `KV_STORE` feature flag and its related assets such as the API endpoint and `keyvalue` table. The main dependency of this feature is the `SHARE_QUERIES_VIA_KV_STORE` feature flag which allows sharing SQL Lab queries without the necessity of saving the query. Our intention is to use the permalink feature to implement this use case before 5.0 and that's why we are deprecating the feature flag now.

- [27434](https://github.com/apache/superset/pull/27434/files): DO NOT USE our docker-compose.*
files for production use cases! While we never really supported
or should have tried to support docker-compose for production use cases, we now actively
have taken a stance against supporting it. See the PR for details.

### Breaking Changes

- [27130](https://github.com/apache/superset/pull/27130): Fixes the DELETE `/database/{id}/ssh_tunnel/`` endpoint to now correctly accept a database ID as a parameter, rather than an SSH tunnel ID.
Expand Down
101 changes: 101 additions & 0 deletions docker-compose-image-tag.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
#
Copy link
Member Author

Choose a reason for hiding this comment

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

this file is effectively the old docker-compose-non-dev.yml just renamed to be more clear

# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# 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}
x-superset-depends-on: &superset-depends-on
- db
- redis
x-superset-volumes:
&superset-volumes # /app/pythonpath_docker will be appended to the PYTHONPATH in the final container
- ./docker:/app/docker
- superset_home:/app/superset_home

version: "3.7"
services:
redis:
image: redis:7
container_name: superset_cache
restart: unless-stopped
volumes:
- redis:/data

db:
env_file: docker/.env
image: postgres:15
container_name: superset_db
restart: unless-stopped
volumes:
- db_home:/var/lib/postgresql/data
- ./docker/docker-entrypoint-initdb.d:/docker-entrypoint-initdb.d

superset:
env_file: docker/.env
image: *superset-image
container_name: superset_app
command: ["/app/docker/docker-bootstrap.sh", "app-gunicorn"]
user: "root"
restart: unless-stopped
ports:
- 8088:8088
depends_on: *superset-depends-on
volumes: *superset-volumes

superset-init:
image: *superset-image
container_name: superset_init
command: ["/app/docker/docker-init.sh"]
env_file: docker/.env
depends_on: *superset-depends-on
user: "root"
volumes: *superset-volumes
healthcheck:
disable: true

superset-worker:
image: *superset-image
container_name: superset_worker
command: ["/app/docker/docker-bootstrap.sh", "worker"]
env_file: docker/.env
restart: unless-stopped
depends_on: *superset-depends-on
user: "root"
volumes: *superset-volumes
healthcheck:
test:
[
"CMD-SHELL",
"celery -A superset.tasks.celery_app:app inspect ping -d celery@$$HOSTNAME",
]

superset-worker-beat:
image: *superset-image
container_name: superset_worker_beat
command: ["/app/docker/docker-bootstrap.sh", "beat"]
env_file: docker/.env
restart: unless-stopped
depends_on: *superset-depends-on
user: "root"
volumes: *superset-volumes
healthcheck:
disable: true

volumes:
superset_home:
external: false
db_home:
external: false
redis:
external: false
31 changes: 20 additions & 11 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,7 +22,13 @@ x-superset-volumes:
- ./docker:/app/docker
- superset_home:/app/superset_home

version: "3.7"
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: "4.0"
services:
redis:
image: redis:7
Expand All @@ -33,7 +38,7 @@ services:
- redis:/data

db:
env_file: docker/.env-non-dev
env_file: docker/.env
image: postgres:15
container_name: superset_db
restart: unless-stopped
Expand All @@ -42,8 +47,9 @@ services:
- ./docker/docker-entrypoint-initdb.d:/docker-entrypoint-initdb.d

superset:
env_file: docker/.env-non-dev
image: *superset-image
env_file: docker/.env
build:
<<: *common-build
container_name: superset_app
command: ["/app/docker/docker-bootstrap.sh", "app-gunicorn"]
user: "root"
Expand All @@ -54,21 +60,23 @@ 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
env_file: docker/.env
depends_on: *superset-depends-on
user: "root"
volumes: *superset-volumes
healthcheck:
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
env_file: docker/.env
restart: unless-stopped
depends_on: *superset-depends-on
user: "root"
Expand All @@ -81,10 +89,11 @@ 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
env_file: docker/.env
restart: unless-stopped
depends_on: *superset-depends-on
user: "root"
Expand Down
32 changes: 23 additions & 9 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,7 +26,13 @@ x-superset-volumes: &superset-volumes
- superset_home:/app/superset_home
- ./tests:/app/tests

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

version: "4.0"
services:
nginx:
image: nginx:latest
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 @@ -120,16 +127,21 @@ services:

superset-node:
image: node:16
environment:
# set this to false if you have perf issues running the npm i; npm run dev in-docker
# if you do so, you have to run this manually on the host, which should perform better!
BUILD_SUPERSET_FRONTEND_IN_DOCKER: ${BUILD_SUPERSET_FRONTEND_IN_DOCKER:-true}
SCARF_ANALYTICS: "${SCARF_ANALYTICS}"
PUPPETEER_SKIP_CHROMIUM_DOWNLOAD: ${BUILD_SUPERSET_FRONTEND_IN_DOCKER:-false}
container_name: superset_node
command: ["/app/docker/docker-frontend.sh"]
env_file: docker/.env
depends_on: *superset-depends-on
environment:
SCARF_ANALYTICS: "${SCARF_ANALYTICS}"
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 +158,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 +171,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
2 changes: 2 additions & 0 deletions docker/.env
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,5 @@ SUPERSET_LOAD_EXAMPLES=yes
CYPRESS_CONFIG=false
SUPERSET_PORT=8088
MAPBOX_API_KEY=''

SUPERSET_SECRET_KEY=TEST_NON_DEV_SECRET
53 changes: 0 additions & 53 deletions docker/.env-non-dev
View file
Open in desktop

This file was deleted.

18 changes: 12 additions & 6 deletions docker/docker-frontend.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,17 @@ set -e

# Packages needed for puppeteer:
apt update
apt install -y chromium
if [ "$PUPPETEER_SKIP_CHROMIUM_DOWNLOAD" = "false" ]; then
apt install -y chromium
fi

cd /app/superset-frontend
npm install -f --no-optional --global webpack webpack-cli
npm install -f --no-optional
if [ "$BUILD_SUPERSET_FRONTEND_IN_DOCKER" = "true" ]; then
cd /app/superset-frontend
npm install -f --no-optional --global webpack webpack-cli
npm install -f --no-optional

echo "Running frontend"
npm run dev
echo "Running frontend"
npm run dev
else
echo "Skipping frontend build steps - YOU RUN IT MANUALLY ON THE HOST!"
fi
2 changes: 1 addition & 1 deletion docs/docs/frequently-asked-questions.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ SUPERSET_WEBSERVER_TIMEOUT = 60
### Why is the map not visible in the geospatial visualization?

You need to register a free account at [Mapbox.com](https://www.mapbox.com), obtain an API key, and add it
to **.env** and **.env-non-dev** at the key MAPBOX_API_KEY:
to **.env** at the key MAPBOX_API_KEY:

```
MAPBOX_API_KEY = "longstringofalphanumer1c"
Expand Down
Loading
Loading