better error for No such user

[ktdreyer]

When koji_tag fails because the packages parameter configures packages for a username that does not exist, we get a large backtrace for koji.GenericError. For example, the error from Koji here is No such user: 'foobar'.

{
    "_ansible_no_log": false,
    "module_stderr": "/tmp/ansible-tmp-1622549651.76-241145048044015/AnsiballZ_koji_tag.py:18: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses\n  import imp\nTraceback (most recent call last):\n  File \"/usr/lib/python3.7/site-packages/koji/__init__.py\", line 2631, in _callMethod\n    return self._sendCall(handler, headers, request)\n  File \"/usr/lib/python3.7/site-packages/koji/__init__.py\", line 2549, in _sendCall\n    return self._sendOneCall(handler, headers, request)\n  File \"/usr/lib/python3.7/site-packages/koji/__init__.py\", line 2595, in _sendOneCall\n    ret = self._read_xmlrpc_response(r)\n  File \"/usr/lib/python3.7/site-packages/koji/__init__.py\", line 2607, in _read_xmlrpc_response\n    result = u.close()\n  File \"/usr/lib64/python3.7/xmlrpc/client.py\", line 656, in close\n    raise Fault(**self._stack[0])\nxmlrpc.client.Fault: <Fault 1000: \"No such user: 'rbehera'\">\n\nDuring handling of the above exception, another exception occurred:\n\nTraceback (most recent call last):\n  File \"/tmp/ansible-tmp-1622549651.76-241145048044015/AnsiballZ_koji_tag.py\", line 114, in <module>\n    _ansiballz_main()\n  File \"/tmp/ansible-tmp-1622549651.76-241145048044015/AnsiballZ_koji_tag.py\", line 106, in _ansiballz_main\n    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)\n  File \"/tmp/ansible-tmp-1622549651.76-241145048044015/AnsiballZ_koji_tag.py\", line 49, in invoke_module\n    imp.load_module('__main__', mod, module, MOD_DESC)\n  File \"/usr/lib64/python3.7/imp.py\", line 234, in load_module\n    return load_source(name, filename, file)\n  File \"/usr/lib64/python3.7/imp.py\", line 169, in load_source\n    module = _exec(spec, sys.modules[name])\n  File \"<frozen importlib._bootstrap>\", line 630, in _exec\n  File \"<frozen importlib._bootstrap_external>\", line 728, in exec_module\n  File \"<frozen importlib._bootstrap>\", line 219, in _call_with_frames_removed\n  File \"/tmp/ansible_koji_tag_payload_pu0bdqnf/__main__.py\", line 634, in <module>\n  File \"/tmp/ansible_koji_tag_payload_pu0bdqnf/__main__.py\", line 630, in main\n  File \"/tmp/ansible_koji_tag_payload_pu0bdqnf/__main__.py\", line 622, in run_module\n  File \"/tmp/ansible_koji_tag_payload_pu0bdqnf/__main__.py\", line 544, in ensure_tag\n  File \"/tmp/ansible_koji_tag_payload_pu0bdqnf/__main__.py\", line 389, in ensure_packages\n  File \"/usr/lib/python3.7/site-packages/koji/__init__.py\", line 2135, in __call__\n    return self.__func(self.__name, args, opts)\n  File \"/usr/lib/python3.7/site-packages/koji/__init__.py\", line 2650, in _callMethod\n    raise err\n__koji__brew-stage.GenericError: No such user: 'foobar'\ncommand terminated with exit code 1\n",
    "changed": false,
    "module_stdout": "",
    "rc": 1,
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error"
}

[ktdreyer]

I'm thinking we should add a NoSuchResource error class to Koji and catch that.

[ktdreyer]

Was reading the Koji Smoky Dingo docs and ran across https://obriencj.preoccupied.net/koji-smoky-dingo/kojismokydingo/#kojismokydingo.NoSuchUser

[ktdreyer]

More discussion in internal ticket CWFCONF-883

[ktdreyer]

From Mike's comments in https://pagure.io/koji/pull-request/3028:

There's a larger issue here of refactoring our exception classes. The tricky part is that adding a new class requires both server and client to know about it. If we add a new exception class, then a client might not know about it and will be unable to convert the Fault.

That's not to say we can't add new exception classes, but it would be better to add them in the client before we start returning them from the hub, if at all possible.

[ktdreyer]

Digging into this more....

Koji's Python library has a koji.GenericException class with various sub-classes.

XML-RPC protocol has a concept of "faults". When the Koji Hub raises an exception, it translates that to an XML-RPC fault and sends that to the client. The Python client unmarshals that as an xmlrpc.client.Fault object.

The koji.convertFault() method will further translate those faults into Koji's custom error classes.

Let's say that we're changing the getUser RPC to raise a new koji.UserNotFoundError instead of koji.GenericError (when strict=True):

New client, Old hub:

• Hub will not send a newer faultCode number. It will use GenericError's faultCode, 1000.
• When client uses koji.convertFault(), it will return the old GenericError.
• Clients (eg CLI) should catch GenericError for old hubs, in addition to the newer UserNotFoundError exception. After a long time (years?), remove the except GenericError: condition from client code and thereby remove support for that old hub version.

Old client, New Hub:

• Client will not understand newer faultCode numbers that the hub sends.
• When client uses koji.convertFault(), it will return .... the bare fault instead of GenericError? Ouch
• What happens? Have to read the exact client code to tell. Worst case, the client newly crashes on an unhandled exception since it's expecting to catch koji exceptions, not untranslated xmlrpc.client.Fault exceptions.

Theoretically this means that we must add a UserNotFoundError to the client a long time ahead of using it in the hub. Once we require all clients to upgrade, then we can start using that exception on the hub. Unclear how long that schedule should be.

In this particular case with getUser(), the strict kwarg is relatively new, so almost no client code uses that. The only thing that calls getUser(..., strict=True) is the koji block-notification sub-command with the --user argument. Two things are in our favor:

1. That command does not try/except GenericError, so there was no exception handling anyway (need to empirically test this to be sure)
2. The --user argument requires admin permissions, so it's likely the Koji administrator would be better equipped to upgrade the koji client package to get the new UserNotFoundError class definition.

[ktdreyer]

Hacking on this a bit more, it's true that almost no client code calls getUser directly with strict=True, but there are many, many RPCs that internally call get_user(..., strict=True), and these will all start returning this new NoSuchUserError error class.

[ktdreyer]

With the current hub code (raising GenericError):

$ kojidev set-pkg-owner kdreyerz ceph-5.1-rhel-9 ceph
2021-10-20 11:49:30,505 [ERROR] koji: GenericError: No such user: 'kdreyerz'

With a modified hub that raises a new fault that the old client cannot translate:

$ kojidev set-pkg-owner kdreyerz ceph-5.1-rhel-9 ceph
2021-10-20 12:12:02,735 [ERROR] koji: Fault: <Fault 1024: "No such user: 'kdreyerz'">

[ktdreyer]

I started a thread on koji-devel. Now the work continues here: https://pagure.io/koji/issue/3147

[ktdreyer]

Maybe it's simpler to just add NoSuchResource to koji-ansible for now.