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.

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 docker multistage build #150

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

MichaelUnknown
Copy link

When playing around with docker in another project, I wanted to to see what some best practices for using C++ and docker might look like and checked out this repo. Unfortunately, #63 seems to have died quite some time ago.

This is more a discussion starter than a complete PR, but comes with my naive approach on how a multi-stage docker build could look like - assuming the greeter standalone executable would be our 'web-application' or 'server'.

Some remarks.

  • I placed the dockerfile in subdirectory and explicitly typed out, which files to copy in the docker image. This helps to keep the root directory clean. However, I feel that having the Dockerfile in the root directory (along with a .dockerignore file) and justing using COPY . . would be more straight forward.
  • I used ubuntu:20.04 because it's a nice compromise between ease of use and size (assuming we want to deploy an actual application). Something like gcc:11.2 would be easier to set up but a lot heavier (78mb vs 1.1gb)
  • I am not sure, what would be the best way (and place) to run the tests in docker. We could run them on the builder image itself or make a different image (as used here). Running the tests with docker itself also feels somewhat redundant because we basically do the same in the GitHub CI actions. But then again, if it was some tiny trimmed-for-size docker image, tests on that platform might be essential.

Feedback is greatly appreciated

@TheLartians
Copy link
Owner

Hey, thanks for the PR! Nice approach using the multi-stage build, that looks like a clean approach to separating the development and production environment in a single Dockerfile.

I placed the dockerfile in subdirectory and explicitly typed out, which files to copy in the docker image. This helps to keep the root directory clean. However, I feel that having the Dockerfile in the root directory (along with a .dockerignore file) and justing using COPY . . would be more straight forward.

While I like the clean separation, I agree that placing the files into the project root would seem cleaner. Also this seems to be much more commonplace from what I've seen.

I am not sure, what would be the best way (and place) to run the tests in docker. We could run them on the builder image itself or make a different image (as used here). Running the tests with docker itself also feels somewhat redundant because we basically do the same in the GitHub CI actions. But then again, if it was some tiny trimmed-for-size docker image, tests on that platform might be essential.

I'm also undecided on this. On one hand, I feel that tests are usually designed to be run on the development environment, so running the tests in the build docker image seems natural. On the other hand it definitely seems like a good idea to run the tests in the same environment as production to ensure that everything still works there.

Finally, I wonder what your thoughts on adding Docker to the starter is. From my personal experience, I feel that Docker is still not very common in the C++ development lifecycle (besides CI) and is often added at a more mature project stage. If Docker would be part of the starter, the Dockerfile would also have to be maintained alongside the project (or removed), adding a bit more overhead for using the starter.

As a compromise, how would you feel about dropping the CI tests for the Dockerfile and instead adding usage instructions to the Readme? I think that way Docker would feel more like an optional feature instead of an essential part of every C++ project.

@MichaelUnknown
Copy link
Author

Finally, I wonder what your thoughts on adding Docker to the starter is. From my personal experience, I feel that Docker is still not very common in the C++ development lifecycle (besides CI) and is often added at a more mature project stage. If Docker would be part of the starter, the Dockerfile would also have to be maintained alongside the project (or removed), adding a bit more overhead for using the starter.

As a compromise, how would you feel about dropping the CI tests for the Dockerfile and instead adding usage instructions to the Readme? I think that way Docker would feel more like an optional feature instead of an essential part of every C++ project.

If we categorize all C++ projects in the three arbitrary categories libraries, client- and server-applications, only the latter has actual practical use for Docker. So one idea could be to change the Standalone application to a simple Hello World REST Service (with some minimal/lightweight framework, e.g., oatpp or Crow). The Dockerfile could then be located in the sub-directory and would not be part of the library / main repository. Some instructions for Docker could then provided in a standalone.md which is also located in the corresponding sub-directory (keeping the main directory and the readme.md clean).
Regarding the second question - after thinking a bit, I like the option 'run tests only locally' (and in the GitHub CI, but and not in the Docker image) the most. This should be sufficient for most people, and, if tests in the Docker image are indeed needed, then the user can easily add them later manually.

@MichaelUnknown
Copy link
Author

I updated the everything and also extracted the Docker CI/CD part into a separate repository.

The CI/CD pipeline features:

  • Multi-stage Docker image build (as discussed above)
  • Multi-platform support with Buildx and QEMU
  • CI workflow runs on (branch) push
    • Uses GitHub actions to cache Docker images
  • CD run on 'release' (push with new tag)
    • CD workflow can be manually started via workflow_dispatch (Button in Workflow/Action section)

The setup is quite easy and requires only the DockerHub username and an access_token as GitHub secrets.

I like the example code (Crow) for the fact, that it's small and easy to understand. The downside is that it depends on Boost. I think it is no problem with Docker - we will use a linux distro almost always and so can just install it as a system lib - but if you want to build the example on Windows, the CPMAddPackage takes quite a while (unless you have it cached already). This might be not optimal for a starter project - so if anyone knows a similar web framework with a nice API, it might be worth to change out the example code.

I removed the tests from the Docker images but kept the local build on the build image. This seems to be what the official Docker documentation recommends when building a dockerized Go application.

Again, feedback on the changes is greatly appreciated.

@MichaelUnknown
Copy link
Author

Is there an interest to merge this? With the CrowCpp example or without?

@TheLartians
Copy link
Owner

TheLartians commented May 14, 2022

Is there an interest to merge this? With the CrowCpp example or without?

Hey, thanks for the changes! Tbh I'm a bit hesitant to merge this, as it adapts the starter for a very specific use-case and library. As the starter is intended for both simple and advanced projects, I feel such a change could make it unattractive for some users. As a compromise, how would you feel about linking to your fork from the readme for those interested in Docker / CrowCpp use?

Comment on lines 15 to +19
include(../cmake/CPM.cmake)

# Crow needs Boost 1.64 and does not provide CPM.cmake integration itself, so we have to get Boost
# first
find_package(Boost 1.64 QUIET)
Copy link
Contributor

Choose a reason for hiding this comment

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

The Dockerfile at this level does not work!
We need access to ../cmake and ../CMakeLists.txt while build standalone

Comment on lines +48 to 49
# get the Greeter lib
CPMAddPackage(NAME Greeter SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/..)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is something like add_subdirectory(.. Greeter)

libboost-all-dev \
;
COPY . .
RUN cmake -S standalone -B build -G Ninja -D CMAKE_BUILD_TYPE=Release
Copy link
Contributor

Choose a reason for hiding this comment

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

configure error here!

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