Skip to content

reinstall: Ensure podman is installed #1105

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
Merged
merged 3 commits into from
Feb 12, 2025
Merged

reinstall: Ensure podman is installed #1105

merged 3 commits into from
Feb 12, 2025

Conversation

omertuc
Copy link
Contributor

@omertuc omertuc commented Feb 11, 2025 •
edited
Loading

See commit messages

ensure_podman_installed_flowchart

Fixes #1104

@omertuc omertuc requested a review from cgwalters February 11, 2025 11:25
@omertuc omertuc changed the title See commit messages reinstall: Ensure podman is installed Feb 11, 2025
@omertuc omertuc added the area/system-reinstall-bootc Issues related to system-reinstall-botoc label Feb 11, 2025
@omertuc omertuc requested a review from vrothberg February 11, 2025 11:26
vrothberg
vrothberg approved these changes Feb 11, 2025
View reviewed changes
Copy link
Contributor

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM, nice work. Thank you!

cgwalters
cgwalters approved these changes Feb 11, 2025
View reviewed changes
Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

I'm OK with this as is and my comments can be followups if preferred

@omertuc
Copy link
Contributor Author

omertuc commented Feb 12, 2025

Settled on: ensure_podman_installed_flowchart

@omertuc omertuc force-pushed the ep branch 5 times, most recently from 70bc91b to b66d5b1 Compare February 12, 2025 11:03
@omertuc
Copy link
Contributor Author

omertuc commented Feb 12, 2025

If we skip an internal prompt of "Would you like to install podman using your package manager?", the experience is a bit jarring:

straight.to.dnf.mp4

What are your thoughts?

@cgwalters
Copy link
Collaborator

cgwalters commented Feb 12, 2025 •
edited
Loading

If we skip an internal prompt of "Would you like to install podman using your package manager?", the experience is a bit jarring:

Agree it is.

But I'm uncertain if we really need to ask them to do this. We already warn them the system will be reinstalled right? Under what scenario are they going to say "no" to installing podman?

So I'm thinking after that after the "system will be reinstalled" prompt we could just run this.

@omertuc
logging: initialize_tracing should max WARN
4ac494a 
Warnings should always appear, so we should set the max level to WARN.

Signed-off-by: Omer Tuchfeld <[email protected]>
@omertuc
utils: Don't use run in run_with_cmd_context
52b8a92 
`run` hides stderr until the command fails, which is not what we want
for all current users of `run_with_cmd_context` - it's currently used
by long running processes where we want the users to see all the output
live.

Signed-off-by: Omer Tuchfeld <[email protected]>
@omertuc
reinstall: Ensure podman is installed
d707b57 
Fixes bootc-dev#1104

Make the podman dependency of system-reinstall-bootc optional

* Change the spec file to recommend podman instead of requiring it (this
  will make it more palatable to have this package included in distros
  by default)

* Now that podman is only recommended, the system-reinstall-bootc binary
  must check whether podman is installed and try to install it. This is
  done by launching the install-podman script that is included with the
  system-reinstall-bootc RPM. The exact location where
  system-reinstall-bootc will look for this script is defined in the
  build environment variable `SYSTEM_REINSTALL_BOOTC_INSTALL_PODMAN_PATH`

Signed-off-by: Omer Tuchfeld <[email protected]>
@omertuc
Copy link
Contributor Author

omertuc commented Feb 12, 2025

If we skip an internal prompt of "Would you like to install podman using your package manager?", the experience is a bit jarring:

Agree it is.

But I'm uncertain if we really need to ask them to do this. We already warn them the system will be reinstalled right? Under what scenario are they going to say "no" to installing podman?

So I'm thinking after that after the "system will be reinstalled" prompt we could just run this.

Following your comment here is the latest experience:

exp.mp4

@omertuc omertuc enabled auto-merge February 12, 2025 13:06
@omertuc omertuc requested a review from cgwalters February 12, 2025 13:06
cgwalters
cgwalters approved these changes Feb 12, 2025
View reviewed changes
Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

The demos and flowcharts are really helpful for review BTW, nice work!

@cgwalters
Copy link
Collaborator

CI failure is unrelated, see #1109
Forcing merge over that

@cgwalters cgwalters disabled auto-merge February 12, 2025 16:56
@cgwalters cgwalters merged commit 6a1ba9f into bootc-dev:main Feb 12, 2025
18 of 23 checks passed
@cgwalters cgwalters mentioned this pull request Apr 8, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cgwalters cgwalters cgwalters approved these changes

@vrothberg vrothberg vrothberg approved these changes

Assignees
No one assigned
Labels
area/system-reinstall-bootc Issues related to system-reinstall-botoc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

reinstall: Support installing podman
3 participants
@omertuc @cgwalters @vrothberg