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.

Sign up for GitHub

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

Jump to bottom

rhel8/9: make edge images properly sysroot.readonly=true #3178

Merged
merged 2 commits into from
Dec 13, 2022
Merged

rhel8/9: make edge images properly sysroot.readonly=true #3178

merged 2 commits into from
Dec 13, 2022

Conversation

achilleas-k
Copy link
Member

Cherry-picked and adapted from @runcom's #3053 on top of #3166.

@achilleas-k achilleas-k requested review from runcom, teg and thozza December 11, 2022 14:15
schutzbot
schutzbot reviewed Dec 11, 2022
View reviewed changes
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 0f22249 with the main merge-base 19ec3be). 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/3455850995/artifacts/browse

thozza
thozza reviewed Dec 12, 2022
View reviewed changes
Copy link
Member

@thozza thozza left a comment

Choose a reason for hiding this comment

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

There was not enough information in the commit message nor in the code to answer some of the questions that popped up, so I added them in-line...

internal/distro/rhel9/images.go Show resolved Hide resolved
internal/distro/rhel9/images.go Show resolved Hide resolved
internal/distro/rhel9/images.go Outdated Show resolved Hide resolved
@achilleas-k
Copy link
Member Author

achilleas-k commented Dec 12, 2022 •
edited
Loading

Thanks @thozza. I added a new commit that just adds a comment to all the places where we set the "rw" kernel option explaining why it's needed since it's a bit counter-intuitive.

EDIT: Actually, I think I'm going to squash it into the first commit instead.

schutzbot
schutzbot reviewed Dec 12, 2022
View reviewed changes
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 36344de with the main merge-base 19ec3be). 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/3459146222/artifacts/browse

runcom and others added 2 commits December 12, 2022 15:22
@runcom @achilleas-k
distro/rhel: make edge images properly sysroot.readonly=true
99a17f8 
Make edge raw images (both the raw image type and the same in the edge
simplified installer) mount the /sysroot as read-only.  This was already
done in Fedora 37+ (547f7a6).

Copied commit message from 6f89e9d to a
comment in all places where the accompanying "rw" kernel option is set
explaining the requirement, since the option is counter-intuitive.

Signed-off-by: Antonio Murdaca <[email protected]>
Signed-off-by: Antonio Murdaca <[email protected]>
Co-Authored-By: Achilleas Koutsou <[email protected]>
@achilleas-k
test: update manifests for edge-raw-image and edge-simplified-installer
5f883da
thozza
thozza approved these changes Dec 12, 2022
View reviewed changes
Copy link
Member

@thozza thozza left a comment

Choose a reason for hiding this comment

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

👍

schutzbot
schutzbot reviewed Dec 12, 2022
View reviewed changes
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 5f883da with the main merge-base 19ec3be). 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/3460244717/artifacts/browse

@thozza thozza merged commit 66ce7e3 into osbuild:main Dec 13, 2022
@7flying 7flying mentioned this pull request Dec 13, 2022
Merged
8 tasks
@achilleas-k achilleas-k deleted the edge/sysroot-ro branch December 13, 2022 15:34
henrywang added a commit to henrywang/rhel-edge that referenced this pull request Feb 18, 2023
@henrywang
Set /sysroot RO on simplified installer and raw image test for
b93f601 
RHEL 8.8 and CS8

The RO setting on RHEL 8.8 and CS8 is not configured by ostree,
but osbuild-composer by PR
osbuild/osbuild-composer#3178
henrywang added a commit to virt-s1/rhel-edge that referenced this pull request Feb 19, 2023
@henrywang
Set /sysroot RO on simplified installer and raw image test for
22a1156 
RHEL 8.8 and CS8

The RO setting on RHEL 8.8 and CS8 is not configured by ostree,
but osbuild-composer by PR
osbuild/osbuild-composer#3178
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@schutzbot schutzbot schutzbot left review comments

@teg teg teg left review comments

@thozza thozza thozza approved these changes

@runcom runcom Awaiting requested review from runcom

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@achilleas-k @teg @thozza @schutzbot @runcom