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

[Eli Mesika]

----- Original Message -----
> From: "Itamar Heim" <iheim at redhat.com>
> To: "Eli Mesika" <emesika at redhat.com>
> Cc: "engine-devel" <engine-devel at ovirt.org>, "Einav Cohen" <ecohen at redhat.com>, "Michael Pasternak"
> <mpastern at redhat.com>
> 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 at redhat.com>
> >> To: "Eli Mesika" <emesika at redhat.com>
> >> Cc: "engine-devel" <engine-devel at ovirt.org>, "Einav Cohen"
> >> <ecohen at redhat.com>, "Michael Pasternak"
> >> <mpastern at redhat.com>
> >> 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.
>