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

[9.1] distro/rhel9: edge images default to LVM #2947

Merged
merged 10 commits into from Sep 15, 2022

Conversation

runcom
Copy link
Member

@runcom runcom commented Aug 31, 2022

backport of #2861

Change partition tables on edgeBase images to use 'LVM partitioning'.
We need to ensure that LVM stages are done before LUKS stages (e.g. remove-key) or the pipelines will break (we cannot open a device when its password has changed).

This PR requires changes on coreos-installer-dracut/dracut/scripts/coreos-installer-growfs(coreos/coreos-installer-dracut#9) required by rhel90-edge images to account for the change from traditional partitioning to LVM, otherwise the image will boot on an emergency shell and will need a manual reboot.

This pull request includes:

  • adequate testing for the new functionality or fixed issue
  • adequate documentation informing people about the change such as

7flying and others added 2 commits August 31, 2022 15:18
Change partition tables on edgeBase images to use
'LVM partitioning'. We need to ensure that LVM
stages are done before LUKS stages (e.g. remove-key)
or the pipelines will break (we cannot open a device
when its password has changed).

Add relevant tests on device_test.go plus a new
test partition table on common_test.go

Signed-off-by: Antonio Murdaca <runcom@linux.com>
Signed-off-by: Antonio Murdaca <runcom@linux.com>
@runcom runcom changed the title Edge lvm 91 [9.1] distro/rhel9: edge images default to LVM Aug 31, 2022
@achilleas-k achilleas-k self-requested a review September 8, 2022 09:28
henrywang and others added 2 commits September 12, 2022 11:30
In podman v4.0.0 the default network backend was switched from cni to
netavark.  However, podman will choose cni if there are already
containers, images, or cni networks preset on a system [1].

Starting with podman v4.2.0, containernetworking-plugins is no longer a
hard requirement for podman.  So when an edge commit is built with an
embedded container, podman v4.2.0+ will choose the cni network and fail
with an error because the plugin isn't installed.

Adding the package explicitly alongside podman to avoid this issue with
future RHEL 9.1 edge builds when they include containers.

This change does not affect test manifests.  The package is already
included in manifests as a dependency of podman < v4.2.0.

See rhbz#2123210

[1] https://github.com/containers/common/blob/a083f790abf21048518f2d66b7e01370cc055790/pkg/config/containers.conf#L275-L278
Since api.ocp.ci.centos.org is going away:
https://lists.centos.org/pipermail/ci-users/2022-June/004547.html.

After discussion, OCP4 test will be removed here, but RHEL for Edge
downstream test will keep OCP4 case
@runcom runcom force-pushed the edge-lvm-91 branch 2 times, most recently from a1712b1 to 1d9c15b Compare September 13, 2022 11:42
Signed-off-by: Antonio Murdaca <runcom@linux.com>
Distro version branches aren't synced to GitLab, so we will need to
fetch them from GitHub directly.
This is required for any PR made against any branch other than main.
Copy link
Collaborator

@schutzbot schutzbot left a comment

Choose a reason for hiding this comment

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

⚠️ This PR introduces changes in at least one manifest (when comparing PR HEAD b0fc6bd with the rhel-9.1.0 merge-base 29f66a2). Please review the changes. The changes can be found in the artifacts of the Manifest-diff job [0] as manifests.diff.

[0] https://gitlab.com/redhat/services/products/image-builder/ci/osbuild-composer/-/jobs/3021975780/artifacts/browse

Due to https://bugzilla.redhat.com/show_bug.cgi?id=2124735 and https://bugzilla.redhat.com/show_bug.cgi?id=2108646
we can't really go ahead with cs9 so disable it and investigate further afterwards.

Signed-off-by: Antonio Murdaca <runcom@linux.com>
Copy link
Collaborator

@schutzbot schutzbot left a comment

Choose a reason for hiding this comment

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

⚠️ This PR introduces changes in at least one manifest (when comparing PR HEAD b30aa3c with the rhel-9.1.0 merge-base 29f66a2). Please review the changes. The changes can be found in the artifacts of the Manifest-diff job [0] as manifests.diff.

[0] https://gitlab.com/redhat/services/products/image-builder/ci/osbuild-composer/-/jobs/3032401577/artifacts/browse

instead of manually updating CIV version every once in a while. Get
always the latest version.

In CIV CI, this test runs before any change can be introduced into the
container image, so no unexpedted errors should come from the CIV side.
We were using `latest` as tag, this can be dangerous as it's the default
tag, an anyone can accidentally update it. Using `prod` is safer.

Also use dev container image if the test script is running in CIV CI.
Copy link
Collaborator

@schutzbot schutzbot left a comment

Choose a reason for hiding this comment

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

⚠️ This PR introduces changes in at least one manifest (when comparing PR HEAD f40178d with the rhel-9.1.0 merge-base 29f66a2). Please review the changes. The changes can be found in the artifacts of the Manifest-diff job [0] as manifests.diff.

[0] https://gitlab.com/redhat/services/products/image-builder/ci/osbuild-composer/-/jobs/3033323991/artifacts/browse

Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

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

LGTM!

@achilleas-k achilleas-k merged commit 1df588f into osbuild:rhel-9.1.0 Sep 15, 2022
@runcom runcom deleted the edge-lvm-91 branch September 15, 2022 19:41
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

7 participants