Merge ~rjschwei/cloud-init:baseNetConfTestSUSE into cloud-init:master

[Robert Schweikert]

It's a start a basic test for network configuration on openSUSE & SLES

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:8ac72b3d6c9188114685c5e0ecdd84a23faf4dd5
https://jenkins.ubuntu.com/server/job/cloud-init-ci/496/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    FAILED: MAAS Compatability Testing

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/496/rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:70a8e1d54ee7b3e4aad085321612e6bc7a723ace
https://jenkins.ubuntu.com/server/job/cloud-init-ci/501/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    FAILED: MAAS Compatability Testing

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/501/rebuild

review: Needs Fixing (continuous-integration)

[Chad Smith (chad.smith)]

man those exist stacks are stinking in that file. I would prefer if we could move more to either the wrap_and_call approach for handling multiple mock of FileSystemMockingTestCase with patchOS patchUtil calls

here's a pastebin of your test reworked with FilesystemMockingTestCase
http://paste.ubuntu.com/26142306/

[Chad Smith (chad.smith)]

Thanks for this Robert, I marked it work in progress just so it pops back up to top of review queue when you get a chance to look over or respond my patch suggestion. I think longterm we'll do a bit of cleanup of all the nested mocks etc where possible and use eiter FileSystemMockingTestCase or wrap_and_call mocking function.

Please just tick "Needs review' when this branch or discussion is ready for more eyes,

review: Needs Information

[Chad Smith (chad.smith)]

Also the Needs Fixing from CI is something that was fixed in master after this branch so a 'git fetch master' 'git rebase master' should fix that too.

[Robert Schweikert (rjschwei)]

Thanks Chad, and thanks for re-working the test, it is much nicer now.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:14a7b2e6d1a6ccdd488d1bf0b53fc5903d5e01cb
https://jenkins.ubuntu.com/server/job/cloud-init-ci/608/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/608/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

I think if we re-factor the code just a bit we can make this much easier to test.

Heres an example:
  http://paste.ubuntu.com/26164253/

The result then is that we can test 'write_network_from_eni' by simply calling:
 tmpd = self.temp_dir()
 devs = write_network_from_eni("your_eni_blob", target=tmpd)
 self.assertEqual(expected_files_in_tmpd, dir2dict(self.tempd))

There are some comments inline, but I think if you go this route its easier to get
test coverage.

[Robert Schweikert (rjschwei)]

Sorry, I think I am missing something in the bigger picture.

My goal was to add a basic test such that network configuration doesn't break again. This is/was intended to be a crutch until we can get openSUSE/SUSE moved to the sysconfig-renderer concept.

It is/was my impression that _write_network() is supposed to go away. Sorry but I am missing the point where using the method as a redirector moves us into the direction where _write_network() gets eliminated. But as I said maybe I am missing the bigger picture and _write_network() will live forever more, in which case I think we should drop the warning that is written to the log file if the code goes down that path.

Anyway, I am somewhat confused.

[Scott Moser (smoser)]

Robert,
I'm sorry for the confusion. I was reading this purely "on its own".

I think the re-factor makes it considerably easier to test, and thus is useful even if the point is only getting tests for somethign that will be replaced.

Chad pointed out to me that using the module level defines as defaults for the function call does not really make testing easier as those values not mockable. so consider this paste instead. Chad let me know if I understood you wrong.

 http://paste.ubuntu.com/26166233/

[Chad Smith (chad.smith)]

Scott, understood. This refactor makes sense, we can do it either as a part of this branch landing, or you can point me at a trivial followup afterward that we can land right after

[Chad Smith (chad.smith)]

review: Approve

diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py
index 8d0b263..9a62b71 100644
--- a/tests/unittests/test_distros/test_netconfig.py
+++ b/tests/unittests/test_distros/test_netconfig.py
@@ -2,6 +2,8 @@
 
 import os
 from six import StringIO
+import stat
+from textwrap import dedent
 
 try:
     from unittest import mock
@@ -12,13 +14,12 @@ try:
 except ImportError:
     from contextlib2 import ExitStack
 
-from cloudinit.tests.helpers import TestCase
-
 from cloudinit import distros
 from cloudinit.distros.parsers.sys_conf import SysConf
 from cloudinit import helpers
 from cloudinit.net import eni
 from cloudinit import settings
+from cloudinit.tests.helpers import FilesystemMockingTestCase
 from cloudinit import util
 
 
@@ -175,7 +176,7 @@ class WriteBuffer(object):
         return self.buffer.getvalue()
 
 
-class TestNetCfgDistro(TestCase):
+class TestNetCfgDistro(FilesystemMockingTestCase):
 
     frbsd_ifout = """\
 hn0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
@@ -771,4 +772,46 @@ ifconfig_vtnet0="DHCP"
             self.assertCfgEquals(expected_buf, str(write_buf))
             self.assertEqual(write_buf.mode, 0o644)
 
+    def test_simple_write_oopensuse(self):
+        """Opensuse network rendering writes appropriate sysconfg files."""
+        tmpdir = self.tmp_dir()
+        self.patchOS(tmpdir)
+        self.patchUtils(tmpdir)
+        distro = self._get_distro('opensuse')
+
+        distro.apply_network(BASE_NET_CFG, False)
+
+        lo_path = os.path.join(tmpdir, 'etc/sysconfig/network/ifcfg-lo')
+        eth0_path = os.path.join(tmpdir, 'etc/sysconfig/network/ifcfg-eth0')
+        eth1_path = os.path.join(tmpdir, 'etc/sysconfig/network/ifcfg-eth1')
+        expected_cfgs = {
+            lo_path: dedent('''
+                STARTMODE="auto"
+                USERCONTROL="no"
+                FIREWALL="no"
+                '''),
+            eth0_path: dedent('''
+                BOOTPROTO="static"
+                BROADCAST="192.168.1.0"
+                GATEWAY="192.168.1.254"
+                IPADDR="192.168.1.5"
+                NETMASK="255.255.255.0"
+                STARTMODE="auto"
+                USERCONTROL="no"
+                ETHTOOL_OPTIONS=""
+                '''),
+            eth1_path: dedent('''
+                BOOTPROTO="dhcp"
+                STARTMODE="auto"
+                USERCONTROL="no"
+                ETHTOOL_OPTIONS=""
+                ''')
+        }
+        for cfgpath in (lo_path, eth0_path, eth1_path):
+            self.assertCfgEquals(
+                expected_cfgs[cfgpath],
+                util.load_file(cfgpath))
+            file_stat = os.stat(cfgpath)
+            self.assertEqual(0o644, stat.S_IMODE(file_stat.st_mode))
+
 # vi: ts=4 expandtab