Icinga / icingaweb2-module-director

The Director aims to be your new favourite Icinga config deployment tool. Director is designed for those who want to automate their configuration deployment and those who want to grant their “point & click” users easy access to the configuration.
https://icinga.com/docs/director/latest
GNU General Public License v2.0
413 stars 203 forks source link

Creating objects with object_name = some auto increment id wreaks havoc #992

Closed rattacresh closed 7 years ago

rattacresh commented 7 years ago

Expected Behavior

Creating a host with a number as object_name should be possible without major side-effects.

Current Behavior

When I create a host with an object_name that happens to match the internal auto increment id of some other host, any objects referring to that host (like services) get stolen by the newly created host.

Possible Solution

Cast $id to int before calling static::getPrefetched($id) in DbObject::loadWithAutoIncId(), thus preventing $id to be searched first in the array of object_names.

Steps to Reproduce (for bugs)

  1. Create some arbitrary host "testhost" and a service for that host
  2. insert a host with an object_name that happens to match director's internal auto increment id of testhost
  3. Deploy configuration
  4. See what host the services for testhost were assigned to.

Context

We want to assign the ids from our host database to the object_name, for simple and reliable identification, and put the hostname into the display_name instead. We can't simply add a prefix since that would change ticket ids for agents etc., requiring us to redo all hosts.

Your Environment

Thomas-Gelf commented 7 years ago

Thanks for catching this one, the fix absolutely makes sense. Will be merged very soon, have to finish some work in the next branch first. Or I'll eventually merge it directly there, just do not want master to diverge too much right now.

Regardless of this, please re-evaluate what you're trying to build. Using id's as names feels like a terribly idea to me. While it is perfectly legal to use whatever you want in Icinga 2 object names, the natural perfect fit would be the fqdn. To me this sounds like you're trying to fix some import/sync issues in a weird way. Try to explain me where you came from, eventually I can come up with a better idea.

And, btw: numeric host names are illegal. Just try to ping 2130706433 or 16777217 on your command line. I hope this helps towards your decision to step away from using IDs as hostnames ;-) Also, when working with web clients it is sometimes necessary to distinguish parameters based on their content. As there are no data types, one easily comes to the conclusion "only digits -> id, otherwise -> name". Sure, it would be wrong again. But one might be tempted to do so for the sake of "nice URLs". So it could easily happen that sooner or later someone might break your environment again.

Said all this, don't get me wrong: it is a bug, you are perfectly right on this.

rattacresh commented 7 years ago

There are several reasons for us to chose this solution. For example, we would like to be able to monitor the same host from different satellites, so we need to have two icinga host objects for the same host with different zones, and would get a name conflict if we used the FQDN.

Also, we would like to allow our customers to be able to create custom checks. If two customers would like to monitor the same host (say, www.google.com, perhaps with different services) we would again get a name conflict if we use the fqdn as host name. Sure, it would be possible to merge hosts, or add a prefix, but after consideration we found that it would complicate things too much and thus we went for the id solution.

And finally we don't want to run into trouble when the hostname changes in our DB. Unfortunately, there is no way for director to track hostname changes, it always treats them as a delete of the host with the old object_name and a create of a host with the new object_name. Also, we can always ask icinga "what is the most recent check result for host ID=x?" without caring about renames, which might not have been synced/deployed yet and might thus not yet be reflected in Icinga.

Thomas-Gelf commented 7 years ago

@rattacresh: well... in case all your customers want to sell monitoring services to Google that's going to be a problem :laughing:

Jokes apart: in the prototype for the upcoming IcingaDB (IDO successor) we made distinct "environments" part of the main logic. That way, distinct Icinga 2 masters can write to the same DB without knowing anything about each other. But you'll still see all objects in the same GUI - based on granted permissions of course. You could have 20 different objects all being named test.example.com - but they would not be the same thing. As performance matters, there is no central autoinc logic. We also decided not to opt for GUIDs for other reasons. So, the "global" 20 Byte unique key for objects is in future:

sha1(sha1(env) + sha1(object name))

At least according current plans. No plans to introduce such a thing in Icinga itself, so to have 20 distinct checks for "Google" this would mean running 20 small Icinga instances in case they should all carry the same name. Director will afterwards be made environment aware. You could either have one Director talking to multiple environments or one Director DB per environment. And probably some way to share common template logic between different Director instances. No concrete plans here yet, we'll finish IcingaDB first.

So, in case you have ideas or suggestions pointing in that direction, that would be interesting for us. Please either open a dedicated issue or mail me directly, because this is no longer related to this issue. Renaming objects is easy, but not an issue at all with IDs. Just, it will still be an issue even with IcingaDB, as keys will still depend on names. However, working this way gives us huge performance benefits. We can write state and history to the DB in parallel without even knowing whether the object exists. Rough goal for our prototyping lab was "With one million checks, it should still perform well".

The schema tracks name history and therefor is prepared to track such changes. Just, you still need someone to "tell" IcingaDB that a rename took place. How should we know that a specific object is the same without someone telling us so? Might work for some people, might not for others.

While I do not really like seeing numeric IDs in the object_name, the idea of having a unique object key across all components (config tool, core, grapher, db) is tempting. It would render the object name useless for the front-end user and be a performance hit at object creation time. One would need one more field to store your fqdn. Relations based on fqdn would no longer be possible, this is something that would still work in our IcingaDB prototype.

I'll stop here, too many thoughts :-) Let me have yours!

Thomas-Gelf commented 7 years ago

Merged to next, thank you!