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