Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vm_image_util: produce qcow2 images for Scaleway #1953

Merged
merged 4 commits into from
Apr 26, 2024
Merged

Conversation

tormath1
Copy link
Contributor

@tormath1 tormath1 commented Apr 24, 2024

For importing Scaleway images, extension needs to be '.qcow2'

See: https://www.scaleway.com/en/docs/compute/instances/how-to/snapshot-import-export-feature/

Make sure that the QCOW / QCOW2 image file you want to import,
uses the file extension .qcow or .qcow2 to avoid issues while importing the image.

Testing done

To be backported on Alpha and Beta.

Copy link
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.

For qcow2 we don't need the bz2 wrapping when introducing a new image format. We can add a case with COMPRESSION_FORMAT=none for scaleway in ci-automation/vms.sh

The change looks good otherwise: As scaleway images are not in Stable yet I think we can switch the output file behavior.

@tormath1
Copy link
Contributor Author

tormath1 commented Apr 24, 2024

For qcow2 we don't need the bz2 wrapping when introducing a new image format. We can add a case with COMPRESSION_FORMAT=none for scaleway in ci-automation/vms.sh

The change looks good otherwise: As scaleway images are not in Stable yet I think we can switch the output file behavior.

Fun, it worked without doing nothing. Only qcow2 images are now provided (no bz2), need to understand "why":
http://bincache.flatcar-linux.net/images/amd64/9999.0.0+tormath1-image-ext/

EDIT: The why:

if [[ "${filename}" =~ \.(img|bin|vdi|vhd|vhdx|vmdk)$ ]]; then

Copy link

github-actions bot commented Apr 24, 2024

Build action triggered: https://github.com/flatcar/scripts/actions/runs/8844961434

Copy link
Member

@krnowak krnowak left a comment

Choose a reason for hiding this comment

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

One nitpick, otherwise looks good.

@@ -421,6 +425,11 @@ _dst_path() {
# Get the proper disk format extension.
_disk_ext() {
local disk_format=$(_get_vm_opt DISK_FORMAT)
local disk_extension=$(_get_vm_opt DISK_EXTENSION)
if [[ ! -z "${disk_extension}" ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Quotes are not necessary in [[ (as opposed to [), and ! -z is basically -n:

Suggested change
if [[ ! -z "${disk_extension}" ]]; then
if [[ -n ${disk_extension} ]]; then

This variable allows to override the disk extension which is initially
based on the DISK_FORMAT.

Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
For importing Scaleway images, extension needs to be '.qcow2'

See: https://www.scaleway.com/en/docs/compute/instances/how-to/snapshot-import-export-feature/
> Make sure that the QCOW / QCOW2 image file you want to import,
> uses the file extension .qcow or .qcow2 to avoid issues while importing the image.

Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
@tormath1 tormath1 force-pushed the tormath1/image-ext branch from 34fd6b6 to 0a7819a Compare April 26, 2024 07:47
@tormath1 tormath1 marked this pull request as ready for review April 26, 2024 07:48
@tormath1 tormath1 merged commit 59842e9 into main Apr 26, 2024
1 check failed
@tormath1 tormath1 deleted the tormath1/image-ext branch April 26, 2024 07:48
@tormath1
Copy link
Contributor Author

Cherry-picked to:

  • flatcar-3941
  • flatcar-3913

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants