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

Branch-based tagging with weekly builds. #17

Conversation

lightswitch05
Copy link
Member

@lightswitch05 lightswitch05 commented Feb 29, 2020

Description

Enables smart docker tagging based on git branches and git tags. Builds are ran automatically on 'push' events. The docker tags will be based on the branch that was pushed. For example, if 'master' was pushed, then the standard tags will get created such as 'latest' and 'x86_64'. If there is a tag on HEAD, then it will also generate the version-based tags, like 'v1.1-x86_64' and 'v1.1'. The builds are also ran weekly for the master branch, pulling in any security updates from the base layers.

Other changes:

  • Improved README.md with more details about the project and links to docker hub
  • Added docker-compose.yaml for easier image building
  • Added failure flags to curl to ensure it exits on error

Github Actions

I've taken a major liberty with this pull request and introduced Github Actions. I realize that it is currently using CircleCI - so perhaps this is an unwelcome aspect of the pull request. If no one on the PiHole team wants Github Actions, then I believe the majority of this pull request would still be useful, but updates would be required. Here is my logic in using GitHub actions:

  • Github Actions live within Github. So anyone looking at the code here should be able to easily find the actions associated with the repo. Where are the Circle CI Builds? The readme says its magic. I'm not being serious, I'm sure I can find them if I spent the time to dig for them - but that's kinda my point here. Its really nice (IMO) to have the repo and the builds located in the same location.
  • Its easy to checkout a repo in Github Actions with an attached HEAD. I do not have any experience with CircleCI, so maybe that is easy too - but I know in Jenkins, CodeBuild, and TravisCI - all the builds are done with a detached head. It doesn't seem like a big deal, but its a pain to find what branch you building when HEAD is detached - since you are not on a branch. A lot of times the build system compensates for this by supplying the branch name as an environment variable, but that's still a pain since its system dependent. I'd rather just get the branch from git - so I can test it locally. /rant
  • I don't have Circle CI experience, and I wasn't in the mood to learn it. This is a terrible reason, and works the same way against Github Actions, but I'm being honest that this was a factor for me- especially given my above explanation about detached heads. If Github Actions is a blocker on this pull request, then someone else will need to figure all that out.

Motivation and Context

  • Regularly pull in updates from base layers, such as security updates
  • Laying the base work for building multiple flavors of Debian (buster & stretch). I believe having this logic in place is an essential first step before multiple flavors of Debian can be supported. In fact, if this pull request is accepted, then I intend to follow up with another pull request to implement buster builds (along side of stretch).
  • Having branch-based tagging will allow a 'dev' branch to get updates faster without affecting down-stream projects

How Has This Been Tested?

I setup the Actions on my own repo. However, I was unable to test docker push commands for obvious reasons. Also, some of the links in this pull request are essentially dead links until it gets merged into master. An example is the 'Build Status' badge added to the README.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Further Actions Needed:

If everyone is on board with Github Actions, then two variables need to be added to the repo settings, under 'Secrets':

  1. DOCKERHUB_USER
  2. DOCKERHUB_PAS

This will enable the builds to push the images. I designed the action specifically to be able to get through the entire build phase before trying to login, this way you can accept the pull request and test out the builds before giving it full push permissions.

Finally, thank you for your time reviewing this pull request. I understand that pull request review is a lot of work- especially for something that no one asked for. This is not a small change, so Thank you.

Enables smart docker tagging based on git branches and git tags. Builds are ran automatically on 'push' events. The docker tags will be based on the branch that was pushed. For example, if 'master' was pushed, then the standard tags will get created such as 'latest' and 'x86_64'. If there is a tag on HEAD, then it will also generate the version-based tags, like 'v1.1-x86_64' and 'v1.1'.  The builds are also ran weekly for the master branch, pulling in any security updates from the base layers.

Other changes:
* Improved README.md with more details about the project and links to docker hub
* Added docker-compose.yaml for easier image building
* Added failure flags to curl to ensure it exits on error

Signed-off-by: Daniel <daniel@developerdan.com>
@lightswitch05
Copy link
Member Author

lightswitch05 commented Feb 29, 2020

@lightswitch05
Copy link
Member Author

I could spend some time looking at circle-ci if github actions is a blocker for this

@dschaper
Copy link
Member

dschaper commented Mar 2, 2020

I don't think it's a blocker, we are just busy with various other things that need to be handled.

@lightswitch05
Copy link
Member Author

lightswitch05 commented Mar 2, 2020

Thanks for the input @dschaper. Another note about this pull request. I implemented the tagging logic in bash, as it seemed more in-line with this and other projects within pihole. However, the end result became more complicated then I had envisioned. If there is any interest in converting tag-and-push.sh to tag-and-push.py (python), then I would be onboard with that.

@dschaper
Copy link
Member

dschaper commented Mar 2, 2020

That would be diginc's purview but he's Py friendly and I'm fine with Py as well.

@diginc
Copy link
Contributor

diginc commented Mar 2, 2020

I'm not against the github conversion, I can't knock it until I've tried it but I do have some questions at the end, mostly related to the scalability and compatibility with other patterns implemented in docker-pi-hole's recent CircleCI conversion.

Similar to what Dan said, I've just been pre-occupied with other things but I like what I see here. It can be ran locally fairly easily with docker-compose and not too much build scripting is inside the CI job, both good. CI systems are fairly exchangeable these days - the environment variables are the main differences - as long as we're not too invested in proprietary CI features we're very portable.

My main questions about github's CI functionality are:

  • Can we make PRs run builds but not pushes or have access to docker login vars (admittedly we didn't do this even on CircleCI yet, that is yet to come)
  • Could we use parallel stages instead of docker-compose parallel? How does the speed compare?
  • Do they have workspaces/files that can be shared by different stages? That would be the big hurdle in migrating the docker-pi-hole repo to github since I used a workspace to help pass information from the individual architecture build stage to the multi-architecture stage.
  • What are the limits if any to amount of runs / cpu hours / etc?

@dschaper
Copy link
Member

dschaper commented Mar 2, 2020

I actually just got a link sent to me in email about https://help.github.com/en/actions/configuring-and-managing-workflows/caching-dependencies-to-speed-up-workflows that I don't think was available before.

@lightswitch05
Copy link
Member Author

Can we make PRs run builds but not pushes or have access to docker login vars (admittedly we didn't do this even on CircleCI yet, that is yet to come)

This is a good idea and should be possible, I'll look into it.

Could we use parallel stages instead of docker-compose parallel? How does the speed compare?

Using free github, you can have 20 concurrent jobs. If exceeded, any additional jobs are queued. So this is possible, if it is faster or not would need testing. They do have a 'Job Matrix' concept which can be used to setup many similar concurrent jobs.

One aspect I like about about doing them all together is that if one fails - they all fail. There is no risk of getting a newer build of x86_64 but then still having an old build of arm hanging around because it failed to build for some reason. Its all-or-nothing at the moment.

Do they have workspaces/files that can be shared by different stages? That would be the big hurdle in migrating the docker-pi-hole repo to github since I used a workspace to help pass information from the individual architecture build stage to the multi-architecture stage.

As Dan pointed out, they do have caching support. However, caching is not currently available for the cron based jobs for some weird reason. They also have artifact support, which probably fits this goal better: https://help.github.com/en/actions/configuring-and-managing-workflows/persisting-workflow-data-using-artifacts

Artifacts are stored for 90 days.

What are the limits if any to amount of runs / cpu hours / etc?

Free account gets 2,000 hours a month and 500 MB of storage for cache, artifacts, and logs.

All runners have these specs:
* 2-core CPU
* 7 GB of RAM memory
* 14 GB of SSD disk space

@diginc
Copy link
Contributor

diginc commented Mar 2, 2020

GH Workflow data sounds like what I did in docker-pi-hole circleci between build and deploy to pass what tags are being deployed. So if we wanted to unify on one CI platform it'd be possible.

Forgot to address py vs bash, I'm cool with bash doing the deploy; I did that in the circle-*.sh scripts on docker-pi-hole repo. For converting that repo to GH, I think the circle-vars.sh script would be the main thing needing updating since I rely heavily on ENV vars but abstracted away all the CI specific environment variables with a layer of defaulting/overwriting.

@lightswitch05
Copy link
Member Author

Wow, running the builds in a matrix brought the build time down to ~9 minutes vs. ~16 minutes. It would be a major effort to ensure all builds were successful before trying to push the images, but it sounds like that isn't a requirement. Matrix-based build: https://github.com/lightswitch05/docker-base-images/actions/runs/48420660

@diginc
Copy link
Contributor

diginc commented Mar 2, 2020

It would be a major effort to ensure all builds were successful before trying to push the images

I think this ties back to my point of only wanting to build the paths that changed. If I add a new image separate from the existing ones and for some unrelated reason the FTL ones are failing - I don't want that to block my progress of releasing my image.

Perhaps a path filter would be a simple fix - e.g. don't build it if it did not change : https://help.github.com/en/actions/configuring-and-managing-workflows/configuring-a-workflow#filtering-for-specific-branches-tags-and-paths

Edit: an exception for this is master. We might need separate merge jobs from the scheduled jobs for this reason.

@lightswitch05
Copy link
Member Author

Just a note, I found out Github Actions does not support YAML anchors: https://github.community/t5/GitHub-Actions/Support-for-YAML-anchors/td-p/30336

@lightswitch05 lightswitch05 force-pushed the feature/branch-based-tagging-with-weekly-updates branch 4 times, most recently from 0e35fd7 to 18982e1 Compare March 3, 2020 04:41
* split up image builds into seperate concurrent build
* build images on pull_request, but never tag and push then

Signed-off-by: Daniel <daniel@developerdan.com>
@lightswitch05 lightswitch05 force-pushed the feature/branch-based-tagging-with-weekly-updates branch from 18982e1 to 4efd444 Compare March 3, 2020 04:45
@lightswitch05
Copy link
Member Author

lightswitch05 commented Mar 3, 2020

I think I have everything working except for the targeted build based on what changed. All builds are attempted on each run, failures do not halt execution of other builds. See: https://github.com/lightswitch05/docker-base-images/runs/481440952?check_suite_focus=true

The bit about not trying to push on a pull_request will need to be tested somehow.

(all failed on the login step)

@lightswitch05
Copy link
Member Author

Thanks for the approval @diginc, I certainly recommend doing some testing before adding DOCKERHUB_USER and DOCKERHUB_PASS to the repo secrets. Once its merged, I can submit another pull request to verify that it runs but doesn't attempt to push

@dschaper
Copy link
Member

dschaper commented Mar 3, 2020

Will happily merge this in after we have a chance to investigate pi-hole/docker-pi-hole#587. I'd like to keep things static until then and not change something while we are researching that image related issue.

@dschaper
Copy link
Member

dschaper commented Mar 5, 2020

Merge when you would like to @diginc

@lightswitch05
Copy link
Member Author

Is there anything else I need to take action on here? Or is it just a timing thing now?

@dschaper
Copy link
Member

I'll merge this now.

@dschaper dschaper merged commit 74d667d into pi-hole:master Mar 10, 2020
@lightswitch05
Copy link
Member Author

Checking in on this. Looks like the weekly builds are still happily chugging along, just missing the required secrets to do the actual docker hub uploads (DOCKERHUB_USER & DOCKERHUB_PASS). Results from last run: https://github.com/pi-hole/docker-base-images/runs/660037206?check_suite_focus=true

@dschaper
Copy link
Member

Okay, we can add the repo Env Vars, let me look a few things over and then make the updates.

@tob123
Copy link

tob123 commented Sep 1, 2020

hello dschaper,

are you still working on this one or are there still some issues ?
(just wanted to add this comment since i saw still quite some vulnerabilities are there in the pihole image)
thanks, thanks, thanks for pihole. the software and the docker image are much appreciated.

@dschaper
Copy link
Member

dschaper commented Sep 1, 2020

Honestly, I'm not the person to ask. @lightswitch05 or @diginc have more knowledge than I.

@lightswitch05
Copy link
Member Author

This is already completed. However, this is only the upstream project of pihole, which does not have regular builds.

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.

4 participants