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

Jump to bottom

Add vtadmin binary to release and Docker images #9405

Merged
merged 2 commits into from
Jan 4, 2022
Merged

Add vtadmin binary to release and Docker images #9405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
141ed42
Add vtadmin binaries to release and Docker images
Dec 16, 2021
05a04c0
add vtadmin frontend to vitess/lite image
Dec 17, 2021
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
4 changes: 2 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,14 @@ endif
install: build
# binaries
mkdir -p "$${PREFIX}/bin"
cp "$${VTROOT}/bin/"{mysqlctl,mysqlctld,vtorc,vtctld,vtctlclient,vtctldclient,vtgate,vttablet,vtworker,vtbackup} "$${PREFIX}/bin/"
cp "$${VTROOT}/bin/"{mysqlctl,mysqlctld,vtorc,vtadmin,vtctld,vtctlclient,vtctldclient,vtgate,vttablet,vtworker,vtbackup} "$${PREFIX}/bin/"

# Install local install the binaries needed to run vitess locally
# Usage: make install-local PREFIX=/path/to/install/root
install-local: build
# binaries
mkdir -p "$${PREFIX}/bin"
cp "$${VTROOT}/bin/"{mysqlctl,mysqlctld,vtorc,vtctl,vtctld,vtctlclient,vtctldclient,vtgate,vttablet,vtworker,vtbackup} "$${PREFIX}/bin/"
cp "$${VTROOT}/bin/"{mysqlctl,mysqlctld,vtorc,vtadmin,vtctl,vtctld,vtctlclient,vtctldclient,vtgate,vttablet,vtworker,vtbackup} "$${PREFIX}/bin/"


# install copies the files needed to run test Vitess using vtcombo into the given directory tree.
Expand Down
1 change: 1 addition & 0 deletions docker/k8s/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ COPY --from=base /vt/bin/vtgate /vt/bin/
COPY --from=base /vt/bin/vttablet /vt/bin/
COPY --from=base /vt/bin/vtworker /vt/bin/
COPY --from=base /vt/bin/vtbackup /vt/bin/
COPY --from=base /vt/bin/vtadmin /vt/bin/
Copy link
Member

@deepthi deepthi Dec 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Are you using the k8s image? The standard image is vitess/lite. You are already adding vtadmin to the make install target, that gets it into all the lite images. This change should not be required.

Copy link
Member

Choose a reason for hiding this comment

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

I publish all the individual component images from the k8s image, so IMO this should stay. There is probably an additional step required to copy out the files needed for the web. This step COPY --from=base $VTROOT/web /vt/web/ is what copies them for the existing UI, so I'd expect to see something similar for vtadmin.

Copy link
Member

Choose a reason for hiding this comment

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

Once this gets merged and we come up with an appropriate Dockerfile, I'll add a new directory and vtadmin image repo on Docker Hub that approximates the existing vtctld. https://github.com/vitessio/vitess/blob/main/docker/k8s/vtctld/Dockerfile

Side note: there's an open issue for consolidating images, but for now we still need to support people using lite and the component images. #7433

Copy link
Author

Choose a reason for hiding this comment

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

@derekperkins
I have added /web/vtadmin to the lite image.

Copy link
Author

@jakon89 jakon89 Dec 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

btw there is a difference between vtadmin and orchestrator web files: orchestrator stores js/css files in final form in the repo, however, for vtadmin we copy the whole frontend project that needs to be built by npm run build - only then we get the final js/css files.

This is not a problem since in my project I use vitess/lite image as a base builder and then copy binaries/web files to final images (vtadmin-web, vtorc, vtgate, vtadmin-server).
Example of how I do it for vtorc:

ARG BASE_REGISTRY=gcr.io
ARG BASE_IMAGE=distroless/base
ARG BASE_TAG=nonroot

FROM vitess/lite:v12.0.1 AS builder

FROM ${BASE_REGISTRY}/${BASE_IMAGE}:${BASE_TAG}

COPY --from=builder vt/bin/vtorc /vtorc
COPY --from=builder vt/web/orchestrator /web/orchestrator

USER 65532
ENTRYPOINT ["/vtorc"]

HEALTHCHECK NONE

but IMO we should include the final build (js/css etc) somehow.

I see three potential ways of fixing it:

  • we copy raw web/vtadmin to docker/lite and it's up to the user of docker/lite to build the frontend. That adds 10M to the docker/lite size. This is what is done in that PR for now - https://github.com/vitessio/vitess/pull/9405/files#diff-bdcbcdaa92c26cad86d024c8d1061d14c36e20f67c5c8e577c17e3bbb196c607R54
  • we run npm run build as a part of docker/lite build and copy build/ dir into final docker/lite. That adds additional 13M to the size of docker/lite (just checked)
  • we start building separate vtadmin-web image:
    • use node Docker image as a builder
    • copy all vtadmin web files
    • run npm install
    • copy over generated js/css files to the final image (and serve them by nginx or another webserver)

cc @doeg ^

I will be happy to send a PR if we decide on the final form.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jakon89 wow, thanks so much for the thorough notes! I really really appreciate it.

Between the three, I don't have a strong opinion. I'm happy to go with whatever is most ergonomic for Vitess operators! 🤔 The third option is the closest to what we do at Slack, though that doesn't mean it's the best for everyone. @jakon89 @derekperkins do either you have a preference?

P.S. There is definitely work to be done to make the front-end build smaller; the TypeScript generated from the protobufs is particularly huge.

Copy link
Member

Choose a reason for hiding this comment

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

#3 is my preference, that for now I would put in /docker/k8s/vtadmin. You mentioned vtadmin-web, are you building separate images for the server and the web ui?

It's probably also worth building into lite per #2, but I'll leave that decision to @deepthi.

Copy link
Member

@deepthi deepthi Dec 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

If we follow the same pattern as we do for Golang binaries, we should do #2. I'm fine with that. Not sure if you want to do that as a separate PR, or include it in this one. If this PR is complete, it LGTM.

Copy link
Author

Choose a reason for hiding this comment

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

@deepthi let's merge that, I will implement building vtadmin-ui in new PR. Thanks


# copy web admin files
COPY --from=base $VTROOT/web /vt/web/
Expand Down
1 change: 1 addition & 0 deletions docker/lite/Dockerfile.mysql57
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ ENV PATH $VTROOT/bin:$PATH
# Copy artifacts from builder layer.
COPY --from=builder --chown=vitess:vitess /vt/install /vt
COPY --from=builder --chown=vitess:vitess /vt/src/vitess.io/vitess/web/orchestrator /vt/web/orchestrator
COPY --from=builder --chown=vitess:vitess /vt/src/vitess.io/vitess/web/vtadmin /vt/web/vtadmin
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why this part was not included in mysql8?


# Create mount point for actual data (e.g. MySQL data dir)
VOLUME /vt/vtdataroot
Expand Down
2 changes: 1 addition & 1 deletion tools/make-release-packages.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ mkdir -p releases

# Copy a subset of binaries from issue #5421
mkdir -p "${RELEASE_DIR}/bin"
for binary in vttestserver mysqlctl mysqlctld query_analyzer topo2topo vtaclcheck vtbackup vtbench vtclient vtcombo vtctl vtctldclient vtctlclient vtctld vtexplain vtgate vttablet vtorc vtworker vtworkerclient zk zkctl zkctld; do
for binary in vttestserver mysqlctl mysqlctld query_analyzer topo2topo vtaclcheck vtadmin vtbackup vtbench vtclient vtcombo vtctl vtctldclient vtctlclient vtctld vtexplain vtgate vttablet vtorc vtworker vtworkerclient zk zkctl zkctld; do
cp "bin/$binary" "${RELEASE_DIR}/bin/"
done;

Expand Down