Use the generic redhat-release package #713

travier · 2022-02-10T16:04:54Z

Ensure that /etc/issue.d exists for console-login-helper-messages

This can be removed once we rebase to RHEL 9 as it will be included in
the redhat-release package.

Use the generic redhat-release package

The redhat-release-coreos package is mostly a copy of the generic
package as all major configuration changes are made in this repo.

Thus move the os-release changes to a step in a postprocess script and
use the generic package to benefit from updates for free.

The changes are:

No longer provide "extracted" GPG keys as they are not used in RHCOS
(was in /usr/share/ostree/trusted.gpg.d/)
containerisvsign.asc will be provided by the RHEL 8.5 version of this
package in /etc/pki/rpm-gpg/
Drop /etc/profile.d/path.sh
Update default presets to their latest version as part of the
redhat-release package

travier · 2022-02-10T16:06:40Z

This will fail CI as it needs a fix in RHCOS CLHM package.

cgwalters

Ahh I understand now. OK. I'm generally 👍 on this - none of my comments should be viewed as blocking.

Actually the more I think about it, this is a huge improvement over what we're doing now - the redhat-release-coreos package is a heavy weighted bag of technical debt that just goes away.

cgwalters · 2022-02-10T16:26:22Z

manifest.yaml

@@ -50,6 +50,7 @@ rpmdb: bdb
 automatic-version-prefix: "411.84.<date:%Y%m%d%H%M>"
 # This ensures we're semver-compatible which OpenShift wants
 automatic-version-suffix: "-"
+# Keep this is sync with the version in postprocess


Hmm I think we could figure out how to pass this through.

Or actually, do we even need to use mutate-os-release here anymore since we're effectively rewriting it in postprocess anyways?

I think...hmm, we could pass down the value of automatic-version-prefix and then split on that or something?

Anyways, not a blocker at all, but would clearly be nice to have a single source-of-truth for major version.

cgwalters · 2022-02-10T16:28:06Z

manifest.yaml

+     OCP_RELEASE="4.11"
+     (
+     . /etc/os-release
+     cat > /usr/lib/os-release <<EOF


I am generally fine with this, but I'd note that while we're replacing most of the fields, we're not replacing all of them (e.g. HOME_URL). I think what we probably want to do is basically carry an "overrides" set, and keep other values unchanged.

We could potentially make this a first-class operation in rpm-ostree so all the parsing/merging doesn't need to be shell script?

miabbott · 2022-02-10T17:20:50Z

This will fail CI as it needs a fix in RHCOS CLHM package.

While I'm in favor of this change, I don't want to merge it quite yet as it will block our ability to do downstream builds. (Even though those are still kind of shaky already)

Let's get the downstream CLHM package fixed, then land this.

jupierce · 2022-02-10T19:10:14Z

/test build-test-qemu

HuijingHei · 2022-02-11T01:59:29Z

Nice job! Anyway, this is a huge improve.

manifest.yaml

HuijingHei · 2022-02-14T09:29:27Z

/test build-test-qemu

cgwalters · 2022-02-15T14:33:13Z

So our CI here is broken because we're waiting on openshift/release#26193 - but if this has been tested locally I'm good with merging.

Thanks so much for doing this!

miabbott · 2022-02-15T16:19:02Z

openshift/release#26193 merged, so let's retest

/retest

miabbott · 2022-02-15T18:20:33Z

/retest

miabbott · 2022-02-15T18:24:36Z

Ah, the CI job is going to keep failing because the script is setup to curl the wrong repo

os/ci/prow-build-test-qemu.sh

Line 33 in ded68a2

    
           curl -L http://base-"${ocpver_mut}"-rhel8.ocp.svc.cluster.local > src/config/ocp.repo

+ curl -L http://base-4-11-rhel8.ocp.svc.cluster.local

So it won't use the new repo from openshift/release#26193 until we update the script

If this has been tested locally, let's override the job and merge it.

miabbott · 2022-02-15T18:30:07Z

So it won't use the new repo from openshift/release#26193 until we update the script

#718

miabbott · 2022-02-24T19:57:25Z

/retest

HuijingHei · 2022-02-25T07:35:06Z

/retest

HuijingHei · 2022-02-25T11:43:17Z

2 cases failed:

rhel-matches-rhcos-build failed as RHEL_VERSION=8.5 does not match rhel version in VERSION=411.84.202202250743-0, should replace 84 to 85 in automatic-version-prefix: "411.84.<date:%Y%m%d%H%M>", maybe there is better solution?
network-online-service failed as network-online.target should be inactive default, but it is active, I do not find any special config for this

[core@cosa-devsh ~]$ systemctl show -p ActiveState network-online.target
ActiveState=active

miabbott · 2022-02-25T14:52:12Z

* `rhel-matches-rhcos-build` failed as `RHEL_VERSION=8.5` does not match rhel version in `VERSION=411.84.202202250743-0`, should replace 84 to 85 in `automatic-version-prefix: "411.84.<date:%Y%m%d%H%M>"`, maybe there is better solution?

The automatic-version-prefix: needs to be updated as we are using RHEL 8.5 content now. It may be worthwhile to examine if the test should be reworked now that we are using the redhat-release package.

This can be removed once we rebase to RHEL 9 as it will be included in the redhat-release package.

The redhat-release-coreos package is mostly a copy of the generic package as all major configuration changes are made in this repo. Thus move the os-release changes to a step in a postprocess script and use the generic package to benefit from updates for free. The changes are: - No longer provide "extracted" GPG keys as they are not used in RHCOS (was in /usr/share/ostree/trusted.gpg.d/) - containerisvsign.asc will be provided by the RHEL 8.5 version of this package in /etc/pki/rpm-gpg/ - Drop /etc/profile.d/path.sh - Update default presets to their latest version as part of the redhat-release package We do not add a VARIANT & VARIANT_ID qualifiers yet as this requires changes in kola.

HuijingHei · 2022-02-28T07:10:48Z

@jlebon would you help to check failed case network-online-service? Thanks!
From log systemctl show -p ActiveState network-online.target should be default inactive, but actually active.
Check 411.84.202202251839-0 using the same systemd-239-51.el8.x86_64 as rhel8.5, but get result actually is inactive

jlebon · 2022-03-01T18:06:36Z

@jlebon would you help to check failed case network-online-service? Thanks!

Looks like this is coming from nfs-client.target, which is now enabled via presets in RHEL 8.5+. Backstory on this is in https://bugzilla.redhat.com/show_bug.cgi?id=1967515. The TL;DR I think is that it used to enable itself in a scriptlet, and was moved to a preset in 8.5 as per packaging guidelines and to fix container-related issues. The scriptlet approach never worked on RHCOS because we explicitly make presets canonical (https://github.com/coreos/fedora-coreos-config/blob/656b9727368317a5adcf49823f9ae3a03d1fe3f5/manifests/ignition-and-ostree.yaml#L39-L48).

It's interesting though that since AFAIK NFS works fine on RHCOS currently, it must mean something in the stack is enabling it on-demand (or I think for NFSv3 it can also be socket activated). I'm not seeing it in e.g. https://github.com/openshift/csi-driver-nfs offhand, but it must be happening somehow.

I think it was a conscious decision though to keep it off by default in RHCOS, so we should keep disabling it as before and let whatever re-enables it on-demand keep kicking in.

Added a commit here which does that!

Its enablement was moved from a scriptlet to a preset in RHEL 8.5+ but we want to keep it disabled as before in RHCOS. See: openshift#713 (comment)

cgwalters · 2022-03-01T19:34:10Z

It's interesting though that since AFAIK NFS works fine on RHCOS currently, it must mean something in the stack is enabling it on-demand (or I think for NFSv3 it can also be socket activated). I'm not seeing it in e.g. https://github.com/openshift/csi-driver-nfs offhand, but it must be happening somehow.

https://github.com/openshift/machine-config-operator/blob/4e7fe38d0db4a3a542a246b6a9eb97f582b91b07/templates/master/01-master-kubelet/_base/units/kubelet.service.yaml#L6

Which was cargo culted from CoreOS Tectonic. We've had multiple BZs about this from people not using NFS, and I think indeed enabling it needs to move into the storage drivers.

cgwalters · 2022-03-01T19:36:19Z

/lgtm

openshift-ci · 2022-03-01T19:39:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, HuijingHei, travier

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [HuijingHei,cgwalters,travier]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jlebon · 2022-03-01T20:06:42Z

/test build-test-qemu

openshift-ci · 2022-03-02T02:21:33Z

@travier: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

coreos#1550 does not work when openshift/os#713 merged, because it will use generic redhat-release package instead of redhat-release-coreos. Need to check RHEL specific version using `VERSION_ID`(for example VERSION_ID=8.6), and clarify rhel or fedora using `ID=rhel`, as this will be executed before override according to https://github.com/openshift/os/blob/master/manifest.yaml#L128-L150

lucab · 2022-03-02T08:49:33Z

Isn't this going to throw off-path all the scriptlets and other postprocess scripts which rely on os-release content?
Specifically, I think some of them will source RHEL-original content now.

jlebon · 2022-03-02T15:02:13Z

Isn't this going to throw off-path all the scriptlets and other postprocess scripts which rely on os-release content? Specifically, I think some of them will source RHEL-original content now.

Yeah, I think you're right. Good catch.

For scriptlets, only those which run at compose time would see the difference. I'd be surprised if there were any in our base set which had RHCOS-specific logic. If we encounter any, we should try to work with the maintainers to rip it out, but in the worst case there's /run/ostree-booted which is likely a more appropriate API.

For postprocess scripts, include semantics means indeed that the postprocess added here will run last. Though AFAICT, none of the ones in this repo or in FCOS use ID. There are the workarounds in https://github.com/coreos/fedora-coreos-config/blob/testing-devel/manifests/shared-workarounds.yaml which use NAME but the conditionals are on Fedora so those don't break (but easily could've!). (Edit: My local checkout was old. I see coreos/fedora-coreos-config@656b972 now and the follow-up in coreos/fedora-coreos-config#1553 which is probably what brought you here.)

So overall I think we're OK. But long-term I agree with #713 (comment) we probably want to fold this into rpm-ostree.

#1550 does not work when openshift/os#713 merged, because it will use generic redhat-release package instead of redhat-release-coreos. Need to check RHEL specific version using `VERSION_ID`(for example VERSION_ID=8.6), and clarify rhel or fedora using `ID=rhel`, as this will be executed before override according to https://github.com/openshift/os/blob/master/manifest.yaml#L128-L150

If `/run/ostree-booted` is present, then the kernel arguments can be updated via `rpm-ostree` command. File `/host/etc/redhat-release` may not be available because it can be an absolute linka (see openshift/os#713). Add test harness to allow running `bindata/scripts/*.sh` files in a mocked filesystem via `make test-bindata-scripts`. Harness includes shUnit2 files and a mocked `rpm-ostree` implementation. Signed-off-by: Andrea Panattoni <apanatto@redhat.com>

If `/run/ostree-booted` is present, then the system is based on rpm-ostree and kernel arguments can be updated via `rpm-ostree` command. File `/host/etc/redhat-release` may not be available because it can be an absolute link (see openshift/os#713). Add test harness to allow running `bindata/scripts/*.sh` files in a mocked filesystem via `make test-bindata-scripts`. Harness includes shUnit2 files and a mocked `rpm-ostree` implementation. Signed-off-by: Andrea Panattoni <apanatto@redhat.com>

If `/run/ostree-booted` is present, then the system is based on rpm-ostree and kernel arguments can be updated via `rpm-ostree` command. File `/host/etc/redhat-release` may not be available because it can be an absolute link (see openshift/os#713). Signed-off-by: Andrea Panattoni <apanatto@redhat.com>

coreos#1550 does not work when openshift/os#713 merged, because it will use generic redhat-release package instead of redhat-release-coreos. Need to check RHEL specific version using `VERSION_ID`(for example VERSION_ID=8.6), and clarify rhel or fedora using `ID=rhel`, as this will be executed before override according to https://github.com/openshift/os/blob/master/manifest.yaml#L128-L150

openshift-ci bot requested review from HuijingHei and ravanelli February 10, 2022 16:12

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 10, 2022

travier force-pushed the redhatrelease branch 2 times, most recently from 4d389b8 to be60387 Compare February 10, 2022 16:26

cgwalters approved these changes Feb 10, 2022

View reviewed changes

travier force-pushed the redhatrelease branch from be60387 to deec5f3 Compare February 10, 2022 16:51

HuijingHei suggested changes Feb 11, 2022

View reviewed changes

manifest.yaml Outdated Show resolved Hide resolved

HuijingHei reviewed Feb 11, 2022

View reviewed changes

manifest.yaml Outdated Show resolved Hide resolved

travier force-pushed the redhatrelease branch from deec5f3 to bdfaec5 Compare February 11, 2022 18:18

HuijingHei approved these changes Feb 15, 2022

View reviewed changes

cgwalters mentioned this pull request Feb 17, 2022

treefile: Add conditional includes coreos/rpm-ostree#3439

Merged

travier added 3 commits February 25, 2022 18:17

Ensure that /etc/issue.d exists for console-login-helper-messages

9e1d7da

This can be removed once we rebase to RHEL 9 as it will be included in the redhat-release package.

manifest: Update for RHEL 8.5

443df6c

travier force-pushed the redhatrelease branch from bdfaec5 to 443df6c Compare February 25, 2022 17:20

overlay.d/05rhcos: disable nfs-client.target by default

e7e75bc

Its enablement was moved from a scriptlet to a preset in RHEL 8.5+ but we want to keep it disabled as before in RHCOS. See: openshift#713 (comment)

openshift-ci bot assigned cgwalters Mar 1, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 1, 2022

openshift-merge-robot merged commit da07aad into openshift:master Mar 2, 2022

HuijingHei mentioned this pull request Mar 2, 2022

multipathd.socket: fix remove workaround for rhel8.6 coreos/fedora-coreos-config#1553

Merged

travier deleted the redhatrelease branch March 2, 2022 15:24

cgwalters mentioned this pull request Dec 15, 2022

Best practices for new runners? osbuild/osbuild#1209

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use the generic redhat-release package #713

Use the generic redhat-release package #713

travier commented Feb 10, 2022 •

edited

Loading

travier commented Feb 10, 2022 •

edited

Loading

cgwalters left a comment

cgwalters Feb 10, 2022

cgwalters Feb 10, 2022

miabbott commented Feb 10, 2022

jupierce commented Feb 10, 2022

HuijingHei commented Feb 11, 2022 •

edited

Loading

HuijingHei commented Feb 14, 2022

cgwalters commented Feb 15, 2022

miabbott commented Feb 15, 2022

miabbott commented Feb 15, 2022

miabbott commented Feb 15, 2022

miabbott commented Feb 15, 2022

miabbott commented Feb 24, 2022

HuijingHei commented Feb 25, 2022

HuijingHei commented Feb 25, 2022

miabbott commented Feb 25, 2022

HuijingHei commented Feb 28, 2022

jlebon commented Mar 1, 2022

cgwalters commented Mar 1, 2022

cgwalters commented Mar 1, 2022

openshift-ci bot commented Mar 1, 2022

jlebon commented Mar 1, 2022

openshift-ci bot commented Mar 2, 2022

lucab commented Mar 2, 2022 •

edited

Loading

jlebon commented Mar 2, 2022 •

edited

Loading

Use the generic redhat-release package #713

Use the generic redhat-release package #713

Conversation

travier commented Feb 10, 2022 • edited Loading

travier commented Feb 10, 2022 • edited Loading

cgwalters left a comment

Choose a reason for hiding this comment

cgwalters Feb 10, 2022

Choose a reason for hiding this comment

cgwalters Feb 10, 2022

Choose a reason for hiding this comment

miabbott commented Feb 10, 2022

jupierce commented Feb 10, 2022

HuijingHei commented Feb 11, 2022 • edited Loading

HuijingHei commented Feb 14, 2022

cgwalters commented Feb 15, 2022

miabbott commented Feb 15, 2022

miabbott commented Feb 15, 2022

miabbott commented Feb 15, 2022

miabbott commented Feb 15, 2022

miabbott commented Feb 24, 2022

HuijingHei commented Feb 25, 2022

HuijingHei commented Feb 25, 2022

miabbott commented Feb 25, 2022

HuijingHei commented Feb 28, 2022

jlebon commented Mar 1, 2022

cgwalters commented Mar 1, 2022

cgwalters commented Mar 1, 2022

openshift-ci bot commented Mar 1, 2022

jlebon commented Mar 1, 2022

openshift-ci bot commented Mar 2, 2022

lucab commented Mar 2, 2022 • edited Loading

jlebon commented Mar 2, 2022 • edited Loading

travier commented Feb 10, 2022 •

edited

Loading

travier commented Feb 10, 2022 •

edited

Loading

HuijingHei commented Feb 11, 2022 •

edited

Loading

lucab commented Mar 2, 2022 •

edited

Loading

jlebon commented Mar 2, 2022 •

edited

Loading