announcement - forking setup-storage

[Gémes Géza]

Hi,
> Hello
>
> tl;dr: reasons for the fork, document current state, asking for
> feedback/help
>
>
> I have started to fork setup-storage. Actually forking isn't the proper
> term, it's more like a rewrite.
>
>
> = Why change the code? =
>
> I'm extremely dissatisfied with the current code since I'm using LVM-volumes
> and the setup-storage 1.4 is fundamentally broken for LVM. It's hard to
> determine how buggy the code is. Patches exist, but there's no guarantee
> they break some other functionality.
>
> = Why fork the code? =
>
> To make it obvious it's not simply a drop-in replacement. I attempt to stay
> as close to the current interface as possible, but deviations may happen. If
> everything goes well, then this fork will turn out to be a drop-in
> replacement, but until then it's safer to consider it something else then
> the successor of 'setup-storage'.
>
> But what about the setup-storage test framework you wrote?
>
> On it's own it's insufficient. Some features can not be tested / are very hard
> to test by emulation through tmpfs-backed disks. (e.g. accessing devices by
> path). But I think it's a good supplement for unit-tests.
>
>   first layer: testing internal interfaces through unit testing
>   second layer: testing real-world configurations through emulation
>   third layer: live environment
>
> (Ideally most bugs can be found in the first and second layer, but some bugs
> will surface only in the real world.)
>
> = Why rewrite the code? =
>
> Because I don't understand the current code. I really tried to but there are
> design decisions and coding practices in the current version that are making
> a successfull understanding of the code really hard. In my humble opinion
> the current code is simply not maintainable by anybody else then the
> original maintainer (and maybe not even by him).
>
> Btw. I think I found an interface bug in regards to the 'devices'-spec. The
> regexp pattern is:
>
>    /([^\d,:\s\-][^,:\s]*(:(spare|missing))*(,[^,:\s]+(:(spare|missing))*)*)/
>
> Please note the two different regular expressions before '(:(spare|missing)',
> which would result in
>
>    abcd3:spare,4bcd3:spare
>
> being a valid value, but if you switch the order to
>
>    4bcd3:spare,abcd3:spare
>
> it becomes an invalid value because the first identifier may not start with a
> digit. I found this bug while trying to understand and re-implement the
> parser.
>
>
> = What are my goals? =
>
> - understand the internal program flow
>
> - enable a true 'dry-run' mode
>
> In spirit this feature is present by ommitting the '-X' switch, but the
> implementation is broken by design. It is not even possible to 'just' check
> the validity of a disk configuration file outside of an actual FAI
> installation, since the config check may fail because the existing LVM
> devices can't be disabled / aren't disabled. Not providing the '-X' may
> raise a false alarm that the current config is invalid although in truth
> everything is fine and setup-storage simply wasn't able/allowed to provide
> some necessary conditions.
>
> - split the code into different modules which can be used/tested in isolation
>
> On the outside the modules Commands.pm, Exec.pm etc. exist, but as soon as
> one takes a closer look there's only one namespace (FAI). One might as well
> concatenate all 'modules' together into a single file, because that's the way
> the perl interpreter sees them.
>
> - abolish the use of package-scoped variables
>
> Currently the internal function communicate mainly via package scoped global
> variables. Testing / understanding a single function is extremely hard
> because one must understand/setup the whole internal configuration state
> beforehand.
>
> - provide a test harness (unit tests)
>
> As of today, setup-storage provides no testing method. The original
> maintainer may have some test framework in place, but it's not publicly
> available. Providing unit tests for each feature makes it possible to find
> and fix bugs faster and with more confidence (by proving that it actually was
> a bug and the fix did not break some other functionality).
>
>
> = current state =
>
> completeness: maybe 2%
>
> At the moment this is just an announcement that I'm working on this. Don't
> expect any useable results soon. (I'm currently rewriting the configuration
> file parser, but making only slow progress due to the limited time that I
> have available.) The code is located in a local git repository, unit tests
> can be executed manually and are also done automatically after committing
> through a local jenkins installation.
>
> There's no reason why this can't be made publicly available if there's an
> actual interest. It just wasn't necessary yet.
>
> = feedback =
>
> Is my reasoning sound?
> Do the goals sound reasonable?
> Is there a different way to achieve them?
>
> = help =
>
> It would be really nice if someone else is also willing to work on this.
> Available topics: Analyze the existing code, design and/or write unit tests,
> write/update/check the documentation. Whatever piques your interest.
>
> requirements:
> - English or German
> - a desire to improve the world
> - knowledge of perl and unit testing appreciated but not strictly necessary
>
> thanks for your time
> thomas
If you don't mind I'll start with a feature request, which would make 
the life of people in need for a windows/*linux dual boot install much 
easier:
write the replacement in a way, that preserve_allways:x would not 
overwrite the partition entry for ?dx, creating an unbootable windows 
install, forcing the dual boot installation sites to prepartition and 
use preserve_allways:all.

Cheers

Geza Gemes