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

Implemented Docker and Docker Compose support #63

Closed
wants to merge 16 commits into from

Conversation

leozz37
Copy link

@leozz37 leozz37 commented Oct 3, 2020

Docker Support

Implemented support to build, running, and testing with Docker containers and docker-compose.

All instructions are included in README.md.

@leozz37
Copy link
Author

leozz37 commented Oct 3, 2020

Btw I have some plans to create a Docker workflow, and let the GCC and CMake version up to the user to decide.

Maybe create a deploy image, with a smaller size and non-root user.

Dockerfile Show resolved Hide resolved
@TheLartians
Copy link
Owner

Hey, thanks for the PR, I see how adding Docker support can simplify deployment!

However, I wonder if it's such a common use case for new C++ projects to be worth maintaining in the starter project. Another thing is that running and building in the Docker container should be tested in CI.

Also, for building, running and testing with a pre-defined environment, we already use GitHub Workflows, which can be run locally as well.

@leozz37
Copy link
Author

leozz37 commented Oct 6, 2020

Hey, I just implemented a GitHub Actions workflow for Docker, for building and running the Docker container!

A common use case for C++ is uploading an image to Docker Hub when sharing it, since setting up third-party libraries can be a headache. Also for deploying to cloud services like ECS and Cloud Run.

@omertarikkoc
Copy link

Docker support would be great!

Copy link
Owner

@TheLartians TheLartians left a comment

Choose a reason for hiding this comment

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

Hey, thanks for the updates and sorry for the delay. I've thought a bit about it and believe it would be great to have Docker support set up in the starter!

I've marked some small changes that would be great for consistency, before merging.

Also, as I feel that many of the starter users are not using Docker, an additional sentence in the readme about the advantages of Docker as well as instructions how to remove it (basically just which files to remove) would be beneficial.

If you want, you could also add Docker support to the feature list at the top of the readme.

Dockerfile Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
README.md Outdated
Comment on lines 141 to 144
### Build and Run Docker Compose

The project has support to Docker Compose, that allows building and running your project with other containers.
This container builds all targets, so you can run any of them passing a command after "sh -c".
Copy link
Owner

Choose a reason for hiding this comment

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

The two paragraphs seem to be very similar. Perhaps we can shorten it by merging them into a single one, and only illustrate the differences between using docker and docker-compose?

Copy link
Owner

Choose a reason for hiding this comment

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

Also perhaps it makes sense to move this into a separate ## Docker section below ## Usage as it is unrelated to library development itself.

Copy link
Author

Choose a reason for hiding this comment

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

Let me know if more changes are needed!

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for the update! I'm still wondering if we require both sections as they are very similar. Maybe only giving instructions for docker-compose would already be sufficient?

Copy link
Author

Choose a reason for hiding this comment

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

I think it's important to have both docker and docker-compose instructions. But we can keep them in the same section.

For example, if someone is running the modern-cpp and a Mongo container from the docker-compose, it's important to know how to execute the just modern-cpp container.

.github/workflows/docker.yml Outdated Show resolved Hide resolved
.github/workflows/docker.yml Outdated Show resolved Hide resolved
leozz37 and others added 7 commits November 11, 2020 12:24
Co-authored-by: Lars Melchior <TheLartians@users.noreply.github.com>
Co-authored-by: Lars Melchior <TheLartians@users.noreply.github.com>
Co-authored-by: Lars Melchior <TheLartians@users.noreply.github.com>
Co-authored-by: Lars Melchior <TheLartians@users.noreply.github.com>
README.md Outdated Show resolved Hide resolved
@TheLartians
Copy link
Owner

Also we should add

- Built-in [Docker support](#docker)

to the # Features section.

Comment on lines +16 to +18
# Copying files
COPY . /app
WORKDIR /app
Copy link
Owner

Choose a reason for hiding this comment

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

We should probably explicitly specify which files and directories to copy to avoid copying unwanted files or previous build artefacts to the container.

Copy link
Author

Choose a reason for hiding this comment

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

Implemented .dockerignore file.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for the updates! Tbh I still prefer explicitly copying the necessary directories and files as opposed to blacklisting.

  • The .dockerignore does not contain any build directories or other non-relevant files that users might create before running docker manually
  • Adding the .dockerignore creates yet another root level file to the starter and must be maintained
  • Explicitly listing files ensures that only relevant files get copied and the CI will complain if we forgot something

Alternatively, we could copy only files tracked by git for a clean container state.

@TheLartians
Copy link
Owner

I wonder if it would make sense to copy / install the executables and remove the source and build directories after building in the container? Especially if there are a lot of external dependencies this could dramatically reduce the size of the final image. Or am I missing the use-case for Docker here?

.gitignore
codecov.yaml
LICENSE
README.md
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing EOF newline! 😱

@leozz37
Copy link
Author

leozz37 commented Nov 22, 2020

I wonder if it would make sense to copy / install the executables and remove the source and build directories after building in the container? Especially if there are a lot of external dependencies this could dramatically reduce the size of the final image. Or am I missing the use-case for Docker here?

Yeah is possible and it's considered a good practice. We will lose the ability to run the unit tests from an outside command, but we can run the tests in the build phase instead.

@TheLartians
Copy link
Owner

I think it's a good idea to get the Docker template as production ready as possible, including removing build artificers and unneeded resources as it's something most users would probably want / expect from the template. Running the unit tests in the build phase also makes perfect sense to me here.

@PolarYair
Copy link

Hello, let me add that the best practices for doing a Dockerfile for a C++ Docker image would follow a multi-stage build:
The first image is used to build the binary, and the second Docker container will not contain source code and will not have any of the development tools. It should have the bare minimum that would enable running the binary.

@leozz37
Copy link
Author

leozz37 commented Nov 27, 2020

If we're using multi-staged builds, we must have static libraries linking instead of dynamic.
An alternative is what @PolarYair said, we can have two docker images: a base image with all its dependencies and one with just the binary, using the first one as the base image.

@TheLartians
Copy link
Owner

I actually don't think static linking is required. We could run cmake --build build --target install after building with a custom CMAKE_INSTALL_PREFIX directory to have CMake install all dependencies and executables into a single target directory. We could then copy the full install directory to the new image. This has the additional advantage that any executables and resources added to CMake will also be available in the target image, provided we set $PATH accordingly.

@atomheartother
Copy link

atomheartother commented May 4, 2021

Hello folks, it looks like this has been abandoned. I happen to think Docker integration would be awesome to have in the starter, would anyone mind me forking this PR and giving a shot at implementing the changes suggested above?

My main problem is as far as I can see this PR suggests no solution to the problem of how to clone git repositories from within docker, does it? At least this Dockerfile fails on my projects. Nevermind, I was thinking of cloning with ssh, cloning with https works fine.

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.

None yet

7 participants