Update windows pkg.[install|remove] error logic

[lorengordon]

What does this PR do?

• Use cmd.run_all to capture the entire return dictionary.
• Send output_loglevel='quiet' to cmd.run_all when installing to suppress its outputting of errors on non-zero return codes.
• Check the return code to determine whether the install failed.
• Catch retcode == 3010 as a successful install that just requires a reboot.
• Improve the error message when a package is not detected in the registry, particularly to mention packages that install as Windows updates.
• Add a message to the return dictionary about successful vs failed installs. This helps minimize confusion where the package is not detected in the registry.

What issues does this PR fix or reference?

Fixes #33298

Tests written?

No

[twangboy]

@lorengordon Could you rebase this? It's getting some strange lint errors.

[lorengordon]

Sure, that is definitely strange, considering I updated my fork from upstream this morning before I started working on this. Gimme a few min, adding similar retcode logic to pkg.remove...

[twangboy]

@lorengordon The testing guy updated the tests a few hours ago

[lorengordon]

@twangboy, right on. Rebased the branch, hopefully it is good now.

[TheBigBear]

@twangboy @lorengordon thanks for this, but error and return codes for msi installers (and other windows installers) are something that I think the pkg installer file should allow for specifically on a case by case basis.

It is very common for windows installers as well as msi installers to return non zero exit codes.

Most windows package managers I worked with sofar have had a 'flag' or 'parameter' that one can pass to the pkg manager to let it know which exit codes (they are really just exit codes not necessarily all 'error' codes per se) are valid and to be considered a success.

and 3010 is just one of a number of common codes, that eman something in msi installs, but sort of hardcoding this into the pkg installer wholesale I think is not the right approach, it won't 'always' mean that for every piece of windows software. another installer might mean something else entirely with 3010.
Please consider using something like a combination of
success: 0,3010 ( to allow lisitng of ANY exit codes that signify 'success')
restart-required: 3010 (to allow signalling to the logic that the machine needs a restart - btw win_wua already allows a specifc check for this machine flag if a machine needs rebooting)
So 3010 (and any other exit codes that require a restart) can flag that up in the specific installer sls file, in the windows world it is unfortunately NOT a case off one number always means the same thing. Unfortuately a maintainer of windows packages does have to explicitly list and define them on a per software pkg basis.

Just have a look at the wpkg or chocolatey or oneget/nuget or pdq deply 'recipes', this sort of 'hard-coding' of 3010 (maybe I misunderstand it?) looks good, but I don't think is safe or the right way to solve/overcome this.

sorry for the rant. I am tired and about to go to bed, but wanted to get this out there before this, as good as it looks at first, makes it into the core of winrepo-ng.

[lorengordon]

@TheBigBear, understood, this just a simple fix to address a bug in a point release. I thought about adding such a flag to the pkg definition, but adding features and flags is something that would go into develop first. I would recommend submitting your comment as an issue requesting a new feature.