Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coding error in salt/utils/cache.py #59437

Open
szjur opened this issue Feb 5, 2021 · 4 comments · May be fixed by #63886
Open

Coding error in salt/utils/cache.py #59437

szjur opened this issue Feb 5, 2021 · 4 comments · May be fixed by #63886
Assignees
Labels
Bug broken, incorrect, or confusing behavior severity-low 4th level, cosemtic problems, work around exists
Milestone

Comments

@szjur
Copy link

szjur commented Feb 5, 2021

Description
There seems to be a bug in salt/utils/cache.py. After sending a number jobs with LocalClient.cmd() I guess CacheRegex.sweep() method in this file is called. What seems to be happening is sort() is called on a list containing _sre.SRE_Pattern objects which don't seem to be sortable. By looking at what and how you store in that cache:

        regex = re.compile("{}{}{}".format(self.prepend, pattern, self.append))
        self.cache[pattern] = [1, regex, pattern, time.time()]

it just can't possibly work what you're trying to do in sweep():

        paterns = list(self.cache.values())
        paterns.sort()

These values are lists containing the compiled pattern amongst them. I guess you really intended to sort by the usage time/count or something and here whole lists are compared. You can compare lists but their elements need to be comparable or you need to be lucky and erlier entries on the compared lists are different. Effectively any 2 cache entries with the same usage count (first element on the list) will cause this error, since the compiled pattern seems to be the next item there.
The code below is from Python 3.7 on a Debian box so exception looks a bit different but on 3.6 on RHEL it contains _sre.SRE_Pattern not re.Pattern.

Python 3.7.3 (default, Jul 25 2020, 13:03:44)
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import re
>>> [1, 2, 3, 4] < [1, 2, 3, 5]
True
>>> [1, 2, re.compile('foo'), 4] < [1, 2, re.compile('bar'), 5]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: '<' not supported between instances of 're.Pattern' and 're.Pattern'

>>> [1, 2, re.compile('foo'), 4] < [1, 3, re.compile('bar'), 5]
True

A simple fix should be to do sort(key=lambda i: i[0]) if your aim was to sort by the usage count. There is also creation timestamp at [3] so you may want some more elaborate lambda function there.

Without that fix this eventually ends up with this unhandled exception (stacktrace from Salt 3000.5, some line numbers here and there may be slightly off due to some other patches)

File "/usr/lib/python3.6/site-packages/salt/client/init.py", line 713, in cmd
**kwargs):
File "/usr/lib/python3.6/site-packages/salt/client/init.py", line 1525, in get_cli_event_returns
**kwargs
File "/usr/lib/python3.6/site-packages/salt/client/init.py", line 1102, in get_iter_returns
for raw in ret_iter:
File "/usr/lib/python3.6/site-packages/salt/client/init.py", line 1036, in get_returns_no_block
no_block=True, auto_reconnect=self.auto_reconnect)
File "/usr/lib/python3.6/site-packages/salt/utils/event.py", line 638, in get_event
ret = self._get_event(wait, tag, match_func, no_block)
File "/usr/lib/python3.6/site-packages/salt/utils/event.py", line 561, in _get_event
if any(pmatch_func(ret['tag'], ptag) for ptag, pmatch_func in self.pending_tags):
File "/usr/lib/python3.6/site-packages/salt/utils/event.py", line 561, in
if any(pmatch_func(ret['tag'], ptag) for ptag, pmatch_func in self.pending_tags):
File "/usr/lib/python3.6/site-packages/salt/utils/event.py", line 511, in _match_tag_regex
return self.cache_regex.get(search_tag).search(event_tag) is not None
File "/usr/lib/python3.6/site-packages/salt/utils/cache.py", line 261, in get
self.sweep()
File "/usr/lib/python3.6/site-packages/salt/utils/cache.py", line 246, in sweep
paterns.sort()
TypeError: '<' not supported between instances of '_sre.SRE_Pattern' and '_sre.SRE_Pattern'

Setup
More or less standard Salt 3000.5 installation (with some custom patches, unlikely relevant for the error).

Steps to Reproduce the behavior
Create LocalClient and start sending jobs with cmd(). In my case I use tgt_type="nodegroup" and some kwarg if that makes a difference.

Expected behavior
No exception thrown in that sweep() function.

Versions Report

Salt Version:
           Salt: 3000.5
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: unknown
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.11.1
        libgit2: Not Installed
       M2Crypto: 0.35.2
           Mako: Not Installed
   msgpack-pure: Not Installed
msgpack-python: 0.6.2
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.6.8 (default, Aug 13 2020, 07:46:32)
   python-gnupg: Not Installed
         PyYAML: 3.13
          PyZMQ: 14.7.0
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.1.4
 
System Versions:
           dist: redhat 7.6 Maipo
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-1160.el7.x86_64
         system: Linux
        version: Red Hat Enterprise Linux Server 7.6 Maipo
@szjur szjur added the Bug broken, incorrect, or confusing behavior label Feb 5, 2021
@szjur szjur changed the title Coding error in salt/utils/cache.py ? Coding error in salt/utils/cache.py Feb 5, 2021
@szjur
Copy link
Author

szjur commented Apr 9, 2021

I guess @keesbos no longer contributes to this project to fix this obvious programming error? Mind you I only just noticed the inconsistency between spelling "pattern" and "paterns" in the aforementioned code :-)

@keesbos
Copy link
Contributor

keesbos commented Apr 10, 2021

Indeed. I'm not really active anymore. The '<' was OK for pattern comparing in python2 (That's what I was using at that time). So, the usage was (is?) the key to sort on an the rest is just there to keep it together. I think it just was quicker to sort this way than to extract sort keys. Thus, from the top of my head, it was only the usage counter that matters as key in this sorting.

@szjur
Copy link
Author

szjur commented Apr 10, 2021

Oh I just realised it indeed won't throw exceptions in Python2, although these comparisons don't make much sense and obviously Python2 is no longer supported by recent Saltstack version. So @krionbsd should really consider fixing that. I can't share my few-line patch due to prohibitive beaurocracy at my employer's who otherwise holds rights for it but the verbal description above should let anyone fix it in a few minutes.

Python 2.7.16 (default, Oct 10 2019, 22:02:15)
[GCC 8.3.0] on linux2
>>> import re
>>> re.compile('foo') < re.compile('bar')
True

Python 3.7.3 (default, Jan 22 2021, 20:04:44)
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import re
>>> re.compile('foo') < re.compile('bar')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: '<' not supported between instances of 're.Pattern' and 're.Pattern'

@sagetherage sagetherage added severity-low 4th level, cosemtic problems, work around exists and removed needs-triage labels Apr 30, 2021
@sagetherage sagetherage added this to the Approved milestone Apr 30, 2021
@msciciel msciciel linked a pull request Mar 15, 2023 that will close this issue
3 tasks
@msciciel
Copy link
Contributor

I was able to fix this issue using #63886. @sagetherage please review if it's acceptable solution to this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior severity-low 4th level, cosemtic problems, work around exists
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants