On Mon, Mar 11, 2013 at 5:14 PM, Michele Tartara <[email protected]> wrote: > On Mon, Mar 11, 2013 at 5:04 PM, Guido Trotter <[email protected]> wrote: >> >> On Mon, Mar 11, 2013 at 4:53 PM, Michele Tartara <[email protected]> >> wrote: >> > 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 >> >> Nack on this part of the change. >> (we only do this later, reviewing which docs have been implemented and >> moving them under the 2.8 document). >> > > Ok. I didn't know it. I'll switch back to 2.7. > >> >> >> > >> >> > .. 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. >> > >> >> Let's try having more consistent names, also. And also avoid capital >> letters and camelcasing. :) > > > They are consistent, if you consider that they are the names of the actual > functions that would create the trail. But I agree that looking at all of > them from here is quite ugly and inconsistent. > >> >> For the actor I would say >> >> gnt:opcode:shutdown >> gnt:client:cli or gnt:client:rapi (or gnt:client:hbal, eg) >> >> Some of these won't have a "why" (opcodes or rpcs don't need one, the >> why is usually at a higher level). > > > Then, for example for "gnt:opcode:shutdown" isn't it better to have just > "gnt:opcode" as the actor and "shutdown" as the reason? This way the > information is already much more structured, and less parsing (as simple as > it would be) is required to use it afterwards. >
Not sure. shutdown is just "which opcode are we talking about" rather than the "reason". :) It seems we are just using the "last" element as reason. Which makes me question whether we want the reason at all. user:mtartara can shut down a cluster for chg:194564 system:virgil can shut down an instance for "user-request" but if it's gnt:opcode I don't see the "reason" as being shutdown. At most the reason for an opcode can be a job number opcode-index. (which the reason that opcode was executed) Thanks, Guido
