Re: [Engine-devel] [Design for 3.2 RFE] Adding support for external events

From: "Itamar Heim" <iheim(a)redhat.com&gt; To: "Eli Mesika" <emesika(a)redhat.com&gt; Cc: "engine-devel" <engine-devel(a)ovirt.org&gt;, "Einav Cohen" <ecohen(a)redhat.com&gt;, "Michael Pasternak" <mpastern(a)redhat.com&gt; Sent: Sunday, November 11, 2012 9:10:32 AM Subject: Re: [Engine-devel] [Design for 3.2 RFE] Adding support for external events On 11/10/2012 11:40 PM, Eli Mesika wrote: > > > ----- Original Message ----- >> From: "Itamar Heim" <iheim(a)redhat.com&gt; >> To: "Eli Mesika" <emesika(a)redhat.com&gt; >> Cc: "engine-devel" <engine-devel(a)ovirt.org&gt;, "Einav Cohen" >> <ecohen(a)redhat.com&gt;, "Michael Pasternak" >> <mpastern(a)redhat.com&gt; >> Sent: Friday, November 9, 2012 10:55:58 AM >> Subject: Re: [Engine-devel] [Design for 3.2 RFE] Adding support >> for external events >> >> On 11/08/2012 05:17 PM, Eli Mesika wrote: >>> Hi >>> >>> Please review , any comments are welcomed (please note that API >>> section is still in TBD) >>> >>> RFE :https://bugzilla.redhat.com/show_bug.cgi?id=873223 >>> Requirements : http://wiki.ovirt.org/wiki/Features/ExternalEvents >> >>> Events are classified as NORMAL , WARNING or ERROR and UI will >>> display different icon according to that. >> any reason to not allow external ALERTs? > > Currently we are using ALERTS only for PM events, do we have to > allow displaying external Alerts in the Alerts TAB as well??? not a must, just asking. if an external system wants to inject an event the temperature of a host is critical, its sounds more like an 'alert' to me than 'error'.

> >> >>> Detailed Design >>> :http://wiki.ovirt.org/wiki/Features/Design/DetailedExternalEvents >> >>> Adding is_external boolean field to audit_log with a default >>> value >>> of false >> >> hmmm, i'm not sure it makes sense to inject any event, just >> flagging >> it >> as external. >> why not just add a new AuditLogType of 'External' (i.e., just >> another >> event id? >> it is easy enough to search for it, etc. > > We are adding not one AuditLogType rather , it will be 3 or 4 > (depends on we support also Alerts) can't we separate the severity from the event id? why are the two tightly coupled?

> The additional is_external is defaulted to false , so , existing > code is not influenced > I think that in the search it will be simpler to refer to > is_external rather to the 3 or 4 specific types > Also , cleanup of old external events should use this column why should cleanup be different for internal and external events?

> > >> >>> New command is exposed currently only to SuperUser >> >> I assume you mean there is a new permission, which by default is >> added >> only to superuser role, and admin can add it to other custom >> roles? >> I also assume this is only relevant to admin users, not to user >> roles? > > Yes > >> >> other questions: >> 1. worth detailing all fields of the event which could be passed >> to >> this. > > Currently only the severity and the message text as stated in the > doc. I don't think this is good enough. idea is to be able to inject events as they relate to entities (host,storage,vm,etc.) > >> 2. can an admin add events to entities they don't have permissions >> on >> (I'm guessing yes, since admins aren't filtered from seeing all >> entities, so implicitly, they have permission to all entities) >> otherwise (if intent is to limit permisisons), an event is an >> action >> on >> multiple entities, so for any field which is passed for the event >> (vm_id, (vds)host_id, etc.), you'd need to check admin has >> 'AddExternalEvent' permission on. >> >> the main reason i think doing permissions may hold merit is it >> will >> allow to give this permission by default in more roles, rather >> than >> only >> for superuser, which may make it more viable for UI plugins that >> would >> try to leverage this. > > > I am missing here something, it should be an external event, i.e. > additional command invoked by any plug-in > What is the relation to current events ? to current entities, not current events. since the external events are about entities...

> >> >> 3. REST API modeling for adding an event (is that a PUT on the >> event >> collection, a POST)? >> can it be done only on root events collection, or on events of >> entities >> as well (taking the entity id from url, rather than as a >> parameter), >> etc. >> > > Again, the only info I have from the requirement is to support an > additional command. > The command invoker is responsible for the event text my question was about the REST modeling of this new command (especially as it is relevant only to the REST API, not to the UI). not sure what you mean by 'event text' here. > It seems that you are talking about invoking our events from > external plug-ins as well well, anything external. in any case they would be using the REST API for this.