[BUG] sysrc.managed test=True TypeError: can only concatenate str (not "bool") to str

[network-shark]

Description
When running sysrc.managed in test mode a bool is returned instead of a string . I could not figure out why , it only happens on one of my systems. Most of the time it works with the current code .

Setup

syssrc_vm_enable:
  sysrc.managed:
    - name: vm_enable
    - value: YES

Steps to Reproduce the behavior
salt '*' state.apply teststate test=True

Expected behavior

          ID: syssrc_vm_enable
    Function: sysrc.managed
        Name: vm_enable
      Result: None
     Comment: The value of "vm_enable" will be changed!
     Started: 20:45:55.254429
    Duration: 23.909 ms
     Changes:
              ----------
              new:
                  vm_enable = True will be set.
              old:
                  None

Versions Report

╰─ salt -V
Salt Version:
          Salt: 3002.6

Dependency Versions:
          cffi: 1.14.5
      cherrypy: Not Installed
      dateutil: Not Installed
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 2.11.2
       libgit2: Not Installed
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: 2.20
      pycrypto: Not Installed
  pycryptodome: 3.10.1
        pygit2: Not Installed
        Python: 3.7.9 (default, Mar 27 2021, 01:17:51)
  python-gnupg: Not Installed
        PyYAML: 5.3.1
         PyZMQ: 22.0.3
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.1

System Versions:
          dist: freebsd 12.2
        locale: UTF-8
       machine: amd64
       release: 12.2-RELEASE-p4
        system: FreeBSD
       version: FreeBSD 12.2

Possible fix is salte/states/sysrc.py line 60

      if __opts__["test"] is True:
         ret["comment"] = 'The value of "{0}" will be changed!'.format(name)
         ret["changes"] = {
             "old": current_state,
             "new": name + " = " + str(value) + " will be set.",
         }

[OrangeDog]

Is there an actual error raised? Have you got the stacktrace?

It's part of the YAML parse that YES gets converted to True.

[network-shark]

Sorry I missed that !

----------
          ID: syssrc_vm_enable
    Function: sysrc.managed
        Name: vm_enable
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/usr/local/lib/python3.7/site-packages/salt/state.py", line 2154, in call
                  *cdata["args"], **cdata["kwargs"]
                File "/usr/local/lib/python3.7/site-packages/salt/loader.py", line 2106, in wrapper
                  return f(*args, **kwargs)
                File "/usr/local/lib/python3.7/site-packages/salt/states/sysrc.py", line 64, in managed
                  "new": name + " = " + value + " will be set.",
              TypeError: can only concatenate str (not "bool") to str
     Started: 20:38:02.691753
    Duration: 26.907 ms
     Changes:

I'm not sure if my provided workaround is correct, because even if I fix that. The state always returns a change. Even if there is no change, but that does happen on all my minions not only on the one with this failure.

----------
          ID: syssrc_vm_enable
    Function: sysrc.managed
        Name: vm_enable
      Result: True
     Comment: The value of "vm_enable" was changed!
     Started: 21:09:05.023020
    Duration: 91.775 ms
     Changes:
              ----------
              new:
                  ----------
                  /etc/rc.conf:
                      ----------
                      vm_enable:
                           YES
              old:
                  ----------
                  /etc/rc.conf:
                      ----------
                      vm_enable:
                          YES

[OrangeDog]

If you put value: 'YES' instead of value: YES, does it then work?

[network-shark]

Summary :

 syssrc_vm_enable:
 sysrc.managed:
   - name: vm_enable
   - value: YES

sysrc_vm_dir:
  sysrc.managed:
    - name: vm_dir
    - value: /home/sharky/vm

State always returns changed
Test=True is broken

 syssrc_vm_enable:
 sysrc.managed:
   - name: vm_enable
   - value: 'YES'
sysrc_vm_dir:
  sysrc.managed:
    - name: vm_dir
    - value: '/home/sharky/vm'

state works
test=true works