Open oliv3r opened 3 years ago
While I like this idea, I think more work will need to be done to the Makefile in order to make things work correctly.
The logic in the Makefile assumes the rules involved with tagging the latest image will never include building the alpine variant, only the default. Thus, the required steps necessary to have an alpine image built may not have been met when the Makefile rules that do the tagging are invoked. The reason the CI builds doesn't fail is because these push/tag events are explicitly not invoked for pull requests or any branch other than the master.
I'll have to take a closer look, but I think the most straightforward way of doing this would be to have a separate set of rules for tagging the alpine variant with the "alpine" flag, or rework the existing rules to properly depend on the VERSION and VARIANT combinations to derive the cases where the latest variant image is built and the tag for that variant is used.
While I like this idea, I think more work will need to be done to the Makefile in order to make things work correctly.
The logic in the Makefile assumes the rules involved with tagging the latest image will never include building the alpine variant, only the default. Thus, the required steps necessary to have an alpine image built may not have been met when the Makefile rules that do the tagging are invoked. The reason the CI builds doesn't fail is because these push/tag events are explicitly not invoked for pull requests or any branch other than the master.
I'll have to take a closer look, but I think the most straightforward way of doing this would be to have a separate set of rules for tagging the alpine variant with the "alpine" flag, or rework the existing rules to properly depend on the VERSION and VARIANT combinations to derive the cases where the latest variant image is built and the tag for that variant is used.
Thanks for your reply @phillipross, and sorry for not responding sooner. I was looking at the bottom of the screen and the pipeline results are below your reply :)
I'm no Makexpert so I'll see if I can figure out what you meant with rework the existing rules to properly depend, as I didn't recognize those straight away.
My thought is, that if you have a (latest) release, you want to ideally indeed push them simultaneously, as you want them as a matching pair. So I'll try to spot what you meant ...
Thanks for your reply @phillipross, and sorry for not responding sooner. I was looking at the bottom of the screen and the pipeline results are below your reply :)
No problem, and thanks for contributing!
I'm no Makexpert so I'll see if I can figure out what you meant with rework the existing rules to properly depend, as I didn't recognize those straight away.
My thought is, that if you have a (latest) release, you want to ideally indeed push them simultaneously, as you want them as a matching pair. So I'll try to spot what you meant ...
The way the build is currently setup, the makefile supports setting VERSION and VARIANT environment vars specifying what versions and variants to be built. If they are omitted, then all will be built. This logic is encapsulated in the Makefile which unfortunately does obscure things for folks who are not so familiar with make. We make attempts to mitigate this with documentation and comments, but obviously we can't eliminate the learning curve entirely.
In cases where a build has only specified the default variant (debian) and not alpine, there will be no alpine images built, so attempts to push or tag them will fail. Rather than just add the commands to push and tag an alpine image in the same rule that does them for the default, a little more work will be necessary to only push/tag the alpine images in the cases where they are being built.
The other issue I foresee is that in cases where there might be a VERSION that does not have an alpine variant, the rules will need to somehow account for that case as well. The current builds don't contain any cases of this, but the roadmap does have items for implementing multiplatform images and/or arm builds, and the debian variants do not support the arm platforms for all versions. Again, this isn't a case that needs to be accounted for immediately, but it would be nice if the shorter term changes to the Makefile keep this case in mind.
So, with that in mind, if you don't mind traversing the learning curve involved with make, I'd be happy to review and offer feedback on whatever you can contribute 😉
So if I understand you correctly, you do prefer having them separate targets (based on the env variables) to offer the most flexibility?
I also hear what you are saying for the future cases, where you may even need more flexibility, but I guess getting to a 'single make' would be even harder in that case?
Let me dabble a bit and see what I can come up with first to wrap my mind around this :) In what you really want and whats the easiest to get going first round.
How do you trigger these commands btw? Are they run locally by a human, or are the tag/event based on pipelines that eventually do this?
I'm not complete stranger to make, but maybe estranged a little :p
So if I understand you correctly, you do prefer having them separate targets (based on the env variables) to offer the most flexibility?
How do you trigger these commands btw? Are they run locally by a human, or are the tag/event based on pipelines that eventually do this?
Ideally the commands can be triggered by humans during development/testing as well as triggered by the CI for eventual building, validating, and pushing the images. Currently the VERSION and VARIANT inputs are able to specifically invoke builds of specific versions and variants which in turn allows the CI to configure a build matrix and test finer-grained steps in the pipeline. This also allows developers to focus on specific combinations for the cases and conditions they are currently developing/debugging.
My feeling is that separate targets would be better as the code would be easier to read for people who are less familiar with Makefiles. But I understand that actively working to implement one approach often leads to exposing details that actually cause one to shy away and explore other better approaches. For this reason, I tend to ask contributors to consider my input and feedback as suggestions rather than prescriptions 😉
It is quite common, and upstream postgres does so as well, to also tag a 'latest' for alpine. In this case, this would result in
postgis:alpine
being the latest stable alpine release. This makes it easier (albeit riskier) to track the latest stable postgis release. The risk should be acceptable however, as the same risk would be taken when trackingpostgis:latest
so it really is up to the user.