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

bootc: Add support for --transient #2155

Open
wants to merge 1 commit into
base: bootc
Choose a base branch
from

Conversation

evan-goode
Copy link
Member

This is just a proof-of-concept. I am not sure whether we want to implement the bootc support this way or keep the original plan of keeping things contained to a DNF plugin. But this approach has the big advantage that we get all DNF operations (builddep, install, remove, upgrade, everything) working in "transient mode" for free without reimplementing any DNF commands within the bootc plugin.

Adds support for the --transient option on all transactions. Passing --transient on a bootc system will call bootc usr-overlay to create a transient writeable /usr and continue the transaction.

Specifying --transient on a non-bootc system will throw an error; we don't want to mislead users to thinking this feature works on non-bootc systems.

If --transient is not specified and the bootc system is in a locked state, the operation will be aborted and a message will be printed suggesting to try again with --transient.

@pep8speaks
Copy link

pep8speaks commented Nov 7, 2024

Hello @evan-goode! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-11-12 19:59:46 UTC

@evan-goode
Copy link
Member Author

Requires rpm-software-management/libdnf#1678.

Copy link
Contributor

@dcantrell dcantrell left a comment

Choose a reason for hiding this comment

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

Since we discussed transient mode is kind of the only workable option, does it need to be exposed as a CLI option? If we're trying to lay down groundwork for future dnf5 handling, that's fine too.

I do like modifying the main command this way, even though it bumps part out in anothter package. I had originally suggested a hybrid approach like that, but I think everyone was more comfortable with the plugin approach.

I would also walk this patch set and pull out common strings and replace them with macros or variable names. I do this with the thinking that down the road we will need additional or different name support for things like /usr or /bootc. Put all those strings in one place and macro name them then get rid of the multiple string duplications.

@evan-goode
Copy link
Member Author

I changed the libdnf option to persistence with the values auto, transient, and persist, with auto being the default. We could possibly rename this to bootc_behavior or something.

Currently, persistence=persist throws an error on bootc systems (locked or unlocked) and persistence=transient throws an error on non-bootc systems. persistence=auto will throw an error on locked bootc systems prompting the user to specify --transient, and proceed as usual on unlocked bootc systems.

Thinking it over some more, I think we should make the user explicitly pass --transient, or set persistence=transient in dnf.conf. Some UX requirements/nice-to-haves are:

  • It should be very clear to bootc users that changes are not going to persist across reboots like they would on regular mutable systems.
  • It should be clear to non-bootc users that --transient doesn't work on their system (i.e. error message printed when they specify --transient).
  • If the usr overlay already exists, DNF should proceed without complaint, even if the user doesn't explicitly specify --transient.
  • Users should be able to make transient mode the default in dnf.conf, e.g. by setting persistence=transient.
  • In the future, if/when we add a --persist flag to DNF (4 or 5), the UX should not have to change much.

I moved a couple strings to variables, although for these cases, if we were to change the strings, we'd probably want to change the variable names also.

@evan-goode evan-goode marked this pull request as ready for review November 12, 2024 19:58
Adds support for the --transient option on all transactions. Passing
--transient on a bootc system will call `bootc usr-overlay` to create a
transient writeable /usr and continue the transaction.

Specifying --transient on a non-bootc system will throw an error; we
don't want to mislead users to thinking this feature works on non-bootc
systems.

If --transient is not specified and the bootc system is in a locked
state, the operation will be aborted and a message will be printed
suggesting to try again with --transient.
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.

3 participants