Skip to content
This repository has been archived by the owner on May 24, 2023. It is now read-only.

Generic feature handling #12

Open
bobapple opened this issue May 28, 2018 · 6 comments
Open

Generic feature handling #12

bobapple opened this issue May 28, 2018 · 6 comments
Labels
enhancement Enhancement of existing code. help wanted Extra attention is needed.
Milestone

Comments

@bobapple
Copy link
Member

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

Following options come to my mind:

  • Use the existing icinga2_feature module. Don't know if this is sufficiant
  • Extend the existing icinga2_feature module.
  • Use a generic template and enable features by adding symlinks, like icinga2 does.

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.

@bobapple bobapple added enhancement Enhancement of existing code. help wanted Extra attention is needed. labels May 28, 2018
@bobapple bobapple added this to the v0.3.0 milestone May 28, 2018
@RossBarnie
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor

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
Copy link

slalomsk8er commented Oct 10, 2018

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
Copy link
Member

mkayontour commented Oct 12, 2018

@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) }}"

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Enhancement of existing code. help wanted Extra attention is needed.
Projects
None yet
Development

No branches or pull requests

4 participants