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      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to part of [comment:30 vorner] before my lunch break...

 > Anyway, I noticed two things:
 >  - The start_all_pracesses method starts only b10-auth, not all the
 xfrin, xfrout, etc. That doesn't seem to matter much, as the configuration
 handler is called soon enough anyway. Is there a reason for this? Anyway,
 this is temporary, so it doesn't matter.

 I thought they started as a result of this:
 {{{
         if self.cfg_start_auth:
             component_config['b10-xfrout'] = { 'kind': 'dispensable',
                                                'address': 'Xfrout' }
             component_config['b10-xfrin'] = { 'kind': 'dispensable',
                                               'address': 'Xfrin' }
             component_config['b10-zonemgr'] = { 'kind': 'dispensable',
                                               'address': 'Zonemgr' }
             self.__propagate_component_config(component_config)
 }}}
 didn't they?

 BTW, in this sense the resolver part was incorrect.  I intended to call
 `self.__propagate_component_config` here, too:
 {{{
         if self.cfg_start_resolver:
             component_config['b10-resolver'] = { 'kind': 'needed',
                                                  'special': 'resolver' }
             self.started_resolver_family = True
 }}}

 And, the reason why the invocation of xfrin, etc, was separated from
 that of auth was partly because I tried to keep the original code
 logic as much as possible, and partly because that would allow xfrin,
 etc to start as a non privileged user while allowing auth (and
 resolver) to have the privilege.

 >  - The start_xfrin in master now has some kind of hack with paths.
 Should we preserve that one?

 Yes, we should, unless #1292 is resolved soon.

 > Replying to [comment:25 jinmei]:
 > > > If someone uses boss to start dhcp, he would just remove auth and
 resolver from configuration and have the dhcp part as needed.
 > >
 > > Perhaps the point to consider is what should be specified as 'needed'
 > > by default in bob.spec. [...]
 >
 > When we introduced the start_auth and start_resolver configuration
 options, we discussed this AFAIK and we decided to have start_auth as true
 as the default. The current (nonincremental) version starts the same set
 of processes as the default. I did want the default to be empty and let
 user start what he wants at the time and we might bring it up sometime for
 wider discussion. But I don't think it should be just changed in the
 ticket, there'll be enough new things for users in that anyway.

 Okay.  (And I agree we should generally focus on realizing the current
 behavior with the new framework at least in this ticket).

 > Replying to [comment:26 jinmei]:

 > > Ack.  One small point in the revised doc:
 > > {{{
 > >         You should also register any processes started within boss.
 > > }}}
 > > If this is something that all derived class (whether overridden or
 > > not) should meet, maybe we should move it to start().
 >
 > Hmm, I'm not sure about that. Actually, I envisioned the component could
 have more than one process running (in which case it would register
 multiple times) or no process at all. So, doing this would decrease the
 flexibility. Should I document this idea somewhere?

 Yes please.  I'd leave it to you whether to update the code itself.

 > > documented.  As for pid(), I guess the (more) proper condition is
 > > `if self._procinfo is not None` here:
 > > {{{
 > >         return self._procinfo.pid if self._procinfo else None
 > > }}}
 >
 > Ah, right. Too much perl education in my case O:-). The original was in
 fact correct, as the _procinfo is an object, so it can be False only if it
 is None, but this is more the python way.

 Right (the original was correct), hence "(more)" proper.  In this
 particular case I don't much care about the difference, but since
 there are actually subtle cases where `not X` is not always
 `X is not None` and the difference can cause a real bug, it would be a
 good practice if we always try to be stricter.

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