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

hostgroup assignments inherited from hosttemplates overwrite each other depending on inheritance sequence #824

Open
bcogel opened this issue Mar 1, 2017 · 12 comments

Comments

@bcogel
Copy link

bcogel commented Mar 1, 2017

We have defined a tree of hosttemplates to which we assign the services that are needed for kind of host. To some of those templates we have assigned also hostgroups.

Example: Our AFS database servers and fileservers

Hosttemplates
|
SERVICE (dummy template)
|
afs (Import: SERVICE, Services: BOS)
|
afsdb (Import: afs, Groups: SYS AFSDB, Services: (BOS from afs) BUSERVER, KASERVER, PTSERVER, VLSERVER)
|
afsfs (Import: afs, Groups: SYS AFSFS, Services: (BOS from afs) AFS Space, RXDEBUG)

Preview gives:

template Host "SERVICE" {
}

template Host "afsdb" {
import "afs"

groups = [ "SYS AFSDB" ]

}

template Host "afsfs" {
import "afs"

groups = [ "SYS AFSFS" ]

}

object HostGroup "SYS AFSDB" {
display_name = "SYS AFS DB Server"
}

object HostGroup "SYS AFSFS" {
display_name = "SYS AFS Fileserver"
}

We have some hosts that are both: AFS database server and fileserver

Host afs.thp.uni-koeln.de

object Host "afs.thp.uni-koeln.de" {
import "linux-rhel-sys"
import "afsdb"
import "afsfs"

...

}

Problem:
Depending on which template comes last in the list of imports, the host is assigned only to the last inherited hostgroup.

Sequence "afsdb afsfs" results in hostgroup "SYS AFSFS", Sequence "afsfs afsdb" results in hostgroup "SYS AFSDB".

To assign hosts like this to more than one hostgroup, I have to edit the group entry of each host manually. Same seems to happen with servicegroups.

In nagios/icinga 1.x it was possible to extend the list of hostgroups by using something like "hostgroups=+afsdb".

@dgoetz
Copy link
Contributor

dgoetz commented Mar 31, 2017

This is also happening for groups at objects or I assume for all attributes of type array.

I would like this to work in the way:

  • It shows inherited values
  • It allows to override and/or add by user decision additional values

@bcogel
Copy link
Author

bcogel commented Mar 31, 2017

What will happen to attributes inherited from different templates? Do they build up a new array by default that is shown to the user? Or can I select which one to use? I'm trying to imagine how you want to realize this.
Like one of those array type input field where I can delete or add entries? Like the import field? What happens if I delete one of the imported entries by mistake? Can I 'reset' the list?

@fabsi88
Copy link

fabsi88 commented Mar 21, 2018

Hey guys,

any update on this? I am facing the same problem (icinga 2.8.1-1) and latest director module (master).
We have several host templates with different hostgroups configured. When adding a host based on 2 host templates just one group will be added to host config. We expect to get both hostgroups configured.

Fabian

@emanuel-b-q
Copy link

Hello Thomas,
we also have that problem, would be great to have a change here

@ohrly
Copy link

ohrly commented Nov 27, 2018

Hallo Team,
to be able to add host groups with different host templates would be great. At the moment it only overwrites all, not what I was expecting.

Thanks!

@rafiwui
Copy link

rafiwui commented Feb 19, 2019

Any updates on this? It is now two years since this issue was opened.
Still not possible to add groups to already inherited groups. (Icinga 2.10.2 with director master-branch)

@jonbulica99
Copy link
Contributor

BUMPING this as well. Surprised to find out it wasn't the default behavior.

@inmediasit
Copy link

inmediasit commented Jun 17, 2019

Hey @Thomas-Gelf, can you please take a look into this issue?

@6d6178
Copy link

6d6178 commented Jul 17, 2019

I am facing the same issue. Would love to get some feedback if this will be fixed/changed. I don't think configuring all Hostgroups in all inherited Groups is the goal of director/inheriting.

@viper539
Copy link

viper539 commented Jul 20, 2019

Hello @Thomas-Gelf, what can i do to help you to fix this?
Except of writing code, because i'm not good at it.
In https://community.icinga.com/t/adding-groups-through-templates/1715?u=viper539 i tried to explain what i try to do. The workaround of Michael (edit conffile) would work, but, to be honest, who wants to work on console when you can have Director? ;-) In the config-file-world a groups += [ "hg1" ] works great. I know you have to find a solution for the string/array-Problem, but i'm sure you find this!

@sol1-matt
Copy link

Hi @Thomas-Gelf,

I've had a go a adding this feature by adding a property operator to the IcingaObjectGroups class. I haven't been able to quite get it working.

I've got a working UI,

Additional destination fields now has under Special Properties
824 - director groups

sync rule stored in the db

MariaDB [director]> select * from sync_property limit 20;
+----+---------+-----------+-----------------------------+-------------------+----------+-------------------+--------------+
| id | rule_id | source_id | source_expression           | destination_field | priority | filter_expression | merge_policy |
+----+---------+-----------+-----------------------------+-------------------+----------+-------------------+--------------+
|  3 |       1 |         4 | hg_manual_managed_firewalls | groups            |        3 | NULL              | override     |
|  9 |       2 |         4 | nbtenant_${tenant.slug}     | groups            |        7 | NULL              | override     |
| 13 |       3 |         5 | hg_manual_managed_firewalls | groupsadd         |        3 | NULL              | override     |

and working config getting written if I set the default value for the class property operator to what I require

object Host "nb_mf_examplehost" {
    import "managed-firewall"

    address = "127.0.0.1"
    groups += [ "hg_manual_managed_firewalls" ]
    vars.device_vm = "virtual machine"
}

Incomplete
The problem is at some point in the process when the sync rules are loaded from the db/cache I need to set the operator property I added on the IcingaObjectGroups class, I've been unable to determine where/how to do this (and it could be my approach is wrong).

I have a forked branch here and can create a pull request if required.

The diff is below if you'd prefer that

diff --git a/application/forms/SyncPropertyForm.php b/application/forms/SyncPropertyForm.php
index 85a9940..ba2404a 100644
--- a/application/forms/SyncPropertyForm.php
+++ b/application/forms/SyncPropertyForm.php
@@ -315,8 +315,6 @@ class SyncPropertyForm extends DirectorObjectForm
             }
             if ($dummy->supportsGroups()) {
                 $special['groups']  = $this->translate('Group membership');
+                $special['groupsadd']  = $this->translate('Group membership (Add)');
+                $special['groupsremove']  = $this->translate('Group membership (Remove)');
             }
             if ($dummy->supportsRanges()) {
                 $special['ranges']  = $this->translate('Time ranges');
diff --git a/library/Director/Import/Sync.php b/library/Director/Import/Sync.php
index 87f3117..afdb0ba 100644
--- a/library/Director/Import/Sync.php
+++ b/library/Director/Import/Sync.php
@@ -507,17 +507,6 @@ class Sync
                     }
                 } elseif ($prop === 'groups') {
                     if ($val !== null) {
+                        $object->groups()->operator("assign");
+                        $object->groups()->add($val);
+                    }
+                } elseif ($prop === 'groupsadd') {
+                    if ($val !== null) {
+                        $object->groups()->operator("add");
+                        $object->groups()->add($val);
+                    }
+                } elseif ($prop === 'groupsremove') {
+                    if ($val !== null) {
+                        $object->groups()->operator("remove");
                         $object->groups()->add($val);
                     }
                 } elseif (substr($prop, 0, 5) === 'vars.') {
diff --git a/library/Director/Objects/IcingaObjectGroups.php b/library/Director/Objects/IcingaObjectGroups.php
index c67292d..8a8510d 100644
--- a/library/Director/Objects/IcingaObjectGroups.php
+++ b/library/Director/Objects/IcingaObjectGroups.php
@@ -24,8 +24,6 @@ class IcingaObjectGroups implements Iterator, Countable, IcingaConfigRenderer

     private $position = 0;

+    private $operator = "add";
+
     protected $idx = array();

     public function __construct(IcingaObject $object)
@@ -78,12 +76,6 @@ class IcingaObjectGroups implements Iterator, Countable, IcingaConfigRenderer
         return array_key_exists($this->position, $this->idx);
     }

+    public function operator($val)
+    {
+        $this->operator = $val;
+    }
+
+
     public function get($key)
     {
         if (array_key_exists($key, $this->groups)) {

@@ -364,13 +356,8 @@ class IcingaObjectGroups implements Iterator, Countable, IcingaConfigRenderer
         if (empty($groups)) {
             return '';
         }
+       if ($this->operator === "add") {
+            return c::renderKeyOperatorValue('groups', "+=", c::renderArray($groups));
+        } elseif ($this->operator === "remove") {
+            return c::renderKeyOperatorValue('groups', "-=", c::renderArray($groups));
+        } else {
+            return c::renderKeyValue('groups', c::renderArray($groups));
+       }
-
-        return c::renderKeyValue('groups', c::renderArray($groups));
     }

     public function toLegacyConfigString($additionalGroups = array())
@@ -382,15 +369,8 @@ class IcingaObjectGroups implements Iterator, Countable, IcingaConfigRenderer
             return '';
         }

+       $type = $this->object->getLegacyObjectType();
+
+       if ($this->operator === "add") {
+            return c::renderKeyOperatorValue('groups', "+=", c::renderArray($groups));
+        } elseif ($this->operator == "remove") {
+            return c::renderKeyOperatorValue('groups', "-=", c::renderArray($groups));
+        } else {
+            return c1::renderKeyValue($type.'groups', c1::renderArray($groups));
+       }
-        $type = $this->object->getLegacyObjectType();
-        return c1::renderKeyValue($type.'groups', c1::renderArray($groups));
     }

I'll note that using group add as the default is incredibly useful, particularly with automation. I agree that the default shouldn't change though which is why I've made the attempt to add this feature.

If I'm on the right track and you can point me in the right direction that would be great, if I have taken the wrong approach but you can see a better way to implement this I'm happy to listen to any suggestions and have a crack at them.

Thanks, Matt

@lesinigo
Copy link

Isn't this related to #344 ?

sol1-matt added a commit to sol1-matt/icingaweb2-module-director that referenced this issue Aug 7, 2023
Add default value 'assign' for default group behaviour which matches group behaviour.
Add logic to IcingaObjectGroups so the class initalizes with the global group behaviour and sets the group operator based on the class operator.
Partial Fix for Icinga#344, Icinga#636, Icinga#824, Icinga#2273
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