Skip to content

lib: Add internal qcow2 compression for Nutanix image#2848

Merged
miabbott merged 2 commits intocoreos:mainfrom
thunderboltsid:ntnx-qcow2-compression
May 12, 2022
Merged

lib: Add internal qcow2 compression for Nutanix image#2848
miabbott merged 2 commits intocoreos:mainfrom
thunderboltsid:ntnx-qcow2-compression

Conversation

@thunderboltsid
Copy link
Copy Markdown
Contributor

@thunderboltsid thunderboltsid commented May 6, 2022

Add a -c convert option for Nutanix image which will help reduce
size of the image for Nutanix.

This PR also sets skip_compression value to True for Nutanix variant
in order to ensure cosa compress doesn't gzip the image.

Fixes coreos/fedora-coreos-tracker#1191

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 6, 2022

Hi @thunderboltsid. Thanks for your PR.

I'm waiting for a coreos member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@thunderboltsid
Copy link
Copy Markdown
Contributor Author

/hold this is a provisional PR

@bgilbert
Copy link
Copy Markdown
Contributor

bgilbert commented May 7, 2022

You'll also need to add a way for a VARIANTS dict to set skip-compression in the image metadata, so cmd-compress doesn't try to recompress the image.

/ok-to-test

@thunderboltsid
Copy link
Copy Markdown
Contributor Author

You'll also need to add a way for a VARIANTS dict to set skip-compression in the image metadata, so cmd-compress doesn't try to recompress the image.

@bgilbert can you take another look? Is this what you were expecting?

@sohankunkerkar
Copy link
Copy Markdown
Contributor

/retest

1 similar comment
@bgilbert
Copy link
Copy Markdown
Contributor

/retest

@thunderboltsid
Copy link
Copy Markdown
Contributor Author

@bgilbert the rhcos test seems to be failing due to openshift/os#795

@bgilbert
Copy link
Copy Markdown
Contributor

This appears to be a different error.

@thunderboltsid
Copy link
Copy Markdown
Contributor Author

/retest-required

2 similar comments
@thunderboltsid
Copy link
Copy Markdown
Contributor Author

/retest-required

@thunderboltsid
Copy link
Copy Markdown
Contributor Author

/retest-required

@dustymabe
Copy link
Copy Markdown
Member

Note: before something like this lands we still need to make an announcement about it to the community.

bgilbert
bgilbert previously approved these changes May 10, 2022
Copy link
Copy Markdown
Contributor

@bgilbert bgilbert left a comment

Choose a reason for hiding this comment

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

This LGTM, though I haven't tested it.

@thunderboltsid
Copy link
Copy Markdown
Contributor Author

/retest-required

@miabbott
Copy link
Copy Markdown
Member

miabbott commented May 11, 2022

Looks like another issue with the repo mirror

Updating rpm-md repo 'rhel-86-appstream': cannot update repo 'rhel-86-appstream': Cannot download repomd.xml: Cannot download repodata/repomd.xml: All mirrors were tried; Last error: Status code: 403 for http://base-4-11-rhel86.ocp.svc.cluster.local/rhel-8-appstream/repodata/repomd.xml (IP: 172.30.132.86)

I put up another PR to switch from the 8.6 Beta content, but I'm not convinced it will solve the error.

openshift/release#28509

I would see if the job on openshift/os#798 passes and then trigger the jobs here if so.

@thunderboltsid
Copy link
Copy Markdown
Contributor Author

/retest-required

@thunderboltsid
Copy link
Copy Markdown
Contributor Author

/hold cancel

Copy link
Copy Markdown
Member

@miabbott miabbott left a comment

Choose a reason for hiding this comment

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

This doesn't work as proposed. I tried building RHCOS 4.11 and generating a Nutanix artifact.

$ cosa buildextend-nutanix                        
[INFO]: CLI is a symlink for cmd-buildextend-nutanix
[INFO]: Target 'NUTANIX' is a Qemu Variant image
[INFO]: Targeting architecture: x86_64      
[INFO]: Targeting build: 411.85.202205121303-0
[INFO]: Processed build for: OpenShift 4 (RHCOS-x86_64) 411.85.202205121303-0
[INFO]: Processing the build artifacts    
[INFO]: Staging temp image: /srv/tmp/tmp2x7ohrl6/rhcos-411.85.202205121303-0-nutanix.x86_64.qcow2
...
+ qemu-img convert -f qcow2 -O qcow2 /srv/tmp/tmp2x7ohrl6/rhcos-411.85.202205121303-0-qemu.x86_64.qcow2.working -c "" /srv/tmp/tmp2x7ohrl6/rhcos-411.85.202205121303-0-nutanix.x86_64.qcow2
qemu-img: Could not open '': The 'file' block driver requires a file name
Error running command qemu-img

It looks like -c option requires a filename

@thunderboltsid
Copy link
Copy Markdown
Contributor Author

thunderboltsid commented May 12, 2022

It looks like -c option requires a filename

I don’t think -c requires a filename for the flag. But, I think because the -c flag is the “key” and "" is the value, the underlying command ends up copying the empty string as "" in the generated command.

@miabbott
Copy link
Copy Markdown
Member

I don’t think -c requires a filename for the flag. But, I think because the -c flag is the “key” and "" is the value, the underlying command ends up not copying the empty string as "" in the generated command.

You are correct; I should avoid doing debug before the first coffee 😛

Comment thread src/cosalib/qemuvariants.py
Add a `-c` convert option for Nutanix image which will help reduce
size of the image for Nutanix.
Add a `skip_compression` property on `QemuVariantImage`  based on
the Set `skip_compression` value of the variant in `VARIANTS` dict.
When `skip_compression` is set to True (default is False), the
`mutate_image` method adds `skip-compression:True` to the `meta_patch`.

This commit also sets `skip_compression` value to True for Nutanix variant.
@thunderboltsid thunderboltsid force-pushed the ntnx-qcow2-compression branch from 8e72e12 to cd53df6 Compare May 12, 2022 14:23
@sohankunkerkar sohankunkerkar requested a review from miabbott May 12, 2022 14:55
Copy link
Copy Markdown
Member

@miabbott miabbott left a comment

Choose a reason for hiding this comment

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

cosa buildextend-nutanix and cosa compress behaved as expected

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.

use internal qcow2 compression for nutanix image

5 participants