Unbreak hostname.RedHatStrategy (#76032)

- On Python 3, decode bytes to str
- Tolerate whitespace around the line.
- Report a missing HOSTNAME line as an error.
- Don't clobber line separators when setting a new hostname.
- Add some tests for this strategy to verify correct operation.

lib/ansible/modules/hostname.py

@@ -78,7 +78,7 @@ from ansible.module_utils.basic import (
 )
 from ansible.module_utils.common.sys_info import get_platform_subclass
 from ansible.module_utils.facts.system.service_mgr import ServiceMgrFactCollector
-from ansible.module_utils.facts.utils import get_file_lines
+from ansible.module_utils.facts.utils import get_file_lines, get_file_content
 from ansible.module_utils._text import to_native, to_text
 from ansible.module_utils.six import PY3, text_type
 
@@ -290,9 +290,13 @@ class RedHatStrategy(BaseStrategy):
     def get_permanent_hostname(self):
         try:
             for line in get_file_lines(self.NETWORK_FILE):
+                line = to_native(line).strip()
                 if line.startswith('HOSTNAME'):
                     k, v = line.split('=')
                     return v.strip()
+            self.module.fail_json(
+                "Unable to locate HOSTNAME entry in %s" % self.NETWORK_FILE
+            )
         except Exception as e:
             self.module.fail_json(
                 msg="failed to read hostname: %s" % to_native(e),
@@ -302,8 +306,10 @@ class RedHatStrategy(BaseStrategy):
         try:
             lines = []
             found = False
-            for line in get_file_lines(self.NETWORK_FILE):
+            content = get_file_content(self.NETWORK_FILE, strip=False) or ""
-                if line.startswith('HOSTNAME'):
+            for line in content.splitlines(True):
+                line = to_native(line)
+                if line.strip().startswith('HOSTNAME'):
                     lines.append("HOSTNAME=%s\n" % name)
                     found = True
                 else:

test/units/modules/test_hostname.py

@@ -1,6 +1,10 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+import os
+import shutil
+import tempfile
+
 from units.compat.mock import patch, MagicMock, mock_open
 from ansible.module_utils import basic
 from ansible.module_utils.common._utils import get_all_subclasses
@@ -33,3 +37,98 @@ class TestHostname(ModuleTestCase):
                 self.assertFalse(
                     m.return_value.write.called,
                     msg='%s called write, should not have' % str(cls))
+
+
+class TestRedhatStrategy(ModuleTestCase):
+    def setUp(self):
+        super(TestRedhatStrategy, self).setUp()
+        self.testdir = tempfile.mkdtemp(prefix='ansible-test-hostname-')
+        self.network_file = os.path.join(self.testdir, "network")
+
+    def tearDown(self):
+        super(TestRedhatStrategy, self).tearDown()
+        shutil.rmtree(self.testdir, ignore_errors=True)
+
+    @property
+    def instance(self):
+        self.module = MagicMock()
+        instance = hostname.RedHatStrategy(self.module)
+        instance.NETWORK_FILE = self.network_file
+        return instance
+
+    def test_get_permanent_hostname_missing(self):
+        self.assertIsNone(self.instance.get_permanent_hostname())
+        self.assertTrue(self.module.fail_json.called)
+        self.module.fail_json.assert_called_with(
+            "Unable to locate HOSTNAME entry in %s" % self.network_file
+        )
+
+    def test_get_permanent_hostname_line_missing(self):
+        with open(self.network_file, "w") as f:
+            f.write("# some other content\n")
+        self.assertIsNone(self.instance.get_permanent_hostname())
+        self.module.fail_json.assert_called_with(
+            "Unable to locate HOSTNAME entry in %s" % self.network_file
+        )
+
+    def test_get_permanent_hostname_existing(self):
+        with open(self.network_file, "w") as f:
+            f.write(
+                "some other content\n"
+                "HOSTNAME=foobar\n"
+                "more content\n"
+            )
+        self.assertEqual(self.instance.get_permanent_hostname(), "foobar")
+
+    def test_get_permanent_hostname_existing_whitespace(self):
+        with open(self.network_file, "w") as f:
+            f.write(
+                "some other content\n"
+                "     HOSTNAME=foobar   \n"
+                "more content\n"
+            )
+        self.assertEqual(self.instance.get_permanent_hostname(), "foobar")
+
+    def test_set_permanent_hostname_missing(self):
+        self.instance.set_permanent_hostname("foobar")
+        with open(self.network_file) as f:
+            self.assertEqual(f.read(), "HOSTNAME=foobar\n")
+
+    def test_set_permanent_hostname_line_missing(self):
+        with open(self.network_file, "w") as f:
+            f.write("# some other content\n")
+        self.instance.set_permanent_hostname("foobar")
+        with open(self.network_file) as f:
+            self.assertEqual(f.read(), "# some other content\nHOSTNAME=foobar\n")
+
+    def test_set_permanent_hostname_existing(self):
+        with open(self.network_file, "w") as f:
+            f.write(
+                "some other content\n"
+                "HOSTNAME=spam\n"
+                "more content\n"
+            )
+        self.instance.set_permanent_hostname("foobar")
+        with open(self.network_file) as f:
+            self.assertEqual(
+                f.read(),
+                "some other content\n"
+                "HOSTNAME=foobar\n"
+                "more content\n"
+            )
+
+    def test_set_permanent_hostname_existing_whitespace(self):
+        with open(self.network_file, "w") as f:
+            f.write(
+                "some other content\n"
+                "     HOSTNAME=spam   \n"
+                "more content\n"
+            )
+        self.instance.set_permanent_hostname("foobar")
+        with open(self.network_file) as f:
+            self.assertEqual(
+                f.read(),
+                "some other content\n"
+                "HOSTNAME=foobar\n"
+                "more content\n"
+            )