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.

Sign up for GitHub

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

Fix master_tops configuration rendering #313

Merged
merged 1 commit into from
Aug 13, 2017
Merged

Fix master_tops configuration rendering #313

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions salt/files/master.d/f_defaults.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -746,14 +746,21 @@ rosters:
{%- do default_keys.append('master_tops') %}
master_tops:
{%- for master in cfg_master['master_tops'] -%}
{%- if cfg_master['master_tops'][master] is string %}
{{ master }}: {{ cfg_master['master_tops'][master] }}
{%- else %}
{{ master}}:
{%- if cfg_master['master_tops'][master] is mapping %}
{{ master }}:
{%- for parameter in cfg_master['master_tops'][master] %}
{{ parameter }}: {{ cfg_master['master_tops'][master][parameter] }}
{%- endfor -%}
{%- endif -%}
{%- elif cfg_master['master_tops'][master] is string %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for a separate test for string. Just let the else handle all the scalar types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0xf10e , I also though like this initially, but it wasn't correct, because string is iterable, so the behaviour was not the expect one.
I think that the correct configuration can be created only if the string is treated before other iterable items.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aboe76 , @0xf10e any feedback? As I said, removing the elif for the string will lead to an unexpected behaviour.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0xf10e what is your opinion ?

{{ master }}: {{ cfg_master['master_tops'][master] }}
{%- elif cfg_master['master_tops'][master] is iterable %}
{{ master }}:
{%- for item in cfg_master['master_tops'][master] %}
- {{ item }}
{%- endfor -%}
{%- else %}
{{ master }}: {{ cfg_master['master_tops'][master] }}
{% endif %}
{%- endfor %}
{% endif %}

Expand Down