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

Jenkinsfile.cloud: +8G the qemu qcow2 #228

Merged
merged 1 commit into from
Aug 14, 2018

Conversation

ashcrow
Copy link
Member

@ashcrow ashcrow commented Aug 14, 2018

The installer requires the image to start off with an extra
+8G developer usage. @eparis noted it would be helpful to have
this happen in the pipeline rather than each developer doing it
on their own.

In the (near) future it would be preferable for the installer
to handle this.

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 14, 2018
@dustymabe
Copy link
Member

/me wonders why we are resizing rather than just making the disk bigger when we create it?

@dustymabe
Copy link
Member

/me wonders why we are resizing rather than just making the disk bigger when we create it?

is it because we only want the qemu image to be the one that is bigger?

@@ -166,6 +166,8 @@ node(NODE) {
parallel par_stages; par_stages = [:]

stage("Finalizing, generate metadata") {
// See https://github.com/openshift/installer/blob/master/Documentation/dev/libvirt-howto.md#13a-rhcos
sh "qemu-img resize ${img_prefix}-qemu.qcow2 +8G"
Copy link
Contributor

Choose a reason for hiding this comment

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

The gzip command below is still taking in ${qcow} which is ${img_prefix}.qcow2

@ashcrow
Copy link
Member Author

ashcrow commented Aug 14, 2018

is it because we only want the qemu image to be the one that is bigger?

That's the only one that I'm aware of that needs to be larger. We may want to bump up the vagrant ones as well.

@cgwalters
Copy link
Member

The default size of the default disk is going to be influenced by the base size of all of the containers in the default install right? It feels like we'll end up needing to track that somewhere; probably the installer, but I also don't see a problem with just making all of our images unconditionally larger.

Doing the latter would be just a matter of tweaking the magic comment at the top of cloud.ks.

@ashcrow
Copy link
Member Author

ashcrow commented Aug 14, 2018

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 14, 2018
@ashcrow
Copy link
Member Author

ashcrow commented Aug 14, 2018

Doing the latter would be just a matter of tweaking the magic comment at the top of cloud.ks.

In that case should I close this in favor of changing cloud.ks to up the size? Is it safe to assume the images will be growable rather than literally adding an extra 8G per artifact?

@dustymabe
Copy link
Member

The default size of the default disk is going to be influenced by the base size of all of the containers in the default install right?

right. To me the best place for this long term is to have the installer request an appropriate size of the image (just like it would do for public cloud) and have that "size" live inside the installer. This PR is a temporary solution IMHO and we have noted that in our previous conversation we had about it.

The installer requires the image to start off with an extra
+8G developer usage. @eparis noted it would be helpful to have
this happen in the pipeline rather than each developer doing it
on their own.

In the (near) future it would be preferable for the installer
to handle this.

Signed-off-by: Steve Milner <smilner@redhat.com>
@ashcrow
Copy link
Member Author

ashcrow commented Aug 14, 2018

This PR is a temporary solution [..]

Agreed.

@ashcrow
Copy link
Member Author

ashcrow commented Aug 14, 2018

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 14, 2018
@ashcrow
Copy link
Member Author

ashcrow commented Aug 14, 2018

I opened openshift/installer#126 so it doesn't get forgotten.

@cgwalters
Copy link
Member

In that case should I close this in favor of changing cloud.ks to up the size?

It feels like we should stay consistent across both local and public cloud as much as possible.

Is it safe to assume the images will be growable rather than literally adding an extra 8G per artifact?

I believe every image format we ship isn't "raw" - qcow2 for example doesn't rely on being "sparse" or filled with zeros. The only one I'm a bit uncertain about is vmdk...and OK, some quick testing by doing:

qemu-img resize rhcos-4.0.4297-qemu.qcow2 +100T
qemu-img convert -f qcow2 -O vmdk -o adapter_type=lsilogic,subformat=streamOptimized,compat6 rhcos-4.0.4297-qemu.qcow2 rhcos-4.0.4297-qemu.vmdk

Does show the vmdk growing to 13G before I killed it; a bit surprising as it does look like the vmdk format talks about "virtual size" just like qcow2.

@cgwalters
Copy link
Member

It looks like AWS supports OVA upload, and hopefully at least that supports being sparse; something to look into later.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 14, 2018
@openshift-merge-robot openshift-merge-robot merged commit ffaa9ff into openshift:master Aug 14, 2018
ashcrow added a commit to ashcrow/installer that referenced this pull request Aug 14, 2018
openshift/os#228 has been added as a
workaround until the installer has the ability to resize images.

Signed-off-by: Steve Milner <smilner@redhat.com>
@jlebon
Copy link
Member

jlebon commented Aug 14, 2018

IIRC, the reason we upload VMDK today is because that's what ore supports (along with raw). Though it shouldn't be very hard to add support for a different type supported by the AWS API.

flaper87 pushed a commit to flaper87/installer that referenced this pull request Aug 17, 2018
openshift/os#228 has been added as a
workaround until the installer has the ability to resize images.

Signed-off-by: Steve Milner <smilner@redhat.com>
wking added a commit to wking/openshift-installer that referenced this pull request Oct 4, 2018
This is the size of the current RHCOS images, but resizing here will
allow the OS folks to stop guessing which size we need [1,2].

Explicitly closing the file before resizing ensures we've flushed the
cache after the earlier copy.  We probably should have been doing that
before the Rename anyway.  With this explict close, the deferred close
call may fail, but that's fine.

Dropping to qemu-img makes me a bit jumpy, but at lest most folks with
a libvirtd running will have qemu-img available.

[1]: openshift#126
[2]: openshift/os#228
wking added a commit to wking/openshift-installer that referenced this pull request Oct 4, 2018
This is the size of the current RHCOS images, but resizing here will
allow the OS folks to stop guessing which size we need [1,2].

Explicitly closing the file before resizing ensures we've flushed the
cache after the earlier copy.  We probably should have been doing that
before the Rename anyway.  With this explict close, the deferred close
call may fail, but that's fine.

Dropping to qemu-img makes me a bit jumpy, but at lest most folks with
a libvirtd running will have qemu-img available.

[1]: openshift#126
[2]: openshift/os#228
wking added a commit to wking/openshift-installer that referenced this pull request Oct 4, 2018
This is the size of the current RHCOS images, but resizing here will
allow the OS folks to stop guessing which size we need [1,2].

Explicitly closing the file before resizing ensures we've flushed the
cache after the earlier copy.  We probably should have been doing that
before the Rename anyway.  With this explict close, the deferred close
call may fail, but that's fine.

Dropping to qemu-img makes me a bit jumpy, but at least most folks
with a libvirtd running will have qemu-img available.

[1]: openshift#126
[2]: openshift/os#228
wking added a commit to wking/openshift-installer that referenced this pull request Oct 15, 2018
This is the size of the current RHCOS images:

  $ qemu-img info ~/.cache/openshift-install/libvirt/image/2454fcdc0a61a9005fbf916b54fbe332
  image: /home/trking/.cache/openshift-install/libvirt/image/2454fcdc0a61a9005fbf916b54fbe332
  file format: qcow2
  virtual size: 16G (17179869184 bytes)
  disk size: 1.6G
  cluster_size: 65536
  Format specific information:
      compat: 1.1
      lazy refcounts: false
      refcount bits: 16
      corrupt: false

Resizing here will allow the OS folks to stop guessing which size we
need [1,2].  And when we need future resizes, we can update here
instead of bugging the OS team.

Also inline the volume in the libvirt Terraform module.  The
libvirt/volume module indirection was unnecessary indirection.

[1]: openshift#126
[2]: openshift/os#228
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants