-
Notifications
You must be signed in to change notification settings - Fork 22
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
Build, test and deploy from GitHub Actions #26
Conversation
on: | ||
push: | ||
pull_request: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could let the workflow run every week automatically to produce updated images (e.g. if Alpine updates).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, that sounds like a good idea. I'd been wondering how other projects handle that. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed with 012887f.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, that sounds like a good idea. I'd been wondering how other projects handle that. 😄
with daily builds. to keep the base system up to date you have to build the container every day/week with updates in the dockerfile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. Sounds like we just need to set up some scheduler somewhere and then mostly forget about it. 😄
@justinclift I think this is ready for review / merge. one thing I noted is that the new ARM64 build is untested. but maybe we can do another PR in the (near) future which runs the tests on a ARM64 runner. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should add some type of versioning to this.
i for example deploy this in a swarm cluster and run renovate against my repo to check for updates, that way i can monitor for updates and check if there are breaking changes.
i would suggest something like {semver}-{pg_version}+{build} = 1.2.3-pg16+20230315 when using daily builds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonder if we'll run into storage space problems with Docker Hub if we do that?
I'm guessing there's some limit to how much they'll let (free) users store.
Oh, the application I submitted to convert the Docker Hub repo to be "OSS Sponsored" never got a response.
I guess since we don't have a million downloads already we're too small or something. That's just a total guess though, could be totally wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as far as i can see is that the only limit a free account has is the pull rate limit and the number of private repos.
and i know other projects that have lots of daily builds
so i would say that shouldnt be a problem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome. Sounds like an approach we should try out then. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a bit split for adding tags for daily builds.
On one hand, it is not much effort to implement, simply derive from the current date and adjust the tag. But we also might up with a lot of Docker tags that nobody really uses. Sure, there is not a storage limit, but still seems kind of wasteful.
Then there is the case of us running pushing a new build, maybe there was something in Alpine updated which could break something, but we do not catch with the automated tests. I have been working with containers in software development for the last five years without running any daily tags, and I would say there were only a couple of cases were this happened, compared to the hours we saved by not having to look what version we are now running exactly.
what I would support is adding a version to the script itself, because there I see we most likely implement a breaking feature. You would need to keep the Postgres version in front, otherwise Renovate will never update because v16 of Postgres > v1.0 of the script.
Ultimately, I do not mind if we happen to implement both, it would still be more "security" than there is now in regards to versions and builds.
@andyundso I'll get the self-hosted ARM64 runner back online today. If you want to try running this PR on that you then can, or we can attack that particular part of things in a follow up PR like you're thinking. 😄 |
@andyundso The ARM64 build runner is back online and hooked up to GitHub Actions in this repo. |
Dockerfile
Outdated
RUN cd postgresql-16.* && \ | ||
./configure --prefix=/usr/local-pg16 --with-openssl=no --without-readline --with-icu --with-lz4 --with-system-tzdata=/usr/share/zoneinfo --enable-debug=no CFLAGS="-Os" && \ | ||
make -j $(nproc) && \ | ||
make install && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs to be install-world
instead of install
.
The -world
make target also installs commonly used PG extensions (as per this commit).
Thinking about that more, we might not need it for the early PG versions people are migrating away from, but it'll definitely be needed for the "target" PG versions people are upgrading to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I adjusted it for all PG versions now, just to keep it consistent, in bad495a.
The build will likely take a while, since it will not be able to use the build images. good test in any case 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really good.
I think the only item that really needs changing is to use install-world
for the PG bits rather than install
, so the extensions get included.
Everything else (potential versioning, different scheduling, etc.) we can figure out later as we see how this goes. 😄
Awesome @andyundso. 😄 Merging this now. The versioning idea that @wuast94 raised seems useful too, but we can figure that out separately from this. 😄 |
Replaces #19.
This PR contains a GitHub Actions configuration that can build a multi-arch version of the
pgautoupgrade
container. In a nutshell:build
step should be cached.