Merge ~blake-rouse/maas:debug-false-means-false-2.6 into maas:2.6

[Blake Rouse]

Fixes LP: #1724096 - Part 2 - Fix regiond.conf and rackd.conf parser to handle StringBool correctly for reading the value from the store.

Backport of f01a97d004b6775cfa3975f1d240ef5de3b479b3.

[Blake Rouse (blake-rouse)]

Self-approving backport.

review: Approve

diff --git a/src/maasserver/config.py b/src/maasserver/config.py
index 1eabe0d..4b0b75f 100644
--- a/src/maasserver/config.py
+++ b/src/maasserver/config.py
@@ -7,10 +7,7 @@ __all__ = [
     "RegionConfiguration",
 ]
 
-from formencode.validators import (
-    Int,
-    StringBool,
-)
+from formencode.validators import Int
 from provisioningserver.config import (
     Configuration,
     ConfigurationFile,
@@ -19,6 +16,7 @@ from provisioningserver.config import (
 )
 from provisioningserver.utils.config import (
     ExtendedURL,
+    OneWayStringBool,
     UnicodeString,
 )
 
@@ -61,7 +59,7 @@ class RegionConfiguration(Configuration, metaclass=RegionConfigurationMeta):
     database_keepalive = ConfigurationOption(
         "database_keepalive",
         "Whether keepalive for database connections is enabled.",
-        StringBool(if_missing=True))
+        OneWayStringBool(if_missing=True))
     database_keepalive_idle = ConfigurationOption(
         "database_keepalive_idle",
         "Time (in seconds) after which keepalives will be started.",
@@ -83,14 +81,14 @@ class RegionConfiguration(Configuration, metaclass=RegionConfigurationMeta):
     # Debug options.
     debug = ConfigurationOption(
         "debug", "Enable debug mode for detailed error and log reporting.",
-        StringBool(if_missing=False))
+        OneWayStringBool(if_missing=False))
     debug_queries = ConfigurationOption(
         "debug_queries",
         "Enable query debugging. Reports number of queries and time for all "
         "actions performed. Requires debug to also be True. mode for detailed "
         "error and log reporting.",
-        StringBool(if_missing=False))
+        OneWayStringBool(if_missing=False))
     debug_http = ConfigurationOption(
         "debug_http",
         "Enable HTTP debugging. Logs all HTTP requests and HTTP responses.",
-        StringBool(if_missing=False))
+        OneWayStringBool(if_missing=False))
diff --git a/src/maasserver/management/commands/tests/test_config.py b/src/maasserver/management/commands/tests/test_config.py
index e738265..0e0b3f9 100644
--- a/src/maasserver/management/commands/tests/test_config.py
+++ b/src/maasserver/management/commands/tests/test_config.py
@@ -126,7 +126,7 @@ class TestConfigurationSet(MAASTestCase):
         elif self.option in [
                 "debug", "debug_queries", "debug_http",
                 "database_keepalive"]:
-            value = random.choice(['true', 'false'])
+            value = random.choice([True, False])
         else:
             value = factory.make_name("foobar")
 
diff --git a/src/maasserver/tests/test_config.py b/src/maasserver/tests/test_config.py
index 1878e10..51b129a 100644
--- a/src/maasserver/tests/test_config.py
+++ b/src/maasserver/tests/test_config.py
@@ -94,7 +94,7 @@ class TestRegionConfigurationDatabaseOptions(MAASTestCase):
         if isinstance(getattr(config, self.option), str):
             example_value = factory.make_name(self.option)
         elif self.option == 'database_keepalive':
-            example_value = random.choice(['true', 'false'])
+            example_value = random.choice([True, False])
         else:
             example_value = factory.pick_port()
         # Argument values will most often be passed in from the command-line,
diff --git a/src/provisioningserver/config.py b/src/provisioningserver/config.py
index 52a3766..d973019 100644
--- a/src/provisioningserver/config.py
+++ b/src/provisioningserver/config.py
@@ -140,13 +140,13 @@ from formencode.declarative import DeclarativeMeta
 from formencode.validators import (
     Number,
     Set,
-    StringBool,
 )
 from provisioningserver.path import get_tentative_data_path
 from provisioningserver.utils import typed
 from provisioningserver.utils.config import (
     DirectoryString,
     ExtendedURL,
+    OneWayStringBool,
     UnicodeString,
     UUIDString,
 )
@@ -795,7 +795,7 @@ class ClusterConfiguration(Configuration, metaclass=ClusterConfigurationMeta):
     # Debug options.
     debug = ConfigurationOption(
         "debug", "Enable debug mode for detailed error and log reporting.",
-        StringBool(if_missing=False))
+        OneWayStringBool(if_missing=False))
 
 
 def is_dev_environment():
diff --git a/src/provisioningserver/tests/test_cluster_config_command.py b/src/provisioningserver/tests/test_cluster_config_command.py
index 7f50836..2a3050e 100644
--- a/src/provisioningserver/tests/test_cluster_config_command.py
+++ b/src/provisioningserver/tests/test_cluster_config_command.py
@@ -226,7 +226,7 @@ class TestUpdateMaasClusterConf(MAASTestCase):
         cluster_config_command.run(self.make_args(debug=expected))
         with ClusterConfiguration.open() as config:
             observed = config.debug
-        self.assertEqual(str(expected).lower(), observed)
+        self.assertEqual(expected, observed)
 
     def test_config_set_debug_without_setting_does_nothing(self):
         with ClusterConfiguration.open() as config:
diff --git a/src/provisioningserver/tests/test_config.py b/src/provisioningserver/tests/test_config.py
index 962c94e..2532bf3 100644
--- a/src/provisioningserver/tests/test_config.py
+++ b/src/provisioningserver/tests/test_config.py
@@ -662,6 +662,30 @@ class TestClusterConfiguration(MAASTestCase):
         # It's also stored in the configuration database.
         self.assertEqual({"cluster_uuid": str(example_uuid)}, config.store)
 
+    def test_default_debug(self):
+        config = ClusterConfiguration({})
+        self.assertFalse(config.debug)
+
+    def test_set_and_get_debug_false(self):
+        config = ClusterConfiguration({})
+        config.debug = False
+        self.assertFalse(config.debug)
+
+    def test_set_and_get_debug_false_str(self):
+        config = ClusterConfiguration({})
+        config.debug = "false"
+        self.assertFalse(config.debug)
+
+    def test_set_and_get_debug_true(self):
+        config = ClusterConfiguration({})
+        config.debug = True
+        self.assertTrue(config.debug)
+
+    def test_set_and_get_debug_true_str(self):
+        config = ClusterConfiguration({})
+        config.debug = "true"
+        self.assertTrue(config.debug)
+
 
 class TestClusterConfigurationGRUBRoot(MAASTestCase):
     """Tests for `ClusterConfiguration.grub_root`."""
diff --git a/src/provisioningserver/utils/config.py b/src/provisioningserver/utils/config.py
index b0985f2..196dbca 100644
--- a/src/provisioningserver/utils/config.py
+++ b/src/provisioningserver/utils/config.py
@@ -185,3 +185,15 @@ class Schema(formencode.Schema):
             return False
         else:
             return super(Schema, self)._value_is_iterator(value)
+
+
+class OneWayStringBool(formencode.validators.StringBool):
+    """A `StringBool` that doesn't convert a boolean back into a string.
+
+    Used for "true" and "false" values, but doesn't convert a boolean back
+    to a string.
+    """
+
+    def from_python(self, value):
+        """Do nothing."""
+        return value
diff --git a/src/provisioningserver/utils/tests/test_config.py b/src/provisioningserver/utils/tests/test_config.py
index e4dfe7d..4a6674c 100644
--- a/src/provisioningserver/utils/tests/test_config.py
+++ b/src/provisioningserver/utils/tests/test_config.py
@@ -314,3 +314,12 @@ class TestExtendedURL(MAASTestCase):
                 netloc="[%s]" % addr.upper(), port=factory.pick_bool())
             self.assertEqual(
                 url, self.validator.to_python(url), "url: %s" % url)
+
+
+class TestOneWayStringBool(MAASTestCase):
+    """Tests for `OneWayStringBool`."""
+
+    def test__from_python(self):
+        validator = config.OneWayStringBool()
+        self.assertFalse(validator.from_python(False))
+        self.assertTrue(validator.from_python(True))