Just uploading result with -u, exit status corresponds to assessment

[AloisMahdal]

If I generate, say, assessment report that has needs_action as "worst" result:

$ preupg --force; echo $?
...
1
$

Then later I upload it using -u command:

$ preupg -u http://some/url; echo $?
...
1
$

This is counter-intuitive: I just performed "upload" action, which succeeded, but got 1 anyway.

[AloisMahdal]

Two things come to my mind; probably should not:

• use one binary for two entirely different things,
• care about result as long as assessment report is valid (no error XCCDF
  result or exception) -- --riskcheck was created for that.

[bocekm]

Currently the return code corresponds to what is in the man page:

0 - preupg works properly and modules exit with the following results: PASS, FIXED, INFORMATIONAL, NEEDS_INSPECTION, NOT_APPLICABLE.
1 - preupg works properly but at least one module finishes with NEEDS_ACTION result.
2 - preupg works properly but at least one module finishes with FAIL or ERROR result.
...
28 - We have detected some troubles with sending the report to WEB-UI. Check if it is installed.

This behaviour was designed long ago by the former Preupgrade Assistant maintainer when he changed the meaning of return codes 0-2 so they reflect the risks in the report.
But I believe the main cause of this unintuitiveness is the fact you've mentioned - one binary is used for two different things.

Anyway, I'm not trying to defend the current solution. I'm for both updates you mention:

• have a separate executable for the upload functionality, e.g. preupg-upload-result
• have return code 0 reserved for "everything went as expected" and the other codes for errors indicating the assessment hasn't finished successfully. And as you mention, --riskcheck is here to provide the return code based on the risks in the report.

[AloisMahdal]

This behaviour was designed long ago by the former Preupgrade Assistant maintainer when he changed the meaning of return codes 0-2 so they reflect the risks in the report.

Yeah, that was actually based on a misunderstood downstream bug from us. We just wanted preupg to exit with non-zero if assessment generation failed (ie. one or more modules gave error state), because in that case the assessment is incomplete, ie. invalid.

~

Regarding splitting binary: preupg now:

• creates report (default),
• uploads report (-u),
• investigates report (--riskcheck),
• lists upgrade paths (-s)
• and it used to generate KS (--kickstart) but that was moved to other binary.

However, if we want to have one-binary-per-task scheme, I'd be happier if it was designed more consistently, upfront. (I had a shot at this downstream---sorry, not public).

Anyway, simply ignoring the report content when uploading (ie. -u means either 0 or 28) would be enough to remove the weirdness.