On Mon, Mar 11, 2013 at 4:22 PM, Iustin Pop <[email protected]> wrote:

> On Mon, Mar 11, 2013 at 04:11:48PM +0100, Michele Tartara wrote:
> > This commit adds the design document for introducing "reason trails",
> > tracing the reason why opcodes are executed, step by step.
> >
> > Signed-off-by: Michele Tartara <[email protected]>
> > ---
> >  Makefile.am                 |    1 +
> >  doc/design-draft.rst        |    3 +-
> >  doc/design-reason-trail.rst |   86
> +++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 89 insertions(+), 1 deletion(-)
> >  create mode 100644 doc/design-reason-trail.rst
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 4020227..644e9f6 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -398,6 +398,7 @@ docinput = \
> >       doc/design-partitioned.rst \
> >       doc/design-query-splitting.rst \
> >       doc/design-query2.rst \
> > +     doc/design-reason-trail.rst \
> >       doc/design-resource-model.rst \
> >       doc/design-restricted-commands.rst \
> >       doc/design-shared-storage.rst \
> > diff --git a/doc/design-draft.rst b/doc/design-draft.rst
> > index 9dd2dfc..ec5303b 100644
> > --- a/doc/design-draft.rst
> > +++ b/doc/design-draft.rst
> > @@ -2,7 +2,7 @@
> >  Design document drafts
> >  ======================
> >
> > -.. Last updated for Ganeti 2.7
> > +.. Last updated for Ganeti 2.8
> >
> >  .. toctree::
> >     :maxdepth: 2
> > @@ -17,6 +17,7 @@ Design document drafts
> >     design-monitoring-agent.rst
> >     design-hroller.rst
> >     design-storagespace.rst
> > +   design-reason-trail.rst
> >
> >  .. vim: set textwidth=72 :
> >  .. Local Variables:
> > diff --git a/doc/design-reason-trail.rst b/doc/design-reason-trail.rst
> > new file mode 100644
> > index 0000000..8e86e88
> > --- /dev/null
> > +++ b/doc/design-reason-trail.rst
> > @@ -0,0 +1,86 @@
> > +===================
> > +Ganeti reason trail
> > +===================
> > +
> > +.. contents:: :depth: 2
> > +
> > +This is a design document detailing the implementation of a way for
> Ganeti to
> > +track the origin and the reason of every executed command, from its
> starting
> > +point (command line, remote API, some htool, etc.) to its actual
> execution
> > +time.
> > +
> > +Current state and shortcomings
> > +==============================
> > +
> > +There is currently no way to track why a job and all the operations
> part of it
> > +were executed, and who or what triggered the execution.
> > +This is an inconvenience in general, and also it makes impossible to
> have
> > +certain information, such as finding the reason why an instance last
> changed its
> > +status (i.e.: why it was started/stopped/rebooted/etc.), or
> distinguishing
> > +an admin request from a scheduled maintenance or an automated tool's
> work.
> > +
> > +Proposed changes
> > +================
> > +
> > +We propose to introduce a new piece of information, that will be called
> "reason
> > +trail", to track the path from the issuing of a command to its
> execution.
> > +
> > +The reason trail will be a list of pairs ``(source, reason)``, with:
> > +
> > +``source``
> > +  The entity deciding to perform (or forward) a command.
> > +  It is represented by an arbitrary string, but strings prepended by
> "gnt:"
> > +  are reserved for Ganeti components, and they will be refused by the
> > +  interfaces towards the external world.
> > +
> > +``reason``
> > +  The reason why the entity decided to perform the operation.
> > +  It is represented by an arbitrary string.
>
> You forgot the timestamp…
>

Actually, it was intentionally omitted, because it seems to me that we were
not 100% sure that a timestamp at each step was needed.
But I'll add it, no problem. :-)


> > +The reason trail will be attached at the OpCode level and will be kept
> as a list
> > +according to the internal representation of lists in the programming
> langauge
>
> typo. Please enable a spell checker :)
>

Ok.


>
> > +implementing that part of the system. When such a list has to be
> serialized
> > +externally (such as on the RAPI interface), it will be serialized in
> JSON
> > +format.
>
> I think you're going into too much detail (about programming languages)
> and too little - JSON format, but what data type? I'd leave out the note
> about the programming language, and just say what data type at JSON
> level, since that's the interoperation point.
>

Actually, I thought about the naive transformation of a list of 2-tuples to
JSON, that is the list of lists with two elements (that are strings, as
already specified in the descriptions of "source" and "reason"). That's why
I didn't specify it.
But, of course, I can add it explicitly.


>
> > +Any component the operation goes through is allowed (but not required)
> to append
> > +it's own reason to the list.
> > +Other than this, the list shouldn't be modified.
>
> Weird formatting again (missing blank line _or_ extra hard break).
>

Will fix it. Too much Latex in the last few months, I guess...


> > +As an example here is the reason trail for a shutdown operation invoked
> from
> > +the command line through the gnt-instance tool::
> > +
> > +  [("user", "Cleanup of unused instances"), ("gnt:gnt-instance",
> "stop"),
> > +   ("gnt:cmdlib", "LUInstanceShutdown"),
> > +   ("gnt:noded", "perspective_instance_reboot"),
> > +   ("gnt:backend", "InstanceShutdown"), ("gnt:hv_xen", "StopInstance")]
>
> This trail is inconsistent. Are you using module names? Entity names?
>

For the internal components, I used file names (without the extension).
"User", on the other hand, is just an arbitrary string. It could be turned
into something more formalized, by, in general, we cannot expect anything
specific above this level: everything would just be user-provided.


>
> Furthermore, this shows trails from the backend, which as we discussed
> in our offline meeting can't be added to the opcode.
>

It can't be added to the opcode, but the opcode might pass the trail as it
is to the backend, as a parameter to the invoked function, and the backend
might go on building it.

It all depends whether this could be of any use or not in the future. For
sure, stopping at noded would be much less work, and much less modification
to the current codebase.

For the trail to be used by the instance status data collector (which is at
the moment the only reason why we need all this), stopping at noded would
be fine.


> > +where the first pair is optional and it is determined by a
> user-specified
> > +message, passed to gnt-instance through a command line parameter.
> > +
> > +The same operation, launched by an external GUI tool, and executed
> through the
> > +remote API, would have a reason trail like::
> > +
> > +  [("user", "Cleanup of unused instances"), ("RemoteToolName",
> "GUI_stop"),
> > +   ("gnt:rapi_client", "ShutdownInstance"),
> > +   ("gnt:rlib2", "instances_name_shutdown"),
> > +   ("gnt:cmdlib", "LUInstanceShutdown"),
> > +   ("gnt:noded", "perspective_instance_reboot"),
> > +   ("gnt:backend", "InstanceShutdown"), ("gnt:hv_xen", "StopInstance")]
> > +
> > +Implementation
> > +==============
> > +
> > +The implementation will start from the operations that affect the
> instance
> > +status. They will be changed so that the "reason" is passed to them.
> > +They will then export the new expected instance status, together
> > +with the associated reason for the monitoring daemon.
> > +
> > +This implementation will have to be done in such a way to be extendable
> to
> > +other opcodes if/when needed.
>
> You left out the actual content here. How/when will this be added to
> opcodes?


What do you mean exactly by "when"?
At what point of the execution?


> How is hv_xen above passing back data and modifies the
> (readonly) opcode?
>

As written above, I think it should not modify the opcode itself after
entering the backend, but just passing on the information of the trail (if
we will decide this is actually needed) for the backend to use and expand.

Thanks,
Michele

Reply via email to