Skip to content

Make image compression format configurable#233

Merged
pothos merged 7 commits into
flatcar:mainfrom
gabriel-samfira:add-configurable-compression
Mar 9, 2022
Merged

Make image compression format configurable#233
pothos merged 7 commits into
flatcar:mainfrom
gabriel-samfira:add-configurable-compression

Conversation

@gabriel-samfira
Copy link
Copy Markdown
Member

@gabriel-samfira gabriel-samfira commented Feb 22, 2022

Make image compression format configurable

This change adds a new flag called --image_compression_format which allows us to output the final VM image as one of the supported formats: bz2 (default), gz, zip or none

If the compression format is "none" or "", the image will not be compressed.

This change also enables the CI to output OpenStack images using gzip compression instead of bz2.

Fixes flatcar/Flatcar#575

How to use

Simply add --image_compression_format=gz when calling image_to_vm.sh. Leaving this option unset will preserve the current behavior.

Testing done

These changes were tested by specifying all supported formats as well as arbitrary bogus strings. The results were as expected. Success on any of the supported formats, error on anything else. Image archive as well as legacy digest files were properly generated.

  • Changelog entries added in the respective changelog/ directory (user-facing change, bug fix, security fix, update)

@pothos
Copy link
Copy Markdown
Member

pothos commented Feb 22, 2022

Thanks, looks good on a first glance but users may have download scripts which will break. I suggest to rework this to support both formats at the same time, i.e., the flag should accept a list of compression formats and then we pass .gz,.bz2 for OpenStack images to generate the gzip images additionally.
Edit: flatcar-install does not consume the .img files, so no change needed there, but the https://github.com/flatcar-linux/flatcar-docs have references to openstack_image.img.bz2

Comment thread jenkins/vms.sh Outdated
Comment thread build_library/release_util.sh
@gabriel-samfira
Copy link
Copy Markdown
Member Author

Thanks, looks good on a first glance but users may have download scripts which will break. I suggest to rework this to support both formats at the same time, i.e., the flag should accept a list of compression formats and then we pass .gz,.bz2 for OpenStack images to generate the gzip images additionally.

Looking at where to best add this. Will probably update the PR tomorrow morning.

Edit: flatcar-install does not consume the .img files, so no change needed there, but the https://github.com/flatcar-linux/flatcar-docs have references to openstack_image.img.bz2

Will also update the docs.

@gabriel-samfira gabriel-samfira force-pushed the add-configurable-compression branch 2 times, most recently from e8286d8 to d80b6df Compare February 23, 2022 16:50
@gabriel-samfira
Copy link
Copy Markdown
Member Author

gabriel-samfira commented Feb 23, 2022

@pothos Updated the PR. The change adds the ability to specify multiple compression formats as a comma separated value. The compression logic was moved outside of the upload function.

I also added zstd as an option. Please take another look.

Running:

sdk@flatcar-sdk-all-3139_0_0_os-alpha-3046_0_0-107-g6d0d7ea2 ~/trunk/src/scripts $ ./image_to_vm.sh \
    --upload_root rsync://ubuntu@localhost:/mnt/test_upload \
    --upload \
    --format openstack \
    --image_compression_formats zip,gz,zstd

results in:

ubuntu@flatcar:/tmp$ ls -lh /mnt/test_upload/boards/amd64-usr/3139.0.0+2022-02-21-1346/
total 2.1G
-rw-r--r-- 1 ubuntu ubuntu 1.5K Feb 23 16:35 flatcar_production_openstack.DIGESTS
-rw-r--r-- 1 ubuntu ubuntu 898M Feb 23 16:35 flatcar_production_openstack_image.img
-rw-r--r-- 1 ubuntu ubuntu 1.5K Feb 23 16:35 flatcar_production_openstack_image.img.DIGESTS
-rw-r--r-- 1 ubuntu ubuntu 406M Feb 23 16:35 flatcar_production_openstack_image.img.gz
-rw-r--r-- 1 ubuntu ubuntu 1.5K Feb 23 16:35 flatcar_production_openstack_image.img.gz.DIGESTS
-rw-r--r-- 1 ubuntu ubuntu 406M Feb 23 16:35 flatcar_production_openstack_image.img.zip
-rw-r--r-- 1 ubuntu ubuntu 1.5K Feb 23 16:35 flatcar_production_openstack_image.img.zip.DIGESTS
-rw-r--r-- 1 ubuntu ubuntu 388M Feb 23 16:35 flatcar_production_openstack_image.img.zstd
-rw-r--r-- 1 ubuntu ubuntu 1.5K Feb 23 16:35 flatcar_production_openstack_image.img.zstd.DIGESTS

Comment thread build_library/release_util.sh Outdated
@gabriel-samfira gabriel-samfira force-pushed the add-configurable-compression branch from d80b6df to 9b30ad3 Compare February 23, 2022 17:04
@pothos
Copy link
Copy Markdown
Member

pothos commented Feb 23, 2022

I think we need a full test run where we compare that we still generate the same files plus two additional files

@gabriel-samfira
Copy link
Copy Markdown
Member Author

I think we need a full test run where we compare that we still generate the same files plus two additional files

Sounds good. Should I do that, or is there a CI that does that? Apologies, I am a newbie :D.

@pothos
Copy link
Copy Markdown
Member

pothos commented Feb 23, 2022

Sounds good. Should I do that, or is there a CI that does that? Apologies, I am a newbie :D.

We have to manually start the CI for now, will do soon :D

Edit: running now http://jenkins.infra.kinvolk.io:8080/job/os/job/manifest/4937/

@pothos
Copy link
Copy Markdown
Member

pothos commented Feb 24, 2022

Something is wrong because flatcar_production_image.bin.bz2.sig wasn't generated and the .bz2 is missing, too

@pothos
Copy link
Copy Markdown
Member

pothos commented Feb 24, 2022

Ah, flatcar_production_image.bin.sig and flatcar_production_image.bin got uploaded instead

@gabriel-samfira
Copy link
Copy Markdown
Member Author

Something is wrong because flatcar_production_image.bin.bz2.sig wasn't generated and the .bz2 is missing, too

Are there any CI logs I can check? The link doesn't seem to be accessible.

Ah, flatcar_production_image.bin.sig and flatcar_production_image.bin got uploaded instead

Ack. Looking into it now.

@gabriel-samfira gabriel-samfira force-pushed the add-configurable-compression branch from 9b30ad3 to 09f86d1 Compare February 24, 2022 13:56
@pothos
Copy link
Copy Markdown
Member

pothos commented Feb 24, 2022

The build_image step is responsible for the plain image: https://github.com/flatcar-linux/scripts/blob/main/jenkins/images.sh#L119
We have to test both, not only the image_to_vm.sh case

@pothos
Copy link
Copy Markdown
Member

pothos commented Feb 24, 2022

The changed upload_image function is called at various places, which is probably where the failures come from.

@gabriel-samfira
Copy link
Copy Markdown
Member Author

The build_image step is responsible for the plain image: https://github.com/flatcar-linux/scripts/blob/main/jenkins/images.sh#L119
We have to test both, not only the image_to_vm.sh case

yup. Saw it. Testing now, but iteration time is slow :D. Will ping in the comments once done.

@gabriel-samfira
Copy link
Copy Markdown
Member Author

Made some changes that also takes care of compressing updates and should handle cases where upload_image() is called. The change makes the code a bit more verbose and repetitive, but it decouples compressing images from uploading them, giving the caller clearer expectations of what to expect when calling upload_image().

Currently, from what I see, upload_image() is used to upload not only images, but also other types of files. It also is responsible for compressing images. The need to be able to compress using multiple formats and retain DIGESTS files in a way that legacy scripts won't fail, has made this a bit more verbose than I expected.

If you prefer that I rework this a bit, let me know. Any suggestions are of course welcome.

@pothos
Copy link
Copy Markdown
Member

pothos commented Mar 1, 2022

Thanks, I started a new build and will forward the results http://jenkins.infra.kinvolk.io:8080/job/os/job/manifest/4990/cldsv/

Can you have a look at the suggestions here (mainly about quoting and the changelog entry)?

@gabriel-samfira
Copy link
Copy Markdown
Member Author

Can you have a look at the suggestions here (mainly about quoting and the changelog entry)?

Yes. Of course! First thing in the morning.

@gabriel-samfira
Copy link
Copy Markdown
Member Author

Added quotes to variables, updated changelog.

I combed through the comments here, but I do not see your suggestions regarding quotes and changelog. Perhaps they were not published?

Comment thread changelog/changes/2022-02-22-configurable-image-compression.md Outdated
Comment thread build_library/vm_image_util.sh Outdated
Comment thread build_library/vm_image_util.sh Outdated
Comment thread build_library/vm_image_util.sh Outdated
Comment thread build_library/vm_image_util.sh Outdated
Comment thread changelog/changes/2022-02-22-configurable-image-compression.md Outdated
Comment thread build_library/build_image_util.sh Outdated
Comment thread build_library/modify_image_util.sh Outdated
Comment thread build_library/prod_image_util.sh Outdated
Comment thread build_library/vm_image_util.sh Outdated
declare -a digest_uploads

for file in "${VM_GENERATED_FILES[@]}";do
if [[ "${file}" =~ \.(img|bin|vdi|vhd|vmdk)$ ]]; then
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This duplicates the logic in compress_disk_images and I think it's better to move it there completely as suggested above

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The duplication of logic here is unfortunate, but necessary if we want to retain backwards compatibility and generate a .DIGESTS file for each archive. We need to know which files got compressed, which files are images and which files can be uploaded as is. We should always pass in images to compress_disk_images(), and expect compressed files in the results array. There is no reason to pass in any other types of files. To be honest, I think we can rename the function to compress_files() and remove the if that matches extensions. What do you think?

There needs to be a separation of concerns, and mixing files of different types leads to situations that are difficult to debug.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this legacy logic is a bit complicated, thanks for your works there. I think it's good now, or?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup. Should be ok. The changes to compress_disk_image() makes is also populate an "extra_files" array with whatever doesn't match that image pattern. Which callers can then concatenate and upload if they wish.

@pothos
Copy link
Copy Markdown
Member

pothos commented Mar 2, 2022

Added quotes to variables, updated changelog.

I combed through the comments here, but I do not see your suggestions regarding quotes and changelog. Perhaps they were not published?

Oh, sorry, that was it

@gabriel-samfira gabriel-samfira force-pushed the add-configurable-compression branch 2 times, most recently from 41268de to 6fd38c2 Compare March 2, 2022 16:12
@pothos
Copy link
Copy Markdown
Member

pothos commented Mar 3, 2022

The last completed successfully but I didn't have a close look at the list of resulting files.
I've started a new test run now with these changes and will paste the file lists/URL to it.

@gabriel-samfira
Copy link
Copy Markdown
Member Author

The last completed successfully but I didn't have a close look at the list of resulting files.

should be the same files, plus an extra .DIGESTS file with the name of the uncompressed file when building an image. Here is the output on my machine:

For:

./image_to_vm.sh --upload_root rsync://ubuntu@localhost:/mnt/test_upload --upload

The result in both cases (main branch and this one):

ubuntu@flatcar:~/scripts$ ls -l /mnt/test_upload/boards/amd64-usr/3139.0.0+2022-03-03-1016.compress_to_image
total 381328
-rw-r--r-- 1 ubuntu ubuntu      1017 Mar  3 10:44 flatcar_production_qemu.DIGESTS
-rw-r--r-- 1 ubuntu ubuntu       535 Mar  3 10:44 flatcar_production_qemu.README
-rwxr-xr-x 1 ubuntu ubuntu      7518 Mar  3 10:44 flatcar_production_qemu.sh
-rw-r--r-- 1 ubuntu ubuntu 390453496 Mar  3 10:44 flatcar_production_qemu_image.img.bz2
-rw-r--r-- 1 ubuntu ubuntu      1017 Mar  3 10:44 flatcar_production_qemu_image.img.bz2.DIGESTS
ubuntu@flatcar:~/scripts$ ls -l /mnt/test_upload/boards/amd64-usr/3139.0.0+2022-03-03-0918.to_image/
total 381792
-rw-r--r-- 1 ubuntu ubuntu      1017 Mar  3 10:13 flatcar_production_qemu.DIGESTS
-rw-r--r-- 1 ubuntu ubuntu       535 Mar  3 10:13 flatcar_production_qemu.README
-rwxr-xr-x 1 ubuntu ubuntu      7518 Mar  3 10:13 flatcar_production_qemu.sh
-rw-r--r-- 1 ubuntu ubuntu 390928218 Mar  3 10:13 flatcar_production_qemu_image.img.bz2
-rw-r--r-- 1 ubuntu ubuntu      1017 Mar  3 10:13 flatcar_production_qemu_image.img.bz2.DIGESTS

For:

./build_image --upload_root rsync://ubuntu@localhost:/mnt/test_upload --upload

The result for both branches:

ubuntu@flatcar:~/scripts$ ls -l /mnt/test_upload/boards/amd64-usr/3139.0.0+2022-03-03-0918.bk/
total 1063860
-rw-r--r-- 1 ubuntu ubuntu 391670006 Mar  3 09:39 flatcar_production_image.bin.bz2
-rw-r--r-- 1 ubuntu ubuntu      3159 Mar  3 09:39 flatcar_production_image.bin.bz2.DIGESTS
-rw-r--r-- 1 ubuntu ubuntu    717304 Mar  3 09:39 flatcar_production_image.grub
-rw-r--r-- 1 ubuntu ubuntu   1285928 Mar  3 09:39 flatcar_production_image.shim
-rw-r--r-- 1 ubuntu ubuntu  52289048 Mar  3 09:39 flatcar_production_image.vmlinuz
-rw-r--r-- 1 ubuntu ubuntu   2306240 Mar  3 09:39 flatcar_production_image_contents.txt
-rw-r--r-- 1 ubuntu ubuntu    152830 Mar  3 09:39 flatcar_production_image_kernel_config.txt
-rw-r--r-- 1 ubuntu ubuntu   1843726 Mar  3 09:39 flatcar_production_image_licenses.json
-rw-r--r-- 1 ubuntu ubuntu     10366 Mar  3 09:37 flatcar_production_image_packages.txt
-rw-r--r-- 1 ubuntu ubuntu     20321 Mar  3 09:39 flatcar_production_image_pcr_policy.zip
-rw-r--r-- 1 ubuntu ubuntu 288111708 Mar  3 09:39 flatcar_production_update.bin.bz2
-rw-r--r-- 1 ubuntu ubuntu       345 Mar  3 09:39 flatcar_production_update.bin.bz2.DIGESTS
-rw-r--r-- 1 ubuntu ubuntu  11547194 Mar  3 09:40 flatcar_production_update.zip
-rw-r--r-- 1 ubuntu ubuntu       333 Mar  3 09:40 flatcar_production_update.zip.DIGESTS
-rw-r--r-- 1 ubuntu ubuntu 339375926 Mar  3 09:40 flatcar_test_update.gz
-rw-r--r-- 1 ubuntu ubuntu       312 Mar  3 09:40 flatcar_test_update.gz.DIGESTS
-rw-r--r-- 1 ubuntu ubuntu       185 Mar  3 09:40 version.txt
-rw-r--r-- 1 ubuntu ubuntu       279 Mar  3 09:40 version.txt.DIGESTS
ubuntu@flatcar:~/scripts$ ls -l /mnt/test_upload/boards/amd64-usr/3139.0.0+2022-03-03-1016.compress_image/
total 1061832
-rw-r--r-- 1 ubuntu ubuntu      3159 Mar  3 10:37 flatcar_production_image.bin.DIGESTS
-rw-r--r-- 1 ubuntu ubuntu 390943703 Mar  3 10:36 flatcar_production_image.bin.bz2
-rw-r--r-- 1 ubuntu ubuntu      3159 Mar  3 10:37 flatcar_production_image.bin.bz2.DIGESTS
-rw-r--r-- 1 ubuntu ubuntu    717304 Mar  3 10:36 flatcar_production_image.grub
-rw-r--r-- 1 ubuntu ubuntu   1285928 Mar  3 10:36 flatcar_production_image.shim
-rw-r--r-- 1 ubuntu ubuntu  52289048 Mar  3 10:36 flatcar_production_image.vmlinuz
-rw-r--r-- 1 ubuntu ubuntu   2306240 Mar  3 10:36 flatcar_production_image_contents.txt
-rw-r--r-- 1 ubuntu ubuntu    152830 Mar  3 10:36 flatcar_production_image_kernel_config.txt
-rw-r--r-- 1 ubuntu ubuntu   1843726 Mar  3 10:36 flatcar_production_image_licenses.json
-rw-r--r-- 1 ubuntu ubuntu     10366 Mar  3 10:35 flatcar_production_image_packages.txt
-rw-r--r-- 1 ubuntu ubuntu     20322 Mar  3 10:36 flatcar_production_image_pcr_policy.zip
-rw-r--r-- 1 ubuntu ubuntu       345 Mar  3 10:37 flatcar_production_update.bin.DIGESTS
-rw-r--r-- 1 ubuntu ubuntu 287332590 Mar  3 10:37 flatcar_production_update.bin.bz2
-rw-r--r-- 1 ubuntu ubuntu       345 Mar  3 10:37 flatcar_production_update.bin.bz2.DIGESTS
-rw-r--r-- 1 ubuntu ubuntu  11547194 Mar  3 10:38 flatcar_production_update.zip
-rw-r--r-- 1 ubuntu ubuntu       333 Mar  3 10:38 flatcar_production_update.zip.DIGESTS
-rw-r--r-- 1 ubuntu ubuntu 338802477 Mar  3 10:38 flatcar_test_update.gz
-rw-r--r-- 1 ubuntu ubuntu       312 Mar  3 10:38 flatcar_test_update.gz.DIGESTS
-rw-r--r-- 1 ubuntu ubuntu       185 Mar  3 10:38 version.txt
-rw-r--r-- 1 ubuntu ubuntu       279 Mar  3 10:38 version.txt.DIGESTS

If the CI could run all the code paths you guys usually run to properly validate that I have not missed something, that would be great.

@pothos
Copy link
Copy Markdown
Member

pothos commented Mar 3, 2022

I got this error when building:

INFO    build_image: Cleaning up mounts
INFO    build_image: Compressing images
INFO    build_image: Compressing flatcar_developer_container.bin to bz2
cp: cannot stat '/home/sdk/build/images/amd64-usr/developer-9999.9.9+kai-test-pr-233-a1/flatcar_developer_container.bin.DIGESTS': No such file or directory
ERROR   build_image: script called: build_image '--board=amd64-usr' '--output_root=/home/sdk/build/images' '--torcx_root=/home/sdk/build/torcx' 'prodtar' 'container'
ERROR   build_image: Backtrace:  (most recent call is last)
ERROR   build_image:   file build_image, line 173, called: create_dev_container 'flatcar_developer_container.bin' 'container' 'https://mirror.release.flatcar-linux.net/' 'developer' 'coreos-base/coreos-dev'
ERROR   build_image:   file dev_container_util.sh, line 128, called: die_err_trap 'cp "${BUILD_DIR}/${image_name}.DIGESTS" "${digest_file}"' '1'
ERROR   build_image: 
ERROR   build_image: Command failed:
ERROR   build_image:   Command 'cp "${BUILD_DIR}/${image_name}.DIGESTS" "${digest_file}"' exited with nonzero code: 1
flatcar-images-amd64-9999.9.9-kai-test-pr-233

(from scripts/ci-automation/image.sh)

@gabriel-samfira
Copy link
Copy Markdown
Member Author

I got this error when building:

Checking. Will update today. Thanks for the debug output!

@gabriel-samfira
Copy link
Copy Markdown
Member Author

@pothos any chance there's a guide on how to set up a 3rd party CI that can mirror yours? I'd like to set one up to not have to bug you.

@pothos
Copy link
Copy Markdown
Member

pothos commented Mar 7, 2022

At the moment we have two setups, one is the inherited using cork, and the new one uses the run_sdk_container script for Docker. The failure happened with the new setup - while not complete I'll attach the Jenkins files here:
jenkins.tar.zip

@gabriel-samfira
Copy link
Copy Markdown
Member Author

At the moment we have two setups, one is the inherited using cork, and the new one uses the run_sdk_container script for Docker. The failure happened with the new setup - while not complete I'll attach the Jenkins files here:

Thanks! Currently adding a function for the legacy digests, testing locally and pushing. I managed to run image_build amd64, which worked until it had to upload files to bincache@bincache.flatcar-linux.net 😁

This change adds a new flag called --image_compression_format which
allows us to output the final VM image as one of the supported formats:

bz2 (default), gz, zip or none

if the compression format is "none" or "", the image will not be compressed.

Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
This change makes the Jenkins job output openstack images using gzip
compression format. This allows OpenStack users to directly consume images
by simply specifying the URL to the image. Glance will then download the
image, unarchive it and add it to it's catalogue.

Fixes flatcar#575

Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
Add the ability to specify a comma separated list of compression formats.

Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
@gabriel-samfira gabriel-samfira force-pushed the add-configurable-compression branch from a514639 to f126deb Compare March 7, 2022 15:18
@gabriel-samfira
Copy link
Copy Markdown
Member Author

Updated the PR. If you could share the error from the previous failure, that would be great.

@pothos
Copy link
Copy Markdown
Member

pothos commented Mar 7, 2022

I think it worked now! There was a just build failure on arm64 which may be due to needing a rebase.

Comment thread build_library/release_util.sh Outdated
@gabriel-samfira gabriel-samfira force-pushed the add-configurable-compression branch from b17d52c to 71e4f1c Compare March 8, 2022 08:07
@gabriel-samfira
Copy link
Copy Markdown
Member Author

I think it worked now! There was a just build failure on arm64 which may be due to needing a rebase.

I rebased on top of main, and squashed most of the commits. Give it another go. Running it locally as well.

@jepio
Copy link
Copy Markdown
Member

jepio commented Mar 8, 2022

arm64 is currently failing due to an issue that im fixing here: flatcar-archive/coreos-overlay#1690.

Rename sztd to zst and amend the changelog. The zstd binary generates a
compressed file with the .zst extension by default.

Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
@gabriel-samfira gabriel-samfira force-pushed the add-configurable-compression branch from 1d8f850 to e88f12c Compare March 8, 2022 14:02
Copy link
Copy Markdown
Member

@pothos pothos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Started another test but I think all is covered now, thanks

@pothos
Copy link
Copy Markdown
Member

pothos commented Mar 9, 2022

(While at it we can also add a gz image for DigitalOcean to address the temporary failures the note in https://www.flatcar.org/docs/latest/installing/cloud/digitalocean/ talks about)

Comment thread jenkins/vms.sh Outdated
Co-authored-by: Kai Lüke <pothos@users.noreply.github.com>
Comment thread changelog/changes/2022-02-22-configurable-image-compression.md
Co-authored-by: Kai Lüke <pothos@users.noreply.github.com>
@pothos pothos merged commit 0c8d96e into flatcar:main Mar 9, 2022
@pothos
Copy link
Copy Markdown
Member

pothos commented Mar 9, 2022

Thanks again :)

@gabriel-samfira
Copy link
Copy Markdown
Member Author

Thanks again :)

My pleasure! Thank you for the guidance and patience! :D

@pothos
Copy link
Copy Markdown
Member

pothos commented Jul 1, 2022

It has been working well so far in Beta and Alpha releases. I'll pick it for the Stable/LTS branches to reduce the build system differences across the branches.

pothos added a commit that referenced this pull request Jul 1, 2022
Make image compression format configurable
pothos added a commit that referenced this pull request Jul 1, 2022
Make image compression format configurable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[RFE] Provide OpenStack images using gzip compression to allow import via web_download to Glance

3 participants