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

Data Type "Dictionary" to allow for nested variables #337

Closed
icinga-migration opened this issue Jul 4, 2016 · 52 comments
Closed

Data Type "Dictionary" to allow for nested variables #337

icinga-migration opened this issue Jul 4, 2016 · 52 comments
Assignees
Milestone

Comments

@icinga-migration
Copy link

This issue has been migrated from Redmine: https://dev.icinga.com/issues/12093

Created by lebvanih on 2016-07-04 11:05:38 +00:00

Assignee: (none)
Status: New
Target Version: (none)
Last Update: 2016-08-10 19:27:13 +00:00 (in Redmine)


Hello,

We are currently trying to import our running configuration in Icinga Director (1.1.0). Our configuration rely on dictionaries and "nested variables" in our hosts. These are used in our "apply for" rules.
The issue is, that as from what we saw (I checked on this tracker and in the example), there is at the moment no options to reflect this kind of configuration in Director. The example provided in the documentation of Director only refer to Array.

Our configuration is mostly based on the following example from the official documentation (see [[http://docs.icinga.org/icinga2/latest/doc/module/icinga2/toc\#!/icinga2/latest/doc/module/icinga2/chapter/monitoring-basics#using-apply-for-custom-attribute-override\]\]):

vars.interfaces["GigabitEthernet0/2"] = {
     iftraffic_units = "g"
     iftraffic_community = IftrafficSnmpCommunity
     iftraffic_bandwidth = 1
     vlan = "internal"
     qos = "disabled"
  }

We also use this kind of structure:

vars.an_ncpa = {
    token = "secret"
    load = { warning=10, critical=20 }
    cpuload = { warning=80, critical=85 }
    disk["/"] = { disk_warning="15%", disk_critical="10%" }
    disk["/home"] = { disk_warning="15%", disk_critical="10%" }
  }

The structure above was used for three reasons in our configuration:

  1. Easy to ready (for us at least)
  2. No need to prefix all variable
  3. Valid with Apply For.

I tried to workaround with a data field named like this: "an_ncpa.token", but it get renamed to "an_npcatoken" as soon as the configuration is rendered.
Is there any plan to allow dictionary creation in Icinga Director and same question for the "nested variable" (dictionary inside a dictionary for example) ?

If this kind of data structure is not "valid" could you please guide us on what we should use instead?

Attachments


Relations:

@icinga-migration
Copy link
Author

Updated by itbess on 2016-08-03 05:08:59 +00:00

+1

@icinga-migration
Copy link
Author

Updated by msmol on 2016-08-03 19:50:09 +00:00

+1
My team (jprivard & cardeois) and I also need this functionality and are ready to develop it. We've discussed several possible solutions and have decided the best approach would be to create a new object similar to the DataList, called DataObject. The difference being DataList contains only "key", and "value", whereas DataObject will contain "key", "value", and "type".

In the aforementioned example :

vars.an_ncpa = {
    token = "secret" 
    load = { warning=10, critical=20 }
    cpuload = { warning=80, critical=85 }
    disk["/"] = { disk_warning="15%", disk_critical="10%" }
    disk["/home"] = { disk_warning="15%", disk_critical="10%" }
  }

The "an_ncpa" form definition could be represented by a DataObject such as:

[{
    "key": "token", 
    "value": "Token",
    "type": String
},
{
    "key": "load", 
    "value": "Load",
    "type": DataObject['notification_threshold']
},
...]

Where "notification_threshold" is another DataObject that can be used generically for both 'load' and 'cpuload' etc.

That being said, the "disk" structure above would be difficult to implement using this approach. A possible workaround would be to use a structure like this instead:

vars.an_ncpa = {
    ...
    disk =  [
        {disk_warning="15%", disk_critical="10%", path="/"}
    ]
    ...
}

This would be generated by a DataObject like so:

{
    "key": "disk", 
    "value": "Disks",
    "type": Array(DataObject['disk'])
}

Would this be an acceptable solution for you? Open to suggestions / alternatives / reasons why this will fail miserably.
Seems like a lot of work but we are willing to get started if there's interest & no objections to our proposed solution.

Cheers,
Mitch

@icinga-migration
Copy link
Author

Updated by tgelf on 2016-08-03 22:05:23 +00:00

Hi Mitch,

thanks a lot for your input, this absolutely makes sense to me. What about calling it "Dictionary" instead of "Data Object" to follow Icinga 2 naming conventions? Should the value really be part of the type? I'd consider leaving it away. Right now, for all data fields default values are specified when assigning a field to a real object. Then, a DataType currently consists of a class name stored in an entry in the datafield table. So why not basing the dictionary on this, creating a list of datafield_ids and related arity?

Let me show you the DB tables that could reflect this:

CREATE TABLE director_dictionary (
  id int(10) unsigned NOT NULL AUTO_INCREMENT,
  dictionary_name varchar(255) NOT NULL,
  PRIMARY KEY (id),
  UNIQUE KEY dictionary_name (dictionary_name)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

CREATE TABLE director_dictionary_field (
  dictionary_id int(10) unsigned NOT NULL,
  datafield_id INT(10) USIGNED NOT NULL,
  is_required ENUM('y','n') NOT NULL,
  allow_multiple ENUM('y','n') NOT NULL,
  PRIMARY KEY (dictionary_id,datafield_id),
  CONSTRAINT director_dictionary_field_dictionary
    FOREIGN KEY (dictionary_id)
    REFERENCES director_dictionary (id)
    ON DELETE CASCADE
    ON UPDATE CASCADE,
  CONSTRAINT director_dictionary_field_datafield
    FOREIGN KEY (datafield_id)
    REFERENCES director_datafield (id)
    ON DELETE RESTRICT
    ON UPDATE CASCADE
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

We could combine is_required and allow_multiple into a single "arity" property or use min/max-count, but as is_required is already used in the *_field table it seemed to fit better. Please note that this schema would also allow one to create two different types for the same key, something that could also be desired. Your initial disk example should perfectly be possible with this, your workaround (Array of disks) isn't. But that's what we should then add as a new DataType "Array of SomeType" while renaming the current Array to "Array of Strings".

Cheers,
Thomas

@icinga-migration
Copy link
Author

Updated by jprivard on 2016-08-04 14:54:02 +00:00

cardeois, msmol and I just reviewed your solution and we all agree to it. We'll start working on it right now.

@icinga-migration
Copy link
Author

Updated by tgelf on 2016-08-04 15:02:40 +00:00

jprivard wrote:

cardeois, msmol and I just reviewed your solution and we all agree to it. We'll start working on it right now.

Fantastic! Can't wait to see the result ;-) This is gonna be a pretty cool feature I guess.

@icinga-migration
Copy link
Author

Updated by msmol on 2016-08-08 19:57:47 +00:00

Hi Thomas,

We started working on this feature and have a a few things done, and a few things left to do.

So far, we've added 2 new tabs in datalist section, for creating Dictionaries and DictionaryFields.

What we're missing is the ability to set a custom attribute with type Dictionary, as we're not really sure how to show this kind of nested data in a reasonably nice way. cardeois tells me that he thinks you're already working on something like this. Can you confirm whether or not you are, and if so if you've made much progress?

You can see our internal PR inside our fork here: internap#1

Cheers,
Mitch

@icinga-migration
Copy link
Author

Updated by tgelf on 2016-08-09 15:20:08 +00:00

  • File added filter_form_playground.png

Hi Mitch!

msmol wrote:

So far, we've added 2 new tabs in datalist section, for creating Dictionaries and DictionaryFields.

Perfect. I had a quick look at it, looks good to me. What was the reason for the "Rename dictionary_field to dictionaryfield" commit? All the other implemented fields use "_field" as a postfix, did you run in any problems with this?

What we're missing is the ability to set a custom attribute with type Dictionary, as we're not really sure how to show this kind of nested data in a reasonably nice way. cardeois tells me that he thinks you're already working on something like this. Can you confirm whether or not you are, and if so if you've made much progress?

One missing part is the data type itself, please have a look into library/Director/DataType for examples and register your new type in configuration.php. Registration might seem useless, but it is pretty cool as it allows other Icinga Web 2 modules to provide their very own data types in an upgrade-safe way.

The DataType must provide a form field, and this is where it starts to become tricky. Such a form field is basically a ZF1 form element. You'll need to create one for Dictionaries in library/Director/Web/Form/Element. That element can specify a custom view helper, you'll also need one, should be placed into application/views/helpers/.

The web form is the trickiest part of your feature request and the main reason why I avoided to start working on it so far ;-) You could have a look at the ExtensibleSet for an example, but that one isn't isolated very good.

What I'm currently working on (I think that's what cardeois was referring to) is a form element for "Filters", allowing one to create deep nested filter definitions while allowing the developer to handle it like a single-element form field. That component got larger than expected, as I want to use it for nested apply rules and added support for data types. Please have a look at the attached screenshot:

filter_form_playground.png

Look & feel might be improved. However, it is pretty cool. Every icon is a form button, you can nest it as far as your screen allows. And still, in your code you just write

$this->addElement('dataFilter', 'element_name', ...);

...and that's is. $element->getValue() gives you a full Filter object that can be serialized and stored to DB. I wanted to have this feature in v1.1.1, even if I haven't been forced to do so. By the end, it became the main reason why I didn't release v1.1.1 yet :p I'm currently on vacation, but I'll continue to work on this these days. Still a little bit unhappy with how I have to deal with data types, but I'll find a solution.

As you can see, the problems I have to tackle for this feature are very similar to yours. So it could absolutely make sense for you to wait for this, but you could also start with this in parallel. Regardless of this I'd suggest to go on with the provided DataType itself. Then make your first steps with a rough custom FormElement. Do not spend to much time there, eventually cheat by using a simple text field asking for JSON in the meantime. Try to figure out whether you can use CustomVariableDictionary for validation/serialization, or what other components might be missing.

I hope this helps, please do not hesitate letting me know when I just confused you instead ;-)

Cheers,
Thomas

@icinga-migration
Copy link
Author

Updated by msmol on 2016-08-10 19:27:13 +00:00

What was the reason for the "Rename dictionary_field to dictionaryfield" commit? All the other implemented fields use "_field" as a postfix, did you run in any problems with this?

Yes, I don't remember exactly what the issue was, but between whatever the issue was (been a few days now), and seeing how "DirectorDatafield" uses "Datafield" and not "DataField", it seemed like a simple solution that solved the issue at the time.

One missing part is the data type itself, please have a look into library/Director/DataType for examples and register your new type in configuration.php. Registration might seem useless, but it is pretty cool as it allows other Icinga Web 2 modules to provide their very own data types in an upgrade-safe way.

Yes, already started work on that, but isn't committed yet. Basically for the reasons you listed, i.e. wasn't sure what to do for the web form, and seeing as cardeois mentioned you were maybe already working on something for this, wrote the comment above :-)

I think I'll take your advice and cheat for now with a simple text field.

Thanks for the help, and enjoy your vacation

Mitch

@icinga-migration
Copy link
Author

Updated by tgelf on 2016-10-25 12:02:39 +00:00

  • Blocks set to 11849

@cbnorman
Copy link

when is this feature likely to be made available?

@cschneemann
Copy link

Still working on this? It would be great to have dictionary-support in director

@tobiias123
Copy link

I'am currently writing my bachelor thesis about the Icinga Director. One point of this is to analyze how to migrate an existing configuration to the Director. There I ran into the issue with the dictionary-support. So are there any news about this? Is there a workaround or something?

@likg
Copy link

likg commented Aug 1, 2017

Propose to add an ability to insert user-defined functions also. Icinga2 core supports complex data types since 2015 (new-features-in-icinga-2-3) and the following vars definition is perfectly valid in Icinga2 world:

vars.http_vhosts["icinga.org"] = {
    http_address = "icinga.org"
    interval = 1m
  }
vars.dummy_text = {{ return "Dummy text here" }}
vars.dummy_state = {{ return 0 }}

Director object inspection already represents functions as:

vars: {
        dummy_state: { type: "Function" },
        dummy_text: { type: "Function" }
      }

Therefore comment sound reasonable to me.

@friesoft
Copy link
Contributor

Any news on dictionaries? :-) We are really missing them...

@marcelfischer
Copy link

We need them too.
For example for filesystem monitoring we have a standard set of filesystems. But we want to change thresholds for a single filesystem for a single or group of hosts.

Currently we have an service set with the defaults and then create a new service set for each exception

@linuxmail
Copy link

Hello,

I wanted to migrate too and stuck now again, after I remembered, why I did not move month ago to the Director module: I need the dictonary support. I have a bigger Ceph storage with many disks (and types) and a lot of switches. For example on some I have SSDs, HDDs, Raid and virtual disks which which can be present as /dev/sda ... I have no idea, how can I solve it without the dictionary type.
The same problem I have for switch ports: from virtual, over standard 1Gbit/s to 100Gb/s ports. So a default array won't work.

Is there workaround for that problem, without moving the whole node to icinga2 conf.d/ again?

cu denny

@freddy4711
Copy link

+1 for that. It would be a really nice feature, if Director supports Dictionaries. This would make life easier.

@SimonHoenscheid
Copy link

Any News? @Thomas-Gelf

@wols
Copy link

wols commented Dec 15, 2017

Is this a blocker for own function Icinga/icinga2#5870 also?

@mkayontour
Copy link
Member

I do need that too, when working with the command "by_ssh" the by_ssh_arguments variable won't work because it expects a dictionary.

@psteffen
Copy link

psteffen commented Feb 1, 2018

+1

2 similar comments
@n-ncls
Copy link

n-ncls commented Feb 11, 2018

+1

@phil-or
Copy link

phil-or commented Feb 16, 2018

+1

@rojobull
Copy link

+1. Need dictionary data type fields in Director.

@lokidaibel
Copy link
Contributor

+1 Realy needed. Would make so much so more easy,

@Thomas-Gelf
Copy link
Contributor

Had related discussions multiple times, trying to sum it up, just for the records:

  • in my believes most of the times you're trying to solve something with dictionaries, you're doing it wrong
  • Icinga 2 configuration examples often show "create Disk/vHost/whatever checks in a loop". This is nice in text-based configuration, but not in a GUI
  • in plaintext config this allows to have things "in one place", UI doesn't have this problem. It shows a hosts services, no matter how they have been defined
  • "Services generated from dictionaries" are redundant, you have all the properties in a dictionary and create on service object for each entry, with the drawback that you cannot modify the resulting service
  • Director instead allows to override thresholds per Host, even when this Service is an Apply Rule
  • You can even blacklist applied services since v1.5.0
  • synchronizing (and purging) single services from external sources also works fine, no need to create weird nested data structures on your hosts
  • single services can easily be searched, listed, sorted, filtered
  • allowing your users to create free-form dictionaries you loose control. No validation, no drop-downs, no suggestions, no restrictions
  • your users should go to a hosts service list, say "Add Service -> HTTPS URL Check", fill in the URL, store. And not "got to one of the many empty host dictionary fields, add one more entry and try to remember what the name of it's properties used to be"

Said all this, sooner or later we might implement this. It has low priority, as we see no gain in it. It will have the form of custom nested data types, allow to combine different data fields with related rules like explained in this and other related issues. We are currently preparing the next generation of our base web form libraries. Ones we have them, weird nested form elements will become easier to implement. We have no plans to allow for free-form Dictionaries, and in our believes not doing so is the right way.

As always, there might be different opinions and use-cases we didn't think about. I'd love to learn more about them!

Cheers,
Thomas

@rojobull
Copy link

The way I accomplished this was not by using apply rules. I created a service template that takes in the partition name, and desired thresholds and then applied one service for each partition for each system. Thomas-Gelf's comment above describes doing exactly this.

@d-stevens
Copy link

But as you said:

applied one service for each partition for each system.

This is very time-consuming and cannot handle imports from Inventory scanning, CMDB or something else.
Icinga can do this. It is only the director addon that doesn't.

@rojobull
Copy link

Even if you use dictionaries, you have to have some way of determining what partitions need to be monitored on each system. If you know this ahead of time, then you can just apply one service for each partition to a host template and you're done. If it needs to be dynamic, as in my case, you need to do something else to determine what partitions each system has.
We did this outside of Icinga/Director. We wrote a python script enforced by Puppet that each host runs a few times per day, detects its partitions and if there are changes, it adds/removes partitions via the Director API. This way it is automated and we don't have to ever worry about manually adding/removing partitions to be monitored. Also, since Director is in place, we still have a point and click interface where we can adjust thresholds on a per system, per partition basis.

@d-stevens
Copy link

ok. I understand. what the director cannot do your third-party tool can.
I wanted to import those informations with sync-rules instead of writing an API-Script that does it.
I think it would be more time-saving to write all the configuration in the config-files and use the abillities of icinga2 then using the director + writing api-scripts. And all that only because dictionaries are not available.

@rojobull
Copy link

You can still use dictionary variables so long as you define the service and apply rules in Icinga2, outside of Director. It just means that you won't be able to manage that particular service and associated thresholds in Director which is not ideal.
I only briefly tested sync'ing and sync rules. In my case I was importing systems from LDAP. It worked great except I didn't see a way to automatically detect each system's partitions. Perhaps there is a way to do it but we just decided to add that logic outside of Director.

@ss6s
Copy link

ss6s commented Oct 18, 2018

WE NEED THIS FEATURE! + 1

@Gianlu
Copy link

Gianlu commented May 5, 2019

It would be very nice to have this feature.

@kwiksand
Copy link

Trying to port my manually generated code to director for "ease" of reuse , but this really seems to be a showstopper. Any movement on this as a feature?

@msokolich-ta
Copy link

Like many others here this is a pretty necessary feature for our environment.

@shake-spear
Copy link

yes - this feature would be quite a scream +1

@drallgood
Copy link

What happened to this?

@Thomas-Gelf Thomas-Gelf changed the title [dev.icinga.com #12093] Data Field - Dictionary type/nested variable currently not possible Data Type "Dictionary" to allow for nested variables Jan 15, 2021
@Thomas-Gelf Thomas-Gelf self-assigned this Jan 15, 2021
@Thomas-Gelf Thomas-Gelf added this to the 1.9.0 milestone Jan 15, 2021
@Thomas-Gelf
Copy link
Contributor

This is going to be a thing.

@lokidaibel
Copy link
Contributor

Hey, it only took 5 Years to get you on our side ;D
Well, constant dripping wears away the stone .

@Thomas-Gelf
Copy link
Contributor

Honestly, I'm still not on your side 🤣 In my believes working this way is an ugly hack and leads to redundancy. But now there is no way back, Pandora's box is wide open 😱 And sure, it is a nice feature that helps getting things done.

Reason for this getting implemented was a customer project where the main task was to ship structured data to a bunch of custom-made plugins, where the "real" multi-instance-logic was in those plugins - and partly in their related notification scripts. So they sponsored a few days, allowing me to work on this.

Some related features will follow very soon, stay tuned ;-) There will be some adjustments related to apply for rules, and related "Overrides" are a (tricky) topic.

API, CLI and Sync already allowed for structured data, that part continues to work the way it did. With the difference that you can now allow your users to work with that data in the UI too.

@lokidaibel
Copy link
Contributor

Though something like that. But i like that you actually closed a issue from over 5 Years ago !
Even you belives are different, hell yeah thats professionalism !
Thx for the fix !
Stay healthy, you are the best !

@Thomas-Gelf
Copy link
Contributor

Dry-aged issues get better over time 🤣 You're welcome, glad that you like it.

Stay healthy, you are the best !

You too! Got Corona-positive in November, no plans to catch it again anytime soon ;-)

@joni1993
Copy link
Contributor

joni1993 commented Jan 27, 2021

I'm really heaving a hard time getting this to work. No matter how i set it up i always run in some kind of exception (either SQL fetch issues or PHP exceptions 😕 ) - Do you prefer this issue to report or should i create a new one?

@dskachan
Copy link

I'm really heaving a hard time getting this to work. No matter how i set it up i always run in some kind of exception (either SQL fetch issues or PHP exceptions 😕 ) - Do you prefer this issue to report or should i create a new one?

confirm. Master is unusable at the moment.

@Thomas-Gelf
Copy link
Contributor

Could you please open a dedicated issue, showing the errors/exceptions you're facing?

@MarcusCaepio
Copy link

Hi @Thomas-Gelf
I am currently looking at the director again and try to "translate" my DSL config to it.
Although with the new dictionary type, I am not able to translate it 1:1. You said, using dictionaries is a redundancy. So I guess, I am doing sth. wrong. Could you please tell me, how I have to configure the director to get the same result as in this DSL snip? Thanks a lot

apply Service "tcp: " for ( tcp => config in host.vars.tcp ) {
  import "generic-service"
  check_command = "tcp"
  vars += config
  assign where host.vars.tcp
}
object Host "abc" {
  [...]
  vars.tcp["123"] = {
    tcp_address = "xxx.xxx.xxx.xxx"
    tcp_port = "123"
  }
  vars.tcp["456"] = {
    tcp_address = "xxx.xxx.xxx.xxx"
    tcp_port = "567"
  }
}

@joni1993
Copy link
Contributor

joni1993 commented Oct 5, 2021

I created a patch, because dictionaries did not work for me on hosts - only on services. (see #2275 (comment))

To translate the host dictionary 1:1 you can do the following (after applying the patch):

  1. create fields "tcp_address" and "tcp_port".
  2. create a host template "host-tcp-service-dictionary" (or any name you like)
  3. assign the created fields to the host template
  4. create another field of type dictionary (template object type=Host, template=host-tcp-service-dictionary)
  5. assign this dictionary field to any host template you like (e.g. "server template")
  6. on host "abc" import "server template"
  7. on host "abc" click "Manage Instances" on your field - add the key (e.g. "123" under name, and for values set "xxx.xxx.xxx.xxx" for "tcp_address" and "123" for "tcp_port" and then "Store"
  8. verify everything is correct in the preview - it should look like:
object Host "abc" {
[...]
    address = "10.4.0.31"
    vars.tcp= {
        123= {
            tcp_address = "xxx.xxx.xxx.xxx"
            tcp_port = "123"
        }
    }
}

Sadly without the adjustments in the "apply-for" section for services you can not recreate the service-apply like in your DSL.
Maybe @Thomas-Gelf can shed some light on this topic.

@joni1993
Copy link
Contributor

joni1993 commented Aug 24, 2022

As if today i was able to create the dict for a host successfully - IcingaDirector successfully creates a host looking like this:

object Host "XXX" {
[...]
    vars.host_interfaces = {
        wan = {
            host_address_ip = "XXX.XXX.XXX.XXX"
            host_address_type = "wan"
        }
        vlan_surveillance = {
            host_address_ip = "YYY.YYY.YYY.YYY"
            host_address_type = "vpn"
        }
    }
}

Sadly you cant create a service apply for dict type in director, but manual config works:

apply Service "if-" for (interface_name => interface_config in host.vars.host_interfaces) {
        import "generic-service"

        check_command = "ping4"
        display_name = "Ping [" + interface_name + "]"

        vars.ping_address = interface_config["host_address_ip"]
}

The only thing that does not work correctly is that IcingaDirector shows "empty [Manage Instances]" for the variable in the host - but opening it reveals the entries.

@MarcusCaepio
Copy link

Coming back to this issue, as I am (again) having a look to the new director version.
@joni1993 I did your patch and can see the dictionary in the host overview. I also can create a new instance, but I am not able to set the properties. Tried to solve my example at #337 (comment) and failed again :(
I really would like to use the director for automation stuff, but using host dictionaries is a must have for me...

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

No branches or pull requests