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/6d898b136b6f612aa197a223155e79a667432...
https://pagure.io/pungi-fedora/blob/6d898b136b6f612aa197a223155e79a667432...
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(a)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
>