-
Notifications
You must be signed in to change notification settings - Fork 108
Rework Agent containers build #2607
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
Conversation
cee822b
to
0d43bf6
Compare
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'm impressed. It's still "Draft" so I won't bother to approve, but it seems to be looking rather comprehensive assuming testing proves it all out in practice.
e448d46
to
0261cba
Compare
6253f4f
to
2ace48d
Compare
Replace cookie-cutter approach with templates and pattern rules Switch from docker.io to quay.io for base images Update default list to: Fedora 34, Fedora 35, and CentOS 8
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.
The make
magic is kinda blinding, but it looks good.
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.
A tour-de-force of makefileology! Opaque but impressive.
A couple of questions:
- Is it possible to produce the Makefile after all the automatic expansions have happened? That might be a good thing to have if one needs to debug it.
- I wonder if some of the opaqueness could be alleviated by using a more expressive language, e.g. by having a program that would write a makefile for some distro (e.g centos-8) that the main Makefile could then run.
In general, I like programs that write programs, but I'm not sure how I feel about Makefiles that create Makefiles (particularly if they don't write them out) :-)
Nevertheless, once the CentOS problems are resolved, I'm ready to approve.
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.
LGTM
You can have Make dump its database (which is pretty voluminous) by using the Also, using If you're really interested, you can use
This is certainly a valid approach, but it's not one that I'm partial to. It might be easier to debug, and it might be easier for someone with limited facility with Make to deal with, but, when things are working, it's a more cumbersome solution which often requires more dependencies and ends up being more fragile. Make is intended to be able to handle things like this without resorting to external super- or sub-structures (although, Make is quite capable of |
The current build procedure for the Agent containers no longer works. The first problem is that it insists upon cross-checking all of its default build targets, including Fedora 32 (even if the requested build doesn't include F32...), which doesn't work now that we've removed our support for Fedora 32 from COPR and COPR itself has deprecated Fedora 33 support. The second problem is that its build targets are specific to the respective target platforms, and so adding a new platform requires cookie-cutter copying the existing targets to create targets for the new platform.
This PR makes the following changes:
docker.io
toquay.io
as the source for base images. Since the repositories have different names, and since the tags have different formats, this required some changes to the plumbing.Note that there is nothing intrinsic to these changes which would prevent Fedora 32 or Fedora 36 from working, but we don't currently provide the required stuff on COPR for these targets. Likewise, there is nothing in these changes which prevents CentOS 7 or CentOS 9 from working -- the CentOS 7 build had stopped working prior to these changes (and its build now fails in exactly the same way with these changes), and
packagecloud.io
, from which we source the Prometheus RPMs, does not yet support CentOS 9.[I've tagged @portante as a reviewer for, oh, so many reasons; @ndokos, I would appreciate it if you could be my second reviewer, and anyone else is welcome to step up.]