announcement - forking setup-storage

[Thomas Neumann]

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