Add an error message if a pkg cannot be removed, fixes #35672

[kustodian]

SUMMARY

When yum cannot remove a package it fails, but the message why the module failed isn't obvious. This adds an error return field which prints exactly which package couldn't be removed.
This PR is a fix for #35672.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

yum

ANSIBLE VERSION

ansible 2.6.0dev0

[ansibot]

cc @Akasurde @berenddeschouwer @mkrizek @verm666
click here for bot help

[mkrizek]

I wonder if it would be better for consistency to append it to res['msg'] rather than creating a new key, since we use msg for errors in the module?

[kustodian]

I thought about it, but in this situation the msg looks like this:

Skipping the running kernel: kernel-3.10.0-693.21.1.el7.x86_64

Which isn't that obvious tbh and if I appended my message it would be easy to miss it. I would leave msg to the yum output.

Also yum module is all over the place regarding the return keys, it would really be nice if we made it consistent and also add the docs about the return keys. I could probably provide some proposal before any work is done on it if you are interested.

[mkrizek]

I don't have strong feelings about this really but I guess my concern was that with this we could end up with having error messages in both msg and error at the same time which seems little confusing to me.

[mkrizek]

Having said that we could do this (add error) for now and do cleanup for the whole module (including documenting the return keys) later.

[bcoca]

if it is an error, use fail_json, otherwise it is a warning, which module.warn() can take care of

[kustodian]

@bcoca I don't understand what you are saying. We are using fail_json, it's in the next line. This will just make sure all the fields which were set before are also returned.

[bcoca]

then that should be the msg= to fail_json, the error key is not a good choice here, i could see a 'failed_pkgs' = [ list of failed] as an additional key, but error seems to be part of a standard that does not exist.

[maxamillion]

Agreed with @bcoca here, I would like to see that land in msg

[kustodian]

As requested I set the error message to be returned inside the msg key, so let's get this merged :)

[ansibot]

cc @maxamillion
click here for bot help

[maxamillion]

@kustodian please rebase and resolve the current conflict, thank you :)

[kustodian]

@maxamillion done.

[maxamillion]

rebuild_merge

[maxamillion]

@kustodian thank you! Sorry about the churn on the code. I'd like to continue to do more incremental clean up now that the major refactor is done but hopefully won't be doing anything that big again. Anyways, thank you again!