Project

General

Profile

Actions

Bug #7883

closed

command_execution_result does not define result classes for "other" return codes than kept/repaired

Added by Janos Mattyasovszky about 8 years ago. Updated almost 2 years ago.

Status:
Rejected
Priority:
N/A
Assignee:
-
Category:
Generic methods
Target version:
Severity:
UX impact:
User visibility:
Effort required:
Priority:
0
Name check:
Fix check:
Regression:

Description

Hi,

Running the testall on v0.x (on commit 7c57f4c), command_execution_result.cf fails:

srv:~/ncf/tests/acceptance # ./testall --verbose ./30_generic_methods/command_execution_result.cf

=== Test environment: ===
AGENT=
CF_PROMISES=
CF_SERVERD=
CF_KEY=
=========================

======================================================================
Testsuite started at 2016-02-08 09:24:35
----------------------------------------------------------------------
Total tests: 1

./30_generic_methods/command_execution_result.cf FAIL

======================================================================
Testsuite finished at 2016-02-08 09:24:39 (4 seconds)

Passed tests: 0
Failed tests: 1
Failed to crash tests: 0
Skipped tests: 0

Apparently Test "E" fails with the exit code 5:

2016-02-08T09:24:38+0100  verbose: /default/default/methods/'any'/default/check: Private classes augmented: ok_B ok_D ok_A ok_C
2016-02-08T09:24:38+0100  verbose: /default/default/methods/'any'/default/check/reports: Evaluating promise '$(this.promise_filename) Pass'
2016-02-08T09:24:38+0100  verbose: /default/default/methods/'any'/default/check/reports: Skipping next promise '$(this.promise_filename) Pass', as context 'ok' is not relevant

Files

7883-test-failed[1].log (124 KB) 7883-test-failed[1].log fail logged with verbose Janos Mattyasovszky, 2016-02-08 09:30
Actions #1

Updated by Janos Mattyasovszky about 8 years ago

Attached the failure's log

Actions #2

Updated by Jonathan CLARKE about 8 years ago

  • Target version set to 0.x

I see what is happening here.

The test is defined as:

vars:
  "kept_codes" string => "0,1, 2";
  "repaired_codes" string => "3, 4";

methods:
  "phE" usebundle => command_execution_result("exit 5", "${kept_codes}", "${repaired_codes}");

But the command_execution_result method uses a classes result body defined as follows:

body classes classes_generic_return_code_list(x, kept_return_codes, repaired_return_codes)                                                                                                                                                    
{
  kept_returncodes => { @{kept_return_codes} };
  repaired_returncodes => { @{repaired_return_codes} };

  promise_repaired => { "promise_repaired_$(x)", "$(x)_repaired", "$(x)_ok", "$(x)_reached" };
  repair_failed => { "repair_failed_$(x)", "$(x)_failed", "$(x)_not_ok", "$(x)_not_kept", "$(x)_not_repaired", "$(x)_reached", "${x}_error" };
  repair_denied => { "repair_denied_$(x)", "$(x)_denied", "$(x)_not_ok", "$(x)_not_kept", "$(x)_not_repaired", "$(x)_reached", "${x}_error"  };  
  repair_timeout => { "repair_timeout_$(x)", "$(x)_timeout", "$(x)_not_ok", "$(x)_not_kept", "$(x)_not_repaired", "$(x)_reached", "${x}_error"  };  
  promise_kept => { "promise_kept_$(x)", "$(x)_kept", "$(x)_ok", "$(x)_not_repaired", "$(x)_reached" };
}

As you can see, "error_returncodes" is not defined.

However, the CFEngine documentation is quite clear about what will happen here (from https://docs.cfengine.com/lts/reference-promise-types.html#failed_returncodes)

If none of the attributes kept_returncodes, repaired_returncodes, or failed_returncodes are set, the default is to consider a return code zero as promise repaired, and nonzero as promise failed.

That means none of them, as in zero. However, we define two, so the default does not come into play. This means that any return codes other than the success and repaired codes specified will not cause result classes to be defined.

We either need to change the default behaviour in CFEngine or find a way to say "failed_returncodes = <others>".

Actions #3

Updated by Jonathan CLARKE about 8 years ago

  • Subject changed from command_execution_result.cf FAIL to command_execution_result does not define result classes for "other" return codes than kept/repaired
Actions #4

Updated by Jonathan CLARKE about 8 years ago

Jonathan CLARKE wrote:

As you can see, "error_returncodes" is not defined.

However, the CFEngine documentation is quite clear about what will happen here (from https://docs.cfengine.com/lts/reference-promise-types.html#failed_returncodes)

If none of the attributes kept_returncodes, repaired_returncodes, or failed_returncodes are set, the default is to consider a return code zero as promise repaired, and nonzero as promise failed.

That means none of them, as in zero. However, we define two, so the default does not come into play. This means that any return codes other than the success and repaired codes specified will not cause result classes to be defined.

We either need to change the default behaviour in CFEngine or find a way to say "failed_returncodes = <others>".

Well, the documentation was pretty clear, but as it turns out, wrong.

This Pull Request in CFEngine https://github.com/cfengine/core/pull/1737 changed the default behaviour in CFEngine 3.7 (rudder-agent >= 3.2) so that any "other" return codes are considered as failure.

Actions #5

Updated by Janos Mattyasovszky about 8 years ago

You could:
  1. parse the kept_codes and repaired_codes into an array -- splitstring()
  2. make an array of all possible exit codes (it's not that many) -- irange()
  3. apply difference() on it to get the the complementer list of exit codes.

I am just not sure if you can mix ilist and slist lists, I'll let this idea be evaluated by the cfengine ninjas you have ;-)

Actions #6

Updated by Alexis Mousset over 7 years ago

  • Category set to Generic methods
Actions #7

Updated by Alexis Mousset about 7 years ago

  • Status changed from New to Rejected

This was fixed in CFEngine 3.6.1 (CFEngine Redmine #5986), so any more recent version will work.

Especially, all currently supported Rudder releases uses a higher version.

Closing.

Actions #8

Updated by Alexis Mousset almost 2 years ago

  • Target version changed from 0.x to ncf-0.x
  • Priority set to 0
Actions #9

Updated by Alexis Mousset almost 2 years ago

  • Project changed from 41 to Rudder
  • Category changed from Generic methods to Generic methods
Actions

Also available in: Atom PDF