Truncate node text fields when too long

Pushing too long messages in the node last_error and maintenance reason can
cause node.save() failures, leaving the node in a transient state, with no
conductor actually handling it anymore.

Change-Id: Id4db377781f83cf4d97564ced9622d5a8a8c67af
Story: #2005377
Task: #30359

ironic/conf/default.py

@@ -167,6 +167,9 @@ exc_log_opts = [
                        'an exception message (a programming error). If True, '
                        'raise an exception; if False, use the unformatted '
                        'message.')),
+    cfg.IntOpt('log_in_db_max_size', default=4096,
+               help=_('Max number of characters of any node '
+                      'last_error/maintenance_reason pushed to database.'))
 ]
 
 hash_opts = [

ironic/objects/node.py

@@ -12,11 +12,13 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
-
+from oslo_config import cfg
+from oslo_log import log
 from oslo_utils import strutils
 from oslo_utils import uuidutils
 from oslo_utils import versionutils
 from oslo_versionedobjects import base as object_base
+import six
 
 from ironic.common import exception
 from ironic.common.i18n import _
@@ -29,6 +31,9 @@ from ironic.objects import notification
 
 REQUIRED_INT_PROPERTIES = ['local_gb', 'cpus', 'memory_mb']
 
+CONF = cfg.CONF
+LOG = log.getLogger(__name__)
+
 
 @base.IronicObjectRegistry.register
 class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
@@ -416,6 +421,16 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
                         object, e.g.: Node(context)
         :raises: InvalidParameterValue if some property values are invalid.
         """
+
+        for attr_name in ('last_error', 'maintenance_reason'):
+            attr_value = getattr(self, attr_name, '')
+            if (attr_value and isinstance(attr_value, six.string_types) and
+                    len(attr_value) > CONF.log_in_db_max_size):
+                LOG.info('Truncating too long %s to %s characters for node %s',
+                         attr_name, CONF.log_in_db_max_size, self.uuid)
+                setattr(self, attr_name,
+                        attr_value[0:CONF.log_in_db_max_size])
+
         updates = self.do_version_changes_for_db()
         self._validate_property_values(updates.get('properties'))
         if 'driver' in updates and 'driver_internal_info' not in updates:

ironic/tests/unit/objects/test_node.py

@@ -23,6 +23,7 @@ from testtools import matchers
 from ironic.common import context
 from ironic.common import exception
 from ironic import objects
+from ironic.objects import node as node_objects
 from ironic.tests.unit.db import base as db_base
 from ironic.tests.unit.db import utils as db_utils
 from ironic.tests.unit.objects import utils as obj_utils
@@ -171,6 +172,64 @@ class TestNodeObject(db_base.DbTestCase, obj_utils.SchemasTestMixIn):
                 self.assertEqual(test_time, res_updated_at)
                 self.assertEqual({}, n.driver_internal_info)
 
+    @mock.patch.object(node_objects, 'LOG', autospec=True)
+    def test_save_truncated(self, log_mock):
+        uuid = self.fake_node['uuid']
+        test_time = datetime.datetime(2000, 1, 1, 0, 0)
+        with mock.patch.object(self.dbapi, 'get_node_by_uuid',
+                               autospec=True) as mock_get_node:
+            mock_get_node.return_value = self.fake_node
+            with mock.patch.object(self.dbapi, 'update_node',
+                                   autospec=True) as mock_update_node:
+                mock_update_node.return_value = db_utils.get_test_node(
+                    properties={'fake': 'property'}, driver='fake-driver',
+                    driver_internal_info={}, updated_at=test_time)
+                n = objects.Node.get(self.context, uuid)
+                self.assertEqual({'private_state': 'secret value'},
+                                 n.driver_internal_info)
+                n.properties = {'fake': 'property'}
+                n.driver = 'fake-driver'
+                last_error = 'BOOM' * 2000
+                maintenance_reason = last_error
+                n.last_error = last_error
+                n.maintenance_reason = maintenance_reason
+
+                n.save()
+
+                self.assertEqual([
+                    mock.call.info(
+                        'Truncating too long %s to %s characters for node %s',
+                        'last_error',
+                        node_objects.CONF.log_in_db_max_size,
+                        uuid),
+                    mock.call.info(
+                        'Truncating too long %s to %s characters for node %s',
+                        'maintenance_reason',
+                        node_objects.CONF.log_in_db_max_size,
+                        uuid)],
+                    log_mock.mock_calls)
+
+                mock_get_node.assert_called_once_with(uuid)
+                mock_update_node.assert_called_once_with(
+                    uuid,
+                    {
+                        'properties': {'fake': 'property'},
+                        'driver': 'fake-driver',
+                        'driver_internal_info': {},
+                        'version': objects.Node.VERSION,
+                        'maintenance_reason':
+                            maintenance_reason[
+                                0:node_objects.CONF.log_in_db_max_size],
+                        'last_error':
+                            last_error[
+                            0:node_objects.CONF.log_in_db_max_size]
+                    }
+                )
+                self.assertEqual(self.context, n._context)
+                res_updated_at = (n.updated_at).replace(tzinfo=None)
+                self.assertEqual(test_time, res_updated_at)
+                self.assertEqual({}, n.driver_internal_info)
+
     def test_save_updated_at_field(self):
         uuid = self.fake_node['uuid']
         extra = {"test": 123}

releasenotes/notes/bug-2005377-5c63357681a465ec.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - Fixes node last_error and maintenance_reason overflow with too long error
+    messages, preventing the object from being correctly committed to database.
+    The maximum message length can be customized through a new configuration
+    parameter, ``[DEFAULT]/log_in_db_max_size`` (default, 4096 characters).