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

Apply rule disk is missing the assign rule. #307

Open
telution opened this issue Jul 26, 2024 · 2 comments
Open

Apply rule disk is missing the assign rule. #307

telution opened this issue Jul 26, 2024 · 2 comments

Comments

@telution
Copy link

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:
- name: disk
type: Service
imports:
- generic-service
apply: true
apply_for: disk => config in host.vars.disks
file: 'zones.d/global-templates/disk-service-apply.conf'
check_command: "disk"
vars: + config

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

Thank you guys.

@telution
Copy link
Author

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

[...]

  • name: ping
    type: Service
    apply: true
    apply_for: config in host.vars.ips
    check_command: ping4
    vars: + config

@Donien
Copy link
Collaborator

Donien commented Oct 9, 2024

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

  • create a Service object directly (computationally better for Icinga 2 since no condition is checked)
  • use assign rules to determine the hosts that should have that Service

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants