Code review comment for lp:~raharper/curtin/trunk.fix-bcache-multi-bdev-to-cdev-lp1514094

On Tue, Nov 17, 2015 at 9:21 AM, Scott Moser <email address hidden> wrote:

> On Tue, 17 Nov 2015, Ryan Harper wrote:
>
> >
> >
> > Diff comments:
> >
> > >
> > > === modified file 'curtin/commands/curthooks.py'
> > > --- curtin/commands/curthooks.py 2015-10-02 19:59:11 +0000
> > > +++ curtin/commands/curthooks.py 2015-11-11 14:47:59 +0000
> > > @@ -687,6 +687,10 @@
> > > "mdadm.conf")
> > > if os.path.exists(mdadm_location):
> > > copy_mdadm_conf(mdadm_location, target)
> > > + # as per
> https://bugs.launchpad.net/ubuntu/+source/mdadm/+bug/964052
> > > + # reconfigure mdadm
> > > + util.subp(['chroot', target, 'dpkg-reconfigure',
> > > + '--frontend=noninteractive', 'mdadm'], data=None)
> >
> > Hrm, unclear to me yet if we end up tripping ourselves up with
> modifications to the target that could potentially stick around.
>
> > Also not clear to me that we won't have issues with configurations to
> installations of packages that might expect something to be present
> > post install that update-initramfs might do to the target filesystem.
>
> update-intiramfs is a focused operation without side effects. If it has
> side effects, those are issues with it and we really should raise it.
>

That's fair; I suppose the package would fail to install if it was checking
for post update-initramfs state since we don't run that at install time.

>
> > I hate to extend the install time, that said, this only hits for mdadm
> case where it is really needed.
>
> It already runs so much though. And each one is a multi-second operation
> with disk io and cpu.
>
> Can we at least try to make this a call to update_initramfs so that we
> explicitly "know" that is what we're doing? That'd mean in the future we
> could possibly disable update-intiramfs during install and then update
> once at the end (just having each update_initramfs call mark that one was
> requested and then running at the end if any were).
>

I'm fine with that, however; the documentation surrounding this currently
says
that mdadm needs to be reconfigured; If we're fine in saying that the only
thing
that matters is getting mdadm.conf into the initramfs for
post-install-first-boot
to ensure that the device we created at install time (say md0 and md1)
actually
end up as md0 and md1 on first boot then OK.

I'm open to alternatives but I don't think we need to exhaustively search
for that
while holding up a critical release issue.

> Scott
>
> --
>
> https://code.launchpad.net/~raharper/curtin/trunk.fix-bcache-multi-bdev-to-cdev-lp1514094/+merge/277255
> You are the owner of
> lp:~raharper/curtin/trunk.fix-bcache-multi-bdev-to-cdev-lp1514094.
>