It's good to document the --log-level and --log-podman flags because
they can give us some flexibility with the logging in future, but it's
still desirable to keep --verbose (and the -vv trick) in the manual.
Toolbox is still a small enough code base that not too many log levels
are actually needed, yet. The complexity of remembering which log
level reveals which detail soon starts to outweigh the simplicity of
dumping as much as possible, since there aren't that many log messages
to begin with. It's a lot easier to type and remember things like
--verbose, -v and -vv, than their newer counterparts, and they are a
reasonably widely used convention (eg., flatpak, nmap, ssh, etc.).
If some day Toolbox grows to have a significantly larger number of log
messages, then it's possible that --verbose would be of less use, but
that's not the case today.
https://github.com/containers/toolbox/pull/814
Currently, the 'enter' command involves two extra invocations of
'podman exec' to detect if the user's chosen shell and current working
directory are present inside the toolbox container. Each invocation is
sufficiently expensive to add a noticeable overhead to the 'enter' and
'run' commands. Moreover, file system operations being inherently racy,
it's always better to detect errors and handle them instead of trying
to pre-emptively avoid them.
Therefore, this shuffles the code around to attempt the non-fallback
invocation, and then handle the errors by attempting a series of
fallbacks for the command and the current working directory.
Unfortunately, in case of a missing command, capsh(1) adds an extra
error message that seems difficult to get rid of:
$ toolbox enter
/bin/sh: /bin/zsh: No such file or directory
Error: command /bin/zsh not found in container fedora-toolbox-34
Using /bin/bash instead.
https://github.com/containers/toolbox/pull/813
This will be used by the subsequent commit to optimize the 'enter' and
'run' commands in the non-fallback case, by attempting the fallback
only if an error was encountered by the main 'podman exec' invocation,
as opposed to pre-emptively setting up the fallback.
https://github.com/containers/toolbox/pull/813
The reason for setting FParseErrWhitelist.UnknownFlags to 'true' was to
prepare for a future when the 'init-container' command would have fewer
options than it does now.
However, there's no need to prepare for it, because the version of
toolbox(1) that's bind mounted into the container is the same as the
one on the host. So, FParseErrWhitelist.UnknownFlags can be set in
future if, or when, the number of flags do get reduced.
This reverts commit 5c2086e9ea.
https://github.com/containers/toolbox/pull/807
This builds upon commit eedfdda535, which added more information
to the error messages presented to the user by including the errors
thrown by the lower layers of the code.
However, if the errors are being thrown by external modules, or are
coming from functions that are too many layers below, then it is
difficult to guarantee their contents. They might be duplicating
information added by the upper layers, or in extreme cases might even
contain JSON blobs, simply because it made sense for the API contracts
of the functions generating the errors.
Therefore, it's better to put them in the debug logs to retain control
over what gets displayed to the user under normal (ie., non-debug)
operation.
https://github.com/containers/toolbox/pull/809
Even though SilenceUsage is set to 'true', to have full control over
what gets shown in the case of an error, there is still (at least?)
one occasion in which the usage function set using SetUsageFunc (ie.,
rootUsage) is used - when an unknown flag is used. For example,
'toolbox --foo'. Oddly enough, an unknown command won't lead to
rootUsage. eg., 'toolbox foo'.
Since rootUsage uses executableBase, that variable needs to be set
earlier, which means that setUpGlobals needs to run before rootUsage.
It turns out that the PersistentPreRunE hook (ie., preRun) doesn't get
invoked when an unknown flag is encountered. Therefore, we can't put
setUpGlobals inside preRun.
This reverts commit 6bbbedf675.
https://github.com/containers/toolbox/pull/802
Some people create images manually. If such created images are recognize
as toolbox images (they have the proper labels) but do not have
a name/tag then 'toolbox list' will panic due to index being out of
range.
https://github.com/containers/toolbox/pull/800
Since /etc/machine-id is bind mounted into the toolbox container from
the host operating system, it doesn't make sense to make it mandatory
for images to have that file. Apparently, (some?) Arch Linux images
don't have /etc/machine-id.
Since a missing containerPath for a directory is handled the same way,
there's no reason not to do the same for regular files. It will make
life a bit easier for those creating toolbox images for different
distributions.
https://github.com/containers/toolbox/pull/710
Errors thrown from 'toolbox init-container' are usually not shown to
the user. One has to use 'podman start --attach ...' to see them.
Therefore, it's worth adding the extra bit of information to the error.
https://github.com/containers/toolbox/pull/710
A subsequent commit will handle a missing containerPath when bind
mounting a regular file like /etc/machine-id. Therefore, it's better to
explicitly state that this code is dealing with a directory.
https://github.com/containers/toolbox/pull/710
Not having the corresponding image for UBI toolbox containers show up
in 'toolbox list' is a rough edge. However, the whole UBI feature is
a bit experimental. It's about a gratis RHEL environment getting
created in a jiffy on any host, which is something that hasn't been
done before, and those containers also suffer from various shortcomings
because of the limited package set of UBI.
So it's not that big of a problem if it takes a release or two to
hammer out the details. Especially since it's likely that there will
be a special Toolbox-specific image that's created out of the UBI RPM
repositories, which will likely have the com.github.containers.toolbox
label.
There's also the issue that 0.1.0 needs to be finished, and for that
the the churn needs to be kept down. Changing the labels can very
likely lead to compatibility issues in the future, because of which it
either can't be removed for a while or the wrong images start to get
listed. Some of the older labels have finally been removed, so it's
better not to add more to the list.
In short, this problem will likely fix itself in the coming months, so
it's wise not to create complications trying to rush through a fix.
This reverts commits 1df36591d0 and
e09de9f3e5.
https://github.com/containers/toolbox/issues/753
UBI[0] does not have the recommend Toolbox labels used to track whether
an image/container is truly a toolbox image/container. Thankfully, they
have a number of labels to choose from that we can use to identify the
image. The "com.redhat.component=ubi8-container" seems to be ideal.
The approach of using the UBI8 label introduces one problem though. If
we were to use only one set of labels for both images and containers,
containers created with Podman and not Toolbox from UBI8 would also be
marked as toolbox containers. This is not desired and therefore there
are now two sets of labels. Ones for images where the new label has been
added and other for containers that stays the same.
Since the rewrite of the system test suite[0] we've relied on the Zuul
playbooks for taking care of caching images using Skopeo for increasing
the reliability of the tests (in the past the instability of the Fedora
registry caused problems). This state is problematic if we want to use
the tests in other environments than the Zuul CI. This moves the caching
from Zuul into the system tests.
Currently, Bats does not support officially suite-wide setup and
teardown functions. The solution I chose was to add two new test files
that are executed before and after all tests. This may complicate the
execution of cherry-picked tests but that is not a very common use case
anyway.
The tests are now to some extent capable of adjusting to the host
environment. This is meant in the sense of: I'm running on RHEL, the
"default image" is UBI; I'm running on Fedora, the "default image" is
fedora-toolbox. This mechanism relies on os-release, which is the same
as what Toolbox itself uses.
[0] https://github.com/containers/toolbox/pull/517https://github.com/containers/toolbox/pull/774
The fedora-toolbox:32 image is the first of images in the renamed
toolbox image repository[0]. With the change we can drop the
pull_image_old() function because it was kept only for the old image.
Seems like newer version of ShellCheck checks the validity of variable
names (SC2153). This caused a false positive, so I silenced it.
[0] https://github.com/containers/toolbox/pull/615https://github.com/containers/toolbox/pull/780
The system test refactor[0] replaced the 'run_toolbox' helper function
with 'run toolbox', which is a normal invocation of Toolbox. This makes
it impossible to override Toolbox used during the tests using env var.
[0] https://github.com/containers/toolbox/pull/693
Instead of executing 'podman ps|images' several times in a row, call
them only once and get output with all images/containers. Then, filter
out the JSON using labels and keep images/containers only with matching
labels.
This simplifies the code significantly and cuts down the execution time
of 'toolbox list'. The speed gain is noticeable:
- the system has 5 images and 10 containers
Before patch: ~1.45s
After patch: ~0.85s
- Update "See also" sections
Toolbox does not use Buildah for a considerable time now[0]. We can stop
referencing it in the "See also" sections of the documentation.
In some places mention podman command man pages where they are relevant.
- Add section about toolbox images/containers
Toolbox only supports certain OCI images. These should be documented.
Also, document the change of fedora-toolbox image name.
- Add a section about toolbox container setup
Toolbox containers are specifically configured OCI containers. This
should be documented so that users know what they're using.
- Remove redundant part documentation
The description of what `toolbox init-container` does is already in
toolbox-init-container(1). There's no need to have it in
toolbox-create(1). Instead, replace the text with a hint to visit the
other part of documentation.
- Clarify behaviour of --image option
The fact that Toolbox by default tries to pull from the Fedora
registry[1] should be noted.
- Update synopsis & description of commands
Mention options passed to `podman exec`. Remove redundant paragraph
about container names (is already dealt with in toolbox-create(1)).
There's no need to mention the name of the default container on Fedora
since Toolbox now also supports RHEL.
Mention the default used image on unrecognised systems.
Emphasize the fact that toolboxes are not a fully sandboxed environment.
Update the wording of the description and splits it into a few
subsections.
The description of the --monitor-host was inaccurate and while the
option will go away in the future[2], it is currently in and should be
more documented.
[0] https://github.com/containers/toolbox/pull/160
[1] https://registry.fedoraproject.org
[2] https://github.com/containers/toolbox/pull/617https://github.com/containers/toolbox/pull/512
Since v0.0.91[0] Toolbox throws an error if $PWD is not available in a
toolbox. While this fixes the problem with 'toolbox enter/run' silently
failing to enter/exec in a container, it still requires an action to be
made by the user. I believe it is better to handle such situations more
gracefully by falling back to entering the user's home folder + printing
a warning about doing so.
[0] https://github.com/containers/toolbox/pull/370
Following patches were made:
- Use toolbox for listing containers/images (assumes the existence of
cut and tail)
- Suggest containers for cmd enter
- Don't suggest --container option
- Update global options
- Don't suggest cmd if already specified
The preferred way to provide of a container in commands enter & create
is via an argument.
Since the rewrite in Go, Toolbox provides the --log-level & --log-podman
options. These options deprecate the --verbose & --very-verbose options.
The completion script with this pops already used global options from
the list, handles better cases with different options and suggests log
levels for the --log-level option.
Toolbox can't be used with multiple commands.
The spinner needs to be explicitly stopped before showing the example
'enter' command for using the container. Otherwise, it gets misprinted:
$ toolbox create foo
Creating container foo: / Created container: foo
Enter with: toolbox enter foo
A comment was added to highlight this, since it might not be obvious at
first sight.
Due to such potential quirks, it might be better to keep the spinner
somewhat tightly encapsulated with the code that necessitates it, which
in this case is 'podman create'. For instance, we already need to be
careful to avoid enclosing the pullImage function with a spinner
because it carries it's own.
The code lying between the 'podman pull' and the 'podman create' is so
light that a human user isn't able to discern the absence of a
spinner. So, it seems worth leaning towards ease of understanding and
avoiding potential traps.
This reverts commit 3aaa1d30f1.
https://github.com/containers/toolbox/pull/746
Shell Toolbox has been replaced by the Go implementation a quite while
ago. It is kept in the repository but is no longer actively developed.
There is no need to continue checking it with ShellCheck.
https://github.com/containers/toolbox/pull/733
Call "meson builddir" makes Meson create a build directory called
"builddir". It does not make it build the project. A subsequent call to
"meson compile" or "ninja" needs to be made. This subtle detail causes
a minor (purely visual) discrepancy in the CI output. Fix this for both
unit-test & system-test job definitions.
