Merge lp:~jaypipes/glance/bug766052 into lp:~hudson-openstack/glance/trunk

[Jay Pipes]

Make sure we use get_option() when dealing with boolean values
read from configuration files...otherwise "False" is True :(

[Brian Waldon (bcwaldon)]

Great job.

review: Approve

[Jay Pipes (jaypipes)]

FYI, the other test case (test_logfile) was removed because other tests in test_curl_api and the new tests in test_logging covered the same code path.

[Dan Prince (dan-prince)]

Hey Jay. I think this looks good. Looks like you've coded it such that the config file settings will override the glance-api and glance-registry command line --debug and --verbose options. Now that we have upstart scripts I'm not sure anybody would even use those options but they may be handy for occasionally running in the foreground. Should we make it so those options override the config file?

review: Needs Information

[Dan Prince (dan-prince)]

Never mind. I was misreading it. Looks great. Approve.

review: Approve

[Jay Pipes (jaypipes)]

The CLI option overrides the config files value, AFAIK.

=== modified file 'glance/common/config.py'
--- glance/common/config.py	2011-04-13 12:56:45 +0000
+++ glance/common/config.py	2011-04-19 15:51:32 +0000
@@ -135,8 +135,10 @@
 
     # If either the CLI option or the conf value
     # is True, we set to True
-    debug = options.get('debug') or conf.get('debug', False)
-    verbose = options.get('verbose') or conf.get('verbose', False)
+    debug = options.get('debug') or \
+            get_option(conf, 'debug', type='bool', default=False)
+    verbose = options.get('verbose') or \
+            get_option(conf, 'verbose', type='bool', default=False)
     root_logger = logging.root
     if debug:
         root_logger.setLevel(logging.DEBUG)
@@ -278,8 +280,10 @@
 
         # We only update the conf dict for the verbose and debug
         # flags. Everything else must be set up in the conf file...
-        debug = options.get('debug') or conf.get('debug', False)
-        verbose = options.get('verbose') or conf.get('verbose', False)
+        debug = options.get('debug') or \
+                get_option(conf, 'debug', type='bool', default=False)
+        verbose = options.get('verbose') or \
+                get_option(conf, 'verbose', type='bool', default=False)
         conf['debug'] = debug
         conf['verbose'] = verbose
 

=== modified file 'tests/functional/__init__.py'
--- tests/functional/__init__.py	2011-04-04 13:59:07 +0000
+++ tests/functional/__init__.py	2011-04-19 15:51:32 +0000
@@ -46,6 +46,8 @@
 
     def setUp(self):
 
+        self.verbose = True
+        self.debug = True
         self.test_id = random.randint(0, 100000)
         self.test_dir = os.path.join("/", "tmp", "test.%d" % self.test_id)
 
@@ -144,8 +146,8 @@
 
         conf_file = tempfile.NamedTemporaryFile()
         conf_contents = """[DEFAULT]
-verbose = True
-debug = True
+verbose = %(verbose)s
+debug = %(debug)s
 
 [app:glance-api]
 paste.app_factory = glance.server:app_factory

=== modified file 'tests/functional/test_logging.py'
--- tests/functional/test_logging.py	2011-03-17 21:43:26 +0000
+++ tests/functional/test_logging.py	2011-04-19 15:51:32 +0000
@@ -15,6 +15,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+"""Functional test case that tests logging output"""
+
 import os
 import unittest
 
@@ -24,56 +26,55 @@
 
 class TestLogging(functional.FunctionalTest):
 
-    """Tests that logging can be configured correctly"""
-
-    def test_logfile(self):
-        """
-        A test that logging can be configured properly from the
-        glance.conf file with the log_file option.
-
-        We start both servers daemonized with a temporary config
-        file that has some logging options in it.
-
-        We then use curl to issue a few requests and verify that each server's
-        logging statements were logged to the one log file
-        """
+    """Functional tests for Glance's logging output"""
+
+    def test_verbose_debug(self):
+        """
+        Test logging output proper when verbose and debug
+        is on.
+        """
+
         self.cleanup()
         api_port, reg_port, conf_file = self.start_servers()
 
-        cmd = "curl -X POST -H 'Content-Type: application/octet-stream' "\
-              "-H 'X-Image-Meta-Name: ImageName' "\
-              "-H 'X-Image-Meta-Disk-Format: Invalid' "\
-              "http://0.0.0.0:%d/images" % api_port
-        ignored, out, err = execute(cmd)
-
-        self.assertTrue('Invalid disk format' in out,
-                        "Could not find 'Invalid disk format' "
-                        "in output: %s" % out)
-
-        self.assertTrue(os.path.exists(self.api_log_file),
-                        "API Logfile %s does not exist!"
-                        % self.api_log_file)
-        self.assertTrue(os.path.exists(self.registry_log_file),
-                        "Registry Logfile %s does not exist!"
-                        % self.registry_log_file)
-
-        api_logfile_contents = open(self.api_log_file, 'rb').read()
-        registry_logfile_contents = open(self.registry_log_file, 'rb').read()
-
-        # Check that BOTH the glance API and registry server
-        # modules are logged to their respective logfiles.
-        self.assertTrue('[glance.server]'
-                        in api_logfile_contents,
-                        "Could not find '[glance.server]' "
-                        "in API logfile: %s" % api_logfile_contents)
-        self.assertTrue('[glance.registry.server]'
-                        in registry_logfile_contents,
-                        "Could not find '[glance.registry.server]' "
-                        "in Registry logfile: %s" % registry_logfile_contents)
-
-        # Test that the error we caused above is in the log
-        self.assertTrue('Invalid disk format' in api_logfile_contents,
-                        "Could not find 'Invalid disk format' "
-                        "in API logfile: %s" % api_logfile_contents)
+        # The default functional test case has both verbose
+        # and debug on. Let's verify that debug statements
+        # appear in both the API and registry logs.
+
+        self.assertTrue(os.path.exists(self.api_log_file))
+
+        api_log_out = open(self.api_log_file, 'r').read()
+
+        self.assertTrue('DEBUG [glance-api]' in api_log_out)
+
+        self.assertTrue(os.path.exists(self.registry_log_file))
+
+        registry_log_out = open(self.registry_log_file, 'r').read()
+
+        self.assertTrue('DEBUG [glance-registry]' in registry_log_out)
+
+        self.stop_servers()
+
+    def test_no_verbose_no_debug(self):
+        """
+        Test logging output proper when verbose and debug
+        is off.
+        """
+
+        self.cleanup()
+        api_port, reg_port, conf_file = self.start_servers(debug=False,
+                                                           verbose=False)
+
+        self.assertTrue(os.path.exists(self.api_log_file))
+
+        api_log_out = open(self.api_log_file, 'r').read()
+
+        self.assertFalse('DEBUG [glance-api]' in api_log_out)
+
+        self.assertTrue(os.path.exists(self.registry_log_file))
+
+        registry_log_out = open(self.registry_log_file, 'r').read()
+
+        self.assertFalse('DEBUG [glance-registry]' in registry_log_out)
 
         self.stop_servers()