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

sysroot: Detect "live" system on /run/ostree-live #1995

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cgwalters
Copy link
Member

@cgwalters cgwalters commented Jan 22, 2020

Support read-only queries, but deny any attempts to make
changes with a clear error.

Now...in the future, we probably do want to support making
"transient" overlays. See:
coreos/fedora-coreos-tracker#354 (comment)

Closes: #1921

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters

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:

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

@cgwalters cgwalters changed the title WIP: ostree live support sysroot: Detect "live" system on /run/ostree-live Mar 19, 2020
@cgwalters
Copy link
Member Author

OK, fixed this up and got it to work. No CI here yet, but I did test this manually with an override and cosa buildextend-live.

I've got some thoughts/code in motion for support for testing on "live" systems - it would be most ergonomic with coreos/ignition#935

@jlebon
Copy link
Member

jlebon commented Mar 23, 2020

I think this needs a rebase too on top of fixed CI.

No functional changes; prep for a future patch
which will load the "live" deployment rather than
reading the bootloader configs.
Support read-only queries, but deny any attempts to make
changes with a clear error.

Now...in the future, we probably do want to support making
"transient" overlays.  See:
coreos/fedora-coreos-tracker#354 (comment)

Closes: ostreedev#1921
@rh-atomic-bot
Copy link

☔ The latest upstream changes (presumably #2045) made this pull request unmergeable. Please resolve the merge conflicts.

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Sorry for the delay on this. This would definitely be nice to fix!

* The existence of this file declares that the system
* is booted "live" (fully in RAM).
*
* Since: 2020.1
Copy link
Member

Choose a reason for hiding this comment

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

2020.4

* OSTREE_SYSROOT_LIVE_PATH:
*
* The existence of this file declares that the system
* is booted "live" (fully in RAM).
Copy link
Member

Choose a reason for hiding this comment

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

I think one important thing worth noting here is that unlike the seemingly similar /run/ostree-booted, this isn't written by OSTree itself. I.e. it's an agreed-upon API between OSTree and OS vendors. Maybe something like:

It is written by external code which drive OSTree in live contexts (e.g. live ISOs or live PXE boots).

?

@@ -284,6 +284,9 @@ gboolean
_ostree_sysroot_ensure_writable (OstreeSysroot *self,
GError **error)
{
if (self->root_is_ostree_live)
return glnx_throw (error, "sysroot does not support persistent changes");
Copy link
Member

Choose a reason for hiding this comment

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

Minor: how about prefixing with found /run/ostree-live?

@jlebon
Copy link
Member

jlebon commented Mar 31, 2020

Hmm OK, so just thinking about this some more and looking at the FCOS side; do we want to think more deeply about what the ideal integration would look like between the OS and libostree?

For example, right now FCOS has to play hacks with the cmdline to fool ostree-prepare-root: https://github.com/coreos/fedora-coreos-config/blob/f49ef1237d6b75f39410147001d14f594daf79cf/overlay.d/05core/usr/lib/dracut/modules.d/20live/ostree-cmdline.sh. Is this how we want things to work long-term?

I'm wondering if we should instead have a ostree=live karg or something that signals ostree-prepare-root to (1) automatically detect the only deployment in /sysroot and prepare root on that, and (2) write the /run/ostree-live stamp file.

/cc @bgilbert

@openshift-ci-robot
Copy link
Collaborator

@jlebon: GitHub didn't allow me to request PR reviews from the following users: bgilbert.

Note that only ostreedev members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Hmm OK, so just thinking about this some more and looking at the FCOS side; do we want to think more deeply about what the ideal integration would look like between the OS and libostree?

For example, right now FCOS has to play hacks with the cmdline to fool ostree-prepare-root: https://github.com/coreos/fedora-coreos-config/blob/f49ef1237d6b75f39410147001d14f594daf79cf/overlay.d/05core/usr/lib/dracut/modules.d/20live/ostree-cmdline.sh. Is this how we want things to work long-term?

I'm wondering if we should instead have a ostree=live karg or something that signals ostree-prepare-root to (1) automatically detect the only deployment in /sysroot and prepare root on that, and (2) write the /run/ostree-live stamp file.

/cc @bgilbert

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.

@cgwalters
Copy link
Member Author

I'm wondering if we should instead have a ostree=live karg or something that signals ostree-prepare-root to (1) automatically detect the only deployment in /sysroot and prepare root on that, and (2) write the /run/ostree-live stamp file.

Yeah, that seems reasonable.

I guess there's a much bigger topic around whether it's worth doing the "deployment root" for live systems at all. It might not be really hard to rework things so that libostree supports just not switching root in this case and operating in read-only mode. But OTOH, FCOS already shipped a live system built this way.

@cgwalters
Copy link
Member Author

OTOH live systems don't support in-place updates, so there aren't any backcompat constraints really; we are free to change the API between live systems and libostree.

@cgwalters
Copy link
Member Author

OK in the meantime I split off the prep patch in #2049

@cgwalters
Copy link
Member Author

cgwalters commented Apr 7, 2020

Gah the rabbit hole here is deep. So today, the FCOS live bits create an overlayfs for /etc. But I just realized that since FCOS also has the readonly sysroot bits on, ostree-remount.service would ordinarily try to set up the bind mount for /etc except it doesn't do that only because ConditionKernelCmdLine=ostree isn't met.

EDIT: Ah but that code already exits early on if / is ro, so I think that's fine.

@openshift-ci-robot
Copy link
Collaborator

@cgwalters: PR needs rebase.

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.

@openshift-ci
Copy link

openshift-ci bot commented Jun 29, 2023

@cgwalters: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/sanity 74089e4 link true /test sanity
ci/prow/fcos-e2e 74089e4 link true /test fcos-e2e
ci/prow/images 74089e4 link true /test images

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make ostree aware of live environments
4 participants