[dev.icinga.com #770] Acknowledge with expire time #369
Comments
Updated by mfriedrich on 2010-09-02 10:29:17 +00:00
Please have a look :-) |
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 ricardo on 2010-09-04 19:40:34 +00:00
Applied in changeset commit:"09e15152b5c60b1e289359b487427321a43ad37f". |
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 ricardo on 2010-09-14 20:12:58 +00:00 I'll try to implement the changes that have to be done to the other components as well. This well be:
hopefully finishing it bevor the next release |
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 ricardo wrote:
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 ricardo on 2010-09-16 13:50:07 +00:00 as far as I could investigate it dnx isn't using any of the nebstruct_acknowledgement_struct. So this shouldn't be a problem. |
Updated by ricardo on 2010-09-16 14:40:36 +00:00 ricardo wrote:
the same for mod_gaerman |
Updated by mfriedrich on 2010-09-16 14:41:53 +00:00 maybe we can adapt the struct changes - add the new params at the end of the struct. but for the rest i figure that it might need a livestatus patch then. |
Updated by magellanic on 2010-09-16 15:15:41 +00:00 ricardo wrote:
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. your thoughts? |
Updated by ricardo on 2010-09-16 22:05:10 +00:00
I agree, sounds reasonable. |
Updated by mfriedrich on 2010-09-17 11:46:13 +00:00 regarding your commit, my fix for #790 will remove the check_program_update define (16) completely. just to mention that. |
Updated by mfriedrich on 2010-10-10 12:13:52 +00:00
|
Updated by ricardo on 2010-10-22 17:41:26 +00:00 reimplemented it into my new rbartels/1.3 branch |
Updated by ricardo on 2010-10-22 19:13:32 +00:00
Applied in changeset commit:"36e0a3369e039f0af2913287c444461c150769d5". |
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. thoughts? |
Updated by ricardo on 2010-11-05 11:33:42 +00:00 this sounds terrific. |
Updated by mfriedrich on 2010-11-05 11:36:15 +00:00
ok, then i'll set this one up for 1.5 as the required changes might take a bunch of time for 1.3 |
Updated by elagon on 2010-11-05 14:23:13 +00:00 +1 for IEB ! |
Updated by Anonymous on 2010-11-05 16:09:27 +00:00 how will this look for mod_gearman / dnx / pnp4nagios? |
Updated by mfriedrich on 2010-11-05 16:13:29 +00:00 pls move the discussion over to #964 as this is just the feature request issue for acknowledgement with expiry time. |
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 mfriedrich on 2011-07-12 18:47:42 +00:00
|
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. Cheers |
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. https://git.icinga.org/?p=icinga-core.git;a=commitdiff;h=55c200f24ea790d78f891de85eb35b9857f087f1#patch14 and one typo - it's called "expiring", not "expiering". the rest looks good, thanks. i'll sneak into within the next days. |
Updated by ricardo on 2011-09-13 17:28:55 +00:00 Thanks for pointing out. fixed in current 'test/core' |
Updated by mfriedrich on 2011-09-16 16:36:36 +00:00
sweet and fancy javascript gui look 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
|
Updated by mfriedrich on 2011-10-06 17:55:00 +00:00 the gui stuff is done, the docs issue is still pending. |
Updated by mfriedrich on 2011-11-11 16:56:49 +00:00
ready for release. thanks again ricardo. |
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
Assignee: ricardo
Status: Resolved (closed on 2011-11-11 16:56:49 +00:00)
Target Version: 1.6
Last Update: 2011-12-03 11:30:09 +00:00 (in Redmine)
http://feedback.icinga.org/forums/50329-general/suggestions/890535-acknowledge-with-expire-time?ref=title
Attachments
Changesets
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
Relations:
The text was updated successfully, but these errors were encountered: