Merge ~jasonzio/cloud-init:mockSLG into cloud-init:master

[Jason Zions]

test_azure: mock util.SeLinuxGuard where needed

Mock util.SeLinuxGuard to do nothing within tests that mock functions
used by the guard, when those mocks confuse the guard. This has no
impact when executing unit tests on systems which do not enable selinux
(e.g. Ubuntu).

LP: #1825253

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

FAILED: Continuous integration, rev:e42320f4baf9764d85c41a97afd17ae4de368ee1
https://jenkins.ubuntu.com/server/job/cloud-init-ci/681/
Executed test runs:
    FAILED: Checkout

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

review: Needs Fixing (continuous-integration)

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

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

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

review: Approve (continuous-integration)

[Dan Watkins (oddbloke)]

I've launched a CentOS VM and confirmed that there were test failures in master, but not with this branch. Thanks!

(I think it's worth noting that if you run tests in a virtualenv or via tox, you shouldn't hit this issue because `import selinux` won't find anything; this is certainly still worth merging to fix the non-virtualenv case!)

review: Approve

[Ryan Harper (raharper)]

Two questions.

1) I think we want to mock this in CiTestCase (cloudinit/tests/helpers.py)
2) Is it not sufficient to set mock_sel.return_value = False?

review: Needs Information

[Jason Zions (jasonzio)]

I didn't try just setting mock_sel.return_value to be false. SeLinuxGuard is a context manager class, and I didn't want this mock to change that; I just wanted to mock the context manager to act as if selinux was absent.

Putting the mock in CiTestCase would render SeLinuxGuard impotent in all unittests, even ones in which there was no filesystem mocking that would otherwise confuse it. I thought it important that the guard context manager be exercised except when it was being lied to in a manner that caused it to misbehave.

As Dan pointed out, running under tox or in a typical virtualenv, SeLinuxGuard does nothing anyway. But anyone developing cloud-init to run in production on a RedHat-based distro will want to run the unit tests in the same environment as cloud-init will run in deployment; it seems best to me that we "turn off" as little as possible when running that way.

-----Original Message-----
From: <email address hidden> <email address hidden> On Behalf Of Ryan Harper
Sent: Thursday, April 18, 2019 8:55 AM
To: <email address hidden>
Subject: Re: [Merge] ~jasonzio/cloud-init:mockSLG into cloud-init:master

Review: Needs Information

Two questions.

1) I think we want to mock this in CiTestCase (cloudinit/tests/helpers.py)
2) Is it not sufficient to set mock_sel.return_value = False?

--
https://nam06.safelinks.protection.outlook.com/?url=https:%2F%2Fcode.launchpad.net%2F~jasonzio%2Fcloud-init%2F%2Bgit%2Fcloud-init%2F%2Bmerge%2F366236&amp;data=02%7C01%7Cjason.zions%40microsoft.com%7C7ac714c1846a4d1c35f908d6c4163a55%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C1%7C636911997241162813&amp;sdata=TBPlUvt196psSU41WIk8tCzl4ohHTV1bi6XGBKa8mNY%3D&amp;reserved=0
You are the owner of ~jasonzio/cloud-init:mockSLG.

[Ryan Harper (raharper)]

Thanks for the comments; that helps.

I think we can defer putting that into CiTestCase until we have more cases where we'd see misbehavior and then we could have the testcase enabling it via class boolean, like with_logs, etc. Disabled by default, enabled when needed.

review: Approve

diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
index 53c56cd..ab77c03 100644
--- a/tests/unittests/test_datasource/test_azure.py
+++ b/tests/unittests/test_datasource/test_azure.py
@@ -1375,12 +1375,15 @@ class TestCanDevBeReformatted(CiTestCase):
         self._domock(p + "util.mount_cb", 'm_mount_cb')
         self._domock(p + "os.path.realpath", 'm_realpath')
         self._domock(p + "os.path.exists", 'm_exists')
+        self._domock(p + "util.SeLinuxGuard", 'm_selguard')
 
         self.m_exists.side_effect = lambda p: p in bypath
         self.m_realpath.side_effect = realpath
         self.m_has_ntfs_filesystem.side_effect = has_ntfs_fs
         self.m_mount_cb.side_effect = mount_cb
         self.m_partitions_on_device.side_effect = partitions_on_device
+        self.m_selguard.__enter__ = mock.Mock(return_value=False)
+        self.m_selguard.__exit__ = mock.Mock()
 
     def test_three_partitions_is_false(self):
         """A disk with 3 partitions can not be formatted."""
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index fd03deb..ca6ef97 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -3269,9 +3269,12 @@ class TestNetplanPostcommands(CiTestCase):
         mock_netplan_generate.assert_called_with(run=True)
         mock_net_setup_link.assert_called_with(run=True)
 
+    @mock.patch('cloudinit.util.SeLinuxGuard')
     @mock.patch.object(netplan, "get_devicelist")
     @mock.patch('cloudinit.util.subp')
-    def test_netplan_postcmds(self, mock_subp, mock_devlist):
+    def test_netplan_postcmds(self, mock_subp, mock_devlist, mock_sel):
+        mock_sel.__enter__ = mock.Mock(return_value=False)
+        mock_sel.__exit__ = mock.Mock()
         mock_devlist.side_effect = [['lo']]
         tmp_dir = self.tmp_dir()
         ns = network_state.parse_net_config_data(self.mycfg,