curtin-dev team mailing list archive

[Ryan Harper <rharper@xxxxxxxxxxx>]

Thimo,

Thanks for the thoughtful reply.  I've replied more below to your questions.
I would say here as a summary (tl;dr):

I'm OK with curtin supporting two-disk RAID5 and RAID10.  I'd like to see
this in the form of adding raid_devices: N field to the type: raid structure.

I don't have direct control over subiquity; but I would strongly advice
against enabling two-disk RAID5/10 in the UI mostly to prevent user confusion
about expected capacity or redundancy.  For the advanced user who does
understand what's happening I would have them use the Automated Installer[1]
option which lets users supply custom storage configurations (non-interactive
install).  Michael, I'm interested in your thoughts on this approach.

> Hi Ryan,
>
> first of all thank you for your answer and sorry for the long reply:
>
> > [...]
> > A two-drive RAID5 is RAID1 with different metadata format. Why
> > not just create a RAID1 with your two disks? IIUC, you can
> > already convert RAID1 to RAID5.
> >
> > I believe users who do understand they are creating RAID5 in
> > degraded mode and that they can grow their RAID5 later also
> > know they can convert a RAID1 into RAID5 as well.
> >
> > The expansion of an array is out-of-scope for the installer where
> > as producing non-degraded in the least confusing configurations
> > is very much in-scope.

> One important point first: A two component RAID5 is absolutely NOT a degraded array.
> ....
>
> So I am _not_ asking for the degraded array feature which is for sure an "expert"/corner case feature.

Thank you for the follow-up.  I was mistaken which I found out later as I
ran a few mdadm commands.

>
> Let me hijack your "semi-educated" user standpoint:
> A user with the following requirements:
>  - Availability (survives one component failure)
>  - Expandability (Can add space on demand)
>
> will stumble quite easily over the "grow" feature of mdadm for RAID5. Thus
> he might - after minor research - select two disk RAID5 happily if the
> installer allows. On the other hand - if the installer only allows RAID1, it
> is not so easy to find/imagine that this actually also satisfies his
> expandability requirement.

I suspect the same "semi-educuated" user will also find the examples in
the manpage for expanding or migrating from mirror to parity arrays as well.


>
> >> As a side-note, "mdadm" complains if the creation is missing some
> >> devices and it is not explicitly told "missing", so I am
> >> generally wondering why it is necessary to repeat the check in
> >> the installer (due to missing dry-run feature of mdadm?).
>
> > The 'missing' keyword is interesting. This does allow creating
> > an array of larger number of disks without all being present.
> > I'm still finding this to be a dubious "feature" for the initial
> > installer.
> > [...]
> As mentioned above, I did not want to propose the creation of degraded
> arrays.  I was wondering, why you implement the checks and error messages in
> curtin instead of just issuing the mdadm command and in case of an error
> propagating back the error messages from mdadm (which are often quite
> explanatory).

The main reason why curtin wraps and validates various configurations is that
it's used inside of other frontend installers which have user-facing UIs
in which not all users will grok what exactly is going wrong.  

The other goal is to catch as many errors in configuration without actually
having to boot and run this. MAAS, for example, uses curtin in bare-metal
deployments where some of the hardware may take quite some time (many 
minutes) to power-cycle, boot, start the install only to find out it
failed due to misconfiguration.

With the configuration language and our storage schema MAAS and Subiquity
can validate the configuration before starting an install.

Things still go wrong sometimes, and of course the error message from the
command we run is captured.

Our experience also shows that some tools issue errors that aren't fatal,
and some tools don't report failure when they've failed.  Such is the way of
things.  We utilize this experience to provide a reliable and repeatable
install infrastructure.


>
> >> Regarding the layout support, I think this is a much appreciated
> >> feature but it is not a dependency to fix the current issue.
> >>
> >> Since we are at the layout topic: I am not sure if there is much
> >> interest for the layout feature beyond RAID10. Thus it might also
> >> be an option to have two(/three) different RAID10 modes like
> >> RAID10near and RAID10far. The main question then would be if it
> >> is safe to assume the number of copies to be 2 (from mdadm man
> >> page: "2 is normal, 3 can be useful").
>
> > For min-drives on RAID5 it's not related. For RAID10 my
> > understanding is that the default layout for RAID10 would not
> > give you the optimal single thread perf the linux-raid thread
> > I found[1] indicated that f2 would be best (but this may be dated).
> >
> > 1. https://marc.info/?l=linux-raid&m=141051176728630&w=2
>
> You are absolutely right and this is still valid information.
> Still, I would propose to handle this in a subsequent PR to not intermix a
> "bugfix" with new feature.

That's fine; it can be implemented separately.

>
> > A big portion of what curtin is providing is a language in which
> > to describe your storage configuration. Subiquity is a consumer
> > of this language and in-short is a user-facing tool to help
> > construct curtin's storage configuration files.
G> >
> > Generally curtin delegates behavior choices to the constructor
> > of the configuration. That is as Michael pointed out, curtin
> > shouldn't second guess that someone wants to construct a two-disk
> > RAID5 array etc. I generally agree with that perspective.
> >
> > The other end of this is that without guidance users end up
> > constructing things that don't actually work or are sub-optimal
> > and the blame falls to the installer for letting users footgun
> > their server. Subiquity goes through great efforts to prevent
> > users from constructing configurations which result in an
> > unbootable server even if curtin is more than happy to do so.
> >
> > If we are to enable these special cases then we must provide
> > configuration language sufficient to describe what is possible
> > rather than only the special cases and be explicit about it.
> >
> > For example, the 2-disk raid5 array config could look like:
> >
> > - type: raid
> >   level: 5
> >   raid_devices: 3
> >   devices:
> >     - /dev/sda
> >     - /dev/sdb
>
> As written above, this would be a degraded raid which I did not want to propose.
> Thus the raid_devices are for my needs redundant (and in my case = 2).

This is a good point.  Yes you're correct it would create the degraded array.
If curtin is going to allow someone to create a two-disk RAID5 which can use
--grow; why wouldn't we support a 3-disk raid5 created degraded so user can
add a hotspare later to recover it? 


> I also mentioned in the subiquity bug report that the impact for the "unintentional"
> two disk RAID5 user is not too severe compared to a RAID1. For details,
> please have a look there.

I've seen that discussion.  While I'm flexible with curtin's role of doing
what it's been told I personally would not allow the two-disk RAID5 or 10
mode to be supported in the UI; rather I would point users to the Automated
Install[1] form of Subiquity where user has provided a custom storage config


>
> > And the RAID10 array with specific layout could be
> >
> > - type: raid
> >   level: 10
> >   raid_devices: 2
> >   devices:
> >     - /dev/sda
> >     - /dev/sdb
> >   layout: f2
> Same here, raid_devices would be unnecessary unless you want to support it.

raid_devices is required if we are to allow independent control over the
value of --raid-devices and telling mdadm how many drives to expect.

>
> > However, we'd like to sanity check these configurations, for
> > example
> >
> > - type: raid
> >   level: 5
> >   raid_devices: 5
> >   devices:
> >     - /dev/sdb
> >     - /dev/sdc
> >
> > Will fail at runtime (as I showed above) but this is a poor
> > experience when curtin's block schema can and should validate
> > the configuration and reject it indicating that RAID5 can only
> > allow one missing device.

> Not sure if it is an option for curtin to run the command "as-is":
>  mdadm --create -l 5 -n 5 /dev/md0 /dev/sdb /dev/sdc
> and pass back the (quite descriptive) mdadm error message "You haven't given
> enough devices (real or missing) to create this array".

The preference is to verify a config so we don't have to wait until runtime
to find out there's an error.


>
> > Likewise, adding layout support will need additional checks
> > and validations so that the install experience isn't painful
> > and the language we use to describe what we want isn't ambiguous
> > allowing curtin to incrementally add feature/function over time.
>
> Again, if it is an option for you, you can just pass everything to mdadm an
> receive a good explanation if something went wrong. This way you avoid
> duplicating the logic and error handling of mdadm.  $ mdadm --create -l 5 -n
> 2 -p argl /dev/md0 /dev/sdb /dev/sdc
> mdadm: layout argl not understood for raid5.

Verify first is our preference.

>
> Apart from that, the "layout" keyword would be really nice.
> As a first step, this could be only valid for RAID10, since the layout of
> the other levels are probably not too interesting (at least I do not see a
> strong argument why anyone would favor right-symmetric over
> left-symmetric or the like).

None-the-less if we are adding capabilities to our config to construct
mdadm commands we should add in what is possible.  I'm generally
comfortable with expanding capabilities as we do exercise various
raid configuration paths in our testing harness.


1. https://ubuntu.com/server/docs/install/autoinstall-reference

-- 
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/400931
Your team curtin developers is requested to review the proposed merge of ~mwhudson/curtin:raid-min-levels into curtin:master.