BIND 10 #213: Change hard-coded process startups to configuration-driven

[BIND 10 Development]

#213: Change hard-coded process startups to configuration-driven
-------------------------------------+-------------------------------------
                   Reporter:  shane  |                 Owner:  jinmei
                       Type:         |                Status:  reviewing
  enhancement                        |             Milestone:
                   Priority:  major  |  Sprint-20111108
                  Component:  Boss   |            Resolution:
  of BIND                            |             Sensitive:  0
                   Keywords:         |           Sub-Project:  Core
            Defect Severity:  N/A    |  Estimated Difficulty:  9
Feature Depending on Ticket:         |           Total Hours:
        Add Hours to Ticket:         |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 Replying to [comment:34 jinmei]:
 > All these three points now seem to be related this point: it was not
 > clear what was the intended guideline to customize the start/stop
 details.
 > Also, as indicated by the missing documentation update to kill(),
 > it's not clear which set of methods are supposed to be overridden at
 > once.  Even though documentation helps, it seems to be more error prone.
 >
 > I'd suggest something like this:
 > - Introduce a separate class, say ComponentInternal.  It has "start",
 >   "stop", "name", and "pid" methods.  This class is an abstract base
 >   class; it's not expected to be used directly, and all of these
 >   methods are expected to be overridden in a derived class.
 > - Define a derived DefaultComponentInternal.  Move the current
 >   _start_internal (renamed to start), _stop_internal (renamed to
 >   stop), name and pid, _procinfo to this class.  This class takes a
 >   callable object (start_func) on initialization to allow small
 >   customization for startup only.
 > - Component is constructed with a concrete ComponentInternal, whose
 >   default is DefaultComponentInternal.  Component.start() calls
 >   component_internal.start(), etc, instead of Component._start().
 >   _procinfo.
 > - Say if the user wants complete customization, define a separate
 >   ComponentInternal derived class and use it with Component.
 >   If the user wants a small customization for startup, only pass a
 >   different callable to the Component (or set
 >   DefaultComponentInternal._start_func to the user's callable in some
 >   way)
 >
 > This way it's clearer what is expected to be overridden as a set.
 > It's also safer in terms of avoiding to accidentally refer to things
 > like _procinfo with partially overridden methods.

 This sounded like too many classes to me, it's hard to follow it. So I
 made a compromise to have a base class which has the methods which should
 not be overriden and took the procinfo and stuff to a separate derived
 class. Would this help?

 > I meant this comment is UNIX (or POSIX) specific while Python's
 > subprocess class is more generic:
 > {{{
 >         If the forcefull is true, it uses SIGKILL instead of SIGTERM.
 > }}}

 I updated it to say „for example“ to make it clear this is not the only
 possible way.

 > > This one is not in there right now, but I might have a proposal for a
 workaround. We may define a special drop-root-privileges component to
 place the dropping in the middle of the startup by the priorities. Does it
 sound reasonable?
 >
 > Probably, as a workaround (by definition as long as it works and is
 > relatively short term, any solution should be reasonable).  Note also
 > that the experimental 213-incremental branch has another workaround
 > (not specifically intending to address this, but as a result of the
 > attempt of keeping the original behavior as much as possible).

 But this workaround won't work if we move to the configurable way
 completely. Because if the list of processes is defined completely by
 configuration, the boss doesn't have the knowledge to know where to put
 the „drop privileges“ call.

 > > Actually, there was a code to start it depending the config, but the
 config wasn't in the spec file yet.
 >
 > Ah, cfg_start_dhcp6 (in master) is always False (unless the test code
 > explicitly modifies it).  Confusing:-)  While it's not a problem of
 > this task per se, it would be nicer to add some comment around here:
 > {{{
 >         if self.cfg_start_dhcp6:
 >             self.start_dhcp6(c_channel_env)
 > }}}

 If this will disappear at this branch, I don't think the comment is needed
 any more.

 Giving it to you, as I'll create a new branch for the restarts branch so
 we don't mix them too much.

-- 
Ticket URL: <http://bind10.isc.org/ticket/213#comment:40>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development