Re: [atomic-devel] Changing partitioning defaults discussion

2017-06-06 Thread Dusty Mabe 


On 06/06/2017 11:29 AM, Colin Walters wrote:
> On Mon, Jun 5, 2017, at 03:12 PM, Dusty Mabe wrote:
>>
>> - Why not stick with the distro default? 
> 
> Well, what is "distro default"?  This is a messy topic; some things
> *are* defaults in blivet, and apparently the default filesystem is ext4,
> but for Fedora Server it's overridden.  I'm honestly not sure a lot
> of serious thought is put into those blivet defaults, it seems to me
> to be a lot more like inertia.
> 
> I've attached an updated patch which links to the Server thread.
> For us, the argument that it's the RHEL default also carries weight,
> but for containers, it's what we test with the most downstream.  
> 
> A good specific technical advantage XFS has over ext4 for this
> use case (with overlayfs) is that inodes are dynamic, and when
> you're doing containers you simply tend to have more filesystem
> trees.
> 
>> - If we have a good reason can we do this in a different commit with a 
>>   commit message that is relevant?
> 
> I updated the existing commit message, I think the "match Fedora Server"
> rationale should cover it, right?
> 


Rationale is sound. Patch LGTM for rawhide.

Re: [atomic-devel] Changing partitioning defaults discussion

2017-06-06 Thread Colin Walters 
On Mon, Jun 5, 2017, at 03:12 PM, Dusty Mabe wrote:
> 
> - Why not stick with the distro default? 

Well, what is "distro default"?  This is a messy topic; some things
*are* defaults in blivet, and apparently the default filesystem is ext4,
but for Fedora Server it's overridden.  I'm honestly not sure a lot
of serious thought is put into those blivet defaults, it seems to me
to be a lot more like inertia.

I've attached an updated patch which links to the Server thread.
For us, the argument that it's the RHEL default also carries weight,
but for containers, it's what we test with the most downstream.  

A good specific technical advantage XFS has over ext4 for this
use case (with overlayfs) is that inodes are dynamic, and when
you're doing containers you simply tend to have more filesystem
trees.

> - If we have a good reason can we do this in a different commit with a 
>   commit message that is relevant?

I updated the existing commit message, I think the "match Fedora Server"
rationale should cover it, right?
From 430a8722acd55c93d8794e93c54c992e2c8a7ee7 Mon Sep 17 00:00:00 2001
From: Colin Walters 
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.

Note this deletes the `/boot` override; while I don't like the new Blivet
default of `1G`, it's a messy topic, and I don't think it's worth carrying
an override in the baremetal path just for that.

Note we also explicitly start matching the Fedora Server default of XFS,
with the same rationale; there's a [long thread](https://lists.fedoraproject.org/archives/list/ser...@lists.fedoraproject.org/thread/W4KWEEKADZDCAFK4XJYEJG34DZLTC3BB/)

For AH, we've been using XFS by default, this just ensures it's used
across all filesystems by default.

One specific reason why XFS is good over ext4 is support for dynamic inodes is
really quite nice when using `overlayfs` and containers.
---
 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"
 
+# 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")
 else:
 autoreq.fstype = storage.default_fstype
 
-- 
2.9.4

Re: [atomic-devel] Changing partitioning defaults discussion

2017-06-05 Thread Dusty Mabe 


On 06/05/2017 03:12 PM, Dusty Mabe wrote:
> 
> 
> On 06/05/2017 02:19 PM, Colin Walters wrote:
>> On Mon, Jun 5, 2017, at 01:58 PM, Dusty Mabe wrote:
>>>
>>> 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
>>
>> Yes.  I think I'm proposing we keep that
>> behavior for F26, and change rawhide to remove the separate LV
>> by default.
> 
> OK. To be clear, all of this is for rawhide and onward, not F26. Got
> it.
> 
> If we are going to officially make that proposal can we open a ticket
> for it in the atomic-wg pagure? I know many of us have talked about
> it, but I would like to formalize that as our strategy.
> 

Note this partially plays into https://pagure.io/atomic-wg/issue/281

Re: [atomic-devel] Changing partitioning defaults discussion

2017-06-05 Thread Dusty Mabe 


On 06/05/2017 02:19 PM, Colin Walters wrote:
> On Mon, Jun 5, 2017, at 01:58 PM, Dusty Mabe wrote:
>>
>> 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
> 
> Yes.  I think I'm proposing we keep that
> behavior for F26, and change rawhide to remove the separate LV
> by default.

OK. To be clear, all of this is for rawhide and onward, not F26. Got
it.

If we are going to officially make that proposal can we open a ticket
for it in the atomic-wg pagure? I know many of us have talked about
it, but I would like to formalize that as our strategy.

> 
> (Hmm, a detail here is that because we frob GROWPART=true
>  in the kickstart, ostree will from then on think it's user-modified
>  and not take on new defaults =(   Will have to think about fixing that)
> 

I agree that this pattern (modifying things in kickstart) is something
we'll have to think about, but this particular case shouldn't worry us
too much because when you are rpm-ostree upgrading your storage should 
have been set up already, right? I guess there are corner cases where 
you blow away your storage and start over. For cloud cases you might as
well blow everything away and start over.

>> One case this would affect is booting the qcow directly on kvm/libvirt
>> without extending the disk image or adding another disk first.
> 
> Yes.  And while I'm sure people do that...I think in practice
> that use case should go into one of:
> 
>  - ISO install into VM ("pet"/"elephant" machines)
>  - vagrant (transient dev/test environments)
> 
> (Of course one can the ISO for dev/test and vagrant for pet/elephant machines
> too but talking about the "defaults" of the tools here)
> 

Yeah, I think if we go with overlayfs on / by default then this isn't
an issue.

>> 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. 
> 
> Yeah, in fact let me break this out as a sub-proposal - the Vagrant
> box for F26 changes to "one big /".
> 

If none of these changes are for f26 then i don't see a reason to
change the vagrant box. One nice thing about the vagrant box being
*somewhat similar* to the cloud base qcow2 is that we have some
overlap for test coverage/finding issues. If we move both the qcow
and the vagrant boxes to "overlayfs on /" for rawhide/f27 will that
be satisfactory?

>> 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?
> 
> Not quite; it also affects kickstart users that use `autopart`.
> 
>>> +defaultFS = "xfs"
>>
>> Not used anywhere that I can see
> 
> I think this is sucked into the blivet layer - so if the user is choosing
> filesystems interactively in the GUI, that's what it'll use for example.
> I bet it also comes back to us as `storage.default_fstype`.

Yeah - I see this in pyanaconda/installclass.py

# The default filesystem type to use.  If None, we will use whatever


# Blivet uses by default.   


defaultFS = None

A few questions: 
- Why not stick with the distro default? 
- If we have a good reason can we do this in a different commit with a 
  commit message that is relevant?

> 
> 
>> Could this be removed in a separate commit to explain why it is being
>> removed?
> 
> Broadly we're matching Server.  You're correct to call this out though -
> it will change to the new Fedora default of 1G (which to me feels
> really excessive but we're not yet changing the cloud environment)
> 

Good info for a commit message :)

Dusty

Re: [atomic-devel] Changing partitioning defaults discussion

2017-06-05 Thread Dusty Mabe 


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 
> 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

[atomic-devel] Changing partitioning defaults discussion

2017-06-05 Thread Colin Walters 
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)
- 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
```

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? 
From ccbe400f0340eda93b7d8f3c052c355dea17aebc Mon Sep 17 00:00:00 2001
From: Colin Walters 
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.
---
 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"
 
+# 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