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

use internal qcow2 compression for nutanix image #1191

Closed
vnephologist opened this issue May 4, 2022 · 25 comments · Fixed by coreos/coreos-assembler#2848
Closed

use internal qcow2 compression for nutanix image #1191

vnephologist opened this issue May 4, 2022 · 25 comments · Fixed by coreos/coreos-assembler#2848

Comments

@vnephologist
Copy link

Feature Request

support qemu-img "-c" option for compressing qcow2 images for nutanix platform

Desired Feature

We would like to add functionality to natively compress Nutanix qcow2 using the "-c" compress option rather than gzip them. This functionality would support the ability of the OpenShift installer to orchestrate the Nutanix Image Service to directly download and import the qcow2 rather than having the installer locally unzip then upload an uncompressed image.

Example Usage

coreos-assembler buildextend-nutanix --compress

Other Information

We need to coordinate this change with OpenShift installer functionality so it would be helpful to temporarily build both compressed qcow2 and gzip'd qcow2 artifacts.

@vnephologist
Copy link
Author

@miabbott @sohankunkerkar See issue here as discussed. Please let us know if we should take up these changes ourselves.

@miabbott
Copy link
Member

miabbott commented May 4, 2022

The buildextend-* commands already support a --compress flag, but that just shells out to gzip and compresses the final artifact.

[coreos-assembler]$ cosa buildextend-nutanix --compress
[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.202205032022-0
[INFO]: Processed build for: OpenShift 4 (RHCOS-x86_64) 411.85.202205032022-0
[INFO]: Processing the build artifacts    
[INFO]: Staging temp image: /srv/tmp/tmpaku4nhgh/rhcos-411.85.202205032022-0-nutanix.x86_64.qcow2
...
+ qemu-img convert -f qcow2 -O qcow2 /srv/tmp/tmpaku4nhgh/rhcos-411.85.202205032022-0-qemu.x86_64.qcow2.working /srv/tmp/tmpaku4nhgh/rhcos-411.85.202205032022-0-nutanix.x86_64.qcow2
+ qemu-img info --output=json /srv/tmp/tmpaku4nhgh/rhcos-411.85.202205032022-0-nutanix.x86_64.qcow2
[INFO]: Moving /srv/tmp/tmpaku4nhgh/rhcos-411.85.202205032022-0-nutanix.x86_64.qcow2 to /srv/builds/411.85.202205032022-0/x86_64/rhcos-411.85.202205032022-0-nutanix.x86_64.qcow2
[INFO]: Calculating metadata for rhcos-411.85.202205032022-0-nutanix.x86_64.qcow2
Targeting build: 411.85.202205032022-0
Compressing: builds/411.85.202205032022-0/x86_64
+ gzip -6 -c builds/411.85.202205032022-0/x86_64/rhcos-411.85.202205032022-0-nutanix.x86_64.qcow2
Compressed: rhcos-411.85.202205032022-0-nutanix.x86_64.qcow2.gz
Skipped artifacts: ostree
Updated: builds/411.85.202205032022-0/x86_64/meta.json
[INFO]: Finished building artifacts

We'd probably want to implement something like --qemu-compress that would pass the -c option to qemu-convert when we go from the qemu image to the target image type.

@dustymabe
Copy link
Member

We would like to add functionality to natively compress Nutanix qcow2 using the "-c" compress option rather than gzip them. This functionality would support the ability of the OpenShift installer to orchestrate the Nutanix Image Service to directly download and import the qcow2 rather than having the installer locally unzip then upload an uncompressed image.

Is the only reason you want the image compressed internally because you want to do the "direct download and import"? I've heard some people claim that using the internal qcow2 compression could have performance impacts, but I have no idea whether that is actually true. It might be worth investigating that before we made a change.

@dustymabe
Copy link
Member

The buildextend-* commands already support a --compress flag, but that just shells out to gzip and compresses the final artifact.

[coreos-assembler]$ cosa buildextend-nutanix --compress
[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.202205032022-0
[INFO]: Processed build for: OpenShift 4 (RHCOS-x86_64) 411.85.202205032022-0
[INFO]: Processing the build artifacts    
[INFO]: Staging temp image: /srv/tmp/tmpaku4nhgh/rhcos-411.85.202205032022-0-nutanix.x86_64.qcow2
...
+ qemu-img convert -f qcow2 -O qcow2 /srv/tmp/tmpaku4nhgh/rhcos-411.85.202205032022-0-qemu.x86_64.qcow2.working /srv/tmp/tmpaku4nhgh/rhcos-411.85.202205032022-0-nutanix.x86_64.qcow2
+ qemu-img info --output=json /srv/tmp/tmpaku4nhgh/rhcos-411.85.202205032022-0-nutanix.x86_64.qcow2
[INFO]: Moving /srv/tmp/tmpaku4nhgh/rhcos-411.85.202205032022-0-nutanix.x86_64.qcow2 to /srv/builds/411.85.202205032022-0/x86_64/rhcos-411.85.202205032022-0-nutanix.x86_64.qcow2
[INFO]: Calculating metadata for rhcos-411.85.202205032022-0-nutanix.x86_64.qcow2
Targeting build: 411.85.202205032022-0
Compressing: builds/411.85.202205032022-0/x86_64
+ gzip -6 -c builds/411.85.202205032022-0/x86_64/rhcos-411.85.202205032022-0-nutanix.x86_64.qcow2
Compressed: rhcos-411.85.202205032022-0-nutanix.x86_64.qcow2.gz
Skipped artifacts: ostree
Updated: builds/411.85.202205032022-0/x86_64/meta.json
[INFO]: Finished building artifacts

We'd probably want to implement something like --qemu-compress that would pass the -c option to qemu-convert when we go from the qemu image to the target image type.

Maybe the best place to make a change like this would be:

https://github.com/coreos/coreos-assembler/blob/0fd89554a1921ddc124c142daca83445e4699f05/src/cosalib/qemuvariants.py#L114-L117

@vnephologist
Copy link
Author

@dustymabe this looks like exactly what we want, I see there are others that already implement "convert options", so we could just do the same with "-c".

@dustymabe
Copy link
Member

I'm going to transfer this to our main issue tracker for awareness.

@dustymabe dustymabe transferred this issue from coreos/coreos-assembler May 4, 2022
@dustymabe dustymabe changed the title support qemu-img "-c" option for compressing qcow2 images for nutanix platform use internal qcow2 compression for nutanix image May 4, 2022
@dustymabe dustymabe added the meeting topics for meetings label May 4, 2022
@bgilbert
Copy link
Contributor

bgilbert commented May 4, 2022

I concur that we should verify there are no performance impacts. Another alternative is simply to skip compressing the artifact altogether.

@vnephologist
Copy link
Author

I concur that we should verify there are no performance impacts. Another alternative is simply to skip compressing the artifact altogether.

I am confirming with our core virt/storage team on the performance aspect.

We would definitely want to skip the gzip compression for the resulting artifact.

@thunderboltsid
Copy link

I think we should skip compressing the artifact with gzip for now and we can late-bind on the qcow2 compression once we've verified the performance aspects.

@bgilbert
Copy link
Contributor

bgilbert commented May 4, 2022

Note that we can't stop shipping the compressed artifact (at least without a deprecation period), since that would break anyone using it. We'll need to add a new one.

@vnephologist
Copy link
Author

Note that we can't stop shipping the compressed artifact (at least without a deprecation period), since that would break anyone using it. We'll need to add a new one.

That's fine. Just note that these images are not yet GA'd from OpenShift perspective anyway. We are planning GA in 4.11.

@bgilbert
Copy link
Contributor

bgilbert commented May 4, 2022

Just note that these images are not yet GA'd from OpenShift perspective anyway. We are planning GA in 4.11.

They're shipping in Fedora CoreOS though.

@vnephologist
Copy link
Author

I have confirmed that there is no performance impact with compression. Our image service converts all qcow2 images to raw for deployed VMs.

@thunderboltsid
Copy link

thunderboltsid commented May 5, 2022

Maybe the best place to make a change like this would be:

https://github.com/coreos/coreos-assembler/blob/0fd89554a1921ddc124c142daca83445e4699f05/src/cosalib/qemuvariants.py#L114-L117

@dustymabe Would something like this be sufficient to ensure internal qcow2 compression?

"nutanix": {
        "image_format": "qcow2",
        "platform": "nutanix",
        "convert_options": {
            '-c': ''
        }
    },

Looking at the nutanix variant, it doesn't specify compression anywhere. So, the actual compression of the artifact is happening elsewhere (explicit --compress flag when running the command?). If so, can you point me to the script?

I'm trying to figure out how do the images assembled from the assembler end up as an artifact in https://github.com/openshift/installer/blob/master/data/data/coreos/rhcos.json#L489-L500

@thunderboltsid
Copy link

I'm trying to figure out how do the images assembled from the assembler end up as an artifact in https://github.com/openshift/installer/blob/master/data/data/coreos/rhcos.json#L489-L500

Looking at the past git commits, these changes to the contents of the rhcos.json file are generated using plume.

@miabbott
Copy link
Member

miabbott commented May 5, 2022

Looking at the nutanix variant, it doesn't specify compression anywhere. So, the actual compression of the artifact is happening elsewhere (explicit --compress flag when running the command?). If so, can you point me to the script?

We do the compression of the artifacts in the RHCOS build pipeline as a separate stage, where we just call cosa compress

https://github.com/coreos/coreos-assembler/blob/main/src/cmd-compress

@thunderboltsid
Copy link

Looking at the nutanix variant, it doesn't specify compression anywhere. So, the actual compression of the artifact is happening elsewhere (explicit --compress flag when running the command?). If so, can you point me to the script?

We do the compression of the artifacts in the RHCOS build pipeline as a separate stage, where we just call cosa compress

https://github.com/coreos/coreos-assembler/blob/main/src/cmd-compress

@miabbott Can we, as part of the RHCOS build pipeline, upload a non-gzipped variant along with the current gzipped variant.

@miabbott
Copy link
Member

miabbott commented May 5, 2022

@miabbott Can we, as part of the RHCOS build pipeline, upload a non-gzipped variant along with the current gzipped variant.

It's possible, though I think we'll need to define a new artifact as noted here #1191 (comment)

@thunderboltsid
Copy link

@miabbott Can we, as part of the RHCOS build pipeline, upload a non-gzipped variant along with the current gzipped variant.

It's possible, though I think we'll need to define a new artifact as noted here #1191 (comment)

Yeah, we're fine with a new artifact. So, what do we need to do to make that happen?

@miabbott
Copy link
Member

miabbott commented May 6, 2022

Yeah, we're fine with a new artifact. So, what do we need to do to make that happen?

There's a WIP checklist that covers some of it - #1170

Introducing a new artifact is not trivial, so I would be prepared for a lot of back and forth.

@bgilbert
Copy link
Contributor

bgilbert commented May 6, 2022

I think there might be a problem with introducing a new artifact in this case. In stream metadata, we'd have both qcow2.xz and qcow2 formats, which is okay. In meta.json, we can have distinct lookup keys for the QCOWs with and without internal compression. But in the filesystem itself, both images would have the same name, fedora-coreos-$ver-nutanix.x86_64.qcow2, until cmd-compress runs.

Considering this some more, I now think we shouldn't add a new artifact. The technical hurdles to introducing one aren't trivial, and I'm guessing there aren't many FCOS users who've started using the existing one. And if we carry a second artifact, there's the implication that we'll continue doing so for the long term. So instead, I think we should announce a short deprecation period on coreos-status and just switch over.

@cgwalters
Copy link
Member

Agree with ⬆️ - being compatible where possible is important, but I think this case doesn't quite warrant it.

(I do think longer term though Nutanix and other platforms ideally learns how to accept e.g. kubevirt-style containerdisks natively instead)

@travier
Copy link
Member

travier commented May 11, 2022

From this week's meeting:

  * AGREED: We will switch the Nutanix artifacts to an internally
    compressed qcow2 image for the next cycle for testing & next and the
    following one for stable. This breaking change will be announced
    ASAP.  (travier, 17:33:45)

@travier travier removed the meeting topics for meetings label May 11, 2022
@dustymabe dustymabe added the status/pending-testing-release Fixed upstream. Waiting on a testing release. label May 12, 2022
@dustymabe
Copy link
Member

The fix for this went into testing stream release 36.20220522.2.1. Please try out the new release and report issues.

@dustymabe dustymabe added status/pending-stable-release Fixed upstream and in testing. Waiting on stable release. and removed status/pending-testing-release Fixed upstream. Waiting on a testing release. labels May 25, 2022
@dustymabe
Copy link
Member

The fix for this went into stable stream release 36.20220522.3.0.

@dustymabe dustymabe removed the status/pending-stable-release Fixed upstream and in testing. Waiting on stable release. label Jun 7, 2022
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 a pull request may close this issue.

7 participants