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

bootloader/zipl: Run in target deployment as container if needed #3104

Merged
merged 4 commits into from
Dec 1, 2023
Merged

bootloader/zipl: Run in target deployment as container if needed #3104

merged 4 commits into from
Dec 1, 2023

Conversation

cgwalters
Copy link
Member

sysroot: Expose deployment container executor internally

Prep for using this for zipl.

bootloader/zipl: Run in target deployment as container if needed

xref https://issues.redhat.com/browse/MGMT-16303

Basically the OCP Assisted installer has now grown code
to try to do OS updates offline post-install, and this means
we need to handle the case of running zipl from the target
root.

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Nov 30, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@cgwalters
Copy link
Member Author

cc @nikita-dubrovskii

@cgwalters
Copy link
Member Author

cgwalters commented Dec 1, 2023 •
edited
Loading

OK with this I get:

(ostree admin undeploy:120445): OSTree-DEBUG: 20:29:15.365: executing zipl in deployment root
Error: Config file '/lib/s390-tools/zipl.conf': Cannot build automenu: no IPL entries available
Looking for components in '/lib/s390-tools'
Using config file '/lib/s390-tools/zipl.conf'
error: Final bootloader swap: Child process exited with code 1

Which...hmm, looks like it's saying no BLS entries were found?
EDIT: Right of course, we're not mounting /boot in the deployment root in this scenario. Will fix

@cgwalters cgwalters marked this pull request as ready for review December 1, 2023 14:22
@cgwalters
Copy link
Member Author

OK, we now correctly bind /boot, and this appears to work as far as successfully running zipl!

nikita-dubrovskii
nikita-dubrovskii approved these changes Dec 1, 2023
View reviewed changes
Copy link
Contributor

@nikita-dubrovskii nikita-dubrovskii left a comment

Choose a reason for hiding this comment

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

LGTM, thx for this!

src/libostree/ostree-bootloader-zipl.c
{
g_debug ("executing zipl in deployment root");
g_autofree char *deployment_path
= ostree_sysroot_get_deployment_dirpath (self->sysroot, target_deployment);
Copy link
Contributor

Choose a reason for hiding this comment

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

As i see, this function uses bwrap but doesn't map/bind /boot , so zipl uses its default zipl.conf. This could be also avoided by using --blsdir option.

@cgwalters
Copy link
Member Author

I've invited @nikita-dubrovskii to be a collaborator, but someone else with commit rights needs to approve... @jmarrero ?

@jmarrero
Copy link
Member

jmarrero commented Dec 1, 2023 

   CCLD     ostree
/usr/bin/ld: ./.libs/libostree-1.so: undefined reference to `_ostree_sysroot_run_in_deployment'
collect2: error: ld returned 1 exit status
make[2]: *** [Makefile:4466: ostree] Error 1
make[2]: Leaving directory '/__w/ostree/ostree'
make[1]: *** [Makefile:8202: all-recursive] Error 1
make[1]: Leaving directory '/__w/ostree/ostree'
make: *** [Makefile:3345: all] Error 2
Error: Process completed with exit code 2.

failure seems legit re-running just in case.

@cgwalters
sysroot: Expose deployment runner outside of selinux
aa635bf 
We will use it in e.g. s390x zipl backend.
@cgwalters
bootloader/zipl: Run in target deployment as container if needed
c3aa295 
xref https://issues.redhat.com/browse/MGMT-16303

Basically the OCP Assisted installer has now grown code
to try to do OS updates offline post-install, and this means
we need to handle the case of running zipl from the target
root.
@cgwalters
Copy link
Member Author

Should be fixed, let's see

jmarrero
jmarrero approved these changes Dec 1, 2023
View reviewed changes
Copy link
Member

@jmarrero jmarrero left a comment

Choose a reason for hiding this comment

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

lgtm

@cgwalters cgwalters enabled auto-merge December 1, 2023 20:15
@cgwalters cgwalters merged commit 15d0777 into ostreedev:main Dec 1, 2023
24 checks passed
@cgwalters cgwalters mentioned this pull request Jun 7, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikita-dubrovskii nikita-dubrovskii nikita-dubrovskii approved these changes

@jmarrero jmarrero jmarrero approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@cgwalters @jmarrero @nikita-dubrovskii