From 412eec8a5fb6208d361c033653015ff3c74ad3ab Mon Sep 17 00:00:00 2001 From: ftimme <ft@falkotimme.com> Date: Thu, 11 Jul 2013 10:59:00 -0400 Subject: [PATCH] - Improved recognition of when a service (Apache, nginx, etc.) was really restarted/reloaded successfully. Checking port 80 is not enough because nginx reloads silently, ignoring faulty configuration files, but returns a value > 0. If we don't check the return value, nginx will refuse to start when someone issues a restart the next time. --- server/mods-available/dns_module.inc.php | 9 +++++---- server/plugins-available/apache2_plugin.inc.php | 7 ++++--- server/lib/classes/services.inc.php | 3 ++- server/mods-available/web_module.inc.php | 9 +++++---- server/plugins-available/nginx_plugin.inc.php | 7 ++++--- interface/web/sites/database_phpmyadmin.php | 6 +++--- 6 files changed, 23 insertions(+), 18 deletions(-) diff --git a/interface/web/sites/database_phpmyadmin.php b/interface/web/sites/database_phpmyadmin.php index 0bd379d..e77c5df 100644 --- a/interface/web/sites/database_phpmyadmin.php +++ b/interface/web/sites/database_phpmyadmin.php @@ -65,13 +65,13 @@ if($global_config['phpmyadmin_url'] != '') { $phpmyadmin_url = $global_config['phpmyadmin_url']; $phpmyadmin_url = str_replace(array('[SERVERNAME]', '[DATABASENAME]'),array($serverData['server_name'], $dbData['database_name']),$phpmyadmin_url); - header('Location:'.$phpmyadmin_url); + header('Location: '.$phpmyadmin_url); } else { isset($_SERVER['HTTPS'])? $http = 'https' : $http = 'http'; if($web_config['server_type'] == 'nginx') { - header('location:' . $http . '://' . $serverData['server_name'] . ':8081/phpmyadmin'); + header('Location: http://' . $serverData['server_name'] . ':8081/phpmyadmin'); } else { - header('location:' . $http . '://' . $serverData['server_name'] . '/phpmyadmin'); + header('Location: ' . $http . '://' . $serverData['server_name'] . '/phpmyadmin'); } } exit; diff --git a/server/lib/classes/services.inc.php b/server/lib/classes/services.inc.php index d10abf2..7de5856 100644 --- a/server/lib/classes/services.inc.php +++ b/server/lib/classes/services.inc.php @@ -55,9 +55,10 @@ $function_name = $this->registered_services[$service_name]['function']; $app->log("Calling function '$function_name' from module '$module_name'.",LOGLEVEL_DEBUG); // call_user_method($function_name,$app->loaded_modules[$module_name],$action); - call_user_func(array($app->loaded_modules[$module_name],$function_name),$action); + return call_user_func(array($app->loaded_modules[$module_name],$function_name),$action); } else { $app->log("Unable to restart $service_name. Service not registered.",LOGLEVEL_WARNING); + return 0; } } diff --git a/server/mods-available/dns_module.inc.php b/server/mods-available/dns_module.inc.php index 01c4526..fde6b60 100644 --- a/server/mods-available/dns_module.inc.php +++ b/server/mods-available/dns_module.inc.php @@ -129,11 +129,11 @@ } if($action == 'restart') { - exec($conf['init_scripts'] . '/' . $daemon . ' restart'); + exec($conf['init_scripts'] . '/' . $daemon . ' restart', $output, $retval); } else { - exec($conf['init_scripts'] . '/' . $daemon . ' reload'); + exec($conf['init_scripts'] . '/' . $daemon . ' reload', $output, $retval); } - + return $retval; } function restartPowerDNS($action = 'restart') { @@ -177,9 +177,10 @@ $daemon = 'pdns'; } - exec($conf['init_scripts'] . '/' . $daemon . ' restart'); + exec($conf['init_scripts'] . '/' . $daemon . ' restart', $output, $retval); // unset $tmps; + return $retval; } diff --git a/server/mods-available/web_module.inc.php b/server/mods-available/web_module.inc.php index dd7aba0..2e51c6f 100644 --- a/server/mods-available/web_module.inc.php +++ b/server/mods-available/web_module.inc.php @@ -211,11 +211,11 @@ } if($action == 'restart') { - exec($conf['init_scripts'] . '/' . $daemon . ' restart'); + exec($conf['init_scripts'] . '/' . $daemon . ' restart', $output, $retval); } else { - exec($conf['init_scripts'] . '/' . $daemon . ' reload'); + exec($conf['init_scripts'] . '/' . $daemon . ' reload', $output, $retval); } - + return $retval; } function restartPHP_FPM($action = 'restart') { @@ -229,7 +229,8 @@ if(!$init_script) $init_script = $conf['init_scripts'].'/'.$web_config['php_fpm_init_script']; - exec($init_script.' '.$action); + exec($init_script.' '.$action, $output, $retval); + return $retval; } } // end class diff --git a/server/plugins-available/apache2_plugin.inc.php b/server/plugins-available/apache2_plugin.inc.php index 4202f04..0df93ef 100644 --- a/server/plugins-available/apache2_plugin.inc.php +++ b/server/plugins-available/apache2_plugin.inc.php @@ -1476,7 +1476,8 @@ $apache_online_status_before_restart = $this->_checkTcp('localhost',80); $app->log('Apache status is: '.$apache_online_status_before_restart,LOGLEVEL_DEBUG); - $app->services->restartService('httpd','restart'); + $retval = $app->services->restartService('httpd','restart'); // $retval is 0 on success and > 0 on failure + $app->log('Apache restart return value is: '.$retval,LOGLEVEL_DEBUG); // wait a few seconds, before we test the apache status again $apache_online_status_after_restart = false; @@ -1488,8 +1489,8 @@ } //* Check if apache restarted successfully if it was online before $app->log('Apache online status after restart is: '.$apache_online_status_after_restart,LOGLEVEL_DEBUG); - if($apache_online_status_before_restart && !$apache_online_status_after_restart) { - $app->log('Apache did not restart after the configuration change for website '.$data['new']['domain'].' Reverting the configuration. Saved non-working config as '.$vhost_file.'.err',LOGLEVEL_WARN); + if($apache_online_status_before_restart && !$apache_online_status_after_restart || $retval > 0) { + $app->log('Apache did not restart after the configuration change for website '.$data['new']['domain'].'. Reverting the configuration. Saved non-working config as '.$vhost_file.'.err',LOGLEVEL_WARN); $app->system->copy($vhost_file,$vhost_file.'.err'); if(is_file($vhost_file.'~')) { //* Copy back the last backup file diff --git a/server/plugins-available/nginx_plugin.inc.php b/server/plugins-available/nginx_plugin.inc.php index 7d45168..ecfe5e4 100644 --- a/server/plugins-available/nginx_plugin.inc.php +++ b/server/plugins-available/nginx_plugin.inc.php @@ -1526,7 +1526,8 @@ $nginx_online_status_before_restart = $this->_checkTcp('localhost',80); $app->log('nginx status is: '.$nginx_online_status_before_restart,LOGLEVEL_DEBUG); - $app->services->restartService('httpd','restart'); + $retval = $app->services->restartService('httpd','restart'); // $retval is 0 on success and > 0 on failure + $app->log('nginx restart return value is: '.$retval,LOGLEVEL_DEBUG); // wait a few seconds, before we test the apache status again sleep(2); @@ -1534,8 +1535,8 @@ //* Check if nginx restarted successfully if it was online before $nginx_online_status_after_restart = $this->_checkTcp('localhost',80); $app->log('nginx online status after restart is: '.$nginx_online_status_after_restart,LOGLEVEL_DEBUG); - if($nginx_online_status_before_restart && !$nginx_online_status_after_restart) { - $app->log('nginx did not restart after the configuration change for website '.$data['new']['domain'].' Reverting the configuration. Saved non-working config as '.$vhost_file.'.err',LOGLEVEL_WARN); + if($nginx_online_status_before_restart && !$nginx_online_status_after_restart || $retval > 0) { + $app->log('nginx did not restart after the configuration change for website '.$data['new']['domain'].'. Reverting the configuration. Saved non-working config as '.$vhost_file.'.err',LOGLEVEL_WARN); $app->system->copy($vhost_file,$vhost_file.'.err'); if(is_file($vhost_file.'~')) { //* Copy back the last backup file -- Gitblit v1.9.1