Re: [atomic-devel] Changing partitioning defaults discussion

[Dusty Mabe <dusty dustymabe com>]

On 06/05/2017 01:28 PM, Colin Walters wrote:
> As a followup to the previous thread on storage, also
> to try to fix this BZ:
> https://bugzilla.redhat.com/show_bug.cgi?id=1391725
> 
> I'd like to revisit partitioning again.  There are a number of
> intersecting factors that have changed since things were introduced;
> the biggest is by far the introduction of overlay2.  The second is
> that system containers may use the ostree repo, but that's stored
> in /.   (The advantage of that is the container content can be deduped
> with the host, the downside is their storage is entangled and needs to
> be budgeted together)
> 
> Partitioning I think is going to bifurcate a bit around physical versus
> virtual/cloud. 
> 
> I'd like to propose the attached patch for immediate commit to rawhide
> which causes us to match Fedora Server for physical installs.  See the
> commit message for rationale.
> 
> For people doing physical installs, they have a choice and
> it seems very likely to me that they want to customize things anyways.
> (For example, some people will want dm-crypt, and the fact that
>  ostree supports this is a definite feature)
> 
> Where things get more interesting is the virtual/cloud area.  Here,
> we choose/hardcode partitioning.  At a technical level, most of it lives here:
> https://pagure.io/fedora-kickstarts/blob/master/f/fedora-atomic.ks#_25
> 
> Except that one aspect that I'd forgotten about that's really
> critical when thinking about the kickstart numbers, which is
> the *total size* of the disk.  It turns out this is defined in the Fedora Pungi
> configs.   And what tricked me even more is that we have different
> defaults for the qcow2 vs vagrant:
> 
> https://pagure.io/pungi-fedora/blob/6d898b136b6f612aa197a223155e79a667432ef8/f/fedora-atomic.conf#_227
> https://pagure.io/pungi-fedora/blob/6d898b136b6f612aa197a223155e79a667432ef8/f/fedora-atomic.conf#_247
> 
> The qcow2 version is also what ends up in the AMI.  Now, the rationale
> for this separation I think is that (AIUI) very common for operations
> people to use separately allocated disks (EBS volumes etc) for storage
> of things they care about.  Concretely, it's likely that Kube/OpenShift
> operators are going to stick Docker storage (both LVM/overlay) in
> a separate disk.
> 
> Of course, we use GROWPART=true with container-storage-setup, which
> will expand the root volume group if one allocates a larger disk - this
>  is also quite common to do in IaaS environments (AWS,GCE etc.) in particular.
> 
> So let me propose this:
> 
> QCOW2/AMI ("Production cloud image")
> - Use overlay2 by default (Already done)

One qualification - we use overlay2 by default, but we are going to be
placing all of /var/lib/docker/ on its own filesystem:

    $ cat /etc/sysconfig/docker-storage-setup 
    # Edit this file to override any configuration options specified in
    # /usr/share/container-storage-setup/container-storage-setup.
    #
    # For more details refer to "man container-storage-setup"
    STORAGE_DRIVER=overlay2
    CONTAINER_ROOT_LV_NAME=docker-root-lv
    CONTAINER_ROOT_LV_MOUNT_PATH=/var/lib/docker
    GROWPART=true

I'm not opposed to changing that and making overlay storage default to
going in the root fs. While there are many reasons why that is not
good, it is simple and we can easily document how to more properly set
up your storage. 

> - Expand the / fully, don't reserve any space in the VG, i.e.:
> ```
>   -logvol / --size=3000 --fstype="xfs" --name=root --vgname=atomicos
>   +logvol / --size=3000 --grow --fstype="xfs" --name=root --vgname=atomicos
> ```

One case this would affect is booting the qcow directly on kvm/libvirt
without extending the disk image or adding another disk first.

> 
> We retain the 6GB disk size default, but in practice, I expect operators to either allocate
> larger root storage disks, or choose separate volumes.  Note that operators
> who want to use devicemapper instead need separate volumes; this should
> be well supported with container-storage-setup.
> 
> Vagrant box:
> I suspect the rationale behind the 40GB number is that Vagrant
> implementations are usually thinly provisioned, and that adding
> separate disks is basically not in the default Vagrant workflow.
> (Although it's certainly possible)
> 
> So let's do the same thing here as for the QCOW2/AMI, except keep
> the 40GB number? 
> 

If the default was to just put overlayfs on the root filesystem then i
think this would work fine. If not, then we are basically forcing the
vagrant user to add another disk for container storage. 


> From ccbe400f0340eda93b7d8f3c052c355dea17aebc Mon Sep 17 00:00:00 2001
> From: Colin Walters <walters verbum org>
> Date: Mon, 5 Jun 2017 12:46:40 -0400
> Subject: [PATCH] Match Fedora Server's partitioning
> 
> The max size of 3GB is pretty restrictive for most bare metal installs where we
> expect peopl to have at least one large physical drive.  This gives
> a lot more headroom for things that may be stored in `/` like e.g. the systemd
> journal.
> 
> The max size of `15GB` ensures that if the user wants to make other LVs, there's
> enough space in the VG to do so.

Am i correct in my assumption that the patch below only affects ISO
installs that don't use a kickstart and just use the defaults?

> ---
>  installclass_atomic.py | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/installclass_atomic.py b/installclass_atomic.py
> index cce1470..c3677ca 100644
> --- a/installclass_atomic.py
> +++ b/installclass_atomic.py
> @@ -36,11 +36,13 @@ class AtomicInstallClass(FedoraBaseInstallClass):
>      name = "Atomic Host"
>      sortPriority = 11000
>      hidden = False
> +    defaultFS = "xfs"

Not used anywhere that I can see

>  
> +    # This is intended right now to match Fedora Server; if changing this,
> +    # please discuss on https://lists.projectatomic.io/projectatomic-archives/atomic-devel/
>      def setDefaultPartitioning(self, storage):
> -        # 3GB is obviously arbitrary, but we have to pick some default.
>          autorequests = [PartSpec(mountpoint="/", fstype=storage.default_fstype,
> -                                size=Size("1GiB"), max_size=Size("3GiB"),
> +                                size=Size("3GiB"), max_size=Size("15GiB"),
>                                  grow=True, lv=True)]
>  
>          bootreqs = platform.set_default_partitioning()
> @@ -56,7 +58,6 @@ class AtomicInstallClass(FedoraBaseInstallClass):
>              if autoreq.fstype is None:
>                  if autoreq.mountpoint == "/boot":
>                      autoreq.fstype = storage.default_boot_fstype
> -                    autoreq.size = Size("300MiB")

Could this be removed in a separate commit to explain why it is being
removed?

>                  else:
>                      autoreq.fstype = storage.default_fstype
>  
> -- 
> 2.9.4
>