ncm-dpmlfc: major rewrite to support DAV configuration

[jouvin]

This PR is a major rewrite of ncm-dpmlfc to support configuring DPM WebDav protocol with ncm-dpmlfc. To do this, the ncm-xrootd rule-based editor has been ported back to DPM to replace the original one. This rule-based editor has been slightly extended but should remain compatible with the ncm-xrootd one. The intent is to move it as a new CAF module (it probably represent 1/3 of the configuration module code currently).

Another improvement in ncm-dpmlfc is in the schema: rather than specifying the protocol options for each node (whith usually the same options for all nodes), it is now possible to define the protocol options globally that will act as a default for each nodes where those options are not specified.

In addition to this feature extension, there was an extensive rewrite of this configuration module to simplify the code. The old one was based on the assumption that both DPM and LFC could be configured on the same machine, something that has never been actually possible as both product have a (different) daemon listening the same port. This was leading to a very complicated processing of the configuration that has been streamlined. A few 100s of lines have been removed!

As part of this rewrite, unit tests have been added both for some of the rule-based editor features and for the whole Configure method with basically all the config files managed by the config module checked. This allowed to uncover quite a number of flaws in the original code and is a pretty good coverage of the whole code. The new version is test friendly: the few LC::Check calls still needed have been protected by testing $CAF::Object::NoAction. When tests are run in verbose mode, the output should produce noERROR` message in addition to passing the tests.

This version has been deployed successfully at LAL yesterday and is almost ready for merging. The two remaining minor things to do are:

• Validate pan schema in unit tests
• Update the pod file for the DAV part and the new global options for protocols.

Having it part of 15.12 would be great! Thus the milestone. But I would understand that this is too late...

[stdweird]

first exists is not needed

[jouvin]

@stdweird what about perl-enum not found during executiong of unit tests? Is it enough to list it as a dependency in the pom file?

[stdweird]

test this please

[stdweird]

@jouvin i added enum, tests pass again. (i forgot to do that yesterday, apologies for the delay)

[stdweird]

the previous conversation was lost

I understand your point but the problem is how do you return a bunch of value from a Perl function (I know how to do it in Python!).

i would convert the curent variables in a hashref, and return the hashref

e.g.

sub _parse_one_rule
{
    ...
    my $res = {};
    $res->{value_fmt} = $value_fmt;
    ...
    return $res;
}

(i'm not sure i understand your question though)

You basically need to return if the rule has to be applied or not and the parameter used to edit the line after the rule is parsed. That means one boolean and one hash. Do you consider that passing the hash as an argument and having it modified by the function is a good practice?

i am talking about a rule parser, i.e. something that converts

[condition->]attribute:option_set[,option_set,...];line_fmt

in

my $rule = {
    condition => x,
    attrinute => y,
    option_set => [option1, ...],
    line_fmt =>,
}

in particular, this method should retrun undef on failure and log error in case the rule is invalid, so it can be used e.g. for unittestting the rules. in particular, all rules should be valid/parsable before a single one is applied.
right now, you apply the rules one by one, and in case there is an issue, you continue, instead of doing a $fh->cancel and return undef/stop.

[jouvin]

With the last commit, the suggestion to have a distinct method for parsing rules has been implemented. Updated in both ncm-dpmlfc and ncm-xrootd. 2 improvements are left for a later version:

• cancel file modification if an error occurs applying of the rules. I agree that this should be the default behaviour but I'd like to implement the option to just ignore errors (apply as many mods as possible) to keep the current behaviour possible.
• Preserve initial whitespace when modifying a line. This should be possible with the customized replace() used and the required feature may be provided by CAF::FileEditor in the future (Add a rule-based file editor to CAF CAF#123).

As for me, this PR is ready for merging.

[jouvin]

BTW, deployed successfully at LAL.

[jouvin]

@jrha I'd really like to see this one going into 15.12, please... It has been under a very detailed review by @stdweird and is in prod at LAL. It will bring only advantages on the previous version!

[jouvin]

A bug was just uncovered at LAL in shift.conf management. I'm fixing it asap. Unfortunately this file has a rule with a specific feature and was not enough unit tested, conversely to other files... Another proof that we should never minimize the effort on unit tests!

[jouvin]

Pb fixed, new version deployed successfully at LAL, missing coverage in unit tests fixed.

@jrha is there any change to have it included in 15.12, I saw that RC2 is out already, I know this is late but there are some important fixes for people using Quattor to manage DPM...

[jrha]

I spend a while thinking about this, but in the end decided that given it only touches one grid component which is known to have issues, it was better off merged.

[jouvin]

Thanks!