-
Notifications
You must be signed in to change notification settings - Fork 917
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
drop el-7, debian-buster, amazonlinux-2, CircleCI #14990
base: master
Are you sure you want to change the base?
Conversation
50272e0
to
70a9a45
Compare
this claim was based on https://endoflife.date/amazon-linux which says 30 Jun 2025 (update submitted). However, https://aws.amazon.com/amazon-linux-2/faqs/ says 30 Jun 2026. That said, I don't think Amazon Linux 2 is reason enough to hang on to all that legacy. I'm not even sure we have any users on it. |
Pull Request Test Coverage Report for Build 12432070620Details
💛 - Coveralls |
I cannot offer a strong review but a weak review it eye balls good to me. There are a few spec file comments that refer to EL7 still (see the systemd runtime dir bits) but thats minor. |
ah yes. I missed at least one comment there, thanks! |
@@ -16,10 +16,8 @@ on: | |||
type: string | |||
# please remember to update build-packages.yml as well | |||
default: >- | |||
el-7 | |||
el-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.
should this list be in sync with the list in builder.yml
?
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.
no, in fact these days perhaps it should explicitly not overlap with it at all I think. builder-dispatch
should contain a good default set for shipping packages (even if we don't builder-dispatch for releases any more). builder
is older than all the more specific ones, and was for daily testing of distros we -don't- autobuild for in any other way.
builder-dispatch should be in sync with builder-packages (or ideally the list would live in one of them, but that's tricky). builder
(which we should maybe give a useful suffix to explain what it does) should then cover distros we do not, or do not yet, ship for
./install_quiche.sh; \ | ||
fi | ||
RUN cd /pdns/builder-support/helpers/ && ./install_rust.sh; \ | ||
yum install -y git cmake clang; \ |
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.
ISTM all these commands want && chaining for resilience, but this is not a new 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.
agreed. Would make sense as a separate commit if we're touching all this anyway.
RUN touch /var/lib/rpm/* && if $(grep -q 'release 7' /etc/redhat-release); then \ | ||
scl enable devtoolset-11 -- builder/helpers/build-specs.sh builder-support/specs/dnsdist.spec; \ | ||
elif $(grep -q 'release 8' /etc/redhat-release); then \ | ||
RUN touch /var/lib/rpm/* && if $(grep -q 'release 8' /etc/redhat-release); 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.
in theory, in this context grep ...
should be equivalent to $(grep...)
@@ -104,9 +91,6 @@ EOF | |||
# The EL7 and 8 systemd actually supports %t, but its version number is older than that, so we do use seperate runtime dirs, but don't rely on RUNTIME_DIRECTORY |
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.
should this comment go?
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.
yes! I think that's what ph1 spotted too
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.
ah yes! the same comment is copied into a second file too
@@ -144,35 +123,27 @@ exit 0 | |||
%if 0%{?suse_version} | |||
%service_add_post %{name}.service | |||
%endif | |||
%if 0%{?rhel} >= 7 |
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.
with the suse context above, this looks like this block might have only been in place for rhel?
Short description
el-7 and debian buster are EOL already. Amazonlinux-2 goes EOL in six months and we never shipped those packages.
We stopped using CircleCI years ago.
To be done (separate PR perhaps): the el7 devtoolset check in misc-dailies should be migrated for el8.
This is entirely untested and needs a strong review because of all the "if el7" things I took out from various files in various syntaxes.
Checklist
I have: