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

Upgrade to Ubuntu 22.04 build container #317

Merged
merged 1 commit into from Jun 6, 2022

Conversation

@timja timja requested a review from a team as a code owner June 3, 2022 18:45
@timja timja marked this pull request as draft June 3, 2022 18:45
@timja

This comment was marked as resolved.

@timja

This comment was marked as resolved.

@timja

This comment was marked as resolved.

@timja

This comment was marked as resolved.

@timja timja changed the title Switch to createrepo_c Upgrade to Ubuntu 22.04 build container Jun 5, 2022
@@ -24,7 +24,7 @@ BuildRoot: %{_tmppath}/build-%{name}-%{version}
# TODO: If re-enable, fix the matcher for Java 11
# Requires: java >= 1:1.8.0
Requires: procps
PreReq: /usr/sbin/groupadd /usr/sbin/useradd
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was deprecated

@@ -25,7 +25,7 @@ BuildRoot: %{_tmppath}/build-%{name}-%{version}
# TODO: Fix the query for Java 11 if it is reenabled
# Requires: java >= 1:1.8.0
Requires: procps
PreReq: /usr/sbin/groupadd /usr/sbin/useradd
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was deprecated

@@ -71,10 +71,10 @@ rm -rf "%{buildroot}"
%__install -D -m0755 "%{SOURCE7}" "%{buildroot}%{_datadir}/%{name}/migrate"

%pre
/usr/sbin/groupadd -r %{name} &>/dev/null || :
/usr/bin/getent group %{name} &>/dev/null || /usr/sbin/groupadd -r %{name} &>/dev/null
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the recommended method for adding a group depending on if it exists or not

# SUSE version had -o here, but in Fedora -o isn't allowed without -u
/usr/sbin/useradd -g %{name} -s /bin/false -r -c "@@SUMMARY@@" \
-d "%{workdir}" %{name} &>/dev/null || :
/usr/bin/getent passwd %{name} &>/dev/null || /usr/sbin/useradd -g %{name} -s /bin/false -r -c "@@SUMMARY@@" \
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the recommended method for adding a user depending on if it exists or not

# on the server
# shellcheck disable=SC2029
ssh "${SSH_OPTS[@]}" "$PKGSERVER" createrepo --update -o "'$RPM_WEBDIR'" "'$RPMDIR/'"
ssh "${SSH_OPTS[@]}" "$PKGSERVER" createrepo_c --update -o "'$RPM_WEBDIR'" "'$RPMDIR/'"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not seem that $PKGSERVER has createrepo_c installed, which is a prerequisite to the successful deployment of this change.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch!
I thought that all was happenning inside the container here, and that the SSH steps were only for basic operation on this VM. You just caught that my assumptions were false.

This machine is puppet managed:

But it is still Ubuntu 18.04 :'(

Since I haven't checked: is there a way to add createrepo_c on 18.04?
If no, could we use a Docker container for this instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If no, could we use a Docker container for this instead?

It is just software. You can do anything you want, if you write the code.

In the short term, I suggest upgrading the box to Ubuntu 22.04.

Copy link

@dduportal dduportal Jun 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the short term, I suggest upgrading the box to Ubuntu 22.04.

(edited message)
This machine also hosts the Update Center: its not an easy task and would require unplanned effort for now.
Not that I would not want, but that would be a huge context switch for the team meaning we would have to plan it for next week: it's not a short term solution alas :'(

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can just switch back to createrepo and symlink it in the container, it’s API compatible as far as I understand

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sense. Many thanks @timja @basil for the help and quick reactions!

Upgrading our boxes from 18.04 to 22.04 is a topic that I would like to see happen quite fast though!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Err, I don't think createrepo is even needed in the docker container?

It is only executed over SSH?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timja timja marked this pull request as ready for review June 5, 2022 08:06
@timja timja requested review from dduportal and basil June 5, 2022 08:07
@timja
Copy link
Member Author

timja commented Jun 5, 2022

nice work on the test framework @basil!

The tests caught 2 issues (since fixed):

  1. Building on Ubuntu no longer works for debian by default as it uses a compression algorithm not supported on debian
  2. rpmsign uses gpgv2 now which wasn't installed and silently failed, tests caught that it wasn't signed anymore

Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

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.

None yet

3 participants