Icinga / ansible-icinga2

Ansible Role for Icinga 2
Apache License 2.0
50 stars 23 forks source link

Generic feature handling #12

Open bobapple opened 6 years ago

bobapple commented 6 years ago

A general helper that can enable and disable features and change configuration for a feature.

Following options come to my mind:

No specific feature options are set here.

A reload/restart of Icinga is triggered if a feature is enabled/disabled or the configuration changes.

Every specific feature allows the configuration of its settings. Some features may have extended functionality, such as importing data schemas.

Example implementation with templates: https://github.com/mkayontour/icinga2-ansible/tree/master/roles/icinga2-feature-handler Another example implementation with templates: https://github.com/chrnie/ansible-role-icinga2

Feedback is very welcome, I don't know what's the best "ansible way" here.

RossBarnie commented 6 years ago

I don't see how the current icinga2_feature module could be extended to support the variation in features and their configuration without specifying all possible options (unfeasible) or making a very generic config argument to handle it. The latter option is more feasible but doesn't provide much value, in my opinion. We'd need documentation on default values so when someone does enable a feature they know what's being configured, but that's difficult when the features are so different (database vs command for example).

I'd prefer the generic templates and symlinks option as that, as you point out, fits in with how Icinga2 behaves. Generally I prefer to use Ansible to maintain standard approaches, rather than introducing new approaches as it's more likely to be understood by future maintainers/sysadmins.

mkayontour commented 6 years ago

Hey, @RossBarnie I agree with you on the generic template. Before I start working on those features I want to make a point clear how I think a generic template look like. As you might have seen my templates which are, i would say, generic - we need to use a hash variable for that. Therefore we iterate through the values an generate everything we need.

https://github.com/mkayontour/icinga2-ansible/blob/master/roles/icinga2-feature-handler/templates/feature-graphite.j2

But in this case I struggle with the Ansible variable precedence. If we define a standard hash as default for the feature:

i2_feature:
  graphite:
    host: 127.0.0.1
    port: 2003
    enable_send_thresholds: "true"
    enable_send_metadata: "false"

As I just want to change one value of the feature, let's say the host parameter. We need to overwrite the whole hash on a higher level like group_vars.

If we overwrite the variable like this:

#group_vars
i2_feature.graphite.host: 10.10.0.2

Results...

i2_feature:
  graphite:
    host: 10.10.0.2

... into a hash which has only one value left. So we need to encourage users to overwrite the whole hash with defaults and their own values. Like so:

#some group vars
i2_feature:
  graphite:
    host: 10.10.0.2
    port: 2003
    enable_send_thresholds: "true"
    enable_send_metadata: "false"

This would be a generic template for every feature we are using, would you agree on that topic?

Or do you suggest by documenting all parameters, that we define every feature with all parameter defaults and use single parameters for every feature-parameter?

i2_feature_graphite_host: 127.0.0.1
i2_feature_graphite_port: 2003 

m'kay

mkayontour commented 6 years ago

Ok, haven't seen that trick before :) i2_constants: "{{ i2_default_constants | combine(i2_custom_constants) }}"

Seems I won't need to worry about the MERGE_BEHAVOUR anymore.

RossBarnie commented 6 years ago

I think that by creating the generic template we'd be saving ourselves from having to keep our templates up to date with Icinga2 (admittedly we'd be passing that responsibility to the user but that's fine in my opinion).

And yes the i2_feature hash is much better to me than the individual variables. I'm not even sure how we'd handle it that way without having to specify every possible feature and parameter.

I haven't seen anyone else use that hash trick but it's worked for me so far! 😄

slalomsk8er commented 6 years ago

I fund it useful to construct endpoints:

  pre_tasks:
    - name: Gather facts from ALL hosts (regardless of limit or tags)
      setup:
      delegate_to: "{{ item }}"
      delegate_facts: True
      when: hostvars[item]['ansible_default_ipv4'] is not defined
      with_items: "{{ groups['icinga'] }}"

    - name: Construct endpoints config
      set_fact:
        icinga2_endpoints: "{{ icinga2_endpoints|default({}) | combine( {item: {'host': hostvars[item]['ansible_default_ipv4']['address']}})  }}"
      loop: "{{ groups['icinga'] }}"

It took me quite a while to figure out how to do this.

Edit: added more context.

mkayontour commented 6 years ago

@RossBarnie I did some tests and it's charming but only with simple hashes. Something like this:

i2_feature_graphite:
  host: "1.2.3.4"
  port: 2003

which fits for most features. When we take a look at the Ido-Mysql feature we notice that the cleanup settings are also defined as hash.

object IdoMysqlConnection "mysql-ido" {
  host = "127.0.0.1"
  port = 3306
  user = "icinga"
  password = "icinga"
  database = "icinga"

  cleanup = {
    downtimehistory_age = 48h
    contactnotifications_age = 31d
  }
}

So I would assume that we should provide the options on that feature in the same way than the others:

i2_feature_ido_mysql:
  host: "127.0.0.1"
  port: "3306"
  cleanup:
    downtimehistory_age: "48h"
    contactnotifications_age: "31d"

EDIT: RTM We can use the combine method for that because we can use recursive=true to also combine lower levels than the first.

Example:

i2_default_features:
  graphite:
    host: "127.0.0.1"
    port: "2003"
  ido-mysql:
    host: "127.0.0.1"
    port: "3306"
    cleanup:
      downtimehistory_age: "48h"
i2_features: "{{ i2_default_features | combine(i2_custom_features, recursive=True) }}"