Code review comment for lp:~ltrager/curtin/lp1640519

On Mon, Nov 14, 2016 at 1:29 PM, dann frazier <email address hidden>
wrote:

> Thanks Lee - I agree that querying flash-kernel would be a more robust
> solution. I'm not sure which sets of problems you are trying to solve, and
> which ones are ancillary, so here's some more background/questions.
>
> flash-kernel installs a kernel hook that runs /usr/sbin/flash-kernel on
> every kernel upgrade. This has caused a problem in the past, because
> flash-kernel will return an error on systems it does not support, causing
> the apt upgrade process to error. We've therefore historically had code
> that tries to recognize the system, and only install flash-kernel if it
> should run. In short - flash-kernel was the problem there, not
> u-boot-tools. u-boot-tools can be installed on any system innocuously. If
> that's the problem you are trying to address (which I don't believe is a
> problem anymore), then I'd suggest removing flash-kernel after this test.
>
> Today, installing flash-kernel and/or u-boot-tools on any MAAS-supported
> system should be fine. The only reason I see to selectively install
> u-boot-tools is to save install space. If space is a concern, then this
> helps to address that. You could also remove flash-kernel if unneeded to
> save more space.
>
> If space isn't an issue, and we believe that installing flash-kernel
> everywhere is safe these days, then the simplest fix might be to just
> install flash-kernel/u-boot-tools on all arm systems.
>

Are there arm systems which *dont* or *wont* use u-boot-tools and
flash-kernel? If so, are they certified for MAAS? Unless we have a
special case, I'd push for just always installing it.

>
> If you do stick with the system-matching approach, you might want to
> consider a more generic "optional package" install method than "if
> flash-kernel-supported then install u-boot-tools". flash-kernel can tell
> you what additional packages it requires on the target platform:
>
> machine="$(get_cpuinfo_hardware)"
> get_machine_field "$machine" "Required-Packages"
>
> This way you would only install u-boot-tools on platforms that really need
> it, and also support other required packages than u-boot-tools.
>

Yeah, that's nicer I think. Will "Requires-Packages" list if flask-kernel
is required? If so, then it does look reasonably tidy

1) install flash kernel in the ephemeral
2) run flash-kernel to detect supported machine
3) if supported, install packages in "Required-Packages" list in-target

It is somewhat unfortunate that the ephemeral environment for arm* doesn't
have flash-kernel always installed since it'd be nice
to not have to download and install that each deploy. But maybe that's a
separate bug once we get this one solved.

--
> https://code.launchpad.net/~ltrager/curtin/lp1640519/+merge/310604
> Your team curtin developers is requested to review the proposed merge of
> lp:~ltrager/curtin/lp1640519 into lp:curtin.
>