[dev.icinga.com #770] Acknowledge with expire time #369
This issue has been migrated from Redmine: https://dev.icinga.com/issues/770
Created by mfriedrich on 2010-09-02 10:28:32 +00:00
2010-09-04 10:26:46 +00:00 by ricardo 09e15152b5c60b1e289359b487427321a43ad37f
2010-10-22 17:25:06 +00:00 by ricardo 36e0a33
2011-09-12 20:25:52 +00:00 by ricardo 55c200f
2011-09-13 17:25:46 +00:00 by ricardo afa2835
2011-09-16 16:42:36 +00:00 by mfriedrich a125426
Updated by ricardo on 2010-09-04 10:39:02 +00:00
Fixed with git commit: 09e15152b5c60b1e289359b487427321a43ad37f
The command string has changed. Added the expire timestamp.
Updated by mfriedrich on 2010-09-14 07:23:26 +00:00
by changing the command string, this will break backwards compatibility.
furthermore, the broker data struct ds is being enhanced too. this might lead into problems for current neb modules.
next to that, idoutils needs to be adapted reading acknowledgement expiry time from the buffer, pushing into the database (query adaption, schema changes). this affects then the api querying the information, and the web displaying this information.
since this is a rather huge change, please discuss.
Updated by mfriedrich on 2010-09-14 21:42:24 +00:00
i would propose the other way around - check if other popular addons like merlin or livestatus still work with that (typecasting hell!) and provide feedback.
i still can implement the changes for idoutils, it's not that much to do/change. for api/web:
Updated by ricardo on 2010-09-16 10:23:26 +00:00
The problem is, I have no idea what kind of modules are out there.
At first I tested mk-livestatus. It crashed icinga. I have written a patch for mk-livestatus and with this one it works fine.
now i gonna try out merlin.
What modules do you all know? Is there a other way to implement this feature?
Updated by mfriedrich on 2010-09-16 10:34:27 +00:00
well everything changing core structures will crash livestatus. can you attach the patch for livestatus to see what needs to be changed?
probably dnx too, but i am not sure if the acknowledgements struct is used over there in regard of typecasting.
well in regard of changing core structures, a proposed way is to add new struct members at the end in order to allow memory mapping and typecasts. but i am not sure if anything else will break that then too.
Updated by ricardo on 2010-09-16 12:45:28 +00:00
here is the patch for livestatus.
All other modules should be working with changed headers. Then they can not request the data but at least they don't crash.
Merlin seems to work fine.
Updated by magellanic on 2010-09-16 15:15:41 +00:00
I've been having a peak at dnx, I agree that this shouldn't effect it, it doesn't seem to do weird struct casting noted in the livestatus stuff.
Updated by mfriedrich on 2010-09-16 15:29:27 +00:00
i think this a very big change/enhancement to the current core which needs to be tested on several platforms and with different addons like you just did.
we can try to do it during the release freeze, but i would like to give addons devs the chance to discuss about that changes and even to implement comaptibility.
for that reason i'd like to add that change into 1.3.x (unstable) and show that to the community - and not 1.2 which should run stable.
Updated by mfriedrich on 2010-11-03 17:05:43 +00:00
still, this breaks the NEB API. i would like to bump for Icinga 2.x instead of pushing that onto 1.3 branch. 1.3 will be the base for 1.4 stable.
We can take care of IDOUtils, but it will also break Merlin or Centreon Broker which are considered to be important addons meanwhile.
we need to figure out a workaround for modules not supporting that, but exporting the old fashioned style of the neb callbacks. possibly by adding a new callback then keeping the old for compatibility reasons!!!
Updated by mfriedrich on 2010-11-05 11:24:09 +00:00
just some more thoughts on this, as you have proposed another feature which might break the neb api.
how about creating an "icinga event broker api" and keeping the old "nagios event broker" api intact?
then we might register idoutils/idomod functions like we want, but all other addons still work with the old grown neb api.
this will need a deep rundown into the core, re-implementing the broker.c and nebmods.c to fit our needs.
using this way, we are able to keep compatibility in various ways, but also offer our own abstracted layer.
i'd also see that as pre-requisite of a new core api in that case, doing a proof of concept and clearly adding abstracted layer functions for such callbacks.
then i'd like to see this feature with the new IEB API instead of NEB API, and IDOUtils will remove compatibility with NEB and step on IEB - which brings the benefit of integrating it even more deep into the core and hackup things like queued events in the broker e.g.
Updated by mfriedrich on 2011-03-14 22:00:38 +00:00
hmmm considering the recent changes on
i have another idea.
if someone changes the compatibility to support that too then, we can still change the old kept functions too.
cmd.cgi and retention.dat shouldn't be a problem then. i also need to add changes to idoutils then.
let's consider this something for the next unstable release!
Updated by ricardo on 2011-09-12 20:37:56 +00:00
Well, finally submitted (1 year later) a new patch to add this function.
It will break the ABI, like "address6" does, but all modules should work fine.
THe commit Message:
Could everyone please test this to make sure it works properly that it can hit 1.6?
After other people tested it and works for them I will add a Issue for icinga-web and icinga-docs.
Updated by mfriedrich on 2011-09-12 20:46:53 +00:00
and how would a neb module get the appropriate callback then?
idomod needs to be aware of this changes, ido2db needs to know that too, and the db schema will require an upgrade, before you can alert web / docs.
Updated by mfriedrich on 2011-09-12 20:53:42 +00:00
ok, ic, replacing the function and keeping the old one.
2 more things. the one for the neb struct new attribute at the end is fine, but you can't leave that as is in objects.h and statusdata.h - this requires this small adaption too.
and one typo - it's called "expiring", not "expiering".
the rest looks good, thanks. i'll sneak into within the next days.
Updated by mfriedrich on 2011-09-16 16:36:36 +00:00
external command example
i've taken the liberty of adding it to idoutils too without creating an extra issue.
idomod debug message sample (note the 267 is a new type at the end)
within the mysql database, a newer example after a debug session.
icinga.debug on the event for expiring
and voila it is gone :-)
the question remains - how should this get cleaned from the database itsself?
the dummy test for the mklivestatus users (using 1,1,9irgendwas)
Updated by mfriedrich on 2011-09-16 16:41:51 +00:00
so, this needs proper testing. and as far as i have seen, you've been adding 2 new external commands, which need to get documented then.
if this hits the release branch for 1.6 we need a docs issue for that then.
Updated by mfriedrich on 2011-09-16 16:46:26 +00:00
and once more, when this is running properly, assign the icinga-web dev team an issue to