stages/org.osbuild.tar: add sparse option #6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lukewarmtemp
force-pushed
the
luyang-sparse-tar
branch
from
eae2d45
to
1f80e47
Compare
While integrating the changes to grub2 config stage options in the `osbuild/images` repository, I noticed that the code for grub2.legacy is embedding the grub2 stage config options struct in its own config options struct [1]. However, the grub2 stage options contain also the `default` entry, while the grub2.legacy stage does not allow this option in its stage options. This change fixes that and ensures that the grub2 stage options config section is a subset of the grub2.legacy stage options config section. [1] https://github.com/osbuild/images/blob/d5644f311537df5965ac0ca0fdbcb2800b8cd8ed/pkg/osbuild/grub2_legacy_stage.go#L85 Signed-off-by: Tomáš Hozza <thozza@redhat.com>
Under certain conditions a bind mount without a specified `rw` or `ro` option gets mounted read-only. We need a way to be explicit about needing a rw mount. We might want to change this in the future to be a single option (mode optional?) with valid values "rw", "ro". It's not entirely clear what the conditions are but it occurs when bind mounting the containers storage into the osbuild store, which we will need for the next few commits.
Add utility function to read the host's container storage config.
This source checks for the existence of a local container in the host's containers-storage. The source first reads the host's `/etc/containers/storage.conf` file for the storage config and then checks if the user has imported the desired container into the local store. Unlike the org.osbuild.containers stource, the org.osbuild.containers-storage source doesn't need any extra data other than the image ID. The ID is all that is used to retrieve the container. The location and other information regarding the storage are read from the host configuration and are not encoded in the manifest There's no need to use the name to resolve it like we do in other sources because containers in the local storage can be directly referenced by their image id (config digest). Other data such as the name of the container will only be relevant in the stage that will use the container as input. The source items are objects instead of simple strings of checksums because we might, in the future, want to add specific options for each source. The content_type for this source is `containers-storage`, which defines the location in the store where the source will bind mount the host's container storage for stages to read. We make this different from the containers content because it will be treated differently enough to need a separate input type. Co-authored-by: Gianluca Zuccarelli <gzuccare@redhat.com> Co-Authored-By: Michael Vogt <michael.vogt@gmail.com>
Implement fetch_all() and fetch_one() as calls to exists() to make sure we check that the containers are available every time they are needed.
An input that pairs with the org.osbuild.containers-storage source. It makes the host container storage available to a stage and a container ID for the stage to use. Unlike all the other input types, this input does not refer to static objects in the osbuild sources store but a dynamically mounted container store that depends on the host storage configuration and the availability of specific containers in that store. Co-authored-by: Achilleas Koutsou <achilleas@koutsou.net> Co-authored-by: Gianluca Zuccarelli <gzuccare@redhat.com>
Containers with the `containers-storage` are bind mounted to the osbuild store. This helper function bind mounts the containers-storage from the store into the `/containers/storage` so that stages that require containers can then make use of these containers.
The system-wide location for the containers storage.conf is /usr/share/containers. The existence of a file in /etc/containers completely overrides this (see containers-storage.conf(5)). If no file is found at /etc/containers/storage.conf then fall back to reading the config from /usr/share/containers/storage.conf. If neither file exists, this is an error since the default config should be packaged with any tool that requires it (skopeo, podman, etc).
Let the caller decide if a reload of the storage configuration is needed and simplify the storage configuration reader.
Similar to the `stage_module` fixture for stage tests this adds a fixture to test sources modules of osbuild. The code from `stage_module` and `sources_module` is similar and could be combined but pytest makes it hard to do this without having a shared root dir. Given that it's just four lines it seems easier to just life with the tiny bit of code duplication.
Make make_container a context manager so we can reliably clean up containers that were created in tests.
When parsing a container input, add the checksum to the data as well. Usually with other inputs, the stage only needs to know the filepath where it can find the source content. In most (all, so far) cases, this is a checksum appended to the content type. In this case, the filepath is the location of the storage bind mount and the checksum is needed to retrieve the container. The name might only be a destination name (a name to use for storing the container in the image), so we can't rely on it being valid in the source.
A manifest (mpp and json) that uses the new source and input with the skopeo stage. This depends on the image we store at ./test/data/stages/skopeo/hello.img The plan is to test this by pulling the hello.img into the host root storage, build the manifest, delete the image from storage, and check the tree.
Copy hello.img into the host storage, build the manifest, export the tree, and check the storage in the tree for the expected files and data.
Parse the manifest to dynamically discover the expected data in the destination so we don't have to keep the test updated if the manifest changes.
The default overlayfs doesn't work inside the container runner which causes the test to fail with Invalid destination name containers-storage:localhost/osbuild-skopeo-test-...: 'overlay' is not supported over overlayfs, a mount_program is required: backing file system is unsupported for this graph driver Changing the driver to vfs solves the issue.
This stage crashes when using `containers-storage` if `/var/tmp` doesn't exist, since the directory is required by `skopeo` for big files. See: https://github.com/containers/image/blob/4ad2f46269f1a1f6ad4067e418b820a1b3453615/storage/storage_src.go#L163 https://github.com/containers/image/blob/2cb3c9d3c29dc1c23b45f93abeed9b3f57f6a8b4/internal/tmpdir/tmpdir.go#L18
Small followup for osbuild#1586 that includes a basic check that the initoverlayfs option calls the right binary.
[skip ci]
|
This LGTM. You might want to update the tar tests to exercise this option. When you propose upstream you might want to mention to them that we'd also be fine making this not an option (i.e. just the default specified) if they think that everyone would want it everytime. |
The source implementation used `subprocess.run()` argument `capture_output`, which was added in Python 3.7. Since the minimum supported Python version for osbuild on RHEL-8 is 3.6, the stage fails with TypeError. Example failure: https://artifacts.dev.testing-farm.io/c147b608-c40e-46ed-bf11-6b15ecf718dc/ Signed-off-by: Tomáš Hozza <thozza@redhat.com>
- Add functions for appending kernel parameters to the Boot Loader Specification (BLS) as needed. Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
- Process all necessary operations related to CoreOS platforms is crucial and specific to CoreOS. This step is essential for CoreOS exclusively. - Our approach to handling 'platforms.json' may change as we advance with the OSBuild work. However, we don't have a clear vision about how it will be in the future yet, particularly as we also manage similar components within the osbuild composer to configure cloud parameters. We probably will know better when we start working with the cloud artifacts. As a summary, let's add it know to unblock us, and if we find a better approach in the future, we can always go back and remove it. Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
[skip ci]
lukewarmtemp
force-pushed
the
luyang-sparse-tar
branch
2 times, most recently
from
994b8ce
to
252322e
Compare
lukewarmtemp
force-pushed
the
luyang-sparse-tar
branch
from
252322e
to
134a4cc
Compare
lukewarmtemp
force-pushed
the
luyang-sparse-tar
branch
from
ab3deba
to
74300be
Compare
Add a sparse option to the tar stage which make files in the archive sparse if they are sparse on disk. This is needed for CoreOS Assember.
lukewarmtemp
force-pushed
the
luyang-sparse-tar
branch
from
74300be
to
e902c5f
Compare
|
Upstream PR: osbuild#1614 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a sparse option to the tar stage which make files in the archive sparse if they are sparse on disk. This is needed for CoreOS Assember.