Skip to content

Conversation

@WaterWhisperer
Copy link
Contributor

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

#2115

What's changed and what's your intention?

support building option to build images base on distroless image and fix some warning in existing Dockerfile to remove noise

PR Checklist

Please convert it to a draft if some of the following conditions are not met.

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR requires documentation updates.
  • API changes are backward compatible.
  • Schema or data changes are backward compatible.

@killme2008
Copy link
Contributor

Thanks.

@sunng87 @lyang24 Please take a look.

@killme2008 killme2008 requested a review from Copilot November 18, 2025 00:21
Copilot finished reviewing on behalf of killme2008 November 18, 2025 00:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for building GreptimeDB Docker images based on distroless images for enhanced security and smaller image size, while also fixing Docker best practice warnings in existing Dockerfiles.

  • Adds two new distroless Dockerfile variants (ci and buildx)
  • Standardizes ENV variable syntax across all Dockerfiles (removes spaces around =)
  • Capitalizes AS keyword in multi-stage builds to follow Docker best practices

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
docker/ci/distroless/Dockerfile New distroless-based image for CI builds
docker/buildx/distroless/Dockerfile New distroless-based image with build-from-source support
docker/ci/ubuntu/Dockerfile Fixed ENV syntax warning
docker/ci/centos/Dockerfile Fixed ENV syntax warning
docker/buildx/ubuntu/Dockerfile Fixed ENV syntax and capitalized AS keyword
docker/buildx/centos/Dockerfile Fixed ENV syntax and capitalized AS keyword

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Collaborator

@lyang24 lyang24 left a comment

Choose a reason for hiding this comment

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

@sunng87
Copy link
Member

sunng87 commented Nov 20, 2025

@WaterWhisperer Just one more concern with the duplicates in ubuntu version and distroless version. Do you think we should switch completely from ubuntu to distroless? Is there any impact?

@daviderli614 what's your ideas about this?

@WaterWhisperer
Copy link
Contributor Author

@WaterWhisperer Just one more concern with the duplicates in ubuntu version and distroless version. Do you think we should switch completely from ubuntu to distroless? Is there any impact?

@daviderli614 what's your ideas about this?

I'd recommend keeping both as options rather than switching completely. Ubuntu is easier debugging and Distroless is smaller, more secure.
Users can choose via BASE_IMAGE variable (ubuntu is default). No breaking changes.

@WaterWhisperer
Copy link
Contributor Author

lgtm can you also add info to the docs repo? like https://github.com/GreptimeTeam/docs/blob/9f492d9c42a47bb519cc70b5a2b1a4f3367271a7/docs/getting-started/installation/greptimedb-standalone.md?plain=1#L71

Okay, I'd like to work on #2217 when this PR doesn't need any changes

@sunng87 sunng87 enabled auto-merge November 26, 2025 05:12
@sunng87 sunng87 added this pull request to the merge queue Nov 26, 2025
Merged via the queue into GreptimeTeam:main with commit 09d1074 Nov 26, 2025
16 checks passed
discord9 pushed a commit to discord9/greptimedb that referenced this pull request Nov 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docker docs-required This change requires docs update. size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants