Unable to render /etc/salt/minion.d/reactor.conf

[rhertzog]

I get this error:

          ID: salt-minion
    Function: file.recurse
        Name: /etc/salt/minion.d
      Result: False
     Comment: #### /etc/salt/minion.d/reactor.conf ####
              Unable to manage file: Problem running salt function in Jinja template: default must be a dictionary or None when merge=True; line 6
              
              ---
              #
              # This file is managed by Salt! Do not edit by hand!
              #
              {# The parameter reactor is kept for backward compatibility -#}
              {%- set reactors = salt['pillar.get']('salt:reactor', []) -%}
              {%- set reactors = salt['pillar.get']('salt:reactors', default=reactors, merge=True) -%}    <======================
              {%- set reactors = salt['pillar.get']('salt:minion:reactors', default=reactors, merge=True) -%}
              
              {%- if reactors %}
              reactor:
                {%- for reactor in reactors %}
              [...]

It seems to be caused by 561eb4c by @javierbertoli ... the change looks wrong.

[javierbertoli]

Hi @rhertzog , reactor and/or reactors should be lists. Am I wrong, or does it seem you're using a dict in reactors, and that's why it's failing?

The line before the one you point, sets the variable reactors to a list (as the default for the formula before the commit you mention used lists for reactor. See the diff for the pillar.example file), so when the line you point tries to merge the dict in salt['pillar.get']('salt:reactors with the list in default=reactors set in the previous line, it fails.

Do I make sense, or am I missing something?

What do you have in your pillar for these parameters?

[rhertzog]

I don't have any pillar value for salt:reactor or salt:reactors on the minion where I got the above error... and the error is rather clear, you can't use a list for the default parameter if you use merge=True. Your commit changed the "None" value I used to have into an empty list, thus the failure.

[rhertzog]

If you look at the engines, beacons, and other similar parameter, they are all true dictionaries in their syntax and using merge=True is thus not a problem. Here for reactors, you have a list of single-key dictionaries. It looks like using merge=True is not an option.

[javierbertoli]

@rhertzog, does the PR #318 fix the issue for you? Any change or comments about it?

[rhertzog]

@javierbertoli Yes, it works. Obviously you get the union of all the reactors but that's okay, we did not really want any override logic I think.

[javierbertoli]

Yeah. First I thought of overriding them, but then I thought that a single beacon might have different reactors for different hosts, and that you'd like to specify them in different places and adding them.

OK, thanks for pointing the error and sorry for the inconveniences. @aboe76, mind you merge that PR?

[aboe76]

@javierbertoli merged it