Merge ~smoser/cloud-init:bug/1703697-python3.6-ubuntu into cloud-init:master

[Scott Moser]

tests: fixes for issues uncovered when moving to python3.6

This includes a few fixes found when testing with python 3.6.
 - fix eni renderer when target is None
   This just uses the util.target_path() in the event that target is None.
 - change test cases to not rely on the cached result of util.get_cmdline()
   and update the base TestCase to unset that cache.
 - mock calls to system_is_snappy from the create_users test cases.
 - drop unused _pp_root in test_simple_run.py

LP: #1703697

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

PASSED: Continuous integration, rev:22f7dc62e99d51d1f3f4185985141841de4d11db
https://jenkins.ubuntu.com/server/job/cloud-init-ci/38/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

[Joshua Powers (powersj)]

LGTM thank you for the fix

review: Approve

[Michael Hudson-Doyle (mwhudson)]

Now I've fixed the python-json-patch thing, this patch is all that's needed to get cloud-init building with python 3.6 as default.

[Scott Moser (smoser)]

We still need one more fix in cloud-init.

test_simple_run.py has some side effects, and makes test_create_users.py (and possibly others) succeed when they should not.
This was found when looking at bug 1704024. It seems that python 3.6 ends
up iterating through tests in a different order than 3.5.

$ python3 -m nose tests/unittests/test_runs/test_simple_run.py tests/unittests/test_distros/test_create_users.py
..........
----------------------------------------------------------------------
Ran 10 tests in 0.065s

OK

$ python3 -m nose tests/unittests/test_distros/test_create_users.pyF........
======================================================================
FAIL: test_basic (tests.unittests.test_distros.test_create_users.TestCreateUser)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/mock/mock.py", line 1305, in patched
    return func(*args, **keywargs)
  File "/home/smoser-public/src/cloud-init/cloud-init/tests/unittests/test_distros/test_create_users.py", line 63, in test_basic
    mock.call(['passwd', '-l', user])])
AssertionError: [call(['systemd-detect-virt', '--quiet', '--conta[116 chars]r'])] != [call(['useradd', 'foouser', '-m'], logstring=['u[57 chars]r'])]
-------------------- >> begin captured logging << --------------------
cloudinit.util: DEBUG: Reading from /etc/os-release (quiet=True)
cloudinit.util: DEBUG: Read 407 bytes from /etc/os-release
cloudinit.util: DEBUG: Reading from /proc/1/cmdline (quiet=False)
cloudinit.util: DEBUG: Read 47 bytes from /proc/1/cmdline
cloudinit.util: DEBUG: Reading from /etc/system-image/channel.ini (quiet=True)
cloudinit.util: DEBUG: Read 0 bytes from /etc/system-image/channel.ini
cloudinit.distros: DEBUG: Adding user foouser
--------------------- >> end captured logging << ---------------------

----------------------------------------------------------------------
Ran 9 tests in 0.023s

FAILED (failures=1)

2c60ffb... by Scott Moser on 2017-07-17

test_create_users.py: correctly mock system_is_snappy.

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

PASSED: Continuous integration, rev:c2145d3b0f1bdac12d6e8cd5f6b3f3227c0eb8cb
https://jenkins.ubuntu.com/server/job/cloud-init-ci/44/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

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

PASSED: Continuous integration, rev:895856e7412e3f72bfc97b15f0489dae39559465
https://jenkins.ubuntu.com/server/job/cloud-init-ci/45/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

008de1d... by Scott Moser on 2017-07-17

remove unused code in test_simple_run

5837847... by Scott Moser on 2017-07-17

reset cached globals in util to initial values from setUp.

PROC_CMDLINE, _DNS_REDIRECT_IP, LSB_RELEASE are the globals
we have right now that get cached.

For test execution, these should have a consistent state rather
that state depending on the order of test execution.

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

PASSED: Continuous integration, rev:7ac6f05b6ca5d5ac568264acb9262c271d436c9f
https://jenkins.ubuntu.com/server/job/cloud-init-ci/46/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

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

PASSED: Continuous integration, rev:c392b94fde1b7b538fc52b478aae2d2d0d5fcd51
https://jenkins.ubuntu.com/server/job/cloud-init-ci/48/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

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

PASSED: Continuous integration, rev:5837847479ff837e17546245cefaec4ee2097466
https://jenkins.ubuntu.com/server/job/cloud-init-ci/51/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

Good fixes, when I got the stale env out of my way all looks good. Thanks for the cleanup of the patch decorator at the top of the class instead of all the repeated patch declarations per unit test.

review: Approve

diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py
index 6754330..9f35b72 100644
--- a/cloudinit/net/netplan.py
+++ b/cloudinit/net/netplan.py
@@ -209,7 +209,8 @@ class Renderer(renderer.Renderer):
         # check network state for version
         # if v2, then extract network_state.config
         # else render_v2_from_state
-        fpnplan = os.path.join(target, self.netplan_path)
+        fpnplan = os.path.join(util.target_path(target), self.netplan_path)
+
         util.ensure_dir(os.path.dirname(fpnplan))
         header = self.netplan_header if self.netplan_header else ""
 
diff --git a/tests/unittests/helpers.py b/tests/unittests/helpers.py
index 569f1ae..6691cf8 100644
--- a/tests/unittests/helpers.py
+++ b/tests/unittests/helpers.py
@@ -82,7 +82,26 @@ def retarget_many_wrapper(new_base, am, old_func):
 
 
 class TestCase(unittest2.TestCase):
-    pass
+    def reset_global_state(self):
+        """Reset any global state to its original settings.
+
+        cloudinit caches some values in cloudinit.util.  Unit tests that
+        involved those cached paths were then subject to failure if the order
+        of invocation changed (LP: #1703697).
+
+        This function resets any of these global state variables to their
+        initial state.
+
+        In the future this should really be done with some registry that
+        can then be cleaned in a more obvious way.
+        """
+        util.PROC_CMDLINE = None
+        util._DNS_REDIRECT_IP = None
+        util._LSB_RELEASE = {}
+
+    def setUp(self):
+        super(unittest2.TestCase, self).setUp()
+        self.reset_global_state()
 
 
 class CiTestCase(TestCase):
diff --git a/tests/unittests/test_distros/test_create_users.py b/tests/unittests/test_distros/test_create_users.py
index 9ded4f6..1d02f7b 100644
--- a/tests/unittests/test_distros/test_create_users.py
+++ b/tests/unittests/test_distros/test_create_users.py
@@ -38,6 +38,8 @@ class MyBaseDistro(distros.Distro):
         raise NotImplementedError()
 
 
[e-mail]("cloudinit.distros.util.system_is_snappy", return_value=False)
[e-mail]("cloudinit.distros.util.subp")
 class TestCreateUser(TestCase):
     def setUp(self):
         super(TestCase, self).setUp()
@@ -53,8 +55,7 @@ class TestCreateUser(TestCase):
                 logcmd[i + 1] = 'REDACTED'
         return mock.call(args, logstring=logcmd)
 
-    @mock.patch("cloudinit.distros.util.subp")
-    def test_basic(self, m_subp):
+    def test_basic(self, m_subp, m_is_snappy):
         user = 'foouser'
         self.dist.create_user(user)
         self.assertEqual(
@@ -62,8 +63,7 @@ class TestCreateUser(TestCase):
             [self._useradd2call([user, '-m']),
              mock.call(['passwd', '-l', user])])
 
-    @mock.patch("cloudinit.distros.util.subp")
-    def test_no_home(self, m_subp):
+    def test_no_home(self, m_subp, m_is_snappy):
         user = 'foouser'
         self.dist.create_user(user, no_create_home=True)
         self.assertEqual(
@@ -71,8 +71,7 @@ class TestCreateUser(TestCase):
             [self._useradd2call([user, '-M']),
              mock.call(['passwd', '-l', user])])
 
-    @mock.patch("cloudinit.distros.util.subp")
-    def test_system_user(self, m_subp):
+    def test_system_user(self, m_subp, m_is_snappy):
         # system user should have no home and get --system
         user = 'foouser'
         self.dist.create_user(user, system=True)
@@ -81,8 +80,7 @@ class TestCreateUser(TestCase):
             [self._useradd2call([user, '--system', '-M']),
              mock.call(['passwd', '-l', user])])
 
-    @mock.patch("cloudinit.distros.util.subp")
-    def test_explicit_no_home_false(self, m_subp):
+    def test_explicit_no_home_false(self, m_subp, m_is_snappy):
         user = 'foouser'
         self.dist.create_user(user, no_create_home=False)
         self.assertEqual(
@@ -90,16 +88,14 @@ class TestCreateUser(TestCase):
             [self._useradd2call([user, '-m']),
              mock.call(['passwd', '-l', user])])
 
-    @mock.patch("cloudinit.distros.util.subp")
-    def test_unlocked(self, m_subp):
+    def test_unlocked(self, m_subp, m_is_snappy):
         user = 'foouser'
         self.dist.create_user(user, lock_passwd=False)
         self.assertEqual(
             m_subp.call_args_list,
             [self._useradd2call([user, '-m'])])
 
-    @mock.patch("cloudinit.distros.util.subp")
-    def test_set_password(self, m_subp):
+    def test_set_password(self, m_subp, m_is_snappy):
         user = 'foouser'
         password = 'passfoo'
         self.dist.create_user(user, passwd=password)
@@ -109,8 +105,7 @@ class TestCreateUser(TestCase):
              mock.call(['passwd', '-l', user])])
 
     @mock.patch("cloudinit.distros.util.is_group")
-    @mock.patch("cloudinit.distros.util.subp")
-    def test_group_added(self, m_subp, m_is_group):
+    def test_group_added(self, m_is_group, m_subp, m_is_snappy):
         m_is_group.return_value = False
         user = 'foouser'
         self.dist.create_user(user, groups=['group1'])
@@ -121,8 +116,7 @@ class TestCreateUser(TestCase):
         self.assertEqual(m_subp.call_args_list, expected)
 
     @mock.patch("cloudinit.distros.util.is_group")
-    @mock.patch("cloudinit.distros.util.subp")
-    def test_only_new_group_added(self, m_subp, m_is_group):
+    def test_only_new_group_added(self, m_is_group, m_subp, m_is_snappy):
         ex_groups = ['existing_group']
         groups = ['group1', ex_groups[0]]
         m_is_group.side_effect = lambda m: m in ex_groups
@@ -135,8 +129,8 @@ class TestCreateUser(TestCase):
         self.assertEqual(m_subp.call_args_list, expected)
 
     @mock.patch("cloudinit.distros.util.is_group")
-    @mock.patch("cloudinit.distros.util.subp")
-    def test_create_groups_with_whitespace_string(self, m_subp, m_is_group):
+    def test_create_groups_with_whitespace_string(
+            self, m_is_group, m_subp, m_is_snappy):
         # groups supported as a comma delimeted string even with white space
         m_is_group.return_value = False
         user = 'foouser'
diff --git a/tests/unittests/test_runs/test_simple_run.py b/tests/unittests/test_runs/test_simple_run.py
index 3132420..55f15b5 100644
--- a/tests/unittests/test_runs/test_simple_run.py
+++ b/tests/unittests/test_runs/test_simple_run.py
@@ -16,24 +16,6 @@ class TestSimpleRun(helpers.FilesystemMockingTestCase):
         self.patchOS(root)
         self.patchUtils(root)
 
-    def _pp_root(self, root, repatch=True):
-        for (dirpath, dirnames, filenames) in os.walk(root):
-            print(dirpath)
-            for f in filenames:
-                joined = os.path.join(dirpath, f)
-                if os.path.islink(joined):
-                    print("f %s - (symlink)" % (f))
-                else:
-                    print("f %s" % (f))
-            for d in dirnames:
-                joined = os.path.join(dirpath, d)
-                if os.path.islink(joined):
-                    print("d %s - (symlink)" % (d))
-                else:
-                    print("d %s" % (d))
-        if repatch:
-            self._patchIn(root)
-
     def test_none_ds(self):
         new_root = tempfile.mkdtemp()
         self.addCleanup(shutil.rmtree, new_root)