Commit graph

1186 commits

Author SHA1 Message Date
Debarshi Ray
8454b31a82 cmd/utils, pkg/utils: Improve the error messages for the distro option
https://github.com/containers/toolbox/issues/937
https://github.com/containers/toolbox/pull/1103
2022-09-02 14:52:44 +02:00
Debarshi Ray
8ca5611942 Increase the validation coverage for the container & release options
Currently, the container name and release are only validated if they
were specified as command line options.  Neither the value of release
in the configuration file nor the container name generated from an
image are validated.

There's also a lot of repeated code in the command front-ends to
validate the container name and release.  This opens the door for
mistakes.  Any adjustment to the code must be repeated elsewhere, and
there are subtle interactions and overlaps between the validation code
and the code to resolve container and image names.

It's worth noting that the container and image name resolution happens
for both the command line and configuration file options, and generates
the container name from the image when necessary.

Therefore, validating everything while resolving cleans up the command
front-ends and increases the coverage of the validation.

This introduces the use of sentinel error values and custom error
implementations to identify the different errors that can occur while
resolving the container and images, so that they can be appropriately
shown to the user.

https://github.com/containers/toolbox/pull/1101
2022-09-02 13:11:32 +02:00
Debarshi Ray
b5474bff84 cmd, pkg/utils: Clarify the error message if the release is invalid
Currently, if --release has an invalid argument, the error message
doesn't give any hints as to what's an acceptable value.  This can be
confusing.  eg., is 36 a valid argument for Fedora?  Or is it f36?  Or
is it F36?  Is 'rawhide' accepted?

https://github.com/containers/toolbox/issues/937
https://github.com/containers/toolbox/pull/1100
2022-09-01 17:57:39 +02:00
Debarshi Ray
aead0023e3 pkg/utils: Rename a variable
This will make the subsequent commit easier to read.

https://github.com/containers/toolbox/issues/937
https://github.com/containers/toolbox/pull/1100
2022-09-01 17:57:35 +02:00
Debarshi Ray
df7e01df10 pkg/utils: Ensure that the distro CLI and config file options are valid
Currently, if an invalid or unsupported string is specified as the
distro on the command line or in the configuration file, then it would
silently fallback to Fedora.  This shouldn't happen.

It should only fallback to Fedora when no distro was specified and
there's no supported Toolbox image matching the host operating system.
If a distro was explicitly specified then it should either be supported
or it should error out.

The test cases were resurrected from commit 8b6418d8aa.

https://github.com/containers/toolbox/issues/937
https://github.com/containers/toolbox/pull/1080
2022-09-01 17:43:20 +02:00
Debarshi Ray
f5bcd22ade pkg/utils: Clarify the default and fallback values
The terms 'default' and 'fallback' are used to mean very specific
things in this context.

The 'default' values are those that are used when the 'create', 'enter'
and 'run' commands were used without any option.  These values are
picked to match the host operating system.

However, if there's no supported Toolbox image matching the host
operating system, and no options were provided to the 'create', 'enter'
and 'run' commands, then the 'fallback' values are used as a last
resort.

Consistently using this terminology leads to a clear mental model and
makes the code easier to read.

This rough arrangement of the code was already being used for
'release', and has now been been extended to 'container name prefix'
and 'distro'.  The suffix for the 'fallback' values was simplified to
'Fallback', instead of 'DefaultFallback'.

https://github.com/containers/toolbox/issues/937
https://github.com/containers/toolbox/pull/1080
2022-09-01 17:43:15 +02:00
Debarshi Ray
344dda6d8d pkg/utils: Re-unify container & image name resolution
The idea of splitting ResolveContainerAndImageNames into two public
functions [1] didn't turn out to be so useful [2].  It pushed the
burden on the callers, who needed to carefully call the split
functions in the right order, because the container, distro, image and
release values are very tightly related.  This opens the door for
mistakes.

A better approach would be to restore ResolveContainerAndImageNames as
the single public API.  If necessary it could be internally split into
smaller private functions.  It would keep things simple for the
callers.

Note that this commit doesn't include the private split.  If necessary,
it can be done in future.

This reverts commit fd756089ef.

[1] Commit fd756089ef
    https://github.com/containers/toolbox/pull/828
    https://github.com/containers/toolbox/pull/838

[2] https://github.com/containers/toolbox/pull/977

https://github.com/containers/toolbox/issues/937
https://github.com/containers/toolbox/pull/1080
2022-09-01 17:43:10 +02:00
Debarshi Ray
0e66af91fe Revert "cmd, pkg/utils: Split distro and release parsing and ..."
The idea of splitting ResolveContainerAndImageNames into two public
functions [1] didn't turn out to be so useful [2].  Splitting things
even further might make it worse.  A better approach might be to
(re-)unify the code further.

This is the first step towards that.

This reverts the following commits:
  * 5c8ad7a7ec
  * 02f45fd3f2
  * 8b6418d8aa

... but retains the test cases that were not tied to the changes in
those commits.

[1] Commit fd756089ef
    https://github.com/containers/toolbox/pull/828
    https://github.com/containers/toolbox/pull/838

[2] https://github.com/containers/toolbox/pull/977

https://github.com/containers/toolbox/issues/937
https://github.com/containers/toolbox/pull/1080
2022-09-01 17:43:04 +02:00
Debarshi Ray
4f78c5ef86 cmd/utils, test/system: Tweak an error message for consistency
Barring the first line, all other lines are terminated with a full stop
elsewhere.

https://github.com/containers/toolbox/pull/1099
2022-09-01 13:02:40 +02:00
Debarshi Ray
59f71219b7 pkg/utils: Mark a private function as such
Figuring out the container name prefix for a given image only needs to
happen as part of resolving the final Toolbox container name from the
given command line and configuration options.

Fallout from c990fb43ca

https://github.com/containers/toolbox/pull/1098
2022-08-31 21:06:51 +02:00
Debarshi Ray
e6c9c0c925 pkg/utils: Be more strict about what is acceptable
https://github.com/containers/toolbox/pull/1097
2022-08-31 20:26:21 +02:00
Debarshi Ray
b864280e42 playbooks: Make all Meson warnings fatal
This should help detect the kind of problem that was fixed in the
previous commit.

https://github.com/containers/toolbox/pull/1096
2022-08-31 19:35:05 +02:00
Debarshi Ray
1567d207c6 build: Silence a WARNING
Otherwise, Meson complains:
  completion/meson.build:4: WARNING: Project targeting '>= 0.58.0' but
    tried to use feature deprecated since '0.56.0':
    dependency.get_pkgconfig_variable. use
    dependency.get_variable(pkgconfig : ...) instead

Fallout from bafbbe81c9

https://github.com/containers/toolbox/pull/1096
2022-08-31 19:35:01 +02:00
Debarshi Ray
4dd73ad160 .zuul, playbooks: Run unit tests on -Dmigration_path_for_coreos_toolbox
The -Dmigration_path_for_coreos_toolbox option enables a different code
path that's currently not tested by the CI at all.  In fact, since it's
a build-time option, the corresponding code path is not even built by
the CI.

To properly support the -Dmigration_path_for_coreos_toolbox option, it
needs to be covered by the CI.  This is a step in that direction by
running the unit tests on it.

https://github.com/containers/toolbox/pull/1095
2022-08-31 13:42:40 +02:00
Debarshi Ray
f3a15c60fe playbooks: Split out the post-configuration steps into a separate file
A subsequent commit will introduce builds performed with the
-Dmigration_path_for_coreos_toolbox option to the CI.  It will be good
to avoid duplicating the build and installation steps for builds with
and without the -Dmigration_path_for_coreos_toolbox option.

https://github.com/containers/toolbox/pull/1095
2022-08-31 13:19:33 +02:00
Debarshi Ray
e965dac9f6 playbooks: Split out the dependencies into a separate file
A subsequent commit will introduce builds performed with the
-Dmigration_path_for_coreos_toolbox option to the CI.  It will be good
to avoid duplicating the installation of RPM packages, Git submodule
handling, and the listing of various debug and version information for
builds with and without -Dmigration_path_for_coreos_toolbox option.

https://github.com/containers/toolbox/pull/1095
2022-08-31 12:46:44 +02:00
Debarshi Ray
6f4e8b97dd Link to the installation guide from the shell Toolbox
This will provide a path forward to those who stumble across the POSIX
shell implementation and don't know how to use the Go implementation.

https://github.com/containers/toolbox/pull/1094
2022-08-29 22:09:15 +02:00
Debarshi Ray
56a18d2a15 Revert "build: Drop ShellCheck on Shell Toolbox"
The subsequent commit will touch the POSIX shell implementation, and
hence ShellCheck needs to be run on it.

As long as the POSIX shell implementation is part of the Git repository,
ShellCheck needs to keep running on it, unless it causes some serious
problems.  The ShellCheck test is very fast, and the reassurance and
mental peace that it provides is invaluable.

This reverts commit 8c1d441916.

https://github.com/containers/toolbox/pull/1094
2022-08-29 22:09:07 +02:00
Debarshi Ray
486762925d cmd: Refactor common code into a function
https://github.com/containers/toolbox/pull/1093
2022-08-29 17:49:00 +02:00
Debarshi Ray
a9a5b96ec6 test/system: Fix typo
This isn't causing any problems at the moment.  However, the test can
break if the order in which the command line arguments are validated
changes.  eg., if the presence of a command is checked before the
release, then the error message will be different.

Fallout from 8b6418d8aa

https://github.com/containers/toolbox/pull/1091
2022-08-26 17:00:56 +02:00
Debarshi Ray
afb002c90c .mailmap: Canonicalize my email
From now on, Debarshi Ray <debarshir@gnome.org> will show up as
Debarshi Ray <rishi@fedoraproject.org>.

Toolbox isn't quite a GNOME project (it doesn't use elements from the
GNOME platform, like GLib), even though it's part of the same
ecosystem and many Toolbox contributors are also GNOME contributors.

Toolbox was conceived to improve the developer experience on Fedora
Silverblue, and expanded over time to cover other use-cases (eg.,
troubleshooting the operating system) and Fedora editions (eg., CoreOS
and Workstation).  Even though there's a growing number of users on
other distributions, they are not the primary reason for Toolbox to
exist.

Toolbox heavily depends on Podman, and as a result is more aligned with
the Containers organization on GitHub than anything else, which is
driven, to a large degree, by Fedora contributors.

Hence, my desire to use my Fedora identity.

https://github.com/containers/toolbox/pull/1083
2022-08-01 18:37:43 +02:00
Debarshi Ray
978bb524e4 test/system: Silence warning with Bats >= 1.7.0
Bats 1.7.0 emits a warning if a command passed to 'run' returns with an
exit code of 127 [1]:
  BW01: `run`'s command `/opt/bin/toolbox run non-existent-command`
    exited with code 127, indicating 'Command not found'. Use run's
    return code checks, e.g. `run -127`, to fix this message.
        (from function `run' in file
          /usr/lib/bats-core/test_functions.bash, line 299,
         in test file test/system/104-run.bats, line 148)

This requires Bats >= 1.5.0, which is present in Fedora >=35, and
supports specifying the exit code as an argument to Bats' 'run'
command [2].

However, bats_require_minimum_version can't be used, because it's
only available from Bats 1.7.0, which is new enough that it's absent
from Fedora 35.

[1] Bats commit c6dc2f88361a4f5b
    https://github.com/bats-core/bats-core/issues/547
    https://bats-core.readthedocs.io/en/stable/warnings/BW01.html

[2] https://github.com/bats-core/bats-core/pull/367
    https://github.com/bats-core/bats-core/pull/507
    https://bats-core.readthedocs.io/en/stable/writing-tests.html

[3] Bats commit 71d6b71cebc3d32b
    https://github.com/bats-core/bats-core/issues/556
    https://bats-core.readthedocs.io/en/stable/warnings/BW02.html

https://github.com/containers/toolbox/pull/1081
2022-08-01 10:47:13 +02:00
Debarshi Ray
03daa8603f .zuul: Drop testing on Fedora 34
Fedora 34 reached End of Life on 7th June 2022:
https://docs.fedoraproject.org/en-US/releases/eol/

The subsequent commit will bump the minimum required Bats version to
1.5.0, which is absent from Fedora 34.

https://github.com/containers/toolbox/pull/1081
2022-08-01 10:47:03 +02:00
Ondřej Míchal
4469774fb1 cmd/completion: Add prefix to command to hide it better
https://github.com/containers/toolbox/pull/1051
2022-05-13 16:33:32 +03:00
Timothée Ravier
e80cba4d3e Images: Create F37 toolbox image from F36
https://github.com/containers/toolbox/pull/1012
2022-03-21 00:26:10 +02:00
Ondřej Míchal
958a2c91af ci: Enable testing on Fedora 36 2022-03-21 00:06:41 +02:00
Oliver Gutierrez
f8e21a31b3 cmd/run, root: Exit with exit code of invoked command
When a command is executed with toolbox run and it returns a non-zero
exit code, it is just ignored if that exit code is not handled. This
prevents users to identify errors when executing commands in toolbox.

With this fix, the exit codes of the invoked command are propagated
and returned by 'toolbox run'. This includes even exit codes returned
by Podman on error.

https://github.com/containers/toolbox/pull/1013

Co-authored-by: Ondřej Míchal <harrymichal@seznam.cz>
2022-03-21 00:05:45 +02:00
Ondřej Míchal
7cba807e45 cmd/run: Launch command with stderr being attached
Without stderr being attached stderr output of the invoked command goes
into stdout.

Behaviour before:
; output="$(toolbox run /etc)"
Error: failed to invoke command /etc in container <name-of-container>
; echo -e "$output"
/bin/sh: line 1: /etc: Is a directory
/bin/sh: line 1: exec: /etc: cannot execute: Is a directory

Behaviour after:
; output="$(toolbox run /etc)"
/bin/sh: line 1: /etc: Is a directory
/bin/sh: line 1: exec: /etc: cannot execute: Is a directory
Error: failed to invoke command /etc in container <name-of-container>
; echo -e "$output"

https://github.com/containers/toolbox/pull/1013
2022-03-21 00:05:45 +02:00
Ondřej Míchal
a22d7821cb cmd/run: Don't allocate pseudo-TTY if not connected to terminal
Passing '--tty' to 'podman exec' unconditionally causes Podman to
allocate a pseudo-TTY for the command execution. This causes problems
with piping (values not being piped in and values being piped out with
carriage return at the end of a line). The solution is to track the
presence of a terminal on stdin/stdout and based on its presence use the
'--tty' flag.

Original behaviour:
; echo foo | toolbox run less

; toolbox echo foo | od -c
0000000   f   o   o  \r  \n
0000005

New behaviour:
; echo foo | toolbox run less
foo

; toolbox echo foo | od -c
0000000   f   o   o  \n
0000004

As seen in the 'Piping in' example, the value gets only printed into
stdout. Not ideal from the point of view of using 'less' (or similar
tools) but still a move forward.

Based on a discussion in Podman's bugtracker[0].

Fixes https://github.com/containers/toolbox/issues/157
Fixes https://github.com/containers/toolbox/issues/848

[0] https://github.com/containers/podman/issues/9718

https://github.com/containers/toolbox/pull/1013
2022-03-21 00:05:45 +02:00
Ondřej Míchal
9d3601e0a6 .zuul: Use labels/nodes without the '-small' suffix 2022-03-20 23:40:58 +02:00
Ondřej Míchal
9bffbb7e13 Revert "ci: Build & Publish Fedora Toolbx images with GitHub Packages"
This revert is done based on discussion happening around the PR that
originally added the change[0].

This reverts commit 818748001c.

[0] https://github.com/containers/toolbox/pull/973

https://github.com/containers/toolbox/pull/1028
2022-03-20 21:11:32 +02:00
Ondřej Míchal
ecd1ced719 cmd/create: Add option --authfile
The option accepts a path to a file that is passed to an internal call
to 'podman pull' via the '--authfile' option. This will make it easier
to pull images from registries with authentication in-place.

Fixes https://github.com/containers/toolbox/issues/689

https://github.com/containers/toolbox/pull/935
2022-03-20 18:08:42 +02:00
Ondřej Míchal
192b9c8265 test/system: Mention where to find available environmental variables
https://github.com/containers/toolbox/pull/1024
2022-03-20 17:11:33 +02:00
Ondřej Míchal
0cdc6b8305 test/system: Add a section about test writing guidelines
https://github.com/containers/toolbox/pull/1024
2022-03-20 17:11:33 +02:00
Ondřej Míchal
dfc38b156c test/system: Adjust the return value of container start test
The output of 'podman logs' does not need to be returned as it can already
be seen in the logs when bats is properly configured.

https://github.com/containers/toolbox/pull/1024
2022-03-20 17:11:33 +02:00
Ondřej Míchal
fc71b9de34 test/system: Skip caching of an image if it is already cached
https://github.com/containers/toolbox/pull/1024
2022-03-20 17:11:33 +02:00
Ondřej Míchal
754c6fb4d1 test/system: Set custom runtime root for rootless Podman
To more completely separate the test environment Podman runtime from the
default system one set also a custom runtime directory.

https://github.com/containers/toolbox/pull/1024
2022-03-20 17:11:33 +02:00
Ondřej Míchal
813f971181 test/system: Don't cleanup tests by resetting Podman
Calling 'podman system cleanup' causes problems with containers/images
in a separate Podman root. Despite being stored elsewhere, they are
still under Podman's influence and the cleanup removes them. Also,
running containers (outside the scope of the tests) still got affected
by this call and e.g., lost the ability to follow terminal size changes.

Despite the raised concerns, to ensure proper cleanup of any Podman
state, the reset still needs to be done. Thus, do it only once during
the test suite teardown, moving the potential source of problems to a
single position..

https://github.com/containers/toolbox/pull/1024
2022-03-20 17:11:33 +02:00
Ondřej Míchal
5012cda506 test/system: Minor fixes & renaming
https://github.com/containers/toolbox/pull/1024
2022-03-20 17:11:33 +02:00
Ondřej Míchal
5c27d73021 .zuul: Run system tests when they are changed
Fallout from c28d902089

https://github.com/containers/toolbox/pull/1024
2022-03-20 17:11:33 +02:00
Ondřej Míchal
bafbbe81c9 Generate & install completion scripts in build system
The previous commit added a means to generating the completion scripts
and this one plugs that into the build system.

A new build option 'install_completions' has been introduced. Set to
'True' by default.

Completions for bash and fish use pkg-config for getting the preferred
install locations for the completions. If the packages are not
available, fallbacks are in-place.

The 'completion' subdir has been kept to work around the ideology of
Meson that does not allow creating/outputing files in subdirectories nor
using the output of custom_target() in install_data().

https://github.com/containers/toolbox/pull/840
2022-02-21 15:15:30 +02:00
Oliver Gutierrez
d69ce6794b cmd: Add shell completion command & generate completion
Cobra (the CLI library) has an advanced support for generating shell
completion. It support Bash, Zsh, Fish and PowerShell. This offering
covers the majority of use cases with some exceptions, of course.

The generated completion scripts have one behavioral difference when
compared to the existing solution: flags (--xxx) are not shown by
default. User needs to type '-' first to get the completion.

https://github.com/containers/toolbox/pull/840

Co-authored-by: Ondřej Míchal <harrymichal@seznam.cz>
2022-02-21 15:15:30 +02:00
Ondřej Míchal
5c8ad7a7ec pkg/utils: Use global default instead of magic value
Found while working on https://github.com/containers/toolbox/issues/937

https://github.com/containers/toolbox/pull/977
2022-02-21 13:43:24 +02:00
Ondřej Míchal
02f45fd3f2 pkg/utils: Use newly introduced API for resolving default distro
https://github.com/containers/toolbox/pull/977
2022-02-21 13:43:24 +02:00
Ondřej Míchal
8b6418d8aa cmd, pkg/utils: Split distro and release parsing and report better errors
Using a non-supported distribution via `--distro` resulted in a silent
fallback to the Fedora image which confuses users. When a faulty release
format was used with `--release` a message without any hint about the
correct format was shown to the user.

This separates distro and release parsing into two chunks that have
greater control over error reporting and provides more accurate error
reports for the user.

Fixes https://github.com/containers/toolbox/issues/937

https://github.com/containers/toolbox/pull/977
2022-02-21 13:43:24 +02:00
pablomh
2af0b30ed3 test/system: Delete .swp file
Delete .swp file that was inadvertenly introduced in [1].

[1] 7a5f3ba2e2

https://github.com/containers/toolbox/pull/996
2022-02-07 01:10:10 +02:00
Debarshi Ray
656deb3677 Canonicalize Ondra's name
From now on, Harry Míchal <harrymichal@seznam.cz> will show up as
Ondřej Míchal <harrymichal@seznam.cz>.

https://github.com/containers/toolbox/pull/1002
2022-01-26 01:44:31 +01:00
Ondřej Míchal
8ae0f9c5c6 doc/toolbox: Add section about supported distribution images
Having a list of supported distributions in the manual has been long
overdue. Complementing it with the expected formats should make lives
of users a bit easier.

https://github.com/containers/toolbox/pull/977
https://github.com/containers/toolbox/pull/986
2022-01-13 22:20:47 +01:00
Ondřej Míchal
ffd365342e doc: Highlight that --distro has to be used with --release
...unless the selected distro matches the host system.

https://github.com/containers/toolbox/pull/977
https://github.com/containers/toolbox/pull/986
2022-01-13 22:20:43 +01:00
Jakub Steiner
4a652d1efd README: Flip the graphic's facing direction for better layout
https://github.com/containers/toolbox/pull/985
2022-01-13 21:22:23 +01:00