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 Container #471

Merged
merged 3 commits into from
Feb 18, 2020
Merged

Add Docker Container #471

merged 3 commits into from
Feb 18, 2020

Conversation

ccremer
Copy link
Contributor

@ccremer ccremer commented Feb 12, 2020

Hello! Thanks for this awesome ZFS management tool! I would like to run znapzend packaged in a (docker) container support

What this PR does

It adds

  • a Dockerfile which builds an Alpine-based container image with znapzend and ZFS in it.
  • Documentation on the its usage
  • changes to include Docker builds in the CI pipeline. The pipeline assumes now following workflow
    • Every branch builds the Perl software, runs unit test and builds the container image
    • Only the master branch pushes the built container image to Docker hub with "master" tag
    • When tagging a commit with a new version release, this tag is pushed to Docker hub.
  • removed old Perl versions in the Pipeline. In my fork, they were failing in Travis, and I'm not fluent in Perl at all to fix the errors I encountered.

The master container tag on Docker Hub is similar to latest, but I often find latest ambiguous, as it's never clear whether "latest" means "latest (dev) build" or "latest stable", or whatever. With master it's clear that it corresponds to the master branch.

Please let me know if there are more dependencies (other than ZFS, mbuffer) needed in the container runtime image.

Why this PR is needed

  • Easier distribution and run of znapzend. Just docker run ... and the tool is installed in seconds.
  • Easier versioning and rollbacks or uninstallations.
  • There are a few Docker images of znapzend already available, but they are often outdated (including mine, but it's for testing this pipeline change). With the integration into this repository, Docker builds become part of the development and thus maintained.

Currently, the container needs to run privileged. I haven't invested the time to investigate why or if it can run unprivileged. The configuration store in ZFS properties comes handy, it removes the need to mount a config file.

What else needs to be done

  • Create and configure a Docker Hub repository "oetiker/znapzend". You already seem to have a docker Hub user, but not the repository.
  • Store the Docker Hub credentials in Travis environment variables in the settings, otherwise Docker push will fail.

@ccremer ccremer changed the title Container Add Docker Container Feb 12, 2020
@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.649% when pulling 338527c on ccremer:container into 62b1079 on oetiker:master.

@ccremer ccremer requested a review from oetiker February 12, 2020 00:53
Copy link
Owner

@oetiker oetiker left a comment

Choose a reason for hiding this comment

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

how about pushing the docker image to the new github package repo and switching to using github actions for building the whole thing while at it ?

running the automated tests with the docker image would be a good thing to veryfy that it is working ...


perl:
Copy link
Owner

Choose a reason for hiding this comment

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

not testing on different perl versions anymore ... a particular reason for this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Older perls were failing in Travis with an error, like this one: https://travis-ci.com/ccremer/znapzend/jobs/285685206. I cannot decipher that.

Most likely the reason is a misconfiguration in travis (config).

@ccremer
Copy link
Contributor Author

ccremer commented Feb 12, 2020

how about pushing the docker image to the new github package repo and switching to using github actions for building the whole thing while at it ?

Switching to Github Actions is also my preferred solution, it's faster, better documented and more consistent. I wasn't sure how attached you were to Travis.

I have tried Github package repo once, I recall some limitation but I need to check what that was... let me try again.

running the automated tests with the docker image would be a good thing to veryfy that it is working ...

We could introduce another build stage after the runtime stage (FROM builder as test) and run the unit tests in the buster image. For master branch and tagged releases we can then only build the runtime image with the docker build -t ... --target runtime command.

Better integration, better documented
@ccremer
Copy link
Contributor Author

ccremer commented Feb 13, 2020

Now I know the limitation:

$ docker pull docker.pkg.github.com/ccremer/znapzend/znapzend:master
Error response from daemon: Get https://docker.pkg.github.com/v2/ccremer/znapzend/znapzend/manifests/master: no basic auth credentials

It's not publicly available without a login :( Also, apparently the downloads costs: https://github.com/settings/billing. So I still recommend Docker Hub.

I have now migrated to GitHub actions and also included the tests in the Docker build. Re-added the matrix build with the different Perl versions, but I know that 5.16 definitely fails, so I left that out.

I'm also not sure anymore if Coveralls becomes broken, I had to create a .coveralls.yml in the CI job. Now it passes with a warning, but hopefully that's just because I haven't set up coveralls for this forked repo.

@oetiker
Copy link
Owner

oetiker commented Feb 13, 2020

cool!

@oetiker
Copy link
Owner

oetiker commented Feb 13, 2020

guess I'll have to merge to see if it wall works :) give me a few days

@oetiker oetiker merged commit d2f0f5b into oetiker:master Feb 18, 2020
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