Re: [atomic-devel] Running docker-storage-setup from a UI

[Marius Vollmer <marius vollmer redhat com>]

Vivek Goyal <vgoyal redhat com> writes:

> On Wed, Apr 13, 2016 at 10:36:40AM +0300, Marius Vollmer wrote:
>
>> [..] So any user-prompting from docker-storage-setup is a bug?  I get
>> a prompt from pvcreate sometimes, for example when the random data on
>> the new partition has a filesystem signature at the beginning.  With
>> what I know now, this is probably unintended.  Should I file an
>> issue?  Lvm likes to add confirmation prompts like this over
>> time... :)
>
> I think if it runs as service, then no terminal is attached and it
> assumes a default answer. I guess yes. Give it a try.

Here what happens with stdin from /dev/null:

    # docker-storage-setup </dev/null
    Checking that no-one is using this disk right now ... OK

    Disk /dev/sda: 2 GiB, 2147483648 bytes, 4194304 sectors
    Units: sectors of 1 * 512 = 512 bytes
    Sector size (logical/physical): 512 bytes / 512 bytes
    I/O size (minimum/optimal): 512 bytes / 512 bytes

    >>> Script header accepted.
    >>> Created a new DOS disklabel with disk identifier 0x79a24362.
    Created a new partition 1 of type 'Linux LVM' and of size 2 GiB.
    /dev/sda2:
    New situation:

    Device     Boot Start     End Sectors Size Id Type
    /dev/sda1        2048 4194303 4192256   2G 8e Linux LVM

    The partition table has been altered.
    Calling ioctl() to re-read partition table.
    Syncing disks.
    WARNING: xfs signature detected on /dev/sda1 at offset 0. Wipe it? [y/n]: n
      Aborted wiping of xfs.
      1 existing signature left on the device.
      Aborting pvcreate on /dev/sda1.

In your opinion, is it a bug or a feature that pvcreate might prompt
like this?

>> Why does d-s-s run during boot, actually?  Shouldn't people run it
>> interactively after changing /e/s/d-s-s?  Or does it only run on
>> first boot?
>
> [ - Get things right automatically during first boot.
>   - Wait for thin pool to come up
>   - Automatically grow pvs backing volume group
> ]

Thanks for the explanation!

>> If you think that a --force-wipe option is not appropriate for d-s-s,
>> Cockpit can wipe the devices itself.  But I think the option is nicer.
>
> I don't know, wiping partitions is always risky by default.

It's an option, it would not be the default behavior.

> User might lose important data. If cockpit can do it, that would be
> great. If there is a strong demand for this, we can add it, but nobody
> has asked for it yet.

I have, and I think there was some demand earlier in the thread.  I
can contribute the code and am happy to look after bugs with it.

> [d-s-s --status]
>
> So all this current config detection can be easily done outside d-s-s
> as well. I think it probably would be better if you run various lvm
> commands to figure out volumes, thin-pools, pvs and display whatever
> information is useful?

I am not sure about "easy". :-) I was hoping that d-s-s can provide a
higher-level abstraction that would make it unnecessary to agree on a
(ever growing) set of low-level conventions.

I'd say we need to have some code somewhere for inspecting the current
state of the docker storage pool.  Having it in d-s-s itself would be
the natural place, IMO, and would make it accessible for others and give
it a fighting chance to keep working.  Having it in Cockpit would create
a pretty high risk that things get out of sync, and nobody else would
benefit from it.

(The same is true for --force-wipe.)

>> I think a reliable "other" flag will be interesting for a potential
>> "docker-storage-setup reset" operation.  This would remove the
>> current pool and create a new one, so that the user can meaningfully
>> remove devices from DEVS again.
>
> If user removes the thin pool, a new one will be created. "lvremove
> docker-vg/docker-pool".

(You need to remove /etc/sysconfig/docker-storage as well before d-s-s
will make a new pool.  Also stopping docker and rm /var/lib/docker are
probably good ideas.)

> I really don't see a need for a new command or option.

But maybe others do.  I hope that counts for something. :-)

> Don't want to make a simple bash script too complicated until and
> unless it is really really needed.

If we would follow your arguments until the end, there would be no
reason to have d-s-s at all. :-) Users can 'easily' create the necessary
thin pools with a few lvm command line calls and write the right
incantation to /e/s/docker-storage.

But we give them a tool since doing all this is in fact non-trivial and
a common use case.  Let's improve the tool to be more robust and cover
more use cases.  That is the fate of all useful software, it will grow.
I would be happy to help.

"d-s-s reset" would allow people to switch storage drivers, switch from
loopback to thinpool, and remove disks from the pool again in a simple
and restricted, but robust and supported way.