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

Simplify role variables by removing one layer #234

Merged
merged 4 commits into from
Dec 6, 2023
Merged

Simplify role variables by removing one layer #234

merged 4 commits into from
Dec 6, 2023

Conversation

rndmh3ro
Copy link
Collaborator

@rndmh3ro rndmh3ro commented Dec 6, 2023

BREAKING CHANGE

previously you could create multiple different objects with the same config:

     icinga_hostgroups:
      - hostgroup_object:
        - "service_abbreviation-environement"
        - "service_abbreviation-environement-web"
        state: present

However internally I don't see any broad adaption of this and it makes it harder to use (the list after the list next to the key-value pairs is confusing) and it is harder to write and maintain.

Compare:

     - name: icinga_hostgroup
       icinga_hostgroup:
         url: "{{ icinga_url }}"
    @@ -11,15 +9,15 @@
         force_basic_auth: "{{ icinga_force_basic_auth | default(omit) }}"
         client_cert: "{{ icinga_client_cert | default(omit) }}"
         client_key: "{{ icinga_client_key | default(omit) }}"
        state: "{{ hostgroup.0.state | default(omit) }}"
        object_name: "{{ hostgroup.1 }}"
        display_name: "{{ hostgroup.0.display_name | default(omit) }}"
        assign_filter: "{{ hostgroup.0.assign_filter | default('host.name=\"' + hostgroup.1 + '-*\"') }}"
       retries: 3
       delay: 3
       register: result
       until: result is succeeded
      loop: "{{ icinga_hostgroups|subelements('hostgroup_object') }}"
       loop_control:
         loop_var: hostgroup
       tags: hostgroup

with:

     - name: icinga_hostgroup
       icinga_hostgroup:
         url: "{{ icinga_url }}"
    @@ -11,15 +9,15 @@
         force_basic_auth: "{{ icinga_force_basic_auth | default(omit) }}"
         client_cert: "{{ icinga_client_cert | default(omit) }}"
         client_key: "{{ icinga_client_key | default(omit) }}"
        state: "{{ hostgroup.state | default(omit) }}"
        object_name: "{{ hostgroup.name }}"
        display_name: "{{ hostgroup.display_name | default(omit) }}"
        assign_filter: "{{ hostgroup.assign_filter | default('host.name=\"' + hostgroup.name + '-*\"') }}"
       retries: 3
       delay: 3
       register: result
       until: result is succeeded
       loop: "{{ icinga_hostgroups }}"
       loop_control:
         loop_var: hostgroup
       tags: hostgroup

previously you could create multiple different objects with the same config:

```
     icinga_hostgroups:
      - hostgroup_object:
        - "service_abbreviation-environement"
        - "service_abbreviation-environement-web"
        state: present
```

However internally I don't see any broad adaption of this and it makes it harder to
use (the list after the list next to the key-value pairs is confusing) and it is harder
to write and maintain.

Compare:

```
 - name: icinga_hostgroup
   icinga_hostgroup:
     url: "{{ icinga_url }}"
@@ -11,15 +9,15 @@
     force_basic_auth: "{{ icinga_force_basic_auth | default(omit) }}"
     client_cert: "{{ icinga_client_cert | default(omit) }}"
     client_key: "{{ icinga_client_key | default(omit) }}"
    state: "{{ hostgroup.0.state | default(omit) }}"
    object_name: "{{ hostgroup.1 }}"
    display_name: "{{ hostgroup.0.display_name | default(omit) }}"
    assign_filter: "{{ hostgroup.0.assign_filter | default('host.name=\"' + hostgroup.1 + '-*\"') }}"
   retries: 3
   delay: 3
   register: result
   until: result is succeeded
  loop: "{{ icinga_hostgroups|subelements('hostgroup_object') }}"
   loop_control:
     loop_var: hostgroup
   tags: hostgroup
```

with:

```
 - name: icinga_hostgroup
   icinga_hostgroup:
     url: "{{ icinga_url }}"
@@ -11,15 +9,15 @@
     force_basic_auth: "{{ icinga_force_basic_auth | default(omit) }}"
     client_cert: "{{ icinga_client_cert | default(omit) }}"
     client_key: "{{ icinga_client_key | default(omit) }}"
    state: "{{ hostgroup.state | default(omit) }}"
    object_name: "{{ hostgroup.name }}"
    display_name: "{{ hostgroup.display_name | default(omit) }}"
    assign_filter: "{{ hostgroup.assign_filter | default('host.name=\"' + hostgroup.name + '-*\"') }}"
   retries: 3
   delay: 3
   register: result
   until: result is succeeded
   loop: "{{ icinga_hostgroups }}"
   loop_control:
     loop_var: hostgroup
   tags: hostgroup
```

Finally it allows us to define a role argument specification. This helps us prevent issues like
#34
@rndmh3ro rndmh3ro changed the title Simplify Simplify role variables by removing one layer Dec 6, 2023
@rndmh3ro rndmh3ro added enhancement New feature or request breaking labels Dec 6, 2023
@rndmh3ro rndmh3ro marked this pull request as ready for review December 6, 2023 12:50
@rndmh3ro rndmh3ro requested a review from a team as a code owner December 6, 2023 12:50
@rndmh3ro rndmh3ro merged commit 8119c52 into main Dec 6, 2023
33 checks passed
@rndmh3ro rndmh3ro deleted the simplify branch December 6, 2023 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants