We recently hit the issue that `osbuild` crashed with:
```
Unable to decode response body "Traceback (most recent call last):
File \"/usr/bin/osbuild\", line 33, in <module>
sys.exit(load_entry_point('osbuild==124', 'console_scripts', 'osbuild')())
File \"/usr/lib/python3.9/site-packages/osbuild/main_cli.py\", line 181, in osbuild_cli
r = manifest.build(
File \"/usr/lib/python3.9/site-packages/osbuild/pipeline.py\", line 477, in build
res = pl.run(store, monitor, libdir, debug_break, stage_timeout)
File \"/usr/lib/python3.9/site-packages/osbuild/pipeline.py\", line 376, in run
results = self.build_stages(store,
File \"/usr/lib/python3.9/site-packages/osbuild/pipeline.py\", line 348, in build_stages
r = stage.run(tree,
File \"/usr/lib/python3.9/site-packages/osbuild/pipeline.py\", line 213, in run
data = ipmgr.map(ip, store)
File \"/usr/lib/python3.9/site-packages/osbuild/inputs.py\", line 94, in map
reply, _ = client.call_with_fds(\"map\", {}, fds)
File \"/usr/lib/python3.9/site-packages/osbuild/host.py\", line 373, in call_with_fds
kind, data = self.protocol.decode_message(ret)
File \"/usr/lib/python3.9/site-packages/osbuild/host.py\", line 83, in decode_message
raise ProtocolError(\"message empty\")
osbuild.host.ProtocolError: message empty
cannot run osbuild: exit status 1" into osbuild result: invalid character 'T' looking for beginning of value
...
input/packages (org.osbuild.files): Traceback (most recent call last):
input/packages (org.osbuild.files): File "/usr/lib/osbuild/inputs/org.osbuild.files", line 226, in <module>
input/packages (org.osbuild.files): main()
input/packages (org.osbuild.files): File "/usr/lib/osbuild/inputs/org.osbuild.files", line 222, in main
input/packages (org.osbuild.files): service.main()
input/packages (org.osbuild.files): File "/usr/lib/python3.11/site-packages/osbuild/host.py", line 250, in main
input/packages (org.osbuild.files): self.serve()
input/packages (org.osbuild.files): File "/usr/lib/python3.11/site-packages/osbuild/host.py", line 284, in serve
input/packages (org.osbuild.files): self.sock.send(reply, fds=reply_fds)
input/packages (org.osbuild.files): File "/usr/lib/python3.11/site-packages/osbuild/util/jsoncomm.py", line 407, in send
input/packages (org.osbuild.files): n = self._socket.sendmsg([serialized], cmsg, 0)
input/packages (org.osbuild.files): ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
input/packages (org.osbuild.files): OSError: [Errno 90] Message too long
```
The underlying issue is that the reply of the `map()` call is too
big for the buffer that `jsoncomm` uses. This problem existed before
for the args of map and was fixed by introducing a temporary file
in https://github.com/osbuild/osbuild/pull/1331 (and similarly
before in https://github.com/osbuild/osbuild/pull/824).
This commit writes the return values also into a file. This should
fix the crash above and make the function more symetrical as well.
Alternative/complementary version of
https://github.com/osbuild/osbuild/pull/1833
Closes: HMS-4537
When `jsoncomm` fails because the message is too big it currently
does not indicate just how big the message was. This commit adds
this information so that it's easier for us to determine what to
do about it.
We could also include a pointer to `/proc/sys/net/core/wmem_defaults`
but it seems we want to not require fiddling with that so let's
not do it for now.
See also https://github.com/osbuild/osbuild/pull/1838
The existing code to record progress was a bit too naive. Instead
of just counting the number os pipelines in a manifest to get the
total steps we need to look at the resolved pipelines.
with this fix `bib` will report the correct number of steps left
when doing e.g. a qcow2 image build. Right now the number of
steps is incorrect because the osbuild manifest contains pipelines
for qcow2,vdmk,raw,ami and all are currently considered steps
that need to be completed. With this commit this is fixed.
This commit adds a new `https_serve_directory()` test helper
and some custom self-signed and worthless certs that are used
during testing. They are not dynamically generated to avoid the
extra compuation time during tests (but they could be).
Generated via:
```
$ openssl req -new -newkey rsa:2048 -nodes -x509 \
-subj "/C=DE/ST=Berlin/L=Berlin/O=Org/CN=localhost" \
-keyout "key1.pem" -out "cert1.pem"
```
This will allow us to test `https` download URLs as well in e.g.
the curl source.
The `test_osbuild_mount_failure_msg` currently fails on fc40 when
run in tmt, see:
https://artifacts.dev.testing-farm.io/c6588a82-a2cb-46df-8ca8-85dd809465f2/
This is because the failure output is slightly different between
a container and a VM/real-machine. The test ensures that we capture
the output of mount and present to the user (for easier debugging).
So this commit updates this test once more for the error string
(that part of the error comes directly from the kernels fsconfig).
If we need another update of the string we should reconsider this
test and e.g. just use `testutil.mock_command()` for this. But
for now it's easier to just add this one more failure string.
This test ensures that the inputs of devices/mounts we generate for
bootc are actually considered valid by the schema. This is a more
blackbox style test compared to `test_get_schema_automatically_added`
which just checks that we get the expected schema but not that the
expected schema actually parses our inputs.
During the work on PR#1752 Florian discovered that make_containers()
is broken for nested containers like:
```
with make_container(tmp_path, {"file1": "file1 from base"}) as base_tag:
with make_container(tmp_path, {"file1": "file1 from final layer"}, base_tag) as cont_tag:
```
It errors with:
```
Error: 5b947de461ee21b858dd5b4224e80442b2f65b6410189147f2445884d9e4e3d8: image not known
```
The reason is that we work with hashes for the image and then call
`podman image rm` which by default will also remove all dangling
references. Those are defined by not having a tag and not referenced
anymore. So the inner container cleanup also removes the outter.
There are many ways to fix this, I went with re-adding tags to the
test containers because it also makes it easy for the user to see if
we left any containers (accidently) around.
With the new `bootc install to-filesystem` support many stages
will need a devices/mount setup to bind mount the deployment root
from the bootc deployment root of the generated image. To make
this globally available just allow "devices/mounts" for all stages
in the schema validation.
Note that `mounts` is already globally allowed so this just adds
devices (this was added in `7e776a076` with ostree as the use-case).
Nothing will change for the filesystem stages that already define
"devices" in a more specialized way.
This is needed because on a mounted `bootc` container `setfiles`
without excluding `/sysroot` will create many warnings like:
```
setfiles: conflicting specifications for /run/osbuild/tree/sysroot/ostree/repo/objects/00/0ef9ada2ee87792e8ba21afd65aa00d79a1253018832652b8694862fb80e84.file and /run/osbuild/tree/usr/lib/firmware/cirrus/cs35l41-dsp1-spk-prot-103c8b8f-r1.bin.xz, using system_u:object_r:lib_t:s0.
```
but simply excluding this dir fixes them.
Instead of requiring only one of the properties to be present require at
least one of them being present; some stages specify both schema
versions (`org.osbuild.rpm`)
Instead of just mocking the binary also write a log of the way
it got called so that tests can use this to check if the right
options are passed.
Note that the API should be improved here, instead of returning
a "naked" path to the calllog file there should be a class wrapping
it. And of course there should be tests.
The current `make_container()` helper is a bit silly (which is
entirely my fault). It requires a container tag as input but all
tests end up creating a random number for this input. So instead
just remove the input and return the container_id from the podman
build in the contextmanager and use that.
Based on the feedback from Tomáš in [0] this commit adds tests
that ensure consistent behavior between the python and the json
loader.
It's not 100% because the python is extremly leaniant and does
not even check if the required pieces of the json are there.
I.e. it will load a module without a SCHEMA or SCHEMA_2 variable
and the json loader code will warn about the issue but not
raise an error.
Fwiw, I have no strong opinion here but I do lean slightly towards
staying close to the original code (but both approaches of failing
with an exectption and continue with a warning have good arguments).
[0] https://github.com/osbuild/osbuild/pull/1618#discussion_r1521141148
Instead of always parsing the python stage to load meta information
allow the user of a new `{stage}-meta.json` file. This is a first
step towards allowing modules to be written in a different language
than python. It also has some practical advantages:
- slightly faster as it avoids calling python to output the schemas
- easier to write schemas as this can be done in a real json editor
now
- more extensible in a future where stages maybe binaries with
shlib dependencies that are only satisfied in the buildroot
but not on the host
This reverts commit 158acaac78.
With https://github.com/osbuild/bootc-image-builder/pull/238 the
original reason to call mknod goes away so we can just revert
it. osbuild now requires not only the loop device but also uses
`losetup --partscan` quite a lot now so the mknod approach becomes
impractical and the consumers of osbuild in a container should
just setup devtmpfs.
Since we support python3.6 we cannot assume that dicts are ordered
in any way. To ensure the `id` is still always valid we pass
sort_keys=True to json.dump().
Thanks to Simon!
Generate log messages with origin "org.osbuild.main" when
pipelines/stages start and finish. This way a higher level
frontend can display high level progress coming from this
origin and filter out e.g. stages based log messages (that
are usually quite technical as they are just stdout/stderr
from the stages).
Tweak the Progress class to be simpler. Given that progress does
not need to support arbitrary depth but only has a single level
the class now just exposes "sub_progress" to the caller.
When the main progress is advanced the sub_progress is now fully
deleted instead of just reset. The rational is that when the main
progress is done and advances a step it is very likely that a
new sub_progress is required and it's most likely an error if
the same sub_progress will get re-used.
This means that `reset()` can be removed as it's not used anymore
(and YAGNI). We can add it back when we have a use-case.
It also change the code so that "total" starts with 0 instead
of `None` (principle of least surprise). This means that now
`progress.incr()` is called in the JSONSeqMonitor() for
`finish()` and `result()` to indicate that the pipeline/stage
is finished.
This commit tweaks Context a bit so that any write will automatically
reset the `_id`. This ensures that we do not forget to reset `_id`
when the code changes.
It also tweaks the naming a bit, before there was a "setter" for
origin and functions to set "pipeline" and "stage". They are all
functions now with a "set_" prefix for symetry mostly.
The class LogLine() is purely used as a dataclass with no state
and the only function on it is `as_dict()`. This got refactored
into a new function `log_entry()` because there is no need for
this to be a class. The function that takes the same inputs.
This is a follow up to #1550 where we enabled a `rw` permissions mode,
which is not ideal since it would theoretically be possible to set both
`ro` and `rw` modes at the same time. This commit fixes the issue by only
allowing one option at a time.
Fixes#1588
The BLS specification [0] says the `options` field is optional and
can also appear multiple times. This commit tweaks the code to
deal with these corner cases and also adds tests that ensure that
this works correctly.
It also tweaks the file handling to be atomic.
[0] https://uapi-group.org/specifications/specs/boot_loader_specification/
The new `testutil.mock_command` context manager can be used to
mock commands in PATH and replace them with arbitrary shell
scripts. This is useful in testing to e.g. simulate exact error
conditions that would be hard to trigger otherwise or to replace
long running commands with faked results.
Example:
```
fake_cmd = textwrap.dedent("""\
do-something
""")
with mock_command("some-cmd", fake_cmd):
your_code
```
This adds a `default: true` option for all cases where OSTree
information is specified in schemas and allows for the information
to be picked up from the filesystem.
This is a safe operation because when building disk images there is
no known case where having two deployments makes sense. In the case
there ever were a case then the osname, ref, and serial options still
exist and can be used.
Co-authored-by: Luke Yang <luyang@redhat.com>
Co-authored-by: Michael Vogt <michael.vogt@gmail.com>
This commit adds code that will remove the least recently used
entries when a store() operation does not succeeds because the
cache is full. To be more efficient it will try to free
twice the requested size (this can be configured in the code).
This is a drive-by change after spending some quality time with the
mount code. The `id` field of `Mount` is calculated only once and
only when creating a `Mount`. This seems slightly dangerous as
any change to an attribute after creation will not update the
id. This means two options:
1. dynamically update the `id` on changes
2. forbid changes after the `id` is calculcated
I went with (2) but happy to discuss of course but it seems more
the spirit of the class.
It also does the same change for "devices.Device"
The test_libc_futimes_works() is failing under RHEL/Centos right
now. To make it more robust a tiny sleep and rounding of the
timestamps is introduced to ensure that we are not run into
floating point comaparison funnines.
The second part of the fix is to open the stamp_file in read-only
mode to ensure that the mtime is not modified by the open itself
which is what lead to the actual test failure.
