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

Image resize support for libvirt/development use cases #126

Closed
ashcrow opened this issue Aug 14, 2018 · 16 comments
Closed

Image resize support for libvirt/development use cases #126

ashcrow opened this issue Aug 14, 2018 · 16 comments

Comments

@ashcrow
Copy link
Member

ashcrow commented Aug 14, 2018

When using qcow2's and libvirt one of the first steps is to resize the image. A temporary workaround was proposed (see openshift/os#228) to keep people from needing to do this manually for development purposes.

A cleaner approach for the future would be to have the installer be able to resize the image to what is requested (maybe with a minimum size limit).

@wking
Copy link
Member

wking commented Oct 4, 2018

I was poking around in this area, since it would be fairly straightforward to slot something in here to perform resizing. But I'm a bit nervous about invoking qemu-img, since it's another dependency that users would have to have on their system for the installer to work. I looked around, but did not find anything that looked like an actively-maintained qcow2 resizer in Go. And I'm not excited about porting this to Go even allowing for cgo. Do we want to statically build an image resizer in C (basically just this code) and provide that binary as a fallback for folks without a local qemu-img? Or will everyone with a libvirtd have a local qemu-img?

wking added a commit to wking/openshift-installer that referenced this issue 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 issue 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 issue 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
Copy link
Member

wking commented Oct 4, 2018

I've pushed #413 if folks want to kick the tires on a qemu-img approach.

@ashcrow
Copy link
Member Author

ashcrow commented Oct 4, 2018

/cc @jlebon and @cgwalters as they were looking at some resizing issues recently as well.

@jlebon
Copy link
Member

jlebon commented Oct 4, 2018

FWIW, qemu-img is a dependency of libvirt, so it should always be there if one is planning to install to libvirt.

@crawford
Copy link
Contributor

crawford commented Oct 4, 2018

I think the RHCOS team is going to be responsible for creating images that are large enough though. Yes, we can resize them for libvirt; but we can't do that for every platform.

@cgwalters
Copy link
Member

And this base image size is primarily a function of...the total size of container images pulled by the installer? Isn't that going to change over time?

Or if the goal is "let's have a saner baseline", well that's easy! Honestly since qcow2s are thin-provisioned, we could make it say 32GB?

@cgwalters
Copy link
Member

but we can't do that for every platform.

Basically every IaaS that's worth paying any money for supports specifying disk size on boot, right? So it's just libvirt that doesn't explicitly have an API for this.

@crawford
Copy link
Contributor

crawford commented Oct 4, 2018

Honestly since qcow2s are thin-provisioned, we could make it say 32GB?

Exactly. It's free for us to make the images bigger. Why don't we?

Basically every IaaS that's worth paying any money for supports specifying disk size on boot, right? So it's just libvirt that doesn't explicitly have an API for this.

But that now pushes the problem onto the end user. We should choose reasonable defaults so that the first experience is a painless one.

@cgwalters
Copy link
Member

Exactly. It's free for us to make the images bigger. Why don't we?

Sure, that's easy:

$ git diff
diff --git a/Jenkinsfile.cloud b/Jenkinsfile.cloud
index 823cd85..6e998a7 100644
--- a/Jenkinsfile.cloud
+++ b/Jenkinsfile.cloud
@@ -183,8 +183,6 @@ 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 ${qcow} +8G"
         sh "gzip < ${qcow} > ${img_prefix}-qemu.qcow2.gz"
         sh "ln -sr ${img_prefix}-qemu.qcow2.gz ${dirpath}/rhcos-qemu.qcow2.gz"
         // Everything above in parallel worked on qcow, we're done with it now
diff --git a/cloud.ks b/cloud.ks
index a6ac31f..c826941 100644
--- a/cloud.ks
+++ b/cloud.ks
@@ -1,5 +1,5 @@
 # This line is interpreted by coreos-virt-install
-#--coreos-virt-install-disk-size-gb: 8
+#--coreos-virt-install-disk-size-gb: 32
 text
 lang en_US.UTF-8
 keyboard us

?

@cgwalters
Copy link
Member

Hmm although now I'm remembering past conversation about this, I think a problem here is that the vhd format we need to upload via the EC2 API isn't sparse, or something like that? So this would probably make our uploads a lot slower...we'd need to investigate if there are any workarounds.

@cgwalters
Copy link
Member

(Big picture, the sooner we can git to having a sane "boot image" and then have the installer always pivot to oscontainer, and then we don't need to upload/deal with VM images very often - the better)

@crawford
Copy link
Contributor

crawford commented Oct 4, 2018

AWS's streaming VMDK API should work now (it was problematic in the past).

wking added a commit to wking/openshift-installer that referenced this issue 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
@crawford
Copy link
Contributor

crawford commented Nov 2, 2018

As I mentioned in #413, it is not the installer's job to make sure that the OS is the correct size; that's the OS team's responsibility. The installer isn't going to be able to resize images in all cases, so we should ensure that they are the correct size to begin with.

/close

@openshift-ci-robot
Copy link
Contributor

@crawford: Closing this issue.

In response to this:

As I mentioned in #413, it is not the installer's job to make sure that the OS is the correct size; that's the OS team's responsibility. The installer isn't going to be able to resize images in all cases, so we should ensure that they are the correct size to begin with.

/close

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.

@cgwalters
Copy link
Member

cgwalters commented Nov 8, 2018

As I mentioned in #413, it is not the installer's job to make sure that the OS is the correct size; that's the OS team's responsibility.

I think something was missed here in the initial discussion, which is that we will definitely want (potentially) different disk sizes between the bootstrap, master, and workers. The installer is currently concerned with bootstrap and initial master, right? Worker disk size should be controlled by the machineset - hmm, I don't see disk size in the examples for EC2, but anyways if it's not supported today it should be trivial.

For libvirt...implementing disk size in the machineset would require some extra effort if it's not done today.

So at a high level, I want to get to a place where the disk size is cluster controlled, and the initial RHCOS size is just concerned with bootstrap.

Does that make sense?

@crawford
Copy link
Contributor

crawford commented Nov 8, 2018

Does that make sense?

Yep. That should be sufficient.

miyamotoh pushed a commit to miyamotoh/installer that referenced this issue Mar 17, 2022
…-config

Create CloudProvider config for Power VS
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.

6 participants