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

[dev.icinga.com #12093] Data Field - Dictionary type/nested variable currently not possible #337

Open
icinga-migration opened this issue Jul 4, 2016 · 36 comments

Comments

Projects
None yet
@icinga-migration
Copy link
Member

commented Jul 4, 2016

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

This comment has been minimized.

Copy link
Member Author

commented Aug 3, 2016

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

+1

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Aug 3, 2016

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

This comment has been minimized.

Copy link
Member Author

commented Aug 3, 2016

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

This comment has been minimized.

Copy link
Member Author

commented Aug 4, 2016

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

This comment has been minimized.

Copy link
Member Author

commented Aug 4, 2016

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

This comment has been minimized.

Copy link
Member Author

commented Aug 8, 2016

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

This comment has been minimized.

Copy link
Member Author

commented Aug 9, 2016

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

This comment has been minimized.

Copy link
Member Author

commented Aug 10, 2016

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

This comment has been minimized.

Copy link
Member Author

commented Oct 25, 2016

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

  • Blocks set to 11849
@cbnorman

This comment has been minimized.

Copy link

commented Apr 12, 2017

when is this feature likely to be made available?

@cschneemann

This comment has been minimized.

Copy link

commented May 12, 2017

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

@tobiias123

This comment has been minimized.

Copy link

commented Jul 17, 2017

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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Contributor

commented Aug 18, 2017

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

@marcelfischer

This comment has been minimized.

Copy link

commented Sep 8, 2017

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

This comment has been minimized.

Copy link

commented Oct 12, 2017

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

This comment has been minimized.

Copy link

commented Oct 18, 2017

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

@SimonHoenscheid

This comment has been minimized.

Copy link

commented Nov 1, 2017

Any News? @Thomas-Gelf

@wols

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2017

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

@mkayontour

This comment has been minimized.

Copy link
Member

commented Jan 30, 2018

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.

@CobbleCity

This comment has been minimized.

Copy link

commented Feb 1, 2018

+1

2 similar comments
@3it-n

This comment has been minimized.

Copy link

commented Feb 11, 2018

+1

@phil-or

This comment has been minimized.

Copy link

commented Feb 16, 2018

+1

@rojobull

This comment has been minimized.

Copy link

commented Jun 13, 2018

+1. Need dictionary data type fields in Director.

@lokidaibel

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2018

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

@Thomas-Gelf

This comment has been minimized.

Copy link
Contributor

commented Jul 13, 2018

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

@lokidaibel

This comment has been minimized.

Copy link
Contributor

commented Jul 13, 2018

Hey Thomas,

Nexted vars helps alot when you work with other Systems that provide Data for the Monitoring.
For example our use case is the following:

To manage our systems we use Jira. We have workflows in Jira for order a host. Also this workflow "lives" as long the Server is in use.

Cause we have all the data of the Servers in Jira we created the subtask Monitoring. In this we have a field where our stuff can write JSON like:

URL Checks in the notation:

"URL_" : "",""

We are applying some rules inside the Director DB to make a loop over all this entry which creates a check like this:

APP_URL
APP_URL2

The problem is that it will publish all check relevant Data to the public (all users that are able to see the service).

But now we also want SNMP Checks to be applyed that way.
But in this case there will be ,, , <pretty_name> which will be a realy long Check name. So i want access only the <pretty_name> for the checks but all i have is $config$ which result in the whole vars displayed.

I dont see how can i accomplish this without nested Vars. But maybe i am doing it wrong as you mentioned

Greetings

Lukas Matecki from Cologne

p.s. I still own you some Gin for the next OSMC. Have found some special here in Cologne. Will bring it with me for this year osmc ;-)

@d-stevens

This comment has been minimized.

Copy link

commented Aug 10, 2018

I don't see any way to deploy Service Apply Rules with individual parameters without nested vars or dictionaries.
Simplest example for that is to check a drive usage, but want to change some theesholds on only one drive or host. Since apply-rules only know one $config$ var it is impossible to declare. So I Have to create one check per drive for all hosts. That's not what apply-rules focus on.
By the way: Using a nested var for the display field helps a lot instead of showing the whole data field.

There are some many usecases that can't be deployed without dictionaries.
WE NEED THIS FEATURE!

@rojobull

This comment has been minimized.

Copy link

commented Aug 10, 2018

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

This comment has been minimized.

Copy link

commented Aug 10, 2018

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

This comment has been minimized.

Copy link

commented Aug 10, 2018

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

This comment has been minimized.

Copy link

commented Aug 10, 2018

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

This comment has been minimized.

Copy link

commented Aug 10, 2018

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

This comment has been minimized.

Copy link

commented Oct 18, 2018

WE NEED THIS FEATURE! + 1

@Gianlu

This comment has been minimized.

Copy link

commented May 5, 2019

It would be very nice to have this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.