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.

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

Conversation

akudiyar
Copy link
Collaborator

@akudiyar akudiyar commented Aug 4, 2023

In this patch:

  • propagate log directory to cartridge CLI so the logs are not created in the local directory
  • fix problems with default Dockerfile on RedHat-like systems

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

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).

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

@ArtDu ArtDu Aug 18, 2023

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.

@dkasimovskiy dkasimovskiy merged commit 3c7163d into master Aug 18, 2023
@dkasimovskiy dkasimovskiy deleted the log-dirctory-in-image branch August 18, 2023 13:31
@ArtDu ArtDu mentioned this pull request Aug 22, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants