Podman script addition #188

psstoyanov · 2022-09-02T20:16:23Z

Basic Podman helpers:

Check if base NixOS image is present
Check if Volume is present

Can be executed in rootless mode following the Podman docs: https://podman.io/getting-started/installation#linux-distributions

No changes made to the existing Docker script

psstoyanov · 2022-09-02T20:18:49Z

Cleaning up afterwards will be nice but I'm not sure how to do that yet (or re-using the existing container/volume)

samueldr · 2022-09-02T20:29:03Z

Cleaning up afterwards will be nice but I'm not sure how to do that yet (or re-using the existing container/volume)

We probably don't want to clean-up, at least not the Nix store volume, as it reduces build time for further invocations. Though it might be relevant to document how to clean it up?

Can someone who knows podman also review and chime in? Thanks!

PureTryOut · 2022-09-03T12:11:10Z

What's your reason for making an entirely different script rather than just changing the 2 Docker commands in the existing script? You can just detect if Podman is installed and if so prefer that one, including the extra command to check for existing volumes.

psstoyanov · 2022-09-03T12:26:27Z

@PureTryOut , separating the two at least for now seems like the safest approach. I don't want to break something that is already working.
I haven't setup Docker to check if it would work with those changes either.

I don't know how readable it would be to hide what is used behind another variable (or how that var would be named). Although it might have the benefit of passing info to the user early as of what can be used 🤔

Note:
I haven't tried to pass which command to be used for other commands in sh script before.

I'm sure there is more that can be improved but it's a start.

psstoyanov · 2022-09-04T18:07:08Z

Alright, complete revamp of the previous iteration, @samueldr

@PureTryOut - now both docker and podman are used within the same script, I hope this will be satisfactory and easy to read.

Added:

Check if the script has been executed as root user
Selection between docker && podman
Same image && volume commands for docker
Errors for the selection
Call build.sh script

Things which I'm uncertain if I implemented correctly:

Exit codes
Style of promps

Other:

Feedback from other systems (MacOS?) will be good
I'm not using docker much so another look there from other users is preferable

Nice to have but most likely for a different PR:

Adding board selection based on the dirs in boards?

support/docker/build.sh

samueldr · 2022-11-03T22:06:51Z

support/docker/build.sh

+	# Check if docker is available
+	if ! command -v docker &> /dev/null;
+	then
+		stderr.printf "Docker not installed\n"
+	else
+		availableManagers+=('docker')
+	fi
+
+
+	# Check if podman is available
+	if ! command -v podman &> /dev/null;
+	then
+		stderr.printf "Podman not installed\n"
+	else
+		availableManagers+=('podman')
+	fi


We might want to add to unavailableManagers and print the list of unavailable managers when availableManagers is empty.

Otherwise we have a spurious non-actionable warning/logging printed.

Or just add to availableManagers in an if without else and not log it, since the error message currently lists the supported options.

Does this behave okay in cases where docker is symlinked to podman? As far as I'm aware it's not uncommon to have a docker executable that just calls podman.

samueldr · 2022-11-03T22:08:27Z

support/docker/build.sh

+	local selectedManager
+	# Use switch statements to select from the available managers
+	stderr.printf "\nAvailable container managers on your system:\n"
+	PS3="Select one of the options or quit: "
+	select manager in ${availableManagers[*]}; do
+		case $manager in
+			docker|podman)
+				stderr.printf "\nSelected:\n $manager\n"
+				selectedManager=$manager
+				break
+				;;
+			Quit)
+				stderr.printf "\nSee you soon!\n"
+				exit 0	
+				;;
+			*)
+				stderr.printf "\nInvalid option\n"
+				;;
+		esac
+	done
+


If I understand this right, this will always stop and ask, right?

This would break the current semantics of acting like a "thick" wrapper to nix-build if I got this right.

We could prefer the first in availableManagers, if some environment variable is not set.

I have to check how to expand this functionality.

I'm thinking of "interactive" mode where the user can do the selections from above.
But to be executed without any required interaction by default.

samueldr · 2022-11-03T22:09:25Z

support/docker/build.sh

+	# Pull the image if it doesn't exists
+	if (( $checkImg == 0 ));
+	then
+		stderr.printf "\nNixOS image is present\n"
+	elif (( $checkImg == 1 ));
+	then
+		stderr.printf "\nCan\'t find NixOS image. Pulling...\n"
+		$selectedManager pull docker.io/nixos/nix
+	fi


Nit: it's the nix image, hosted under the NixOS organization. I guess to make this easier to reason about, always printing nixos/nix when referring to the image is better, since that is the identifier.

Will change it to match the expected image.

samueldr · 2022-11-03T22:11:28Z

support/docker/build.sh

+		select yn in "Yes" "No"; do
+			case $yn in
+				Yes) 
+					stderr.printf "\nProceeding as root user\n"
+					break
+					;;
+				No)
+					stderr.printf "\nQuiting the build script...\n"
+					exit 0
+					;;
+			esac
+		done


Note: This is a case where stopping and asking the user is probably fine? I don't know enough about docker/podman and friends to know if there are drawbacks that warrant stopping running as root.

While it would ideally be a good spot to always request permission, most Docker && Podman starter guides are for root setups.
Will change it to always print something visible. But remove the forced interaction by default.

support/docker/build.sh

samueldr · 2022-11-03T22:17:47Z

Sorry it took this long until I could take a look.

I left some notes about the script.

Though it's possible I missed things. Also note I haven't ran it yet.

One main consideration with the script is it should never* stop and ask things. Otherwise it'll differ too greatly from the usual nix-build commands. It should be a drop-in replacement.

*: except maybe for that thing about running as root, I don't know.

psstoyanov · 2022-11-03T23:01:34Z

Thank you for the great feedback @samueldr !
I will see to block a few hours on Saturday to go through all the notes.

At the top of my head, perhaps an interactive flag could be a solution. It would allow a flexible route when desired by the user while adhering to the "no-stop to ask" rule as a default.

Most guides for starting with Docker and Podman don't add a mention for rootless mode. Most likely to lower the barrier for entry.
Probably a prominent warning that the script is running root mode would be enough.

- Shorter sentences for larger output blocks - Provide more accurate information in the output Co-authored-by: Samuel Dionne-Riel <samuel@dionne-riel.com>

ArenM

It looks like progress on this has stalled, would it be helpful if I open up a new pr with a simplified version of this?

ArenM · 2023-10-08T12:57:04Z

support/docker/build.sh

+	# Check if docker is available
+	if ! command -v docker &> /dev/null;
+	then
+		stderr.printf "Docker not installed\n"
+	else
+		availableManagers+=('docker')
+	fi
+
+
+	# Check if podman is available
+	if ! command -v podman &> /dev/null;
+	then
+		stderr.printf "Podman not installed\n"
+	else
+		availableManagers+=('podman')
+	fi


Does this behave okay in cases where docker is symlinked to podman? As far as I'm aware it's not uncommon to have a docker executable that just calls podman.

psstoyanov added 2 commits September 2, 2022 20:56

Podman helpers

e24e161

Cleanup and fixed if image block

31d02fd

psstoyanov changed the title ~~Podman addition~~ Podman script addition Sep 2, 2022

samueldr added 4. type: enhancement Accepted new feature * help wanted Extra attention is needed labels Sep 2, 2022

psstoyanov added 2 commits September 4, 2022 18:53

V2 of podman addition

db55daf

Fix calling wrong build script

072c40d

Fix error message listing only podman

369b75c

samueldr added this to the 2022.07-006 milestone Nov 3, 2022

samueldr reviewed Nov 3, 2022

View reviewed changes

support/docker/build.sh Outdated Show resolved Hide resolved

samueldr reviewed Nov 3, 2022

View reviewed changes

support/docker/build.sh Outdated Show resolved Hide resolved

samueldr reviewed Nov 3, 2022

View reviewed changes

support/docker/build.sh Outdated Show resolved Hide resolved

support/docker/build.sh Outdated Show resolved Hide resolved

samueldr modified the milestones: 2022.07-006, 20YY.MM-007 Nov 3, 2022

docker: fixes: Wording user facing output

85a9f25

- Shorter sentences for larger output blocks - Provide more accurate information in the output Co-authored-by: Samuel Dionne-Riel <samuel@dionne-riel.com>

ArenM reviewed Oct 8, 2023

View reviewed changes

samueldr modified the milestones: 2023.07-007, 20YY.MM-008 Oct 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Podman script addition #188

Podman script addition #188

psstoyanov commented Sep 2, 2022

psstoyanov commented Sep 2, 2022

samueldr commented Sep 2, 2022 •

edited

PureTryOut commented Sep 3, 2022

psstoyanov commented Sep 3, 2022 •

edited

psstoyanov commented Sep 4, 2022 •

edited

samueldr Nov 3, 2022

ArenM Oct 8, 2023

samueldr Nov 3, 2022 •

edited

psstoyanov Nov 10, 2022

samueldr Nov 3, 2022

psstoyanov Nov 10, 2022

samueldr Nov 3, 2022

psstoyanov Nov 10, 2022

samueldr commented Nov 3, 2022 •

edited

psstoyanov commented Nov 3, 2022

ArenM left a comment

ArenM Oct 8, 2023

Podman script addition #188

Are you sure you want to change the base?

Podman script addition #188

Conversation

psstoyanov commented Sep 2, 2022

psstoyanov commented Sep 2, 2022

samueldr commented Sep 2, 2022 • edited

PureTryOut commented Sep 3, 2022

psstoyanov commented Sep 3, 2022 • edited

psstoyanov commented Sep 4, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samueldr Nov 3, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samueldr commented Nov 3, 2022 • edited

psstoyanov commented Nov 3, 2022

ArenM left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samueldr commented Sep 2, 2022 •

edited

psstoyanov commented Sep 3, 2022 •

edited

psstoyanov commented Sep 4, 2022 •

edited

samueldr Nov 3, 2022 •

edited

samueldr commented Nov 3, 2022 •

edited