[CFE-2162] testing needed

[Dimitrios Apostolou]

Hello list,

As part of fixing CFE-2162 [1] I have rewritten the iteration engine of
CFEngine, i.e. the backend that makes sure promises are actuated over each
element of slists or containers.

[1] https://tracker.mender.io/browse/CFE-2162

The changes go deep inside the evaluator, and behavioural changes are
expected, like "cf_null" no longer being treated specially, and other
more subtle things! So I've prepared an Alpha build of my private branch
and all testing would be appreciated. You can find the builds at:

https://cfengine-package-repos.s3.amazonaws.com/experimental/CFE-2162/cfengine-community-3.10.0-0.a1.0.22.i386.rpm
https://cfengine-package-repos.s3.amazonaws.com/experimental/CFE-2162/cfengine-community-3.10.0-0.a1.0.22.x86_64.rpm
https://cfengine-package-repos.s3.amazonaws.com/experimental/CFE-2162/cfengine-community_3.10.0~a1~22_amd64.deb
https://cfengine-package-repos.s3.amazonaws.com/experimental/CFE-2162/cfengine-community_3.10.0~a1~22_i386.deb

Just remember that this is highly experimental code, in preliminary
stage, so take precautions and run your tests in a VM. And debug output
(-d) is way too dense at this point, you've been warned. :-) The main
reason I'm putting this out is to get feedback on compatibility with
your existing policy.

The big difference of this build should be much better performance in
policies with nested expansions of slists/containers.

All feedback is appreciated - positive or negative, on all aspects like
performance, correctness etc.


Thanks!
Dimitris

P.S. Sorry for hijacking help-cfengine mailing list, the only purpose is to catch as much attention as possible; Please respond only on dev-cfengine which is more relevant to the subject.

[Alex Georgopoulos]

A quick test on my policy gave me this error message.  

   error: Bundle 'autorun' listed in the bundlesequence is not a defined bundle

   error: Fatal CFEngine error: Errors in promise bundles: could not verify bundlesequence

I don't use autorun and this worked on 3.8.2.  My policy is pretty close to masterfiles-git

[Dimitrios Apostolou]

On Sat, May 21, 2016 at 7:03 PM, Alex Georgopoulos <ageo...@gmail.com> wrote:

A quick test on my policy gave me this error message.  

   error: Bundle 'autorun' listed in the bundlesequence is not a defined bundle

   error: Fatal CFEngine error: Errors in promise bundles: could not verify bundlesequence

I don't use autorun and this worked on 3.8.2.  My policy is pretty close to masterfiles-git

Weird, I tested latest masterfiles with no apparent problems. I'll see if I can figure out what's wrong, assuming that your policy is similar to masterfiles. The debug ( -d ) log would help as well.

Thanks for testing!

Dimitris

[Alexis Mousset]

Hello,

I have run the ncf tests against this build, and found no regression related to iterations. However, some tests are broken because strings containing unexpandable variables are skipped. Is it a intended change of behavior in 3.10?

I did a quick comparison with 3.9b1 and found a speedup around 7% with this build, but the tests do not use a lot of nested iterations.

Alexis

--
You received this message because you are subscribed to the Google Groups "dev-cfengine" group.
To unsubscribe from this group and stop receiving emails from it, send an email to dev-cfengine...@googlegroups.com.
To post to this group, send email to dev-cf...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/dev-cfengine/CALSpUb2ae7hRuervDrxJae%3DtERob30jBNm%3D9DBsxmAoBcWLLVA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--

[Nick Anderson]

On 05/23/2016 08:31 AM, Alexis Mousset wrote:
> Hello,
>
> I have run the ncf tests against this build, and found no regression
> related to iterations. However, some tests are broken because strings
> containing unexpandable variables are skipped. Is it a intended change
> of behavior in 3.10?

It might be helpful if you can provide self contained reproductions for
the things that aren't working as expected. How are you using these
unexpanded variables?

> I did a quick comparison with 3.9b1 and found a speedup around 7% with
> this build, but the tests do not use a lot of nested iterations.

Thats pretty significant I would say. Thanks for testing!

[Alexis Mousset]

On 05/23/2016 03:38 PM, Nick Anderson wrote:

On 05/23/2016 08:31 AM, Alexis Mousset wrote:

Hello,

I have run the ncf tests against this build, and found no regression
related to iterations. However, some tests are broken because strings
containing unexpandable variables are skipped. Is it a intended change
of behavior in 3.10?

It might be helpful if you can provide self contained reproductions for
the things that aren't working as expected. How are you using these
unexpanded variables?

Actually, it seems to happen only with nested variables. A simple case is:

bundle agent main {
vars:
"string1" string => "test1";

reports:
"$(test simple)";
"$(test $(string1))";
"$(test $(string2))";
}

Which displayed:

R: $(test simple)
R: $(test test1)
R: $(test $(string2))

before, and only:

R: $(test simple)

in this build.

I did a quick comparison with 3.9b1 and found a speedup around 7% with
this build, but the tests do not use a lot of nested iterations.

Thats pretty significant I would say. Thanks for testing!

--

[Dimitrios Apostolou]

On Mon, May 23, 2016 at 3:31 PM, Alexis Mousset <alexis....@normation.com> wrote:

I have run the ncf tests against this build, and found no regression related to iterations. However, some tests are broken because strings containing unexpandable variables are skipped. Is it a intended change of behavior in 3.10?

*Empty* slists are completely skipped, there is no iteration with $(varname) as the string.

Unexpanded string variables are not skipped though. IMHO it would make sense to skip all promises that contain unexpanded strings, I would really like to see that change happening. But I held back because I'm not sure, how would you distinguish a string containing the dollar-parenthesis characters on purpose?

 

I did a quick comparison with 3.9b1 and found a speedup around 7% with this build, but the tests do not use a lot of nested iterations.

Great! Speedup for the common case should increase even more once I remove all the debug hooks I've spread in the codebase and get a release build out.

Thanks for testing!

Dimitris

[Dimitrios Apostolou]

On Mon, May 23, 2016 at 4:10 PM, Alexis Mousset <alexis....@normation.com> wrote:

Actually, it seems to happen only with nested variables. A simple case is:

Thanks for the contained example, it helps a lot. This subtle behaviour change is a side-effect I guess. :-)

Dimitris

[Alex Georgopoulos]

It is weird.  I did a fresh install and fresh checkout and those worked.  Something subtle in my policy.  I'll dig in and figure it out

[Dimitrios Apostolou]

On Mon, May 23, 2016 at 6:02 PM, Alex Georgopoulos <ageo...@gmail.com> wrote:

It is weird.  I did a fresh install and fresh checkout and those worked.  Something subtle in my policy.  I'll dig in and figure it out

No need, I just reproduced it. I'm only getting it using my package with 3.8.x policy. First thing is to see whether it's also happening in master-branch (as opposed to jimis-branch :-).

Dimitris

[Bas van der Vlies]

I just tested it and my setup to include the proper config files for different clusters does not work anymore. Here is a reduced examples. The class LISA_CLUSTER is defined by init_node and now the
cluster_name is unresolved: /var/cfengine/bin/cf-agent -KI -f ./test_promise.cf -DRUN
{{{
error: Unresolved variable '$(cluster_name)/cluster.cf' in input list, cannot parse
error: Unresolved variable '$(cluster_name)/cluster.cf' in input list, cannot parse
R: run_test
}}}

{{{
body common control
{
version => "$Revision: 4450 $ $Author: bas $";
ignore_missing_bundles => "true";

## Default handshake protocol to connect to server
protocol_version => "2";


inputs => {
@(g.cluster_inputs),
};

bundlesequence => {
@(run.sara_bundles)
};
#
## End
}

bundle common g
{
classes:

## do not execute node_init if one of the classes is set via command line
#
"GOT_INIT"
or => {
"LISA_CLUSTER",
};

## Use realpath instead of variable name else the classes are activated to late
#
!GOT_INIT::
"GOT_INIT" expression => usemodule("init_node","/etc/node_status/cluster");

vars:

UNDEFINED::
"cluster_name" string => "undefined";
"cluster_inputs" slist => {
"undefined/dummy.cf",
};

LISA_CLUSTER::
"cluster_name" string => "lisa";

any::
"cluster_inputs" slist => {
"$(cluster_name)/cluster.cf",
};
}

bundle agent run
{
vars:

any::
"quarterly_bundle" slist => {
"run_test"
};

"sara_bundles" slist => { @(quarterly_bundle) },
policy => "overridable";
}

bundle agent run_test
{
reports:
"run_test";

> --
> You received this message because you are subscribed to the Google Groups "help-cfengine" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to help-cfengin...@googlegroups.com.
> To post to this group, send email to help-c...@googlegroups.com.
> Visit this group at https://groups.google.com/group/help-cfengine.

> For more options, visit https://groups.google.com/d/optout.

---
Bas van der Vlies
| Operations, Support & Development | SURFsara | Science Park 140 | 1098 XG Amsterdam
| T +31 (0) 20 800 1300 | bas.van...@surfsara.nl | www.surfsara.nl |

[Dimitrios Apostolou]

Thanks Bas. This is a known issue I'm facing when a variable is defined twice: once under "any" context and once under "LISA_CLUSTER" context, in your case.

In theory redefining a variable should be forbidden, because convergence becomes uncertain.

In practice though the variable can get redefined with a new value on each pass. Then which value should you use in the reports (or any other) promise that uses that variable? Should you use both, and report a different message on each pass? I.e. execute the promise twice?

Dimitris

[Bas van der Vlies]

The variable is used in the “any” section not defined. If I change it to:
{{{

LISA_CLUSTER::
"cluster_name" string => "lisa";

"cluster_inputs" slist => {
"$(cluster_name)/cluster.cf",
};
}}}

I get the following error message:
12:48 r41n1.lisa.surfsara.nl:/var/cfengine
root# /var/cfengine/bin/cf-agent -KI -f ./test_promises.cf -DRUN
error: Unresolved variable '@(g.cluster_inputs)' in input list, cannot parse
error: Unresolved variable '@(g.cluster_inputs)' in input list, cannot parse
R: run_test lisa

So its looks like the “body common control” can not resolve it.

[Dimitrios Apostolou]

On Mon, May 23, 2016 at 4:10 PM, Alexis Mousset <alexis....@normation.com> wrote:

reports:
"$(test simple)";
"$(test $(string1))";
"$(test $(string2))";
}

Which displayed:

R: $(test simple)
R: $(test test1)
R: $(test $(string2))

before, and only:

R: $(test simple)

in this build.

What would you guys think as the best behaviour?

In my opinion completely skipping promises with unexpanded variables would make sense, because it seems that convergence was not achieved.

Dimitris

[Bas van der Vlies]

I agree. It could do more damage then good in my opinion.

That said we have still this issue:
{{{
bundle agent main
{
vars:
any::
#"split1" slist => string_split("one:two:three", ":", "10");
"test_string" string => "
ExecStartPre={ path=/bin/mkdir ; argv[]=/bin/mkdir /var/run/surfsara-slapd ; ignore_errors=yes ; start_time=[n/a] ; stop_time=[n/a] ; pid=0 ; code=(null) ; status=0/0 }
ExecStart={ path=/opt/ldap/current/lib/slapd ; argv[]=/opt/ldap/current/lib/slapd -h ${SLAPD_SERVICES} $SLAPD_OPTIONS ; ignore_errors=no ; start_time=[n/a] ; stop_time=[n/a] ; pid=0 ; code=(null) ; status=0/0 }
";
"test_list" slist => string_split("$(test)", "\n", "150");

reports:
any::
"$(test_string)";
"$(test_list)";
}
}}}

This will also fail. I have to fill in the issue. some command will return text with variables. It was issue:
* https://tracker.mender.io/browse/CFE-2086

But we solved that differently.


> Dimitris

>
>
>
> --
> You received this message because you are subscribed to the Google Groups "dev-cfengine" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to dev-cfengine...@googlegroups.com.
> To post to this group, send email to dev-cf...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/dev-cfengine/CALSpUb11BkoKYFRPN3rTo8nzVHRVUUvWYGLD13%3DVbD%2B1Qbd88Q%40mail.gmail.com.

> For more options, visit https://groups.google.com/d/optout.

[Nick Anderson]

On 05/24/2016 08:22 AM, Dimitrios Apostolou wrote:
> What would you guys think as the best behaviour?
>
> In my opinion completely skipping promises with unexpanded variables
> would make sense, because it seems that convergence was not achieved.

To me it makes sense to skip it until the final pass of convergence at
which time the literal value could be used. At least that makes sense to
me for a default behavior from the perspective of trying to do the
correct thing without any specific instruction. I also think it would
make sense to have the ability to indicate that a promise that contains
an unexpanded var is not actuated (to allow for easier avoidance of bad
behavior when variables don't get defined).

Maybe thats too magical.

[Eystein Måløy Stenberg]

I think skipping promises with unexpanded variables is a safe and simple
approach.

I have seen too many issues with configurations (and thus services)
being broken due to a variables not expanded for some reason. It is
better to leave old and working configurations in this case.

Unexpanded variables is a bug (either CFEngine or policy) and I don't
think we should proceed, but flag the given promise as not kept.

--

Eystein

[Ted Zlatanov]

On Wed, 25 May 2016 12:58:52 +0200 Eystein Måløy Stenberg <eystein.mal...@cfengine.com> wrote:

EMS> I think skipping promises with unexpanded variables is a safe and
EMS> simple approach.

EMS> I have seen too many issues with configurations (and thus services)
EMS> being broken due to a variables not expanded for some reason. It is
EMS> better to leave old and working configurations in this case.

EMS> Unexpanded variables is a bug (either CFEngine or policy) and I don't
EMS> think we should proceed, but flag the given promise as not kept.

I agree, but also think the promise outcome should be a new one:
"skipped". Or there should be no outcome at all. Otherwise, "promise not
kept" is merging configuration language problems with agent problems
like file permissions.

I propose `mystring` to be a new "literal" string format that indicates
variable expansion should not be done inside, for unusual cases where
you really do need `$(foo)` in a Makefile etc.

Usually this is 'mystring' in other languages but that's already taken
in CFEngine. So when a string is specified with `mystring`, it can be
skipped by the expander. But if it's inserted in a non-literal string,
the "literal" flag won't be preserved, so there is only one level of
complexity.

Ted

[Dimitrios Apostolou]

On Wed, May 25, 2016 at 4:14 PM, Ted Zlatanov <t...@lifelogs.com> wrote:

I propose `mystring` to be a new "literal" string format that indicates
variable expansion should not be done inside, for unusual cases where
you really do need `$(foo)` in a Makefile etc.

That's an interesting approach but involves changing the language, so lets skip it for now. How about escaping the dollar, does it work today? If not, would that be easy to fix?

Dimitris

[Ted Zlatanov]

On Wed, 25 May 2016 16:28:36 +0200 Dimitrios Apostolou <ji...@cfengine.com> wrote:

DA> On Wed, May 25, 2016 at 4:14 PM, Ted Zlatanov <t...@lifelogs.com> wrote:
>> I propose `mystring` to be a new "literal" string format that indicates
>> variable expansion should not be done inside, for unusual cases where
>> you really do need `$(foo)` in a Makefile etc.

DA> That's an interesting approach but involves changing the language, so lets
DA> skip it for now. How about escaping the dollar, does it work today? If not,
DA> would that be easy to fix?

Ultimately the issue is that the evaluation is not deterministic.

Backslashes don't work, see the first report below.

Only `$(const.dollar)` works, and it only protects one expansion. So if
CFEngine decides to expand again, you're busted, see how avoid2 ends up
expanding twice in the second report. edit_line promises are expanded,
so this can get really tricky.

#+begin_src cfengine3
bundle agent main
{
vars:
"avoid" string => "AVOID";
"avoid2" string => "$(const.dollar)(avoid)";

reports:
"$(this.bundle): trying to avoid with a backslash \$(avoid)";
"$(this.bundle): trying to avoid $(avoid2) in a second expansion";
"$(this.bundle): trying to avoid with const.dollar $(const.dollar)(avoid)";
"$(this.bundle): trying to avoid the second expansion $(const.dollar)(avoid2)";
}
#+end_src

Output:

#+begin_src text
% cf-agent -KI ./test_avoid_expand.cf
R: main: trying to avoid with a backslash \AVOID
R: main: trying to avoid AVOID in a second expansion
R: main: trying to avoid with const.dollar $(avoid)
R: main: trying to avoid the second expansion $(avoid2)
#+end_src

The other approach I can suggest is to use Mustache templates to write a
file, but if any variables in the templates are touched by the string
evaluator, you risk expansion.

Ted

[Dimitrios Apostolou]

Thanks Ted, very useful info. There should *definitely* be a clear and always working way of distinguishing whether dollar-parenthesis means variable expansion, or not. Do you know of a ticket for that?

Ted

--
You received this message because you are subscribed to the Google Groups "dev-cfengine" group.
To unsubscribe from this group and stop receiving emails from it, send an email to dev-cfengine...@googlegroups.com.
To post to this group, send email to dev-cf...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/dev-cfengine/87wpmiqjzx.fsf%40lifelogs.com.

[Nick Anderson]

On 05/25/2016 05:58 AM, Eystein Måløy Stenberg wrote:
> I think skipping promises with unexpanded variables is a safe and simple
> approach.
>
> I have seen too many issues with configurations (and thus services)
> being broken due to a variables not expanded for some reason. It is
> better to leave old and working configurations in this case.
>
> Unexpanded variables is a bug (either CFEngine or policy) and I don't
> think we should proceed, but flag the given promise as not kept.

Generally I think this would be a better user experience if we
consistently skipped actuating promises with unresolved variables.

My biggest concern is backwards compatibility. Secondarily to that its
the edge cases of wanting to have specific content somewhere that looks
like a cfengine variable.

[Ted Zlatanov]

On Wed, 25 May 2016 16:46:30 +0200 Dimitrios Apostolou <ji...@cfengine.com> wrote:

DA> Thanks Ted, very useful info. There should *definitely* be a clear and
DA> always working way of distinguishing whether dollar-parenthesis means
DA> variable expansion, or not. Do you know of a ticket for that?

No, sorry. There are many bugs about broken expansion, but not about
this.

Ted