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

Rework Red Hat resolved package list to be more future proof #468

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

jcpunk
Copy link
Contributor

@jcpunk jcpunk commented Jun 6, 2024

Pull Request (PR) description

CentOS Stream10 continues to have systemd-resolved in its own package.

This Pull Request (PR) fixes the following issues

@jcpunk jcpunk added the enhancement New feature or request label Jun 6, 2024
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I'd lean to leaving it out of metadata.json until we can verify it in CI.

Having said that, all the Red Hat family except EL 8 have systemd-resolved. Perhaps we should set it in data/RedHat-family.yaml and then set it to ~ in data/RedHat-family-8.yaml to unset it again. That way we're future proof.

At least, I see that on Fedora 39 & 40 systemd-resolved is a separate package as well that we don't have in Fedora.yaml right now.

metadata.json Outdated
@@ -58,7 +58,8 @@
"operatingsystemrelease": [
"7",
"8",
"9"
"9",
"10"
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps this is a good excuse to start supporting CentOS Stream 10 in our CI for acceptance tests, though I doubt there are Puppet packages available though :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can confirm there is no puppet agent for CS10. I pulled the one from Fedora to test it.

@jcpunk
Copy link
Contributor Author

jcpunk commented Jun 6, 2024

Took a stab at moving the package name up a slot per your comment.

I'm not sure how to fix the case to also work for newer fedora in the spec tests.

@jcpunk jcpunk requested a review from ekohl June 6, 2024 21:09
@jcpunk jcpunk force-pushed the EL10 branch 4 times, most recently from 7677990 to 67ff82b Compare June 11, 2024 17:01
@@ -48,7 +48,7 @@
end

case [facts[:os]['family'], facts[:os]['release']['major']]
when %w[RedHat 7], %w[RedHat 9], %w[Debian 12]
when %w[RedHat 7], %w[RedHat 9], %w[Debian 12], %w[Fedora \d+]
Copy link
Member

@evgeni evgeni Jun 11, 2024

Choose a reason for hiding this comment

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

I think the \d+ doesn't match. Did you try

Suggested change
when %w[RedHat 7], %w[RedHat 9], %w[Debian 12], %w[Fedora \d+]
when %w[RedHat 7], %w[RedHat 9], %w[Debian 12], %w[Fedora /\d+/]

But even then, I think %w[a b] always produces strings, so you'd need to be explicit and say

Suggested change
when %w[RedHat 7], %w[RedHat 9], %w[Debian 12], %w[Fedora \d+]
when %w[RedHat 7], %w[RedHat 9], %w[Debian 12], %w[Fedora 37], %w[Fedora 38]

Or maybe

Suggested change
when %w[RedHat 7], %w[RedHat 9], %w[Debian 12], %w[Fedora \d+]
when %w[RedHat 7], %w[RedHat 9], %w[Debian 12], ["Fedora", /\d+/]

Copy link
Member

@evgeni evgeni Jun 11, 2024

Choose a reason for hiding this comment

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

Wait, lol, no!

family is RedHat for Fedora too. So how about

Suggested change
when %w[RedHat 7], %w[RedHat 9], %w[Debian 12], %w[Fedora \d+]
when %w[RedHat 7], ["RedHat", 9..999], %w[Debian 12]

Copy link
Member

Choose a reason for hiding this comment

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

Adding my brain dump from IRC:

19:19 < Zhenech> ooooh
19:20 < Zhenech> that major not mathing 9..999 is because it's a string, not an int
19:21 < Zhenech> mh, no, even that doesn't help
19:21 < Zhenech> fuckit
19:24 < Zhenech> okay, yeah, it constructs an Array, and that Array, as an object, must match, it can't contain matchers itself

@jcpunk jcpunk force-pushed the EL10 branch 7 times, most recently from c3251ec to c9aa96e Compare June 11, 2024 20:59
spec/classes/init_spec.rb Outdated Show resolved Hide resolved
@bastelfreak
Copy link
Member

@jcpunk any chance you can add factsets to https://github.com/voxpupuli/facterdb as soon as CentOS offers vagrant images? That's required for us to properly test it.

@jcpunk
Copy link
Contributor Author

jcpunk commented Jun 12, 2024

Stream 10 is still gathering shape, vagrant may be a ways off.

@bastelfreak
Copy link
Member

to quote from IRC:

15:38:29 Zhenech | https://mirror.stream.centos.org/10-stream/ is now a thing, so I assume containers should show up soon too and we can look into CI

@ekohl
Copy link
Member

ekohl commented Jun 12, 2024

https://quay.io/repository/centos/centos?tab=tags has stream10-development and stream10-development-minimal. But as I said earlier, we'll need Puppet packages. rpm-software-management/mock#1383 is open, which is probably a requirement to easily build RPMs.

@jcpunk
Copy link
Contributor Author

jcpunk commented Jun 12, 2024

https://kojihub.stream.centos.org/koji/packageinfo?packageID=3493 is also probably worth noting here.

@ekohl
Copy link
Member

ekohl commented Jun 12, 2024

Note our CI relies on Vagrant with Docker as a backend, so containers (which exist) are sufficient for that.

@bastelfreak
Copy link
Member

Vagrant with Docker as a backend

Our CI uses docker directly, without vagrant (if you're speaking about the vox pupuli module pipeline). But we need Vagrant to generate factsets for facterdb (and that uses the facter ruby gems and doesn't require the puppet rpms).

@jcpunk jcpunk changed the title Initial support for RHEL10 (via CentOS Stream 10) Rework package list to be more future proof Jun 12, 2024
@ekohl
Copy link
Member

ekohl commented Jun 13, 2024

Our CI uses docker directly, without vagrant (if you're speaking about the vox pupuli module pipeline). But we need Vagrant to generate factsets for facterdb (and that uses the facter ruby gems and doesn't require the puppet rpms).

Oh yes, I indeed meant Beaker with Docker backend.

@ekohl ekohl changed the title Rework package list to be more future proof Rework Red Hat resolved package list to be more future proof Jun 13, 2024
@ekohl ekohl merged commit 38bd9da into voxpupuli:master Jun 13, 2024
31 checks passed
@jcpunk jcpunk deleted the EL10 branch June 13, 2024 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants