Skip to content

Allow custom base image#227

Merged
mosabua merged 1 commit intotrinodb:mainfrom
andythsu:docker
Feb 7, 2024
Merged

Allow custom base image#227
mosabua merged 1 commit intotrinodb:mainfrom
andythsu:docker

Conversation

@andythsu
Copy link
Copy Markdown
Member

@andythsu andythsu commented Jan 31, 2024

Setting a custom base image allows building a docker image in a siloed environment with no access to DockerHub or replacing the image with a custom version of the base image. Or both..

@cla-bot cla-bot bot added the cla-signed label Jan 31, 2024
Comment thread docker/Dockerfile Outdated
Comment thread docker/build.sh Outdated

TAG_PREFIX="trino-gateway:${TRINO_GATEWAY_VERSION}"

# Use ubi9 minimal as it's more secure
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

remove this comment .. more secure than what?

Comment thread docker/build.sh Outdated
@mosabua
Copy link
Copy Markdown
Member

mosabua commented Feb 1, 2024

Please also add this to the README in the docker folder

@andythsu andythsu force-pushed the docker branch 2 times, most recently from 4b03f30 to b32ce52 Compare February 1, 2024 01:02
Comment thread docker/README.md
Comment on lines +86 to +88
export TRINO_GATEWAY_BASE_IMAGE=<image>
./build.sh
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

build.sh already offers some arguments h,a,r,j to changer the behavior. Why do we need to use an environment variable for the base image?

Copy link
Copy Markdown
Member Author

@andythsu andythsu Feb 1, 2024

Choose a reason for hiding this comment

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

do we want to create a new argument to change the base image then? Because right now the base image is using the one on dockerhub, but we will need to use our own base image to build the docker image.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The script also uses a bunch of env variables.. so its kinda inconsistent .. I think of the trino image more like the java home path to be honest

Comment thread docker/README.md Outdated
./build.sh -r 4
```

Set the environment variable `TRINO_GATEWAY_BASE_IMAGE` to use a specific base image to build Trino Gateway image.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

80 char width wrap please

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed

Copy link
Copy Markdown
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

Looks good now.

@mosabua mosabua merged commit 4d4b825 into trinodb:main Feb 7, 2024
@github-actions github-actions bot added this to the 6 milestone Feb 7, 2024
@mosabua mosabua mentioned this pull request Feb 15, 2024
@andythsu andythsu deleted the docker branch September 18, 2024 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants