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

Remove ostree= as a hard dependancy from the systemd perspective #3190

Closed

Conversation

ericcurtin
Copy link
Collaborator

@ericcurtin ericcurtin commented Feb 21, 2024

We've been working on re-enabling our Android Boot builds since composefs integration. This problem reared it's head again. The Android Boot images don't use the "ostree=" karg at all (they alternatively use the androidboot.slot_suffix= karg to determine what OSTree to boot into). We've been working around this by faking an ostree= argument of sorts in hacky ways. Ideally we could do a ostree= or androidboot.slot_suffix= karg check but systemd doesn't support that. Checks for ostree= or androidboot.slot_suffix= are already contained in binaries such as ostree-prepare-root, etc. So there are still checks, the systemd check is too limited.

We've been working on re-enabling our Android Boot builds since composefs
integration. This problem reared it's head again. The Android Boot
images don't use the "ostree=" karg at all (they alternatively use the
androidboot.slot_suffix= karg to determine what OSTree to boot into).
We've been working around this by faking an ostree= argument of sorts in
hacky ways. Ideally we could do a ostree= or androidboot.slot_suffix=
karg check but systemd doesn't support that. Checks for ostree= or
androidboot.slot_suffix= are already contained in binaries such as
ostree-prepare-root, etc. So there are still checks, the systemd check
is too limited.

Signed-off-by: Eric Curtin <ecurtin@redhat.com>
@jmarrero
Copy link
Member

jmarrero commented Feb 21, 2024

This scares me a bit, I am not sure if doing this would affect systems with ostree installed because they use flatpaks. Like if we remove this conditional the services could trigger on cases like that? Also not sure of the historical reasons this is here.
@jlebon WDYT?

@ericcurtin
Copy link
Collaborator Author

Flatpaks are pretty orthogonal to this change, they don't use systemd

@ericcurtin
Copy link
Collaborator Author

What this inclusion basically does:

ConditionKernelCommandLine=ostree

is cause things to fail early, but in our Android Boot Images we don't use this karg, we were faking it...

We still fail when we exclude this option, just at a slightly later point.

@ericcurtin
Copy link
Collaborator Author

Using ostree via flatpak does not automatically trigger the inclusion of an ostree= karg it's worth pointing out explicitly also.

@jmarrero
Copy link
Member

jmarrero commented Feb 21, 2024

Flatpaks are pretty orthogonal to this change, they don't use systemd

What I mean is, if I am running packaged based Fedora and install ostree because of flatpaks or development or anything I am doing. Does systemd is now aware of these services or we only get them if a deployment is made.

@alexlarsson
Copy link
Member

Yeah, I don't think this is a good idea as is. It will mean these services are started on boot on any system with the ostree package installed. That is not good, these are only meant to be started when the system is booted using ostree.

@ericcurtin
Copy link
Collaborator Author

Ok, lets fix this downstream, I'll just place a blank

ostree=true

or something like this, just to satisfy this....

@ericcurtin ericcurtin closed this Feb 21, 2024
@ericcurtin ericcurtin deleted the remove-ostree-karg-condition branch February 21, 2024 12:52
@dbnicholson
Copy link
Member

We've been working on re-enabling our Android Boot builds since composefs integration. This problem reared it's head again. The Android Boot images don't use the "ostree=" karg at all (they alternatively use the androidboot.slot_suffix= karg to determine what OSTree to boot into). We've been working around this by faking an ostree= argument of sorts in hacky ways. Ideally we could do a ostree= or androidboot.slot_suffix= karg check but systemd doesn't support that. Checks for ostree= or androidboot.slot_suffix= are already contained in binaries such as ostree-prepare-root, etc. So there are still checks, the systemd check is too limited.

You can create an OR of the conditions like:

ConditionKernelCommandLine=|ostree
ConditionKernelCommandLine=|androidboot.slot_suffix

However, it's not quite expressive enough if you want to add other triggering conditions like in ostree-boot-complete.service since the desired semantics are that one of command line options is required in addition to any other triggering conditions.

Another obvious option would be a generator that can do a more thorough investigation of the environment and enable the units when needed. I also think that allowing the services to run and deferring the checks is entirely reasonable, but the programs would have to exit successfully so the service doesn't fail. For example, ostree-prepare-root currently fails if it doesn't find an appropriate ostree argument in the kernel command line.

One thing I notice is that everything except ostree-prepare-root can be changed to use ConditionPathExists=/run/ostree-booted instead of ConditionKernelCommandLine=ostree since ostree-prepare-root is the process that creates /run/ostree-booted. That doesn't entirely solve your problem, but it does focus the issue entirely to ostree-prepare-root.

@ericcurtin
Copy link
Collaborator Author

ericcurtin commented Feb 21, 2024

Thanks @dbnicholson I can go back to this if ostree=true doesn't work out...

The next problem is ostree-system-generator fstab generator expects that very specific ostree= karg (which we were hackily faking in the past)...

But... We were talking about removing /etc/fstab entirely in our Automotive distro, so I'm gonna take the opportunity to try and not execute the fstab generation stuff altogether somehow (just for Automotive to start)

@dbnicholson
Copy link
Member

One thing I notice is that everything except ostree-prepare-root can be changed to use ConditionPathExists=/run/ostree-booted instead of ConditionKernelCommandLine=ostree since ostree-prepare-root is the process that creates /run/ostree-booted. That doesn't entirely solve your problem, but it does focus the issue entirely to ostree-prepare-root.

I think a reasonable fix would be:

  • Change all the systemd units except ostree-prepare-root.service to use ConditionPathExists=/run/ostree-booted instead of ConditionKernelCommandLine=ostree.
  • Change ostree-prepare-root to exit successfully (EXIT_SUCCESS rather than EXIT_FAILURE) when the ostree target can't be determined from the kernel command line. I'd keep the errx calls but rephrase them as warnings. This would unfortunately leave an error message in the initrd logs when the ostree dracut module is enabled but an ostree deployment isn't being booted. Hopefully it's rare that the dracut module is being enabled on systems that don't intend to boot from ostree. In Debian, for instance, all the boot components are in a separate ostree-boot package to allow installing the library and CLI without implying that the system will boot an ostree deployment. I also see in the Fedora spec file that the dracut configuration enabling the ostree module isn't packaged. Presumably the dracut module is enabled externally for CoreOS and Silverblue. There's a --with-dracut=yesbutnoconf build option just for this scenario.
  • Remove ConditionKernelCommandLine=ostree from ostree-prepare-root.service.

@dbnicholson
Copy link
Member

The next problem is ostree-system-generator fstab generator expects that very specific ostree= karg (which we were hackily faking in the past)...

Looks like that was just changed to exit gracefully in 15ec339.

@ericcurtin
Copy link
Collaborator Author

The next problem is ostree-system-generator fstab generator expects that very specific ostree= karg (which we were hackily faking in the past)...

Looks like that was just changed to exit gracefully in 15ec339.

Yeah it was... If we get this in:

https://github.com/ostreedev/ostree/pull/3192/files

The Android Boot environment matches the UEFI one as close as we possibly can, I've worked around this in the past by using downstream hacks. The above fixes a lot of these hacks...

@cgwalters
Copy link
Member

Change all the systemd units except ostree-prepare-root.service to use ConditionPathExists=/run/ostree-booted instead of ConditionKernelCommandLine=ostree.

Yep, I think this would be a generally good idea to reduce our dependency on the ostree kernel argument.

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

5 participants