saltstack
[BUG] salt.thorium.reg.clear clears a level of dict too much
Description
I try to fire a single action in response to many events (one from each minion). So I use the following example code:
patch_grp2:
reg.set:
- add: host
- match: custom/system/up
check.contains:
- value: mysystem1
- value: mysystem2
shell_test:
local.cmd:
- tgt: '*'
- func: cmd.run
- arg:
- echo 'thorium success' > /tmp/thorium.txt
- require:
- check: patch_grp2
reg.clear:
- name: patch_grp2
And on mysystem1/2 I do:
salt-call event.send custom/system/up "{host: $(hostname -s)}"
It works as expected: /tmp/thorium.txt is created after both minions sent the event.
But since the action is triggered multiple times (e.g. if I rm /tmp/thorium.txt on a minion, file is instantly recreated), I do the reg.clear in my action.
If seems this would solve my problem, but there seems to be a bug:
The clear triggers and I get the following error in Log
2020-07-02 14:59:11,525 [salt.state :322 ][ERROR ][1412] An exception occurred in this state: Traceback (most recent call last):
File "/usr/local/lib/python3.6/dist-packages/salt/state.py", line 1981, in call
**cdata['kwargs'])
File "/usr/local/lib/python3.6/dist-packages/salt/loader.py", line 1977, in wrapper
return f(*args, **kwargs)
File "/usr/local/lib/python3.6/dist-packages/salt/thorium/check.py", line 270, in contains
if value in __reg__[name]['val']:
KeyError: 'val'
After that the Thorium register doesn't work at all until master restart.
I guess you want to clear the contents of __reg__[name]['val'] but actually you do clear __reg__[name] ?
Versions Report
salt --versions-report
(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)Salt Version:
Salt: 3000.2
Dependency Versions:
cffi: Not Installed
cherrypy: unknown
dateutil: 2.6.1
docker-py: Not Installed
gitdb: 2.0.3
gitpython: 2.1.8
Jinja2: 2.11.2
libgit2: Not Installed
M2Crypto: Not Installed
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.9 (default, Apr 18 2020, 01:56:04)
python-gnupg: 0.4.1
PyYAML: 5.3.1
PyZMQ: 19.0.0
smmap: 2.0.3
timelib: Not Installed
Tornado: 4.5.3
ZMQ: 4.3.2
System Versions:
dist: Ubuntu 18.04 bionic
locale: UTF-8
machine: x86_64
release: 4.15.0-96-generic
system: Linux
version: Ubuntu 18.04 bionic
btw: another "minor bug" is that save.file throws a "TypeError: Object of type 'set' is not JSON serializable" error for reg.set
A possible fix for this could be:
--- file.py.old 2020-07-02 15:41:14.722003824 +0200
+++ file.py 2020-07-02 16:12:56.586766516 +0200
@@ -41,6 +41,7 @@
# import python libs
from __future__ import absolute_import, print_function, unicode_literals
import os
+import json
# Import salt libs
import salt.utils.data
@@ -48,6 +49,13 @@
import salt.utils.json
+class SetEncoder(json.JSONEncoder):
+ def default(self, obj):
+ if isinstance(obj, set):
+ return str(obj)
+ return json.JSONEncoder.default(self, obj)
+
+
def save(name, filter=False):
'''
Save the register to <salt cachedir>/thorium/saves/<name>, or to an
@@ -80,7 +88,7 @@
os.makedirs(tgt_dir)
with salt.utils.files.fopen(fn_, 'w+') as fp_:
if filter is True:
- salt.utils.json.dump(salt.utils.data.simple_types_filter(__reg__), fp_)
+ salt.utils.json.dump(salt.utils.data.simple_types_filter(__reg__), fp_, cls=SetEncoder)
else:
- salt.utils.json.dump(__reg__, fp_)
+ salt.utils.json.dump(__reg__, fp_, cls=SetEncoder)
return ret
I intentionally added this logic to thorium/file.py instead of utils/json.py since I can not assess the possible side effects.
Yes, this seems to fix the issue:
/usr/local/lib/python3.6/dist-packages/salt/thorium/reg.py
--- reg.py.old 2020-07-02 15:17:42.970803604 +0200
+++ reg.py 2020-07-02 15:13:40.268064514 +0200
@@ -150,7 +150,7 @@
'comment': '',
'result': True}
if name in __reg__:
- __reg__[name].clear()
+ __reg__[name]['val'].clear()
return ret
Actually by fixing the issues I recognized my code does not work.
I have to add a require to reg.clear to prevent it running each time.
And value mysystem2 overrides mysystem1 so , it is waiting for just mysystem2 event instead of both.
The following notation doesn't work either.
patch_grp2:
reg.set:
- add: host
- match: custom/system/up
check.contains:
- value:
- mysystem1
- mysystem2
shell_test:
local.cmd:
- tgt: '*'
- func: cmd.run
- arg:
- echo 'thorium success' > /tmp/thorium.txt
- require:
- check: patch_grp2
reg.clear:
- name: patch_grp2
- require:
- check: patch_grp2
Deal: I fixed 2 of your issues , can you please help me with mine? :)
Ok I found the solution by looking into thorium/check.py (use eq instead of contains), and actually another bug, too. ;)
And here is the fix:
--- check.py.old 2020-07-02 16:29:24.054136343 +0200
+++ check.py 2020-07-02 16:34:35.558769666 +0200
@@ -42,6 +42,8 @@
ret['result'] = False
ret['comment'] = 'Value {0} not in register'.format(name)
return ret
+ if isinstance(__reg__[name]['val'], set):
+ value = set(value)
if __reg__[name]['val'] > value:
ret['result'] = True
return ret
@@ -75,6 +77,8 @@
ret['result'] = False
ret['comment'] = 'Value {0} not in register'.format(name)
return ret
+ if isinstance(__reg__[name]['val'], set):
+ value = set(value)
if __reg__[name]['val'] >= value:
ret['result'] = True
return ret
@@ -108,6 +112,8 @@
ret['result'] = False
ret['comment'] = 'Value {0} not in register'.format(name)
return ret
+ if isinstance(__reg__[name]['val'], set):
+ value = set(value)
if __reg__[name]['val'] < value:
ret['result'] = True
return ret
@@ -141,6 +147,8 @@
ret['result'] = False
ret['comment'] = 'Value {0} not in register'.format(name)
return ret
+ if isinstance(__reg__[name]['val'], set):
+ value = set(value)
if __reg__[name]['val'] <= value:
ret['result'] = True
return ret
@@ -174,6 +182,8 @@
ret['result'] = False
ret['comment'] = 'Value {0} not in register'.format(name)
return ret
+ if isinstance(__reg__[name]['val'], set):
+ value = set(value)
if __reg__[name]['val'] == value:
ret['result'] = True
return ret
@@ -207,6 +217,8 @@
ret['result'] = False
ret['comment'] = 'Value {0} not in register'.format(name)
return ret
+ if isinstance(__reg__[name]['val'], set):
+ value = set(value)
if __reg__[name]['val'] != value:
ret['result'] = True
return ret
Disclaimer: I didn't check if '<' and '>' are actually defined for sets, but eq (and therefore ne) works.
@Heiko-San thanks for the bug repot and possible fixes for multiple issues. For your disclaimer sets don’t have __lt__ or __gt__
