Icinga / ansible-collection-icinga

Collection to setup and manage components of the Icinga software stack
Apache License 2.0
46 stars 35 forks source link

Apply rule disk is missing the assign rule. #307

Open telution opened 4 months ago

telution commented 4 months ago

Hi. I tried to configure this.

apply Service for (disk => config in host.vars.disks) { import "generic-service" check_command = "disk" vars += config }

The collection throws an error "FAILED! => {"msg": "Apply rule disk is missing the assign rule."}" with the following object definition:

icinga2_objects: hostname.de:

It seems to be bug, because assign shouldnt be necessary?

Thank you guys.

telution commented 4 months ago

By the way. Docu (https://github.com/Icinga/ansible-collection-icinga/blob/main/doc/role-icinga2/objects.md)

[...]

Donien commented 1 month ago

TLDR:
I'd say it's more of a decision than a technical problem. An apply rule (without iterations) wouldn't make sense since you could simply add a single Service to every host (which this collection would do for you).

Apply for could be discussed though. Here we'd have an implicit assign rule due to for (config in host.vars.SOMEVARIABLE. A host must have SOMEVARIABLE, so stating the same condition in the assign rule would be superfluous.

If you really don't want any assign rule, just give it a assign: true.

Any other opinion on that matter, @mkayontour?


First point:
KEY => VALUE in host.vars.DICTIONARY_VARIABLE always expects the variable to be a dictionary. If you simply loop over a list, use ITEM in host.vars.ARRAY_VARIABLE. vars: + config` is not possible here since it's not a dictionary.

If you use KEY => VALUE in host.vars.DICTIONARY_VARIABLE in apply_for then KEY will be treated as a variable, meaning your check_command: "disk" results in check_command = disk. The latter should be check_command = "disk" though. Just be careful with the names, is what I want to say.

Example using a dictionary:

  check_command = "dummy"
  vars.disks.sda1 = {
        "warning" = "warning_value"
        "critical" = "critical_value"
  }
  vars.disks.sda2 = {
        "warning" = "warning_value"
        "critical" = "critical_value"
  }
}

apply Service for (sdX => config in host.vars.disks)  {
  // sdX would be sda1, sda2, sda3, ....
  // config would be { "warning" = "warning_value", "critical" = "critical_value" } for the given iteration

  check_command = "disk"
  display_name = sdX
  vars += config
  assign where host.name == "dummy"

Thanks about the hint to the docs. Looks wrong to me. I'll change that.


Now to your actual question: Technically, from Icinga 2's point of view, an assign rule is not necessary.
However, normally you want to either

Looking at the problem from Ansible's point of view I think it would be smarter to use hostvars to determine for example what disks a host has. Afterwards one can create Services based on that information instead of letting Icinga 2 check conditions (again) by using apply rules.

With the collection you can also simply do something like assign: host.vars.disks to assign the Service to every host that actually has the variable disks set.
Alternatively you can be even more generous by providing assign: true which will match every host.