On 11/11/2012 02:20 PM, Eli Mesika wrote:


----- Original Message -----
From: "Itamar Heim" <ih...@redhat.com>
To: "Eli Mesika" <emes...@redhat.com>
Cc: "engine-devel" <engine-devel@ovirt.org>, "Einav Cohen" <eco...@redhat.com>, 
"Michael Pasternak"
<mpast...@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" <ih...@redhat.com>
To: "Eli Mesika" <emes...@redhat.com>
Cc: "engine-devel" <engine-devel@ovirt.org>, "Einav Cohen"
<eco...@redhat.com>, "Michael Pasternak"
<mpast...@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 ?

an audit log event today can specify multiple entities (well, one of each type) it is relevant to.
why wouldn't an external event be able to?

use case?
external host monitoring system to inject an event that a host fan has malfunctioned.
external antivirus appliance scan to inject an event a vm has a virus.
etc.

there are two questions here:
1. API modeling wrt which action is "adding an event" (PUT on event
   collection, or POST for an action). if allowing entities, than
   probably entity is assumed from the action happening on the entity
   events collection.

2. permission check
- inject with single entity - requires permission for this action on
  the relevant entity). this permission could probably part of regular
  roles?
- inject with multiple entities - need to check the permission for each
  entity
- inject with no entity - requires system level permission to inject.
  this by default would only be in super user role.

btw - all are the exact same single permission.






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.



_______________________________________________
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel

Reply via email to