[2956] Created the initial, implementation of DHCP-DDNS service controller

 class, D2Controller, the base class DControllerBase, and unit tests.

src/bin/d2/d2_controller.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -19,12 +19,12 @@
 namespace isc {
 namespace d2 {
 
-DControllerBasePtr& 
+DControllerBasePtr&
 D2Controller::instance() {
-    // If the instance hasn't been created yet, create it.  Note this method 
+    // If the instance hasn't been created yet, create it.  Note this method
     // must use the base class singleton instance methods.  The base class
     // must own the singleton in order to use it within BIND10 static function
-    // callbacks. 
+    // callbacks.
     if (!getController()) {
         setController(new D2Controller());
     }

src/bin/d2/d2_controller.h

@@ -22,7 +22,7 @@ namespace d2 {
 
 /// @brief Process Controller for D2 Process
 /// This class is the DHCP-DDNS specific derivation of DControllerBase. It 
-/// creates and manages an instance of the DCHP-DDNS application process, 
+/// creates and manages an instance of the DHCP-DDNS application process, 
 /// D2Process.  
 /// @TODO Currently, this class provides only the minimum required specialized
 /// behavior to run the DHCP-DDNS service. It may very well expand as the 
@@ -36,7 +36,7 @@ public:
     /// base class singleton instance member.  It instantiates the singleton 
     /// and sets the base class instance member upon first invocation. 
     ///
-    /// @return returns the a pointer reference to the singleton instance.
+    /// @return returns the pointer reference to the singleton instance.
     static DControllerBasePtr& instance();
 
     /// @brief Destructor.
@@ -47,7 +47,7 @@ private:
     /// process.  This method is invoked during the process initialization
     /// step of the controller launch.
     ///  
-    /// @return returns a DProcessBase* to the applicatio process created.
+    /// @return returns a DProcessBase* to the application process created.
     /// Note the caller is responsible for destructing the process. This
     /// is handled by the base class, which wraps this pointer with a smart
     /// pointer.

src/bin/d2/d2_log.cc

@@ -19,7 +19,11 @@
 namespace isc {
 namespace d2 {
 
+/// @brief Defines the service name which is used in the controller constructor
+/// and ultimately defines the BIND10 module name.
 const char* const D2_MODULE_NAME = "b10-d2";
+
+/// @brief Defines the logger used within D2.
 isc::log::Logger d2_logger(D2_MODULE_NAME);
 
 } // namespace d2

src/bin/d2/d2_log.h

@@ -22,7 +22,8 @@
 namespace isc {
 namespace d2 {
 
-/// @TODO need brief
+/// @brief Defines the executable name, ultimately this is the BIND10 module 
+/// name.
 extern const char* const D2_MODULE_NAME;
 
 /// Define the logger for the "d2" module part of b10-d2.  We could define

src/bin/d2/d2_messages.mes

@@ -26,6 +26,10 @@ following a shut down (normal or otherwise) of the DDHCP-DDNS process.
 This is a debug message issued when the service process has been instructed
 to shut down by the controller.
 
+% D2PRC_PROCESS_INIT DHCP-DDNS application init invoked
+This is a debug message issued when the D2 process enters it's
+init method. 
+
 % D2PRC_RUN_ENTER process has entered the event loop 
 This is a debug message issued when the D2 process enters it's
 run method. 

src/bin/d2/d2_process.cc

@@ -21,7 +21,7 @@ using namespace asio;
 namespace isc {
 namespace d2 {
 
-D2Process::D2Process(const char* name, IOServicePtr io_service) 
+D2Process::D2Process(const char* name, IOServicePtr io_service)
     : DProcessBase(name, io_service) {
 };
 
@@ -29,12 +29,12 @@ void
 D2Process::init() {
 };
 
-int
+void
 D2Process::run() {
     // Until shut down or an fatal error occurs, wait for and
     // execute a single callback. This is a preliminary implementation
     // that is likely to evolve as development progresses.
-    // To use run(), the "managing" layer must issue an io_service::stop 
+    // To use run(), the "managing" layer must issue an io_service::stop
     // or the call to run will continue to block, and shutdown will not
     // occur.
     LOG_DEBUG(d2_logger, DBGLVL_START_SHUT, D2PRC_RUN_ENTER);
@@ -44,39 +44,38 @@ D2Process::run() {
             io_service->run_one();
         } catch (const std::exception& ex) {
             LOG_FATAL(d2_logger, D2PRC_FAILED).arg(ex.what());
-            return (EXIT_FAILURE); 
+            isc_throw (DProcessBaseError,
+                std::string("Process run method failed:") + ex.what());
         }
     }
 
     LOG_DEBUG(d2_logger, DBGLVL_START_SHUT, D2PRC_RUN_EXIT);
-    return (EXIT_SUCCESS);
 };
 
-int 
+void
 D2Process::shutdown() {
     LOG_DEBUG(d2_logger, DBGLVL_START_SHUT, D2PRC_SHUTDOWN);
     setShutdownFlag(true);
-    return (0);
-}    
+}
 
-isc::data::ConstElementPtr 
+isc::data::ConstElementPtr
 D2Process::configure(isc::data::ConstElementPtr config_set) {
     // @TODO This is the initial implementation which simply accepts
-    // any content in config_set as valid.  This is sufficient to 
+    // any content in config_set as valid.  This is sufficient to
     // allow participation as a BIND10 module, while D2 configuration support
     // is being developed.
-    LOG_DEBUG(d2_logger, DBGLVL_TRACE_BASIC, 
+    LOG_DEBUG(d2_logger, DBGLVL_TRACE_BASIC,
               D2PRC_CONFIGURE).arg(config_set->str());
 
     return (isc::config::createAnswer(0, "Configuration accepted."));
 }
 
-isc::data::ConstElementPtr 
+isc::data::ConstElementPtr
 D2Process::command(const std::string& command, isc::data::ConstElementPtr args){
     // @TODO This is the initial implementation.  If and when D2 is extended
     // to support its own commands, this implementation must change. Otherwise
-    // it should reject all commands as it does now. 
-    LOG_DEBUG(d2_logger, DBGLVL_TRACE_BASIC, 
+    // it should reject all commands as it does now.
+    LOG_DEBUG(d2_logger, DBGLVL_TRACE_BASIC,
               D2PRC_COMMAND).arg(command).arg(args->str());
 
     return (isc::config::createAnswer(COMMAND_INVALID, "Unrecognized command:"
@@ -86,5 +85,5 @@ D2Process::command(const std::string& command, isc::data::ConstElementPtr args){
 D2Process::~D2Process() {
 };
 
-}; // namespace isc::d2 
+}; // namespace isc::d2
 }; // namespace isc

src/bin/d2/d2_process.h

@@ -20,79 +20,82 @@
 namespace isc {
 namespace d2 {
 
-/// @brief @TODO DHCP-DDNS Application Process 
+/// @brief DHCP-DDNS Application Process
 ///
-/// D2Process provides the top level application logic for DHCP-driven DDNS 
-/// update processing.  It provides the asynchronous event processing required 
-/// to receive DNS mapping change requests and carry them out.   
+/// D2Process provides the top level application logic for DHCP-driven DDNS
+/// update processing.  It provides the asynchronous event processing required
+/// to receive DNS mapping change requests and carry them out.
 /// It implements the DProcessBase interface, which structures it such that it
-/// is a managed "application", controlled by a management layer. 
+/// is a managed "application", controlled by a management layer.
 
 class D2Process : public DProcessBase {
 public:
     /// @brief Constructor
     ///
     /// @param name name is a text label for the process. Generally used
-    /// in log statements, but otherwise arbitrary. 
+    /// in log statements, but otherwise arbitrary.
     /// @param io_service is the io_service used by the caller for
     /// asynchronous event handling.
     ///
-    /// @throw DProcessBaseError is io_service is NULL. 
+    /// @throw DProcessBaseError is io_service is NULL.
     D2Process(const char* name, IOServicePtr io_service);
 
-    /// @brief Will be used after instantiation to perform initialization 
-    /// unique to D2. This will likely include interactions with QueueMgr and 
-    /// UpdateMgr, to prepare for request receipt and processing.
+    /// @brief Will be used after instantiation to perform initialization
+    /// unique to D2. @TODO This will likely include interactions with
+    /// QueueMgr and UpdateMgr, to prepare for request receipt and processing.
+    /// Current implementation successfully does nothing.
+    /// @throw throws a DProcessBaseError if the initialization fails.
     virtual void init();
 
-    /// @brief Implements the process's event loop. 
-    /// The initial implementation is quite basic, surrounding calls to 
+    /// @brief Implements the process's event loop.
+    /// The initial implementation is quite basic, surrounding calls to
     /// io_service->runOne() with a test of the shutdown flag.
-    /// Once invoked, the method will continue until the process itself is 
-    /// exiting due to a request to shutdown or some anomaly forces an exit.   
-    /// @return  returns 0 upon a successful, "normal" termination, non
-    /// zero to indicate an abnormal termination.    
-    virtual int run();
+    /// Once invoked, the method will continue until the process itself is
+    /// exiting due to a request to shutdown or some anomaly forces an exit.
+    /// @throw throws a DProcessBaseError if an error is encountered.
+    virtual void run();
 
-    // @TODO need brief
-    virtual int shutdown();
+    /// @brief Implements the process's shutdown processing. When invoked, it
+    /// should ensure that the process gracefully exits the run method.
+    /// Current implementation simply sets the shutdown flag monitored by the
+    /// run method. @TODO this may need to expand as the implementation evolves.
+    /// @throw throws a DProcessBaseError if an error is encountered.
+    virtual void shutdown();
 
-    // @TODO need brief
-    /// @brief Processes the given configuration. 
-    /// 
+    /// @brief Processes the given configuration.
+    ///
     /// This method may be called multiple times during the process lifetime.
     /// Certainly once during process startup, and possibly later if the user
     /// alters configuration. This method must not throw, it should catch any
     /// processing errors and return a success or failure answer as described
-    /// below. 
+    /// below.
     ///
     /// @param config_set a new configuration (JSON) for the process
     /// @return an Element that contains the results of configuration composed
     /// of an integer status value (0 means successful, non-zero means failure),
-    /// and a string explanation of the outcome. 
+    /// and a string explanation of the outcome.
     virtual isc::data::ConstElementPtr configure(isc::data::ConstElementPtr
                                                  config_set);
 
-    // @TODO need brief
-    /// @brief Processes the given command. 
-    /// 
-    /// This method is called to execute any custom commands supported by the 
-    /// process. This method must not throw, it should catch any processing 
+    /// @brief Processes the given command.
+    ///
+    /// This method is called to execute any custom commands supported by the
+    /// process. This method must not throw, it should catch any processing
     /// errors and return a success or failure answer as described below.
     ///
     /// @param command is a string label representing the command to execute.
     /// @param args is a set of arguments (if any) required for the given
-    /// command. 
+    /// command.
     /// @return an Element that contains the results of command composed
     /// of an integer status value (0 means successful, non-zero means failure),
-    /// and a string explanation of the outcome.  
-    virtual isc::data::ConstElementPtr command(const std::string& command, 
+    /// and a string explanation of the outcome.
+    virtual isc::data::ConstElementPtr command(const std::string& command,
                                                isc::data::ConstElementPtr args);
-    // @TODO need brief
+    /// @brief Destructor
     virtual ~D2Process();
 };
 
-}; // namespace isc::d2 
+}; // namespace isc::d2
 }; // namespace isc
 
 #endif

src/bin/d2/d_controller.cc

@@ -26,12 +26,12 @@ namespace d2 {
 DControllerBasePtr DControllerBase::controller_;
 
 // Note that the constructor instantiates the controller's primary IOService.
-DControllerBase::DControllerBase(const char* name) 
-    : name_(name), stand_alone_(false), verbose_(false), 
+DControllerBase::DControllerBase(const char* name)
+    : name_(name), stand_alone_(false), verbose_(false),
     spec_file_name_(""), io_service_(new isc::asiolink::IOService()){
 }
 
-void 
+void
 DControllerBase::setController(DControllerBase* controller) {
     if (controller_) {
         // This shouldn't happen, but let's make sure it can't be done.
@@ -45,61 +45,66 @@ DControllerBase::setController(DControllerBase* controller) {
 
 int
 DControllerBase::launch(int argc, char* argv[]) {
-    int ret = EXIT_SUCCESS;
+    int ret = d2::NORMAL_EXIT;
 
     // Step 1 is to parse the command line arguments.
     try {
         parseArgs(argc, argv);
     } catch (const InvalidUsage& ex) {
         usage(ex.what());
-        return (EXIT_FAILURE);
+        return (d2::INVALID_USAGE);
     }
 
 #if 1
     //@TODO During initial development default to max log, no buffer
-    isc::log::initLogger(name_, isc::log::DEBUG, 
+    isc::log::initLogger(name_, isc::log::DEBUG,
                          isc::log::MAX_DEBUG_LEVEL, NULL, false);
 #else
     // Now that we know what the mode flags are, we can init logging.
     // If standalone is enabled, do not buffer initial log messages
     isc::log::initLogger(name_,
-                         ((verbose_ && stand_alone_) 
+                         ((verbose_ && stand_alone_)
                           ? isc::log::DEBUG : isc::log::INFO),
                          isc::log::MAX_DEBUG_LEVEL, NULL, !stand_alone_);
 #endif
 
     LOG_DEBUG(d2_logger, DBGLVL_START_SHUT, D2CTL_STARTING).arg(getpid());
     try {
-        // Step 2 is to create and init the application process.
+        // Step 2 is to create and initialize the application process.
         initProcess();
+    } catch (const std::exception& ex) {
+        LOG_ERROR(d2_logger, D2CTL_SESSION_FAIL).arg(ex.what());
+        return (PROCESS_INIT_ERROR);
+    }
 
-        // Next we connect if we are running integrated.
-        if (!stand_alone_) {
-            try {
-                establishSession();
-            } catch (const std::exception& ex) {
-                LOG_ERROR(d2_logger, D2CTL_SESSION_FAIL).arg(ex.what());
-                return (EXIT_FAILURE);
-            }
-        } else {
-            LOG_DEBUG(d2_logger, DBGLVL_START_SHUT, D2CTL_STANDALONE);
+    // Next we connect if we are running integrated.
+    if (stand_alone_) {
+        LOG_DEBUG(d2_logger, DBGLVL_START_SHUT, D2CTL_STANDALONE);
+    } else {
+        try {
+            establishSession();
+        } catch (const std::exception& ex) {
+            LOG_ERROR(d2_logger, D2CTL_SESSION_FAIL).arg(ex.what());
+            return (d2::SESSION_START_ERROR);
         }
+    }
 
-        // Everything is clear for launch, so start the application's
-        // event loop.
+    // Everything is clear for launch, so start the application's
+    // event loop.
+    try {
         runProcess();
     } catch (const std::exception& ex) {
         LOG_FATAL(d2_logger, D2CTL_FAILED).arg(ex.what());
-        ret = EXIT_FAILURE;
+        ret = d2::RUN_ERROR;
     }
 
     // If running integrated, always try to disconnect.
     if (!stand_alone_) {
-        try { 
+        try {
             disconnectSession();
         } catch (const std::exception& ex) {
             LOG_ERROR(d2_logger, D2CTL_DISCONNECT_FAIL).arg(ex.what());
-            ret = EXIT_FAILURE;
+            ret = d2::SESSION_END_ERROR;
         }
     }
 
@@ -111,11 +116,11 @@ DControllerBase::launch(int argc, char* argv[]) {
 void
 DControllerBase::parseArgs(int argc, char* argv[])
 {
-    // Iterate over the given command line options. If its a stock option 
+    // Iterate over the given command line options. If its a stock option
     // ("s" or "v") handle it here.  If its a valid custom option, then
     // invoke customOption.
     int ch;
-    opterr = 0; 
+    opterr = 0;
     optind = 1;
     std::string opts(":vs" + getCustomOpts());
     while ((ch = getopt(argc, argv, opts.c_str())) != -1) {
@@ -139,13 +144,13 @@ DControllerBase::parseArgs(int argc, char* argv[])
             isc_throw(InvalidUsage,tmp.str());
             break;
             }
-            
+
         default:
             // We hit a valid custom option
             if (!customOption(ch, optarg)) {
-                // this would be a programmatic err
+                // This would be a programmatic error.
                 std::stringstream tmp;
-                tmp << " Option listed but implemented?: [" << 
+                tmp << " Option listed but implemented?: [" <<
                         (char)ch << "] " << (!optarg ? "" : optarg);
                 isc_throw(InvalidUsage,tmp.str());
             }
@@ -156,12 +161,12 @@ DControllerBase::parseArgs(int argc, char* argv[])
     // There was too much information on the command line.
     if (argc > optind) {
         std::stringstream tmp;
-        tmp << "extraneous command line information"; 
+        tmp << "extraneous command line information";
         isc_throw(InvalidUsage,tmp.str());
     }
 }
 
-bool 
+bool
 DControllerBase::customOption(int /* option */, char* /*optarg*/)
 {
     // Default implementation returns false.
@@ -171,26 +176,26 @@ DControllerBase::customOption(int /* option */, char* /*optarg*/)
 void
 DControllerBase::initProcess() {
     LOG_DEBUG(d2_logger, DBGLVL_START_SHUT, D2CTL_INIT_PROCESS);
-  
-    // Invoke virtual method to instantiate the application process. 
+
+    // Invoke virtual method to instantiate the application process.
     try {
-        process_.reset(createProcess()); 
+        process_.reset(createProcess());
     } catch (const std::exception& ex) {
         isc_throw (DControllerBaseError, std::string("createProcess failed:")
-                    + ex.what()); 
+                    + ex.what());
     }
 
     // This is pretty unlikely, but will test for it just to be safe..
     if (!process_) {
         isc_throw (DControllerBaseError, "createProcess returned NULL");
-    }  
+    }
 
-    // Invoke application's init method
-    // @TODO This call may throw DProcessError 
+    // Invoke application's init method (Note this call should throw
+    // DProcessBaseError if it fails).
     process_->init();
 }
 
-void 
+void
 DControllerBase::establishSession() {
     LOG_DEBUG(d2_logger, DBGLVL_START_SHUT, D2CTL_CCSESSION_STARTING)
               .arg(spec_file_name_);
@@ -200,13 +205,13 @@ DControllerBase::establishSession() {
                              io_service_->get_io_service()));
 
     // Create the BIND10 config session with the stub configuration handler.
-    // This handler is internally invoked by the constructor and on success 
+    // This handler is internally invoked by the constructor and on success
     // the constructor updates the current session with the configuration that
     // had been committed in the previous session. If we do not install
-    // the dummy handler, the previous configuration would be lost. 
+    // the dummy handler, the previous configuration would be lost.
     config_session_ = ModuleCCSessionPtr(new isc::config::ModuleCCSession(
                                          spec_file_name_, *cc_session_,
-                                         dummyConfigHandler, commandHandler, 
+                                         dummyConfigHandler, commandHandler,
                                          false));
     // Enable configuration even processing.
     config_session_->start();
@@ -224,13 +229,13 @@ DControllerBase::establishSession() {
     // Parse the answer returned from the configHandler.  Log the error but
     // keep running. This provides an opportunity for the user to correct
     // the configuration dynamically.
-    int ret = 0; 
-    isc::data::ConstElementPtr comment = isc::config::parseAnswer(ret, answer); 
+    int ret = 0;
+    isc::data::ConstElementPtr comment = isc::config::parseAnswer(ret, answer);
     if (ret) {
         LOG_ERROR(d2_logger, D2CTL_CONFIG_LOAD_FAIL).arg(comment->str());
     }
 
-    // Lastly, call onConnect. This allows deriving class to execute custom 
+    // Lastly, call onConnect. This allows deriving class to execute custom
     // logic predicated by session connect.
     onSessionConnect();
 }
@@ -243,14 +248,15 @@ DControllerBase::runProcess() {
         isc_throw(DControllerBaseError, "Process not initialized");
     }
 
-    // Invoke the applicatio process's run method. This may throw DProcessError
+    // Invoke the application process's run method. This may throw
+    // DProcessBaseError
     process_->run();
 }
 
 void DControllerBase::disconnectSession() {
     LOG_DEBUG(d2_logger, DBGLVL_START_SHUT, D2CTL_CCSESSION_ENDING);
 
-    // Call virtual onDisconnect. Allows deriving class to execute custom 
+    // Call virtual onDisconnect. Allows deriving class to execute custom
     // logic prior to session loss.
     onSessionDisconnect();
 
@@ -279,7 +285,7 @@ DControllerBase::configHandler(isc::data::ConstElementPtr new_config) {
             .arg(new_config->str());
 
     if (!controller_) {
-        // This should never happen as we install the handler after we 
+        // This should never happen as we install the handler after we
         // instantiate the server.
         isc::data::ConstElementPtr answer =
             isc::config::createAnswer(1, "Configuration rejected,"
@@ -293,14 +299,14 @@ DControllerBase::configHandler(isc::data::ConstElementPtr new_config) {
 
 // Static callback which invokes non-static handler on singleton
 isc::data::ConstElementPtr
-DControllerBase::commandHandler(const std::string& command, 
+DControllerBase::commandHandler(const std::string& command,
                             isc::data::ConstElementPtr args) {
 
     LOG_DEBUG(d2_logger, DBGLVL_COMMAND, D2CTL_COMMAND_RECEIVED)
               .arg(command).arg(args->str());
 
     if (!controller_ )  {
-        // This should never happen as we install the handler after we 
+        // This should never happen as we install the handler after we
         // instantiate the server.
         isc::data::ConstElementPtr answer =
             isc::config::createAnswer(1, "Command rejected,"
@@ -315,14 +321,14 @@ DControllerBase::commandHandler(const std::string& command,
 isc::data::ConstElementPtr
 DControllerBase::updateConfig(isc::data::ConstElementPtr new_config) {
     isc::data::ConstElementPtr full_config;
-    if (stand_alone_) { 
+    if (stand_alone_) {
         // @TODO Until there is a configuration manager to provide retrieval
         // we'll just assume the incoming config is the full configuration set.
         // It may also make more sense to isolate the controller from the
         // configuration manager entirely. We could do something like
         // process_->getFullConfig() here for stand-alone mode?
         full_config = new_config;
-    } else { 
+    } else {
         if (!config_session_) {
             // That should never happen as we install config_handler
             // after we instantiate the server.
@@ -343,13 +349,13 @@ DControllerBase::updateConfig(isc::data::ConstElementPtr new_config) {
     // be set if the definition of this option is set. If someone removes
     // an existing option definition then the partial configuration that
     // removes that definition is triggered while a relevant option value
-    // may remain configured. This eventually results in the 
+    // may remain configured. This eventually results in the
     // configuration being in the inconsistent state.
     // In order to work around this problem we need to merge the new
     // configuration with the existing (full) configuration.
 
     // Let's create a new object that will hold the merged configuration.
-    boost::shared_ptr<isc::data::MapElement> 
+    boost::shared_ptr<isc::data::MapElement>
                             merged_config(new isc::data::MapElement());
 
     // Merge an existing and new configuration.
@@ -362,9 +368,9 @@ DControllerBase::updateConfig(isc::data::ConstElementPtr new_config) {
 
 
 isc::data::ConstElementPtr
-DControllerBase::executeCommand(const std::string& command, 
+DControllerBase::executeCommand(const std::string& command,
                             isc::data::ConstElementPtr args) {
-    // Shutdown is univeral.  If its not that, then try it as
+    // Shutdown is universal.  If its not that, then try it as
     // an custom command supported by the derivation.  If that
     // doesn't pan out either, than send to it the application
     // as it may be supported there.
@@ -375,7 +381,7 @@ DControllerBase::executeCommand(const std::string& command,
         // It wasn't shutdown, so may be a custom controller command.
         int rcode = 0;
         answer = customControllerCommand(command, args);
-        isc::config::parseAnswer(rcode, answer); 
+        isc::config::parseAnswer(rcode, answer);
         if (rcode == COMMAND_INVALID)
         {
             // It wasn't controller command, so may be an application command.
@@ -387,7 +393,7 @@ DControllerBase::executeCommand(const std::string& command,
 }
 
 isc::data::ConstElementPtr
-DControllerBase::customControllerCommand(const std::string& command, 
+DControllerBase::customControllerCommand(const std::string& command,
                                      isc::data::ConstElementPtr /* args */) {
 
     // Default implementation always returns invalid command.
@@ -395,14 +401,14 @@ DControllerBase::customControllerCommand(const std::string& command,
                                       "Unrecognized command:" + command));
 }
 
-isc::data::ConstElementPtr 
+isc::data::ConstElementPtr
 DControllerBase::shutdown() {
-    // @TODO TKM - not sure about io_service_->stop here
+    // @TODO (tmark) - not sure about io_service_->stop here
     // IF application is using this service for all of its IO, stopping
     // here would mean, no more work by the application.. UNLESS it resets
     // it. People have discussed letting the application finish any in-progress
-    // updates before shutting down.  If we don't stop it here, then 
-    // application can't use io_service_->run(), it will never "see" the 
+    // updates before shutting down.  If we don't stop it here, then
+    // application can't use io_service_->run(), it will never "see" the
     // shutdown.
     io_service_->stop();
     if (process_) {
@@ -425,13 +431,10 @@ DControllerBase::usage(const std::string & text)
 
     std::cerr << "Usage: " << name_ <<  std::endl;
     std::cerr << "  -v: verbose output" << std::endl;
-    std::cerr << "  -s: stand-alone mode (don't connect to BIND10)" 
+    std::cerr << "  -s: stand-alone mode (don't connect to BIND10)"
               << std::endl;
-   
-    std::cerr << getUsageText() << std::endl; 
-
-    exit(EXIT_FAILURE);
 
+    std::cerr << getUsageText() << std::endl;
 }
 
 DControllerBase::~DControllerBase() {

src/bin/d2/d_controller.h

@@ -31,6 +31,23 @@
 namespace isc {
 namespace d2 {
 
+/// @brief DControllerBase launch exit status values.  Upon service shutdown
+/// normal or otherwise, the Controller's launch method will return one of
+/// these values.
+
+/// @brief Indicates normal shutdown.
+static const int NORMAL_EXIT = 0;
+/// @brief Indicates invalid command line.
+static const int INVALID_USAGE = 1;
+/// @brief Failed to create and initialize application process.
+static const int PROCESS_INIT_ERROR = 2;
+/// @brief Could not connect to BIND10 (integrated mode only).
+static const int SESSION_START_ERROR = 3;
+/// @brief A fatal error occurred in the application process.
+static const int RUN_ERROR = 4;
+/// @brief Error occurred disconnecting from BIND10 (integrated mode only).
+static const int SESSION_END_ERROR = 5;
+
 /// @brief Exception thrown when the command line is invalid.
 class InvalidUsage : public isc::Exception {
 public:
@@ -46,7 +63,7 @@ public:
 };
 
 
-/// @brief Defines a shared pointer to DControllerBaseBase.
+/// @brief Defines a shared pointer to DControllerBase.
 class DControllerBase;
 typedef boost::shared_ptr<DControllerBase> DControllerBasePtr;
 
@@ -60,25 +77,25 @@ typedef boost::shared_ptr<isc::config::ModuleCCSession> ModuleCCSessionPtr;
 /// @brief Application Controller
 ///
 /// DControllerBase is an abstract singleton which provides the framework and
-/// services for managing an application process that implements the 
-/// DProcessBase interface.  It allows the process to run either in 
+/// services for managing an application process that implements the
+/// DProcessBase interface.  It allows the process to run either in
 /// integrated mode as a BIND10 module or stand-alone. It coordinates command
-/// line argument parsing, process instantiation and intialization, and runtime 
-/// control through external command and configuration event handling.  
-/// It creates the io_service_ instance which is used for runtime control 
-/// events and passes the io_service into the application process at process 
-/// creation.  In integrated mode it is responsible for establishing BIND10 
+/// line argument parsing, process instantiation and initialization, and runtime
+/// control through external command and configuration event handling.
+/// It creates the io_service_ instance which is used for runtime control
+/// events and passes the io_service into the application process at process
+/// creation.  In integrated mode it is responsible for establishing BIND10
 /// session(s) and passes this io_service_ into the session creation method(s).
 /// It also provides the callback handlers for command and configuration events.
 /// NOTE: Derivations must supply their own static singleton instance method(s)
 /// for creating and fetching the instance. The base class declares the instance
 /// member in order for it to be available for BIND10 callback functions. This
 /// would not be required if BIND10 supported instance method callbacks.
-class DControllerBase : public boost::noncopyable { 
+class DControllerBase : public boost::noncopyable {
 public:
-    /// @brief Constructor 
+    /// @brief Constructor
     ///
-    /// @param name name is a text label for the controller. Typically this 
+    /// @param name name is a text label for the controller. Typically this
     /// would be the BIND10 module name.
     DControllerBase(const char* name);
 
@@ -97,13 +114,20 @@ public:
     ///
     /// It is intended to be called from main() and be given the command line
     /// arguments. Note this method is deliberately not virtual to ensure the
-    /// proper sequence of events occur. 
-    /// 
-    /// @param argc  is the number of command line arguments supplied 
+    /// proper sequence of events occur.
+    ///
+    /// @param argc  is the number of command line arguments supplied
     /// @param argv  is the array of string (char *) command line arguments
     ///
-    /// @retrun returns EXIT_SUCCESS upon normal application shutdown and
-    /// EXIT_FAILURE under abnormal terminations.
+    /// @return returns one of the following integer values:
+    /// d2::NORMAL_EXIT - Indicates normal shutdown.
+    /// d2::INVALID_USAGE - Indicates invalid command line.
+    /// d2::PROCESS_INIT_ERROR  - Failed to create and initialize application
+    /// process
+    /// d2::SESSION_START_ERROR  - Could not connect to BIND10 (integrated mode
+    /// only).
+    /// d2::RUN_ERROR - An fatal error occurred in the application process
+    /// d2::SESSION_END_ERROR = 4;
     int launch(int argc, char* argv[]);
 
     /// @brief A dummy configuration handler that always returns success.
@@ -131,7 +155,7 @@ public:
     ///
     /// As a pointer to this method is used as a callback in ASIO for
     /// ModuleCCSession, it has to be static.  It acts as a wrapper around
-    /// the virtual instance method, updateConfig. 
+    /// the virtual instance method, updateConfig.
     ///
     /// @param new_config textual representation of the new configuration
     ///
@@ -141,9 +165,9 @@ public:
 
     /// @brief A callback for handling all incoming commands.
     ///
-    /// As a pointer to this method is used as a callback in ASIO for 
+    /// As a pointer to this method is used as a callback in ASIO for
     /// ModuleCCSession, it has to be static.  It acts as a wrapper around
-    /// the virtual instance method, executeCommand. 
+    /// the virtual instance method, executeCommand.
     ///
     /// @param command textual representation of the command
     /// @param args parameters of the command
@@ -152,43 +176,43 @@ public:
     static isc::data::ConstElementPtr
     commandHandler(const std::string& command, isc::data::ConstElementPtr args);
 
-    /// @brief Instance method invoked by the configuration event handler and 
-    /// which processes the actual configuration update.  Provides behaviorial 
-    /// path for both integrated and stand-alone modes. The current 
-    /// implementation will merge the configuration update into the existing 
+    /// @brief Instance method invoked by the configuration event handler and
+    /// which processes the actual configuration update.  Provides behavioral
+    /// path for both integrated and stand-alone modes. The current
+    /// implementation will merge the configuration update into the existing
     /// configuration and then invoke the application process' configure method.
     ///
     /// @TODO This implementation is will evolve as the D2 configuration
-    /// management task is implemented (trac #h2957). 
-    /// 
-    /// @param  new_config is the new configuration 
-    /// 
+    /// management task is implemented (trac #2957).
+    ///
+    /// @param  new_config is the new configuration
+    ///
     /// @return returns an Element that contains the results of configuration
-    /// update composed of an integer status value (0 means successful, 
-    /// non-zero means failure), and a string explanation of the outcome.  
-    virtual isc::data::ConstElementPtr 
+    /// update composed of an integer status value (0 means successful,
+    /// non-zero means failure), and a string explanation of the outcome.
+    virtual isc::data::ConstElementPtr
     updateConfig(isc::data::ConstElementPtr new_config);
 
 
     /// @brief Instance method invoked by the command event handler and  which
-    /// processes the actual command directive.  
-    /// 
+    /// processes the actual command directive.
+    ///
     /// It supports the execution of:
     ///
     ///   1. Stock controller commands - commands common to all DControllerBase
-    /// derivations.  Currently there is only one, the shutdown command. 
+    /// derivations.  Currently there is only one, the shutdown command.
     ///
     ///   2. Custom controller commands - commands that the deriving controller
     /// class implements.  These commands are executed by the deriving
-    /// controller.  
+    /// controller.
     ///
     ///   3. Custom application commands - commands supported by the application
     /// process implementation.  These commands are executed by the application
-    /// process. 
+    /// process.
     ///
     /// @param command is a string label representing the command to execute.
     /// @param args is a set of arguments (if any) required for the given
-    /// command. 
+    /// command.
     ///
     /// @return an Element that contains the results of command composed
     /// of an integer status value and a string explanation of the outcome.
@@ -198,41 +222,41 @@ public:
     ///   failure.
     ///   D2::COMMAND_INVALID - Command is not recognized as valid be either
     ///   the controller or the application process.
-    virtual isc::data::ConstElementPtr 
+    virtual isc::data::ConstElementPtr
     executeCommand(const std::string& command, isc::data::ConstElementPtr args);
 
 protected:
     /// @brief Virtual method that provides derivations the opportunity to
     /// support additional command line options.  It is invoked during command
     /// line argument parsing (see parseArgs method) if the option is not
-    /// recognized as a stock DControllerBase option. 
+    /// recognized as a stock DControllerBase option.
     ///
     /// @param option is the option "character" from the command line, without
-    /// any prefixing hypen(s)
+    /// any prefixing hyphen(s)
     /// @optarg optarg is the argument value (if one) associated with the option
     ///
-    /// @return must return true if the option was valid, false is it is 
-    /// invalid. (Note the default implementation always returns false.)  
+    /// @return must return true if the option was valid, false is it is
+    /// invalid. (Note the default implementation always returns false.)
     virtual bool customOption(int option, char *optarg);
 
-    /// @brief Abstract method that is responsible for instantiating the 
+    /// @brief Abstract method that is responsible for instantiating the
     /// application process instance. It is invoked by the controller after
-    /// command line argument parsing as part of the process initialization 
+    /// command line argument parsing as part of the process initialization
     /// (see initProcess method).
-    /// 
+    ///
     /// @return returns a pointer to the new process instance (DProcessBase*)
-    /// or NULL if the create fails.   
+    /// or NULL if the create fails.
     /// Note this value is subsequently wrapped in a smart pointer.
     virtual DProcessBase* createProcess() = 0;
 
     /// @brief Virtual method that provides derivations the opportunity to
     /// support custom external commands executed by the controller.  This
-    /// method is invoked by the processCommand if the received command is 
+    /// method is invoked by the processCommand if the received command is
     /// not a stock controller command.
     ///
     /// @param command is a string label representing the command to execute.
     /// @param args is a set of arguments (if any) required for the given
-    /// command. 
+    /// command.
     ///
     /// @return an Element that contains the results of command composed
     /// of an integer status value and a string explanation of the outcome.
@@ -240,8 +264,8 @@ protected:
     ///   D2::COMMAND_SUCCESS - Command executed successfully
     ///   D2::COMMAND_ERROR - Command is valid but suffered an operational
     ///   failure.
-    ///   D2::COMMAND_INVALID - Command is not recognized as a valid custom 
-    ///   controller command. 
+    ///   D2::COMMAND_INVALID - Command is not recognized as a valid custom
+    ///   controller command.
     virtual isc::data::ConstElementPtr customControllerCommand(
             const std::string& command, isc::data::ConstElementPtr args);
 
@@ -250,19 +274,19 @@ protected:
     /// derivation to execute any custom behavior associated with session
     /// establishment.
     ///
-    /// Note, it is not called  when running stand-alone. 
+    /// Note, it is not called  when running stand-alone.
     ///
-    /// @throw should hrow a DControllerBaseError if it fails.
+    /// @throw should throw a DControllerBaseError if it fails.
     virtual void onSessionConnect(){};
 
     /// @brief Virtual method which is invoked as the first action taken when
     /// the controller is terminating the session(s) with BIND10.  It provides
-    /// an opportunity for the derivation to execute any custom behavior 
+    /// an opportunity for the derivation to execute any custom behavior
     /// associated with session termination.
     ///
-    /// Note, it is not called  when running stand-alone. 
+    /// Note, it is not called  when running stand-alone.
     ///
-    /// @throw should hrow a DControllerBaseError if it fails.
+    /// @throw should throw a DControllerBaseError if it fails.
     virtual void onSessionDisconnect(){};
 
     /// @brief Virtual method which can be used to contribute derivation
@@ -275,7 +299,7 @@ protected:
     }
 
     /// @brief Virtual method which returns a string containing the option
-    /// letters for any custom command line options supported by the derivaiton.
+    /// letters for any custom command line options supported by the derivation.
     /// These are added to the stock options of "s" and "v" during command
     /// line interpretation.
     ///
@@ -305,14 +329,14 @@ protected:
         stand_alone_ = value;
     }
 
-    /// @brief Supplies whether or not verbose logging is enabled. 
+    /// @brief Supplies whether or not verbose logging is enabled.
     ///
     /// @return returns true if verbose logging is enabled.
     bool isVerbose() {
         return (verbose_);
     }
 
-    /// @brief Method for enabling or disabling verbose logging. 
+    /// @brief Method for enabling or disabling verbose logging.
     ///
     /// @param value is the new value to assign the flag.
     void setVerbose(bool value) {
@@ -329,7 +353,7 @@ protected:
     /// @brief Getter for fetching the name of the controller's BIND10 spec
     /// file.
     ///
-    /// @return returns a the file name string.
+    /// @return returns the file name string.
     const std::string& getSpecFileName() {
         return (spec_file_name_);
     }
@@ -357,26 +381,26 @@ protected:
     /// instance a second time.
     static void setController(DControllerBase* controller);
 
-private: 
+private:
     /// @brief Processes the command line arguments. It is the first step
     /// taken after the controller has been launched.  It combines the stock
     /// list of options with those returned by getCustomOpts(), and uses
     /// cstdlib's getopt to loop through the command line.  The stock options
     /// It handles stock options directly, and passes any custom options into
     /// the customOption method.  Currently there are only two stock options
-    /// -s for stand alone mode, and -v for verbose logging. 
+    /// -s for stand alone mode, and -v for verbose logging.
     ///
-    /// @param argc  is the number of command line arguments supplied 
+    /// @param argc  is the number of command line arguments supplied
     /// @param argv  is the array of string (char *) command line arguments
     ///
-    /// @throw throws InvalidUsage when there are usage errors. 
+    /// @throw throws InvalidUsage when there are usage errors.
     void parseArgs(int argc, char* argv[]);
 
     /// @brief Instantiates the application process and then initializes it.
-    /// This is the second step taken during launch, following successful 
+    /// This is the second step taken during launch, following successful
     /// command line parsing. It is used to invoke the derivation-specific
-    /// implementation of createProcess, following by an invoking of the 
-    /// newly instanatiated process's init method.
+    /// implementation of createProcess, following by an invoking of the
+    /// newly instantiated process's init method.
     ///
     /// @throw throws DControllerBaseError or indirectly DProcessBaseError
     /// if there is a failure creating or initializing the application process.
@@ -386,52 +410,52 @@ private:
     /// invoked during launch, if running in integrated mode, following
     /// successful process initialization.  It is responsible for establishing
     /// the BIND10 control and config sessions. During the session creation,
-    /// it passes in the controller's IOService and the callbacks for command 
+    /// it passes in the controller's IOService and the callbacks for command
     /// directives and config events.  Lastly, it will invoke the onConnect
     /// method providing the derivation an opportunity to execute any custom
-    /// logic associated with session establishment. 
+    /// logic associated with session establishment.
     ///
-    /// @throw the BIND10 framework may throw std::exceptions. 
+    /// @throw the BIND10 framework may throw std::exceptions.
     void establishSession();
 
     /// @brief Invokes the application process's event loop,(DBaseProcess::run).
-    /// It is called during launch only after successfully completing the 
-    /// requisted setup: comamnd line parsing, application initialization,
-    /// and session establishment (if not stand-alone). 
-    /// The process event loop is expected to only return upon application 
+    /// It is called during launch only after successfully completing the
+    /// requested setup: command line parsing, application initialization,
+    /// and session establishment (if not stand-alone).
+    /// The process event loop is expected to only return upon application
     /// shutdown either in response to the shutdown command or due to an
-    /// unrecovarable error.
+    /// unrecoverable error.
     ///
     // @throw throws DControllerBaseError or indirectly DProcessBaseError
     void runProcess();
 
-    /// @brief Terminates connectivity with BIND10. This method is invoked 
+    /// @brief Terminates connectivity with BIND10. This method is invoked
     /// in integrated mode after the application event loop has exited. It
-    /// first calls the onDisconnect method providing the derivation an 
+    /// first calls the onDisconnect method providing the derivation an
     /// opportunity to execute custom logic if needed, and then terminates the
-    /// BIND10 config and control sessions. 
+    /// BIND10 config and control sessions.
     ///
     /// @throw the BIND10 framework may throw std:exceptions.
     void disconnectSession();
 
     /// @brief Initiates shutdown procedure.  This method is invoked
     /// by executeCommand in response to the shutdown command. It will invoke
-    /// the application process's shutdown method, which causes the process to 
-    /// exit it's event loop. 
+    /// the application process's shutdown method, which causes the process to
+    /// exit it's event loop.
     ///
-    /// @return returns an Element that contains the results of shutdown 
-    /// attempt composed of an integer status value (0 means successful, 
-    /// non-zero means failure), and a string explanation of the outcome. 
+    /// @return returns an Element that contains the results of shutdown
+    /// attempt composed of an integer status value (0 means successful,
+    /// non-zero means failure), and a string explanation of the outcome.
     isc::data::ConstElementPtr shutdown();
 
     /// @brief Prints the program usage text to std error.
-    /// 
+    ///
     /// @param text is a string message which will preceded the usage text.
     /// This is intended to be used for specific usage violation messages.
     void usage(const std::string & text);
 
 private:
-    /// @brief Text label for the controller. Typically this would be the 
+    /// @brief Text label for the controller. Typically this would be the
     /// BIND10 module name.
     std::string name_;
 
@@ -459,10 +483,10 @@ private:
     /// @brief Session that receives configuration and commands.
     ModuleCCSessionPtr config_session_;
 
-    /// @brief Singleton instance value. 
+    /// @brief Singleton instance value.
     static DControllerBasePtr controller_;
 
-// DControllerTest is named a friend class to faciliate unit testing while
+// DControllerTest is named a friend class to facilitate unit testing while
 // leaving the intended member scopes intact.
 friend class DControllerTest;
 };

src/bin/d2/d_process.h

@@ -40,26 +40,26 @@ static const std::string SHUT_DOWN_COMMAND("shutdown");
 
 /// @brief Application Process Interface
 ///
-/// DProcessBase is an abstract class represents the primary "application" 
-/// level object in a "managed" asynchronous application. It provides a uniform 
-/// interface such that a managing layer can construct, intialize, and start
+/// DProcessBase is an abstract class represents the primary "application"
+/// level object in a "managed" asynchronous application. It provides a uniform
+/// interface such that a managing layer can construct, initialize, and start
 /// the application's event loop.  The event processing is centered around the
-/// use of isc::asiolink::io_service. The io_service is shared between the 
-/// the managing layer and the DProcessBase.  This allows management layer IO 
-/// such as directives to be sensed and handled, as well as processing IO 
-/// activity specific to the application.  In terms of management layer IO,
-/// there are methods shutdown, configuration updates, and commands unique
-/// to the application.  
+/// use of isc::asiolink::io_service. The io_service is shared between the
+/// managing layer and the DProcessBase.  This allows management layer IO such
+/// as directives to be sensed and handled, as well as processing IO activity
+/// specific to the application.  In terms of management layer IO, there are
+/// methods shutdown, configuration updates, and commands unique to the
+/// application.
 class DProcessBase {
 public:
     /// @brief Constructor
     ///
     /// @param name name is a text label for the process. Generally used
-    /// in log statements, but otherwise arbitrary. 
+    /// in log statements, but otherwise arbitrary.
     /// @param io_service is the io_service used by the caller for
     /// asynchronous event handling.
     ///
-    /// @throw DProcessBaseError is io_service is NULL. 
+    /// @throw DProcessBaseError is io_service is NULL.
     DProcessBase(const char* name, IOServicePtr io_service) : name_(name),
         io_service_(io_service), shut_down_flag_(false) {
 
@@ -69,78 +69,91 @@ public:
     };
 
     /// @brief May be used after instantiation to perform initialization unique
-    /// to application. It must be invoked prior to invoking run. This would 
-    /// likely include the creation of additional IO sources and their 
-    /// integration into the io_service. 
-    virtual void init() = 0; 
-
-    /// @brief Implements the process's event loop. In its simplest form it 
-    /// would an invocation io_service_->run().  This method should not exit 
-    /// until the process itself is exiting due to a request to shutdown or 
-    /// some anomaly is forcing an exit.   
-    /// @return  returns EXIT_SUCCESS upon a successful, normal termination, 
-    /// and EXIT_FAILURE to indicate an abnormal termination.    
-    virtual int run() = 0; 
-
-    /// @brief Implements the process's shutdown processing. When invoked, it 
-    /// should ensure that the process gracefully exits the run method. 
-    virtual int shutdown() = 0;
-
-    /// @brief Processes the given configuration. 
-    /// 
+    /// to application. It must be invoked prior to invoking run. This would
+    /// likely include the creation of additional IO sources and their
+    /// integration into the io_service.
+    /// @throw throws a DProcessBaseError if the initialization fails.
+    virtual void init() = 0;
+
+    /// @brief Implements the process's event loop. In its simplest form it
+    /// would an invocation io_service_->run().  This method should not exit
+    /// until the process itself is exiting due to a request to shutdown or
+    /// some anomaly is forcing an exit.
+    /// @throw throws a DProcessBaseError if an operational error is encountered.
+    virtual void run() = 0;
+
+    /// @brief Implements the process's shutdown processing. When invoked, it
+    /// should ensure that the process gracefully exits the run method.
+    /// @throw throws a DProcessBaseError if an operational error is encountered.
+    virtual void shutdown() = 0;
+
+    /// @brief Processes the given configuration.
+    ///
     /// This method may be called multiple times during the process lifetime.
     /// Certainly once during process startup, and possibly later if the user
     /// alters configuration. This method must not throw, it should catch any
     /// processing errors and return a success or failure answer as described
-    /// below. 
+    /// below.
     ///
     /// @param config_set a new configuration (JSON) for the process
     /// @return an Element that contains the results of configuration composed
     /// of an integer status value (0 means successful, non-zero means failure),
-    /// and a string explanation of the outcome. 
+    /// and a string explanation of the outcome.
     virtual isc::data::ConstElementPtr configure(isc::data::ConstElementPtr
-                                                 config_set) = 0; 
+                                                 config_set) = 0;
 
-    /// @brief Processes the given command. 
-    /// 
-    /// This method is called to execute any custom commands supported by the 
-    /// process. This method must not throw, it should catch any processing 
+    /// @brief Processes the given command.
+    ///
+    /// This method is called to execute any custom commands supported by the
+    /// process. This method must not throw, it should catch any processing
     /// errors and return a success or failure answer as described below.
     ///
     /// @param command is a string label representing the command to execute.
     /// @param args is a set of arguments (if any) required for the given
-    /// command. 
+    /// command.
     /// @return an Element that contains the results of command composed
     /// of an integer status value (0 means successful, non-zero means failure),
-    /// and a string explanation of the outcome.  
+    /// and a string explanation of the outcome.
     virtual isc::data::ConstElementPtr command(
-            const std::string& command, isc::data::ConstElementPtr args) = 0; 
+            const std::string& command, isc::data::ConstElementPtr args) = 0;
 
-    /// @brief Destructor 
+    /// @brief Destructor
     virtual ~DProcessBase(){};
 
-    bool shouldShutdown() { 
-        return (shut_down_flag_); 
+    /// @brief Checks if the process has been instructed to shut down.
+    ///
+    /// @return returns true if process shutdown flag is true.
+    bool shouldShutdown() {
+        return (shut_down_flag_);
     }
 
-    void setShutdownFlag(bool value) { 
-        shut_down_flag_ = value; 
+    /// @brief Sets the process shut down flag to the given value.
+    ///
+    /// @param value is the new value to assign the flag.
+    void setShutdownFlag(bool value) {
+        shut_down_flag_ = value;
     }
 
+    /// @brief Fetches the name of the controller.
+    ///
+    /// @return returns a reference the controller's name string.
     const std::string& getName() const {
         return (name_);
     }
 
+    /// @brief Fetches the controller's IOService.
+    ///
+    /// @return returns a reference to the controller's IOService.
     IOServicePtr& getIoService() {
         return (io_service_);
     }
 
 private:
-    /// @brief Text label for the process. Generally used in log statements, 
-    /// but otherwise can be arbitrary. 
+    /// @brief Text label for the process. Generally used in log statements,
+    /// but otherwise can be arbitrary.
     std::string name_;
 
-    /// @brief The IOService to be used for asynchronous event handling. 
+    /// @brief The IOService to be used for asynchronous event handling.
     IOServicePtr io_service_;
 
     /// @brief Boolean flag set when shutdown has been requested.
@@ -150,7 +163,7 @@ private:
 /// @brief Defines a shared pointer to DProcessBase.
 typedef boost::shared_ptr<DProcessBase> DProcessBasePtr;
 
-}; // namespace isc::d2 
+}; // namespace isc::d2
 }; // namespace isc
 
 #endif

src/bin/d2/main.cc

@@ -23,11 +23,19 @@
 using namespace isc::d2;
 using namespace std;
 
-/// This file contains entry point (main() function) for standard DHCP-DDNS 
-/// process, b10-dhcp-ddns, component for BIND10 framework.  It fetches 
-/// the D2Controller singleton instance and turns control over to it.
-/// The controller will return with upon shutdown with a avlue of either 
-/// EXIT_SUCCESS or EXIT_FAILURE.
+/// This file contains entry point (main() function) for standard DHCP-DDNS
+/// process, b10-dhcp-ddns, component for BIND10 framework.  It fetches
+/// the D2Controller singleton instance and invokes its launch method.
+/// The exit value of the program will the return value of launch:
+/// d2::NORMAL_EXIT - Indicates normal shutdown.
+/// d2::INVALID_USAGE - Indicates invalid command line.
+/// d2::PROCESS_INIT_ERROR  - Failed to create and initialize application
+/// process
+/// d2::SESSION_START_ERROR  - Could not connect to BIND10 (integrated mode
+/// only).
+/// d2::RUN_ERROR - A fatal error occurred in the application process
+/// d2::SESSION_END_ERROR - Error occurred disconnecting from BIND10 (integrated
+/// mode only).
 int
 main(int argc, char* argv[]) {
 
@@ -35,9 +43,6 @@ main(int argc, char* argv[]) {
     DControllerBasePtr& controller = D2Controller::instance();
 
     // Launch the controller passing in command line arguments.
-    int ret = controller->launch(argc, argv);
-
     // Exit program with the controller's return code.
-    return (ret);
+    return (controller->launch(argc, argv));
 }
-

src/bin/d2/spec_config.h

@@ -1,15 +0,0 @@
-// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
-//
-// Permission to use, copy, modify, and/or distribute this software for any
-// purpose with or without fee is hereby granted, provided that the above
-// copyright notice and this permission notice appear in all copies.
-//
-// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
-// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
-// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
-// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
-// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
-// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
-// PERFORMANCE OF THIS SOFTWARE.
-
-#define D2_SPECFILE_LOCATION "/labs/toms_lab/var/test_1/share/bind10/d2.spec"

src/bin/d2/tests/d2_controller_unittests.cc

@@ -17,6 +17,7 @@
 #include <d2/d2_controller.h>
 #include <d2/spec_config.h>
 
+#include <boost/pointer_cast.hpp>
 #include <boost/date_time/posix_time/posix_time.hpp>
 #include <gtest/gtest.h>
 
@@ -28,59 +29,91 @@ using namespace boost::posix_time;
 namespace isc {
 namespace d2 {
 
-//typedef DControllerTestWrapper<D2Controller> D2ControllerTest;
-
+/// @brief Test fixture class for testing D2Controller class. This class
+/// derives from DControllerTest and wraps a D2Controller.  Much of the
+/// underlying functionality is in the DControllerBase class which has an
+/// extensive set of unit tests that are independent of DHCP-DDNS.
+/// @TODO Currently These tests are relatively light and duplicate some of
+/// the testing done on the base class.  These tests are sufficient to ensure
+/// that D2Controller properly derives from its base class and to test the
+/// logic that is unique to D2Controller. These tests will be augmented and
+/// or new tests added as additional functionality evolves.
+/// Unlike the stub testing, there is no use of SimFailure to induce error
+/// conditions as this is production code.
 class D2ControllerTest : public DControllerTest {
 public:
     /// @brief Constructor
+    /// Note the constructor passes in the static D2Controller instance
+    /// method.
     D2ControllerTest() : DControllerTest(D2Controller::instance) {
     }
 
-    /// @brief Destructor 
+    /// @brief Destructor
     ~D2ControllerTest() {
     }
 };
 
-/// @brief basic instantiation  
-// @TODO This test is simplistic and will need to be augmented
+/// @brief Basic Controller instantiation testing.
+/// Verfies that the controller singleton gets created and that the
+/// basic derivation from the base class is intact.
 TEST_F(D2ControllerTest, basicInstanceTesting) {
+    // Verify the we can the singleton instance can be fetched and that
+    // it is the correct type.
     DControllerBasePtr& controller = DControllerTest::getController();
     ASSERT_TRUE(controller);
+    ASSERT_NO_THROW(boost::dynamic_pointer_cast<D2Controller>(controller));
+
+    // Verify that controller's name is correct.
     EXPECT_TRUE(checkName(D2_MODULE_NAME));
+
+    // Verify that controller's spec file name is correct.
     EXPECT_TRUE(checkSpecFileName(D2_SPECFILE_LOCATION));
+
+    // Verify that controller's IOService exists.
     EXPECT_TRUE(checkIOService());
 
-    // Process should NOT exist yet
+    // Verify that the Process does NOT exist.
     EXPECT_FALSE(checkProcess());
 }
 
-/// @TODO brief Verifies command line processing. 
+/// @brief Tests basic command line processing.
+/// Verfies that:
+/// 1. Standard command line options are supported.
+/// 2. Invalid options are detected.
 TEST_F(D2ControllerTest, commandLineArgs) {
     char* argv[] = { (char*)"progName", (char*)"-s", (char*)"-v" };
     int argc = 3;
 
+    // Verify that both flags are false initially.
     EXPECT_TRUE(checkStandAlone(false));
     EXPECT_TRUE(checkVerbose(false));
 
+    // Verify that standard options can be parsed without error.
     EXPECT_NO_THROW(parseArgs(argc, argv));
 
+    // Verify that flags are now true.
     EXPECT_TRUE(checkStandAlone(true));
     EXPECT_TRUE(checkVerbose(true));
 
-    char* argv2[] = { (char*)"progName", (char*)"-bs" };
+    // Verify that an unknown option is detected.
+    char* argv2[] = { (char*)"progName", (char*)"-x" };
     argc = 2;
     EXPECT_THROW (parseArgs(argc, argv2), InvalidUsage);
 }
 
-/// @TODO brief initProcess testing. 
+/// @brief Tests application process creation and initialization.
+/// Verifies that the process can be successfully created and initialized.
 TEST_F(D2ControllerTest, initProcessTesting) {
     ASSERT_NO_THROW(initProcess());
     EXPECT_TRUE(checkProcess());
 }
 
-/// @TODO brief test launch 
-TEST_F(D2ControllerTest, launchDirectShutdown) {
-    // command line to run standalone 
+/// @brief Tests launch and normal shutdown (stand alone mode).
+/// This creates an interval timer to generate a normal shutdown and then
+/// launches with a valid, stand-alone command line and no simulated errors.
+/// Launch exit code should be d2::NORMAL_EXIT.
+TEST_F(D2ControllerTest, launchNormalShutdown) {
+    // command line to run standalone
     char* argv[] = { (char*)"progName", (char*)"-s", (char*)"-v" };
     int argc = 3;
 
@@ -97,7 +130,7 @@ TEST_F(D2ControllerTest, launchDirectShutdown) {
     ptime stop = microsec_clock::universal_time();
 
     // Verify normal shutdown status.
-    EXPECT_EQ(EXIT_SUCCESS, rcode);
+    EXPECT_EQ(d2::NORMAL_EXIT, rcode);
 
     // Verify that duration of the run invocation is the same as the
     // timer duration.  This demonstrates that the shutdown was driven
@@ -107,53 +140,34 @@ TEST_F(D2ControllerTest, launchDirectShutdown) {
                 elapsed.total_milliseconds() <= 2100);
 }
 
-/// @TODO brief test launch 
-TEST_F(D2ControllerTest, launchRuntimeError) {
-    // command line to run standalone 
-    char* argv[] = { (char*)"progName", (char*)"-s", (char*)"-v" };
-    int argc = 3;
-
-    // Use an asiolink IntervalTimer and callback to generate the
-    // shutdown invocation. (Note IntervalTimer setup is in milliseconds).
-    isc::asiolink::IntervalTimer timer(*getIOService());
-    timer.setup(genFatalErrorCallback, 2 * 1000);
-
-    // Record start time, and invoke launch().
-    ptime start = microsec_clock::universal_time();
-    int rcode = launch(argc, argv);
-
-    // Record stop time.
-    ptime stop = microsec_clock::universal_time();
-
-    // Verify normal shutdown status.
-    EXPECT_EQ(EXIT_SUCCESS, rcode);
-
-    // Verify that duration of the run invocation is the same as the
-    // timer duration.  This demonstrates that the shutdown was driven
-    // by an io_service event and callback.
-    time_duration elapsed = stop - start;
-    EXPECT_TRUE(elapsed.total_milliseconds() >= 1900 &&
-                elapsed.total_milliseconds() <= 2100);
-}
-
-/// @TODO brief test configUpateTests 
+/// @brief Configuration update event testing.
 /// This really tests just the ability of the handlers to invoke the necessary
-/// chain, and error conditions. Configuration parsing and retrieval should be
-/// tested as part of the d2 configuration management implementation.   
+/// chain of methods and handle error conditions. Configuration parsing and
+/// retrieval should be tested as part of the d2 configuration management
+/// implementation.  Note that this testing calls the configuration update event
+/// callback, configHandler, directly.
+/// This test verifies that:
+/// 1. Configuration will be rejected in integrated mode when there is no
+/// session established. (This is a very contrived situation).
+/// 2. In stand-alone mode a configuration update results in successful
+/// status return.
+/// 3. That an application process error in configuration updating is handled
+/// properly.
 TEST_F(D2ControllerTest, configUpdateTests) {
     int rcode = -1;
     isc::data::ConstElementPtr answer;
 
+    // Initialize the application process.
     ASSERT_NO_THROW(initProcess());
     EXPECT_TRUE(checkProcess());
 
-    // Create a configuration set. Content is arbitrary, just needs to be 
+    // Create a configuration set. Content is arbitrary, just needs to be
     // valid JSON.
     std::string config = "{ \"test-value\": 1000 } ";
     isc::data::ElementPtr config_set = isc::data::Element::fromJSON(config);
 
     // We are not stand-alone, so configuration should be rejected as there is
-    // no session.  This is a pretty contrived situation that shouldn't be 
+    // no session.  This is a pretty contrived situation that shouldn't be
     // possible other than the handler being called directly (like this does).
     answer = DControllerBase::configHandler(config_set);
     isc::config::parseAnswer(rcode, answer);
@@ -166,25 +180,35 @@ TEST_F(D2ControllerTest, configUpdateTests) {
     EXPECT_EQ(0, rcode);
 }
 
+/// @brief Command execution tests.
+/// This really tests just the ability of the handler to invoke the necessary
+/// chain of methods and to handle error conditions. Note that this testing
+/// calls the command callback, commandHandler, directly.
+/// This test verifies that:
+/// 1. That an unrecognized command is detected and returns a status of
+/// d2::COMMAND_INVALID.
+/// 2. Shutdown command is recognized and returns a d2::COMMAND_SUCCESS status.
 TEST_F(D2ControllerTest, executeCommandTests) {
     int rcode = -1;
     isc::data::ConstElementPtr answer;
     isc::data::ElementPtr arg_set;
 
+    // Initialize the application process.
     ASSERT_NO_THROW(initProcess());
     EXPECT_TRUE(checkProcess());
 
-    // Verify that shutdown command returns CommandSuccess response.
-    //answer = executeCommand(SHUT_DOWN_COMMAND, isc::data::ElementPtr());
-    answer = DControllerBase::commandHandler(SHUT_DOWN_COMMAND, arg_set); 
-    isc::config::parseAnswer(rcode, answer);
-    EXPECT_EQ(COMMAND_SUCCESS, rcode);
-
-    // Verify that an unknown command returns an InvalidCommand response.
+    // Verify that an unknown command returns an COMMAND_INVALID response.
     std::string bogus_command("bogus");
     answer = DControllerBase::commandHandler(bogus_command, arg_set);
     isc::config::parseAnswer(rcode, answer);
     EXPECT_EQ(COMMAND_INVALID, rcode);
+
+    // Verify that shutdown command returns COMMAND_SUCCESS response.
+    //answer = executeCommand(SHUT_DOWN_COMMAND, isc::data::ElementPtr());
+    answer = DControllerBase::commandHandler(SHUT_DOWN_COMMAND, arg_set);
+    isc::config::parseAnswer(rcode, answer);
+    EXPECT_EQ(COMMAND_SUCCESS, rcode);
+
 }
 
 }; // end of isc::d2 namespace

src/bin/d2/tests/d2_process_unittests.cc

@@ -43,7 +43,7 @@ public:
         process_.reset(new D2Process("TestProcess", io_service_));
     }
 
-    /// @brief Destructor 
+    /// @brief Destructor
     ~D2ProcessTest() {
         io_service_.reset();
         process_.reset();
@@ -59,7 +59,7 @@ public:
         isc_throw (DProcessBaseError, "simulated fatal error");
     }
 
-    /// @brief IOService for event processing. Fills in for IOservice
+    /// @brief IOService for event processing. Fills in for IOService
     /// supplied by management layer.
     IOServicePtr io_service_;
 };
@@ -83,36 +83,36 @@ TEST(D2Process, construction) {
 }
 
 /// @brief Verifies basic configure method behavior.
-// @TODO This test is simplistic and will need to be augmented
-// as configuration ability is implemented.
+/// @TODO This test is simplistic and will need to be augmented as configuration
+/// ability is implemented.
 TEST_F(D2ProcessTest, configure) {
     // Verify that given a configuration "set", configure returns
     // a successful response.
     int rcode = -1;
     string config = "{ \"test-value\": 1000 } ";
     isc::data::ElementPtr json = isc::data::Element::fromJSON(config);
-    isc::data::ConstElementPtr answer = process_->configure(json); 
+    isc::data::ConstElementPtr answer = process_->configure(json);
     isc::config::parseAnswer(rcode, answer);
     EXPECT_EQ(0, rcode);
 }
 
-/// @brief Verifies basic command method behavior. 
-// @TODO IF the D2Process is extended to support extra commands
-// this testing will need to augmented accordingly.
+/// @brief Verifies basic command method behavior.
+/// @TODO IF the D2Process is extended to support extra commands this testing
+/// will need to augmented accordingly.
 TEST_F(D2ProcessTest, command) {
-    // Verfiy that the process will process unsupported command and
+    // Verify that the process will process unsupported command and
     // return a failure response.
     int rcode = -1;
     string args = "{ \"arg1\": 77 } ";
     isc::data::ElementPtr json = isc::data::Element::fromJSON(args);
-    isc::data::ConstElementPtr answer = 
-                                    process_->command("bogus_command", json); 
+    isc::data::ConstElementPtr answer =
+                                    process_->command("bogus_command", json);
     parseAnswer(rcode, answer);
     EXPECT_EQ(COMMAND_INVALID, rcode);
 }
 
-/// @brief Verifies that an "external" call to shutdown causes 
-/// the run method to exit gracefully with a return value of EXIT_SUCCESS.
+/// @brief Verifies that an "external" call to shutdown causes the run method
+/// to exit gracefully.
 TEST_F(D2ProcessTest, normalShutdown) {
     // Use an asiolink IntervalTimer and callback to generate the
     // shutdown invocation. (Note IntervalTimer setup is in milliseconds).
@@ -121,24 +121,21 @@ TEST_F(D2ProcessTest, normalShutdown) {
 
     // Record start time, and invoke run().
     ptime start = microsec_clock::universal_time();
-    int rcode = process_->run();
+    EXPECT_NO_THROW(process_->run());
 
     // Record stop time.
     ptime stop = microsec_clock::universal_time();
 
-    // Verify normal shutdown status.
-    EXPECT_EQ(EXIT_SUCCESS, rcode);
-
     // Verify that duration of the run invocation is the same as the
     // timer duration.  This demonstrates that the shutdown was driven
     // by an io_service event and callback.
     time_duration elapsed = stop - start;
-    EXPECT_TRUE(elapsed.total_milliseconds() >= 1900 && 
+    EXPECT_TRUE(elapsed.total_milliseconds() >= 1900 &&
                 elapsed.total_milliseconds() <= 2100);
 }
 
-/// @brief Verifies that an "uncaught" exception thrown during event loop 
-/// processing is treated as a fatal error.
+/// @brief Verifies that an "uncaught" exception thrown during event loop
+/// execution is treated as a fatal error.
 TEST_F(D2ProcessTest, fatalErrorShutdown) {
     // Use an asiolink IntervalTimer and callback to generate the
     // the exception.  (Note IntervalTimer setup is in milliseconds).
@@ -147,19 +144,16 @@ TEST_F(D2ProcessTest, fatalErrorShutdown) {
 
     // Record start time, and invoke run().
     ptime start = microsec_clock::universal_time();
-    int rcode = process_->run();
+    EXPECT_THROW(process_->run(), DProcessBaseError);
 
     // Record stop time.
     ptime stop = microsec_clock::universal_time();
 
-    // Verify failure status.
-    EXPECT_EQ(EXIT_FAILURE, rcode);
-
     // Verify that duration of the run invocation is the same as the
     // timer duration.  This demonstrates that the anomaly occurred
-    // during io callback processing. 
+    // during io callback processing.
     time_duration elapsed = stop - start;
-    EXPECT_TRUE(elapsed.total_milliseconds() >= 1900 && 
+    EXPECT_TRUE(elapsed.total_milliseconds() >= 1900 &&
                 elapsed.total_milliseconds() <= 2100);
 }
 

src/bin/d2/tests/d_controller_unittests.cc

@@ -27,75 +27,157 @@ using namespace boost::posix_time;
 namespace isc {
 namespace d2 {
 
-/// @brief basic instantiation  
-// @TODO This test is simplistic and will need to be augmented
+/// @brief Test fixture class for testing DControllerBase class. This class
+/// derives from DControllerTest and wraps a DStubController.  DStubController
+/// has been constructed to exercise DControllerBase.
+class DStubControllerTest : public DControllerTest {
+public:
+
+    /// @brief Constructor.
+    /// Note the constructor passes in the static DStubController instance
+    /// method.
+    DStubControllerTest() : DControllerTest (DStubController::instance) {
+    }
+
+    virtual ~DStubControllerTest() {
+    }
+};
+
+/// @brief Basic Controller instantiation testing.
+/// Verfies that the controller singleton gets created and that the
+/// basic derivation from the base class is intact.
 TEST_F(DStubControllerTest, basicInstanceTesting) {
+    // Verify that the singleton exists and it is the correct type.
     DControllerBasePtr& controller = DControllerTest::getController();
     ASSERT_TRUE(controller);
+    ASSERT_NO_THROW(boost::dynamic_pointer_cast<DStubController>(controller));
+
+    // Verify that controller's name is correct.
     EXPECT_TRUE(checkName(D2_MODULE_NAME));
+
+    // Verify that controller's spec file name is correct.
     EXPECT_TRUE(checkSpecFileName(D2_SPECFILE_LOCATION));
+
+    // Verify that controller's IOService exists.
     EXPECT_TRUE(checkIOService());
 
-    // Process should NOT exist yet
+    // Verify that the Process does NOT exist.
     EXPECT_FALSE(checkProcess());
 }
 
-/// @TODO brief Verifies command line processing. 
+/// @brief Tests basic command line processing.
+/// Verifies that:
+/// 1. Standard command line options are supported.
+/// 2. Custom command line options are supported.
+/// 3. Invalid options are detected.
+/// 4. Extraneous command line information is detected.
 TEST_F(DStubControllerTest, commandLineArgs) {
-    char* argv[] = { (char*)"progName", (char*)"-s", (char*)"-v" };
-    int argc = 3;
 
+    // Verify that both flags are false initially.
     EXPECT_TRUE(checkStandAlone(false));
     EXPECT_TRUE(checkVerbose(false));
 
+    // Verify that standard options can be parsed without error.
+    char* argv[] = { (char*)"progName", (char*)"-s", (char*)"-v" };
+    int argc = 3;
     EXPECT_NO_THROW(parseArgs(argc, argv));
 
+    // Verify that flags are now true.
     EXPECT_TRUE(checkStandAlone(true));
     EXPECT_TRUE(checkVerbose(true));
 
-    char* argv1[] = { (char*)"progName", (char*)"-x" };
+    // Verify that the custom command line option is parsed without error.
+    char xopt[3]="";
+    sprintf (xopt, "-%c", *DStubController::stub_option_x_);
+    char* argv1[] = { (char*)"progName", xopt};
     argc = 2;
     EXPECT_NO_THROW (parseArgs(argc, argv1));
 
+    // Verify that an unknown option is detected.
     char* argv2[] = { (char*)"progName", (char*)"-bs" };
     argc = 2;
     EXPECT_THROW (parseArgs(argc, argv2), InvalidUsage);
+
+    // Verify that extraneous information is detected.
+    char* argv3[] = { (char*)"progName", (char*)"extra", (char*)"information" };
+    argc = 3;
+    EXPECT_THROW (parseArgs(argc, argv3), InvalidUsage);
+
+
 }
 
-/// @TODO brief initProcess testing. 
+/// @brief Tests application process creation and initialization.
+/// Verifies that:
+/// 1. An error during process creation is handled.
+/// 2. A NULL returned by process creation is handled.
+/// 3. An error during process initialization is handled.
+/// 4. Process can be successfully created and initialized.
 TEST_F(DStubControllerTest, initProcessTesting) {
-
+    // Verify that a failure during process creation is caught.
     SimFailure::set(SimFailure::ftCreateProcessException);
     EXPECT_THROW(initProcess(), DControllerBaseError);
     EXPECT_FALSE(checkProcess());
 
+    // Verify that a NULL returned by process creation is handled.
     SimFailure::set(SimFailure::ftCreateProcessNull);
     EXPECT_THROW(initProcess(), DControllerBaseError);
     EXPECT_FALSE(checkProcess());
 
+    // Re-create controller, verify that we are starting clean
     resetController();
+    EXPECT_FALSE(checkProcess());
+
+    // Verify that an error during process initialization is handled.
     SimFailure::set(SimFailure::ftProcessInit);
     EXPECT_THROW(initProcess(), DProcessBaseError);
 
+    // Re-create controller, verify that we are starting clean
     resetController();
     EXPECT_FALSE(checkProcess());
 
+    // Verify that the application process can created and initialized.
     ASSERT_NO_THROW(initProcess());
     EXPECT_TRUE(checkProcess());
 }
 
-/// @TODO brief establishSession failure testing. 
-/// Testing with BIND10 is out of scope for unit tests 
-TEST_F(DStubControllerTest, sessionFailureTesting) {
-    ASSERT_NO_THROW(initProcess());
-    EXPECT_TRUE(checkProcess());
+/// @brief Tests launch handling of invalid command line.
+/// This test launches with an invalid command line which should exit with
+/// an status of d2::INVALID_USAGE.
+TEST_F(DStubControllerTest, launchInvalidUsage) {
+    // Command line to run integrated
+    char* argv[] = { (char*)"progName",(char*) "-z" };
+    int argc = 2;
+
+    // Launch the controller in integrated mode.
+    int rcode = launch(argc, argv);
+
+    // Verify session failure exit status.
+    EXPECT_EQ(d2::INVALID_USAGE, rcode);
+}
+
+/// @brief Tests launch handling of failure in application process
+/// initialization.  This test launches with a valid command line but with
+/// SimFailure set to fail during process creation.  Launch exit code should
+/// be d2::PROCESS_INIT_ERROR.
+TEST_F(DStubControllerTest, launchProcessInitError) {
+    // Command line to run integrated
+    char* argv[] = { (char*)"progName", (char*)"-s", (char*)"-v" };
+    int argc = 3;
+
+    // Launch the controller in stand alone mode.
+    SimFailure::set(SimFailure::ftCreateProcessException);
+    int rcode = launch(argc, argv);
 
-    EXPECT_THROW(establishSession(), std::exception);
+    // Verify session failure exit status.
+    EXPECT_EQ(d2::PROCESS_INIT_ERROR, rcode);
 }
 
-/// @TODO brief test launch 
-TEST_F(DStubControllerTest, launchDirectShutdown) {
-    // command line to run standalone 
+/// @brief Tests launch and normal shutdown (stand alone mode).
+/// This creates an interval timer to generate a normal shutdown and then
+/// launches with a valid, stand-alone command line and no simulated errors.
+/// Launch exit code should be d2::NORMAL_EXIT.
+TEST_F(DStubControllerTest, launchNormalShutdown) {
+    // command line to run standalone
     char* argv[] = { (char*)"progName", (char*)"-s", (char*)"-v" };
     int argc = 3;
 
@@ -112,7 +194,7 @@ TEST_F(DStubControllerTest, launchDirectShutdown) {
     ptime stop = microsec_clock::universal_time();
 
     // Verify normal shutdown status.
-    EXPECT_EQ(EXIT_SUCCESS, rcode);
+    EXPECT_EQ(d2::NORMAL_EXIT, rcode);
 
     // Verify that duration of the run invocation is the same as the
     // timer duration.  This demonstrates that the shutdown was driven
@@ -122,9 +204,12 @@ TEST_F(DStubControllerTest, launchDirectShutdown) {
                 elapsed.total_milliseconds() <= 2100);
 }
 
-/// @TODO brief test launch 
+/// @brief Tests launch with an operational error during application execution.
+/// This test creates an interval timer to generate a runtime exception during
+/// the process event loop. It launches wih a valid, stand-alone command line
+/// and no simulated errors.  Launch exit code should be d2::RUN_ERROR.
 TEST_F(DStubControllerTest, launchRuntimeError) {
-    // command line to run standalone 
+    // command line to run standalone
     char* argv[] = { (char*)"progName", (char*)"-s", (char*)"-v" };
     int argc = 3;
 
@@ -140,8 +225,8 @@ TEST_F(DStubControllerTest, launchRuntimeError) {
     // Record stop time.
     ptime stop = microsec_clock::universal_time();
 
-    // Verify normal shutdown status.
-    EXPECT_EQ(EXIT_SUCCESS, rcode);
+    // Verify abnormal shutdown status.
+    EXPECT_EQ(d2::RUN_ERROR, rcode);
 
     // Verify that duration of the run invocation is the same as the
     // timer duration.  This demonstrates that the shutdown was driven
@@ -151,24 +236,51 @@ TEST_F(DStubControllerTest, launchRuntimeError) {
                 elapsed.total_milliseconds() <= 2100);
 }
 
-/// @TODO brief test configUpateTests 
+/// @brief Tests launch with a session establishment failure.
+/// This test launches with a valid command line for integrated mode and no.
+/// Attempting to connect to BIND10 should fail, even if BIND10 is running
+/// UNLESS the test is run as root.  Launch exit code should be
+/// d2::SESSION_START_ERROR.
+TEST_F(DStubControllerTest, launchSessionFailure) {
+    // Command line to run integrated
+    char* argv[] = { (char*)"progName" };
+    int argc = 1;
+
+    // Launch the controller in integrated mode.
+    int rcode = launch(argc, argv);
+
+    // Verify session failure exit status.
+    EXPECT_EQ(d2::SESSION_START_ERROR, rcode);
+}
+
+/// @brief Configuration update event testing.
 /// This really tests just the ability of the handlers to invoke the necessary
-/// chain, and error conditions. Configuration parsing and retrieval should be
-/// tested as part of the d2 configuration management implementation.   
+/// chain of methods and handle error conditions. Configuration parsing and
+/// retrieval should be tested as part of the d2 configuration management
+/// implementation.  Note that this testing calls the configuration update event
+/// callback, configHandler, directly.
+/// This test verifies that:
+/// 1. Configuration will be rejected in integrated mode when there is no
+/// session established. (This is a very contrived situation).
+/// 2. In stand-alone mode a configuration update results in successful
+/// status return.
+/// 3. That an application process error in configuration updating is handled
+/// properly.
 TEST_F(DStubControllerTest, configUpdateTests) {
     int rcode = -1;
     isc::data::ConstElementPtr answer;
 
+    // Initialize the application process.
     ASSERT_NO_THROW(initProcess());
     EXPECT_TRUE(checkProcess());
 
-    // Create a configuration set. Content is arbitrary, just needs to be 
+    // Create a configuration set. Content is arbitrary, just needs to be
     // valid JSON.
     std::string config = "{ \"test-value\": 1000 } ";
     isc::data::ElementPtr config_set = isc::data::Element::fromJSON(config);
 
     // We are not stand-alone, so configuration should be rejected as there is
-    // no session.  This is a pretty contrived situation that shouldn't be 
+    // no session.  This is a pretty contrived situation that shouldn't be
     // possible other than the handler being called directly (like this does).
     answer = DControllerBase::configHandler(config_set);
     isc::config::parseAnswer(rcode, answer);
@@ -185,41 +297,71 @@ TEST_F(DStubControllerTest, configUpdateTests) {
     answer = DControllerBase::configHandler(config_set);
     isc::config::parseAnswer(rcode, answer);
     EXPECT_EQ(1, rcode);
-
 }
 
+/// @brief Command execution tests.
+/// This really tests just the ability of the handler to invoke the necessary
+/// chain of methods and to handle error conditions. Note that this testing
+/// calls the command callback, commandHandler, directly.
+/// This test verifies that:
+/// 1. That an unrecognized command is detected and returns a status of
+/// d2::COMMAND_INVALID.
+/// 2. Shutdown command is recognized and returns a d2::COMMAND_SUCCESS status.
+/// 3. A valid, custom controller command is recognized a d2::COMMAND_SUCCESS
+/// status.
+/// 4. A valid, custom process command is recognized a d2::COMMAND_SUCCESS
+/// status.
+/// 5. That a valid controller command that fails returns a d2::COMMAND_ERROR.
+/// 6. That a valid process command that fails returns a d2::COMMAND_ERROR.
 TEST_F(DStubControllerTest, executeCommandTests) {
     int rcode = -1;
     isc::data::ConstElementPtr answer;
     isc::data::ElementPtr arg_set;
 
+    // Initialize the application process.
     ASSERT_NO_THROW(initProcess());
     EXPECT_TRUE(checkProcess());
 
-    // Verify that shutdown command returns CommandSuccess response.
+    // Verify that an unknown command returns an d2::COMMAND_INVALID response.
+    std::string bogus_command("bogus");
+    answer = DControllerBase::commandHandler(bogus_command, arg_set);
+    isc::config::parseAnswer(rcode, answer);
+    EXPECT_EQ(COMMAND_INVALID, rcode);
+
+    // Verify that shutdown command returns d2::COMMAND_SUCCESS response.
     answer = DControllerBase::commandHandler(SHUT_DOWN_COMMAND, arg_set);
     isc::config::parseAnswer(rcode, answer);
     EXPECT_EQ(COMMAND_SUCCESS, rcode);
 
-    // Verify that a valid custom controller command returns CommandSuccess
-    // response.
+    // Verify that a valid custom controller command returns
+    // d2::COMMAND_SUCCESS response.
     answer = DControllerBase::commandHandler(DStubController::
-                                             custom_ctl_command_, arg_set);
+                                             stub_ctl_command_, arg_set);
     isc::config::parseAnswer(rcode, answer);
     EXPECT_EQ(COMMAND_SUCCESS, rcode);
 
-    // Verify that an unknown command returns an InvalidCommand response.
-    std::string bogus_command("bogus");
-    answer = DControllerBase::commandHandler(bogus_command, arg_set);
-    isc::config::parseAnswer(rcode, answer);
-    EXPECT_EQ(COMMAND_INVALID, rcode);
-
-    // Verify that a valid custom process command returns CommandSuccess
+    // Verify that a valid custom process command returns d2::COMMAND_SUCCESS
     // response.
     answer = DControllerBase::commandHandler(DStubProcess::
-                                             custom_process_command_, arg_set);
+                                             stub_proc_command_, arg_set);
     isc::config::parseAnswer(rcode, answer);
     EXPECT_EQ(COMMAND_SUCCESS, rcode);
+
+    // Verify that a valid custom controller command that fails returns
+    // a d2::COMMAND_ERROR.
+    SimFailure::set(SimFailure::ftControllerCommand);
+    answer = DControllerBase::commandHandler(DStubController::
+                                             stub_ctl_command_, arg_set);
+    isc::config::parseAnswer(rcode, answer);
+    EXPECT_EQ(COMMAND_ERROR, rcode);
+
+    // Verify that a valid custom process command that fails returns
+    // a d2::COMMAND_ERROR.
+    SimFailure::set(SimFailure::ftProcessCommand);
+    answer = DControllerBase::commandHandler(DStubProcess::
+                                             stub_proc_command_, arg_set);
+    isc::config::parseAnswer(rcode, answer);
+    EXPECT_EQ(COMMAND_ERROR, rcode);
 }
 
 }; // end of isc::d2 namespace

src/bin/d2/tests/d_test_stubs.cc

@@ -20,28 +20,31 @@ using namespace asio;
 namespace isc {
 namespace d2 {
 
+// Initialize the static failure flag.
 SimFailure::FailureType SimFailure::failure_type_ = SimFailure::ftNoFailure;
 
-const std::string DStubProcess::custom_process_command_("valid_prc_command");
+// Define custom process command supported by DStubProcess.
+const std::string DStubProcess::stub_proc_command_("cool_proc_cmd");
 
-DStubProcess::DStubProcess(const char* name, IOServicePtr io_service) 
+DStubProcess::DStubProcess(const char* name, IOServicePtr io_service)
     : DProcessBase(name, io_service) {
 };
 
 void
 DStubProcess::init() {
+    LOG_DEBUG(d2_logger, DBGLVL_START_SHUT, D2PRC_PROCESS_INIT);
     if (SimFailure::shouldFailOn(SimFailure::ftProcessInit)) {
-        // Simulates a failure to instantiate the process. 
-        isc_throw(DProcessBaseError, "DStubProcess simulated init failure");
+        // Simulates a failure to instantiate the process.
+        isc_throw(DProcessBaseError, "DStubProcess simulated init() failure");
     }
 };
 
-int
+void
 DStubProcess::run() {
     // Until shut down or an fatal error occurs, wait for and
     // execute a single callback. This is a preliminary implementation
     // that is likely to evolve as development progresses.
-    // To use run(), the "managing" layer must issue an io_service::stop 
+    // To use run(), the "managing" layer must issue an io_service::stop
     // or the call to run will continue to block, and shutdown will not
     // occur.
     LOG_DEBUG(d2_logger, DBGLVL_START_SHUT, D2PRC_RUN_ENTER);
@@ -51,27 +54,31 @@ DStubProcess::run() {
             io_service->run_one();
         } catch (const std::exception& ex) {
             LOG_FATAL(d2_logger, D2PRC_FAILED).arg(ex.what());
-            return (EXIT_FAILURE); 
+            isc_throw (DProcessBaseError,
+                std::string("Process run method failed:") + ex.what());
         }
     }
 
     LOG_DEBUG(d2_logger, DBGLVL_START_SHUT, D2PRC_RUN_EXIT);
-    return (EXIT_SUCCESS);
 };
 
-int 
+void
 DStubProcess::shutdown() {
     LOG_DEBUG(d2_logger, DBGLVL_START_SHUT, D2PRC_SHUTDOWN);
+    if (SimFailure::shouldFailOn(SimFailure::ftProcessShutdown)) {
+        // Simulates a failure during shutdown process.
+        isc_throw(DProcessBaseError, "DStubProcess simulated shutdown failure");
+    }
     setShutdownFlag(true);
-    return (0);
-}    
+}
 
-isc::data::ConstElementPtr 
+isc::data::ConstElementPtr
 DStubProcess::configure(isc::data::ConstElementPtr config_set) {
-    LOG_DEBUG(d2_logger, DBGLVL_TRACE_BASIC, 
+    LOG_DEBUG(d2_logger, DBGLVL_TRACE_BASIC,
               D2PRC_CONFIGURE).arg(config_set->str());
 
     if (SimFailure::shouldFailOn(SimFailure::ftProcessConfigure)) {
+        // Simulates a process configure failure.
         return (isc::config::createAnswer(1,
                 "Simulated process configuration error."));
     }
@@ -79,16 +86,18 @@ DStubProcess::configure(isc::data::ConstElementPtr config_set) {
     return (isc::config::createAnswer(0, "Configuration accepted."));
 }
 
-isc::data::ConstElementPtr 
-DStubProcess::command(const std::string& command, isc::data::ConstElementPtr args){
-    LOG_DEBUG(d2_logger, DBGLVL_TRACE_BASIC, 
+isc::data::ConstElementPtr
+DStubProcess::command(const std::string& command,
+                      isc::data::ConstElementPtr args) {
+    LOG_DEBUG(d2_logger, DBGLVL_TRACE_BASIC,
               D2PRC_COMMAND).arg(command).arg(args->str());
 
     isc::data::ConstElementPtr answer;
     if (SimFailure::shouldFailOn(SimFailure::ftProcessCommand)) {
+        // Simulates a process command execution failure.
         answer = isc::config::createAnswer(COMMAND_ERROR,
                                           "SimFailure::ftProcessCommand");
-    } else if (command.compare(custom_process_command_) == 0) {
+    } else if (command.compare(stub_proc_command_) == 0) {
         answer = isc::config::createAnswer(COMMAND_SUCCESS, "Command accepted");
     } else {
         answer = isc::config::createAnswer(COMMAND_INVALID,
@@ -103,10 +112,15 @@ DStubProcess::~DStubProcess() {
 
 //************************** DStubController *************************
 
-const std::string DStubController::custom_ctl_command_("valid_ctrl_command");
+// Define custom controller command supported by DStubController.
+const std::string DStubController::stub_ctl_command_("spiffy");
+
+// Define custom command line option command supported by DStubController.
+const char* DStubController::stub_option_x_ = "x";
 
 DControllerBasePtr&
 DStubController::instance() {
+    // If the singleton hasn't been created, do it now.
     if (!getController()) {
         setController(new DStubController());
     }
@@ -128,22 +142,22 @@ DStubController::DStubController()
 bool
 DStubController::customOption(int option, char* /* optarg */)
 {
-    // Default implementation returns false
-    if (option == 'x') {
-        return (true);         
+    // Check for the custom option supported by DStubController.
+    if ((char)(option) == *stub_option_x_) {
+        return (true);
     }
-    
+
     return (false);
 }
 
 DProcessBase* DStubController::createProcess() {
     if (SimFailure::shouldFailOn(SimFailure::ftCreateProcessException)) {
-        // Simulates a failure to instantiate the process due to exception. 
+        // Simulates a failure to instantiate the process due to exception.
         throw std::runtime_error("SimFailure::ftCreateProcess");
     }
 
     if (SimFailure::shouldFailOn(SimFailure::ftCreateProcessNull)) {
-        // Simulates a failure to instantiate the process. 
+        // Simulates a failure to instantiate the process.
         return (NULL);
     }
 
@@ -156,9 +170,10 @@ DStubController::customControllerCommand(const std::string& command,
                                      isc::data::ConstElementPtr /* args */) {
     isc::data::ConstElementPtr answer;
     if (SimFailure::shouldFailOn(SimFailure::ftControllerCommand)) {
+        // Simulates command failing to execute.
         answer = isc::config::createAnswer(COMMAND_ERROR,
                                           "SimFailure::ftControllerCommand");
-    } else if (command.compare(custom_ctl_command_) == 0) {
+    } else if (command.compare(stub_ctl_command_) == 0) {
         answer = isc::config::createAnswer(COMMAND_SUCCESS, "Command accepted");
     } else {
         answer = isc::config::createAnswer(COMMAND_INVALID,
@@ -169,14 +184,15 @@ DStubController::customControllerCommand(const std::string& command,
 }
 
 const std::string DStubController::getCustomOpts(){
-    return (std::string("x"));
+    // Return the "list" of custom options supported by DStubController.
+    return (std::string(stub_option_x_));
 }
 
 DStubController::~DStubController() {
 }
 
-DControllerBasePtr DControllerTest::controller_under_test_;
+// Initialize controller wrapper's static instance getter member.
 DControllerTest::InstanceGetter DControllerTest::instanceGetter_ = NULL;
 
-}; // namespace isc::d2 
+}; // namespace isc::d2
 }; // namespace isc

src/bin/d2/tests/d_test_stubs.h

@@ -26,23 +26,24 @@
 namespace isc {
 namespace d2 {
 
-/// @brief Class is used to set a globally accessible value that indicates 
+/// @brief Class is used to set a globally accessible value that indicates
 /// a specific type of failure to simulate.  Test derivations of base classes
-/// can excercize error handling code paths by testing for specific SimFailure
+/// can exercise error handling code paths by testing for specific SimFailure
 /// values at the appropriate places and then causing the error to "occur".
 /// The class consists of an enumerated set of failures, and static methods
 /// for getting, setting, and testing the current value.
 class SimFailure {
-public: 
+public:
     enum FailureType {
-        ftUnknown = -1, 
+        ftUnknown = -1,
         ftNoFailure = 0,
-        ftCreateProcessException, 
+        ftCreateProcessException,
         ftCreateProcessNull,
         ftProcessInit,
         ftProcessConfigure,
         ftControllerCommand,
-        ftProcessCommand
+        ftProcessCommand,
+        ftProcessShutdown
     };
 
     /// @brief Sets the SimFailure value to the given value.
@@ -52,9 +53,9 @@ public:
        failure_type_ = value;
     }
 
-    /// @brief Gets the current global SimFailure value 
+    /// @brief Gets the current global SimFailure value
     ///
-    /// @return returns the current SimFailure value 
+    /// @return returns the current SimFailure value
     static enum FailureType get() {
        return (failure_type_);
     }
@@ -95,40 +96,40 @@ class DStubProcess : public DProcessBase {
 public:
 
     /// @brief Static constant that defines a custom process command string.
-    static const std::string custom_process_command_;
+    static const std::string stub_proc_command_;
 
     /// @brief Constructor
     ///
     /// @param name name is a text label for the process. Generally used
-    /// in log statements, but otherwise arbitrary. 
+    /// in log statements, but otherwise arbitrary.
     /// @param io_service is the io_service used by the caller for
     /// asynchronous event handling.
     ///
-    /// @throw DProcessBaseError is io_service is NULL. 
+    /// @throw DProcessBaseError is io_service is NULL.
     DStubProcess(const char* name, IOServicePtr io_service);
 
-    /// @brief Invoked after process instantiation to perform initialization. 
-    /// This implementation supports simulating an error initializing the 
-    /// process by throwing a DProcesssBaseError if SimFailure is set to
-    /// ftProcessInit.  
+    /// @brief Invoked after process instantiation to perform initialization.
+    /// This implementation supports simulating an error initializing the
+    /// process by throwing a DProcessBaseError if SimFailure is set to
+    /// ftProcessInit.
     virtual void init();
 
-    /// @brief Implements the process's event loop. 
-    /// This implementation is quite basic, surrounding calls to 
-    /// io_service->runOne() with a test of the shutdown flag. Once invoked, 
-    /// the method will continue until the process itself is exiting due to a 
+    /// @brief Implements the process's event loop.
+    /// This implementation is quite basic, surrounding calls to
+    /// io_service->runOne() with a test of the shutdown flag. Once invoked,
+    /// the method will continue until the process itself is exiting due to a
     /// request to shutdown or some anomaly forces an exit.
     /// @return  returns 0 upon a successful, "normal" termination, non-zero to
-    /// indicate an abnormal termination.    
-    virtual int run();
+    /// indicate an abnormal termination.
+    virtual void run();
 
     /// @brief Implements the process shutdown procedure. Currently this is
     /// limited to setting the instance shutdown flag, which is monitored in
     /// run().
-    virtual int shutdown();
+    virtual void shutdown();
 
-    /// @brief Processes the given configuration. 
-    /// 
+    /// @brief Processes the given configuration.
+    ///
     /// This implementation fails if SimFailure is set to ftProcessConfigure.
     /// Otherwise it will complete successfully.  It does not check the content
     /// of the inbound configuration.
@@ -136,26 +137,26 @@ public:
     /// @param config_set a new configuration (JSON) for the process
     /// @return an Element that contains the results of configuration composed
     /// of an integer status value (0 means successful, non-zero means failure),
-    /// and a string explanation of the outcome. 
+    /// and a string explanation of the outcome.
     virtual isc::data::ConstElementPtr configure(isc::data::ConstElementPtr
                                                  config_set);
 
-    /// @brief Executes the given command. 
-    /// 
+    /// @brief Executes the given command.
+    ///
     /// This implementation will recognizes one "custom" process command,
-    /// custom_process_command_.  It will fail if SimFailure is set to 
-    /// ftProcessCommand. 
+    /// stub_proc_command_.  It will fail if SimFailure is set to
+    /// ftProcessCommand.
     ///
     /// @param command is a string label representing the command to execute.
     /// @param args is a set of arguments (if any) required for the given
-    /// command. 
+    /// command.
     /// @return an Element that contains the results of command composed
-    /// of an integer status value and a string explanation of the outcome.  
+    /// of an integer status value and a string explanation of the outcome.
     /// The status value is:
-    /// COMMAND_SUCCESS if the command is recongized and executes successfully.
-    /// COMMAND_ERROR if the command is recongized but fails to execute.
+    /// COMMAND_SUCCESS if the command is recognized and executes successfully.
+    /// COMMAND_ERROR if the command is recognized but fails to execute.
     /// COMMAND_INVALID if the command is not recognized.
-    virtual isc::data::ConstElementPtr command(const std::string& command, 
+    virtual isc::data::ConstElementPtr command(const std::string& command,
                                                isc::data::ConstElementPtr args);
 
     // @brief Destructor
@@ -166,66 +167,71 @@ public:
 /// @brief Test Derivation of the DControllerBase class.
 ///
 /// DControllerBase is an abstract class and therefore requires a derivation
-/// for testing.  It allows testing the majority of the base classs code 
-/// without polluting production derivations (e.g. D2Process).  It uses 
-/// D2StubProcess as its application process class.  It is a full enough
-/// implementation to support running both stand alone and integrated. 
+/// for testing.  It allows testing the majority of the base class code
+/// without polluting production derivations (e.g. D2Process).  It uses
+/// DStubProcess as its application process class.  It is a full enough
+/// implementation to support running both stand alone and integrated.
 /// Obviously BIND10 connectivity is not available under unit tests, so
 /// testing here is limited to "failures" to communicate with BIND10.
 class DStubController : public DControllerBase {
 public:
     /// @brief Static singleton instance method. This method returns the
-    /// base class singleton instance member.  It instantiates the singleton 
-    /// and sets the base class instance member upon first invocation. 
+    /// base class singleton instance member.  It instantiates the singleton
+    /// and sets the base class instance member upon first invocation.
     ///
-    /// @return returns the a pointer reference to the singleton instance.
+    /// @return returns a pointer reference to the singleton instance.
     static DControllerBasePtr& instance();
 
-    /// @brief Defines a custom controller command string. This is a 
+    /// @brief Defines a custom controller command string. This is a
     /// custom command supported by DStubController.
-    static const std::string custom_ctl_command_;
+    static const std::string stub_ctl_command_;
+
+    /// @brief Defines a custom command line option supported by
+    /// DStubController.
+    static const char* stub_option_x_;
 
 protected:
     /// @brief Handles additional command line options that are supported
     /// by DStubController.  This implementation supports an option "-x".
     ///
     /// @param option is the option "character" from the command line, without
-    /// any prefixing hypen(s)
+    /// any prefixing hyphen(s)
     /// @optarg optarg is the argument value (if one) associated with the option
     ///
-    /// @return returns true if the option is "x", otherwise ti returns false. 
+    /// @return returns true if the option is "x", otherwise ti returns false.
     virtual bool customOption(int option, char *optarg);
 
-    /// @brief Instantiates an instance of DStubProcess. 
-    /// 
-    /// This implementation will fail if SimFailure is set to 
+    /// @brief Instantiates an instance of DStubProcess.
+    ///
+    /// This implementation will fail if SimFailure is set to
     /// ftCreateProcessException OR ftCreateProcessNull.
-    /// 
+    ///
     /// @return returns a pointer to the new process instance (DProcessBase*)
     /// or NULL if SimFailure is set to ftCreateProcessNull.
-    /// @throw throws std::runtime_error if SimFailure is ste to
+    /// @throw throws std::runtime_error if SimFailure is set to
     /// ftCreateProcessException.
     virtual DProcessBase* createProcess();
 
-    /// @brief Executes custom controller commands are supported by 
-    /// DStubController. This implementation supports one custom controller 
-    /// command, custom_controller_command_.  It will fail if SimFailure is set
+    /// @brief Executes custom controller commands are supported by
+    /// DStubController. This implementation supports one custom controller
+    /// command, stub_ctl_command_.  It will fail if SimFailure is set
     /// to ftControllerCommand.
     ///
     /// @param command is a string label representing the command to execute.
     /// @param args is a set of arguments (if any) required for the given
-    /// command. 
+    /// command.
     /// @return an Element that contains the results of command composed
-    /// of an integer status value and a string explanation of the outcome.  
+    /// of an integer status value and a string explanation of the outcome.
     /// The status value is:
-    /// COMMAND_SUCCESS if the command is recongized and executes successfully.
-    /// COMMAND_ERROR if the command is recongized but fails to execute.
+    /// COMMAND_SUCCESS if the command is recognized and executes successfully.
+    /// COMMAND_ERROR if the command is recognized but fails to execute.
     /// COMMAND_INVALID if the command is not recognized.
     virtual isc::data::ConstElementPtr customControllerCommand(
             const std::string& command, isc::data::ConstElementPtr args);
 
-    /// @brief Provides a string of the additional command line options 
-    /// supported by DStubController. 
+    /// @brief Provides a string of the additional command line options
+    /// supported by DStubController.  DStubController supports one
+    /// addition option, stub_option_x_.
     ///
     /// @return returns a string containing the option letters.
     virtual const std::string getCustomOpts();
@@ -238,133 +244,161 @@ public:
     virtual ~DStubController();
 };
 
-/// @brief Test fixture class that wraps a DControllerBase.  This class 
+/// @brief Abstract Test fixture class that wraps a DControllerBase. This class
 /// is a friend class of DControllerBase which allows it access to class
-/// content to facilitate testing.  It provides numerous wrapper methods for 
-/// the protected and private methods and member of the base class.  Note that
-/// this class specifically supports DStubController testing, it is designed
-/// to be reused, by simply by overriding the init_controller and 
-/// destroy_controller methods. 
+/// content to facilitate testing.  It provides numerous wrapper methods for
+/// the protected and private methods and member of the base class.
 class DControllerTest : public ::testing::Test {
 public:
 
+    /// @brief Defines a function pointer for controller singleton fetchers.
     typedef DControllerBasePtr& (*InstanceGetter)();
-    static InstanceGetter instanceGetter_;
 
-#if 0
-    /// @brief Constructor - Invokes virtual init_controller method which
-    /// initializes the controller class's singleton instance.
-    DControllerTest() {
-       init_controller(); 
-    }
-#endif
-
-    DControllerTest(InstanceGetter instanceGetter) {
-        instanceGetter_ = instanceGetter;
-    }
-
-    /// @brief Destructor - Invokes virtual init_controller method which
-    /// initializes the controller class's singleton instance.  This is
-    /// essential to ensure a clean start between tests.
-    virtual ~DControllerTest() {
-       destroy_controller();
-    }
+    /// @brief Static storage of the controller class's singleton fetcher.
+    /// We need this this statically available for callbacks.
+    static InstanceGetter instanceGetter_;
 
-    /// @brief Virtual method which sets static copy of the controller class's 
-    /// singleton instance.  We need this copy so we can generically access
-    /// the singleton in derivations. 
-    void init_controller() {
-#if 0
-        std::cout << "Calling DController test init_controller" << std::endl;
-        controller_under_test_ = DStubController::instance();
-#else
+    /// @brief Constructor
+    ///
+    /// @param instance_getter is a function pointer to the static instance
+    /// method of the DControllerBase derivation under test.
+    DControllerTest(InstanceGetter instance_getter) {
+        // Set the static fetcher member, then invoke it via getController.
+        // This ensures the singleton is instantiated.
+        instanceGetter_ = instance_getter;
         getController();
-#endif
     }
 
-
-    void destroy_controller() {
-#if 0
-        std::cout << "Calling DController test destroy_controller" << std::endl;
-        DStubController::instance().reset();
-#else
+    /// @brief Destructor
+    /// Note the controller singleton is destroyed. This is essential to ensure
+    /// a clean start between tests.
+    virtual ~DControllerTest() {
         getController().reset();
-#endif
     }
 
+    /// @brief Convenience method that destructs and then recreates the
+    /// controller singleton under test.  This is handy for tests within
+    /// tests.
     void resetController() {
-        destroy_controller();
-        init_controller();
+        getController().reset();
+        getController();
     }
 
+    /// @brief Static method which returns the instance of the controller
+    /// under test.
+    /// @return returns a reference to the controller instance.
     static DControllerBasePtr& getController() {
-#if 0
-        return (controller_under_test_);
-#else
         return ((*instanceGetter_)());
-#endif
     }
 
+    /// @brief Returns true if the Controller's name matches the given value.
+    ///
+    /// @param should_be is the value to compare against.
+    ///
+    /// @return returns true if the values are equal.
     bool checkName(const std::string& should_be) {
         return (getController()->getName().compare(should_be) == 0);
     }
 
+    /// @brief Returns true if the Controller's spec file name matches the
+    /// given value.
+    ///
+    /// @param should_be is the value to compare against.
+    ///
+    /// @return returns true if the values are equal.
     bool checkSpecFileName(const std::string& should_be) {
         return (getController()->getSpecFileName().compare(should_be) == 0);
     }
 
+    /// @brief Tests the existence of the Controller's application process.
+    ///
+    /// @return returns true if the process instance exists.
     bool checkProcess() {
         return (getController()->process_);
     }
 
+    /// @brief Tests the existence of the Controller's IOService.
+    ///
+    /// @return returns true if the IOService exists.
     bool checkIOService() {
         return (getController()->io_service_);
     }
 
+    /// @brief Gets the Controller's IOService.
+    ///
+    /// @return returns a reference to the IOService
     IOServicePtr& getIOService() {
         return (getController()->io_service_);
     }
 
+    /// @brief Compares stand alone flag with the given value.
+    ///
+    /// @param value
+    ///
+    /// @return returns true if the stand alone flag is equal to the given
+    /// value.
     bool checkStandAlone(bool value) {
         return (getController()->isStandAlone() == value);
     }
 
+    /// @brief Sets the controller's stand alone flag to the given value.
+    ///
+    /// @param value is the new value to assign.
+    ///
     void setStandAlone(bool value) {
         getController()->setStandAlone(value);
     }
 
+    /// @brief Compares verbose flag with the given value.
+    ///
+    /// @param value
+    ///
+    /// @return returns true if the verbose flag is equal to the given value.
     bool checkVerbose(bool value) {
         return (getController()->isVerbose() == value);
     }
 
-
+    /// @Wrapper to invoke the Controller's parseArgs method.  Please refer to
+    /// DControllerBase::parseArgs for details.
     void parseArgs(int argc, char* argv[]) {
         getController()->parseArgs(argc, argv);
     }
 
+    /// @Wrapper to invoke the Controller's init method.  Please refer to
+    /// DControllerBase::init for details.
     void initProcess() {
         getController()->initProcess();
     }
 
+    /// @Wrapper to invoke the Controller's establishSession method.  Please
+    /// refer to DControllerBase::establishSession for details.
     void establishSession() {
         getController()->establishSession();
     }
 
+    /// @Wrapper to invoke the Controller's launch method.  Please refer to
+    /// DControllerBase::launch for details.
     int launch(int argc, char* argv[]) {
         optind = 1;
         return (getController()->launch(argc, argv));
     }
 
+    /// @Wrapper to invoke the Controller's disconnectSession method.  Please
+    /// refer to DControllerBase::disconnectSession for details.
     void disconnectSession() {
         getController()->disconnectSession();
     }
 
-    isc::data::ConstElementPtr updateConfig(isc::data::ConstElementPtr 
+    /// @Wrapper to invoke the Controller's updateConfig method.  Please
+    /// refer to DControllerBase::updateConfig for details.
+    isc::data::ConstElementPtr updateConfig(isc::data::ConstElementPtr
                                             new_config) {
         return (getController()->updateConfig(new_config));
     }
 
-    isc::data::ConstElementPtr executeCommand(const std::string& command, 
+    /// @Wrapper to invoke the Controller's executeCommand method.  Please
+    /// refer to DControllerBase::executeCommand for details.
+    isc::data::ConstElementPtr executeCommand(const std::string& command,
                                        isc::data::ConstElementPtr args){
         return (getController()->executeCommand(command, args));
     }
@@ -380,25 +414,9 @@ public:
     static void genFatalErrorCallback() {
         isc_throw (DProcessBaseError, "simulated fatal error");
     }
-
-    /// @brief Static member that retains a copy of controller singleton 
-    /// instance.  This is needed for static asio callback handlers.
-    static DControllerBasePtr controller_under_test_;
-};
-
-class DStubControllerTest : public DControllerTest {
-public:
-
-    DStubControllerTest() : DControllerTest (DStubController::instance) {
-    }
-
-    virtual ~DStubControllerTest() {
-    }
 };
 
-
-
-}; // namespace isc::d2 
+}; // namespace isc::d2
 }; // namespace isc
 
 #endif