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

----- 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: 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'. OK,make sense , I will add it to the design > >> >>> >>>> 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? No we can't currently , we are assigning each AuditLogType a certain severity, so , I will have to define here 4 for NORMAL , WARNING , ERROR and ALERT > >> 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? I think that since those are external events ,it may clean up on a different period and forced by the regular application event clean-up value > >> >> >>> >>>> 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... How entities are passed to the engine? Do we have a full use-case for that ?

> >> >>> >>> 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. >