-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Add vtctldclient
container image
#16318
Add vtctldclient
container image
#16318
Conversation
Signed-off-by: Sam Wronski <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Sam Wronski <[email protected]>
1e9b048
to
fba947f
Compare
We do not have that and I don't think we necessarily need one, from the user point of view: they go to DockerHub and use the images available there. |
Sorry, I marked this as |
Updates copyright of vtctldclient dockerfile Co-authored-by: Florent Poinsard <[email protected]> Signed-off-by: Sam Wronski <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me! Thank you for this contribution 🙌🏻
$> docker build -t vitess/test-vtctldclient:latest ./docker/binaries/vtctldclient
$> docker run -it docker.io/vitess/test-vtctldclient:latest bash
root@0cddf19e73fe:/# which vtctldclient
/usr/bin/vtctldclient
root@0cddf19e73fe:/# vtctldclient --version
vtctldclient version Version: 21.0.0-SNAPSHOT (Git revision 780abdd9c4017f08c29f26c7883abd5081b525d2 branch 'main') built on Tue Jul 2 19:23:58 UTC 2024 by vitess@buildkitsandbox using go1.22.4 linux/amd64
RUN apt-get update && \ | ||
apt-get upgrade -qq && \ | ||
apt-get install jq curl -qq --no-install-recommends && \ | ||
apt-get autoremove && \ | ||
apt-get clean && \ | ||
rm -rf /var/lib/apt/lists/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming jq and curl are in here for debugging? I know they're in the other Dockerfiles, so I don't want to gate this change on it, but should we think about moving to distroless for most of these binaries? The only ones offhand I can think of that need Linux might be vttablet since it uses installs the mysql client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eventually we should think about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are mostly to align with the existing vtctlclient
image. I can remove the entire RUN
layer since I don't think any of the apt-get functionality is required for this to work if that'd be preferred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what you are proposing right now is fine. IMHO moving to distroless should eventually be done to all images in a subsequent PR so we have something consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, this PR is great as is to stay consistent with vtctlclient
Introduces the
vtctldclient
image.Description
vtctldclient
is a typed replacement ofvtctlclient
which has been deprecated since Vitess v18. Avtctlclient
image has continued to be published however and avtctldclient
image is absent from the projects published images. This introducesvtctldclient
to resolve this and so thatvtctlclient
can be removed at a later point.Related Issue(s)
Fixes #16288
Checklist
Deployment Notes
This change should not impact the broader vitess code and is isolated to the container image and the docker build processes (adds a new image). A description of the image will need to be added (the
vtctlclient
description is minimal but could be reused for this)