Skip to content

Propagate log directory to cartridge-cli command #83

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

Merged
merged 1 commit into from
Aug 18, 2023
Merged

Propagate log directory to cartridge-cli command #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 5 additions & 1 deletion src/main/java/org/testcontainers/containers/TarantoolCartridgeContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,10 @@ public class TarantoolCartridgeContainer extends GenericContainer<TarantoolCartr
private static final String ENV_TARANTOOL_SERVER_GID = "TARANTOOL_SERVER_GID";
private static final String ENV_TARANTOOL_WORKDIR = "TARANTOOL_WORKDIR";
private static final String ENV_TARANTOOL_RUNDIR = "TARANTOOL_RUNDIR";
private static final String ENV_TARANTOOL_LOGDIR = "TARANTOOL_LOGDIR";
private static final String ENV_TARANTOOL_DATADIR = "TARANTOOL_DATADIR";
private static final String ENV_TARANTOOL_INSTANCES_FILE = "TARANTOOL_INSTANCES_FILE";
private static final String ENV_TARANTOOL_CLUSTER_COOKIE = "TARANTOOL_CLUSTER_COOKIE";

private final CartridgeConfigParser instanceFileParser;
private final TarantoolContainerClientHelper clientHelper;
Expand Down Expand Up @@ -239,8 +241,10 @@ private static Map<String, String> mergeBuildArguments(Map<String, String> build
ENV_TARANTOOL_SERVER_GID,
ENV_TARANTOOL_WORKDIR,
ENV_TARANTOOL_RUNDIR,
ENV_TARANTOOL_LOGDIR,
ENV_TARANTOOL_DATADIR,
ENV_TARANTOOL_INSTANCES_FILE
ENV_TARANTOOL_INSTANCES_FILE,
ENV_TARANTOOL_CLUSTER_COOKIE
)) {
String variableValue = System.getenv(envVariable);
if (variableValue != null && !args.containsKey(envVariable)) {
Expand Down
29 changes: 19 additions & 10 deletions src/main/resources/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
ARG TARANTOOL_VERSION=2.10.5
ARG TARANTOOL_VERSION=2.11.0
FROM tarantool/tarantool:${TARANTOOL_VERSION}-centos7 AS cartridge-base

# system preparations because docker mount directory as a root
Expand All @@ -8,19 +8,28 @@ USER $TARANTOOL_SERVER_USER:$TARANTOOL_SERVER_GROUP
RUN groupadd $TARANTOOL_SERVER_GROUP && useradd -m -s /bin/bash $TARANTOOL_SERVER_USER || true

# install dependencies
RUN yum -y install cmake make gcc gcc-c++ git unzip cartridge-cli && \
# a yum bug requires setting ulimit, see https://bugzilla.redhat.com/show_bug.cgi?id=1537564
RUN ulimit -n 1024 && \
yum -y install cmake make gcc gcc-c++ git unzip cartridge-cli && \
yum clean all
RUN cartridge version

# build
# build and run
FROM cartridge-base AS cartridge-app
ARG CARTRIDGE_SRC_DIR="cartridge"
ARG TARANTOOL_WORKDIR="/app"
ARG TARANTOOL_RUNDIR="/tmp/run"
ARG TARANTOOL_DATADIR="/tmp/data"
ARG TARANTOOL_LOGDIR="/tmp/log"
ARG TARANTOOL_INSTANCES_FILE="./instances.yml"
ARG TARANTOOL_CLUSTER_COOKIE="testapp-cluster-cookie"
ENV TARANTOOL_WORKDIR=$TARANTOOL_WORKDIR
ENV TARANTOOL_RUNDIR=$TARANTOOL_RUNDIR
ENV TARANTOOL_DATADIR=$TARANTOOL_DATADIR
ENV TARANTOOL_LOGDIR=$TARANTOOL_LOGDIR
ENV TARANTOOL_INSTANCES_FILE=$TARANTOOL_INSTANCES_FILE
ENV TARANTOOL_CLUSTER_COOKIE=$TARANTOOL_CLUSTER_COOKIE
COPY $CARTRIDGE_SRC_DIR $TARANTOOL_WORKDIR
WORKDIR $TARANTOOL_WORKDIR
RUN cartridge build --verbose

ENV TARANTOOL_RUNDIR="/tmp/run"
ENV TARANTOOL_DATADIR="/tmp/data"
ENV TARANTOOL_INSTANCES_FILE="./instances.yml"

CMD cartridge build && cartridge start --run-dir=$TARANTOOL_RUNDIR --data-dir=$TARANTOOL_DATADIR --cfg=$TARANTOOL_INSTANCES_FILE
CMD cartridge build && cartridge start --run-dir=$TARANTOOL_RUNDIR --data-dir=$TARANTOOL_DATADIR \
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it better to install rocks in an image so that the container of this image can be started immediately?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't completely get what you suggest. The rocks will be installed prior to launching the application according to CMD statement. If the contents of CARTRIDGE_SRC_DIR are not changed, the container will run almost immediately because the cartridge build command will just do nothing and return quickly.

Copy link
Contributor

@ArtDu ArtDu Aug 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I realized that my previous code isn't working as I wanted it.

We specify CARTRIDGE_SRC_DIR in an image assembling phase. So I wanted to build the application(install rocks) in the assembling phase.
But it doesn't work now, because we use COPY(not ADD) to copy files from the directory to a container.

In my conception, it would be great if we can collect rocks in an image-building phase. Then check whether nothing changed on the container starting after an image has been built.

Your changes don't change behavior so that you can merge them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe that building the rocks on image building phase is not completely correct since the users can further change the Cartridge app dependencies. That should not lead to rebuilding the image, since only rocks will need to be rebuilt, and this is faster than the whole image rebuild.

Copy link
Contributor

Choose a reason for hiding this comment

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

I realized that my previous code isn't working as I wanted it.

We specify CARTRIDGE_SRC_DIR in an image assembling phase. So I wanted to build the application(install rocks) in the assembling phase. But it doesn't work now, because we use COPY(not ADD) to copy files from the directory to a container.

In my conception, it would be great if we can collect rocks in an image-building phase. Then check whether nothing changed on the container starting after an image has been built.

Your changes don't change behavior so that you can merge them.

I was wrong. Copy work during image building

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that building the rocks on image building phase is not completely correct since the users can further change the Cartridge app dependencies. That should not lead to rebuilding the image, since only rocks will need to be rebuilt, and this is faster than the whole image rebuild.

And that's why I left 2 lines with cartridge build. The first one in RUN command (during image building). The second one in CMD command(on container starting)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can add one more stage, making COPY + RUN a separate layer that will be rebuilt without all the deps.
Also as an idea of future improvement, we may also specify the deps to be installed by yum in an external file so that we would be able automatically rebuild the whole image too in case if we want an external dependency for our Cartridge app (like a newer SSL library or whatever).

--log-dir=$TARANTOOL_LOGDIR --cfg=$TARANTOOL_INSTANCES_FILE