Merge pull request #2269 from Azure/release-2.3.0.0

 Merge Release 2.3.0.2 into master

azurelinuxagent/common/persist_firewall_rules.py

@@ -32,13 +32,14 @@
 class PersistFirewallRulesHandler(object):
 
     __SERVICE_FILE_CONTENT = """
-# This unit file was created by the Azure VM Agent.
+# This unit file (Version={version}) was created by the Azure VM Agent.
 # Do not edit.
 [Unit]
 Description=Setup network rules for WALinuxAgent 
 Before=network-pre.target
 Wants=network-pre.target
 DefaultDependencies=no
+ConditionPathExists={binary_path}
 
 [Service]
 Type=oneshot
@@ -67,6 +68,10 @@ class PersistFirewallRulesHandler(object):
 
     _FIREWALLD_RUNNING_CMD = ["firewall-cmd", "--state"]
 
+    # The current version of the unit file; Update it whenever the unit file is modified to ensure Agent can dynamically
+    # modify the unit file on VM too
+    _UNIT_VERSION = "1.2"
+
     @staticmethod
     def get_service_file_path():
         osutil = get_osutil()
@@ -113,6 +118,13 @@ def setup(self):
             # In case of a failure, this would throw. In such a case, we don't need to try to setup our custom service
             # because on system reboot, all iptable rules are reset by firewalld.service so it would be a no-op.
             self._setup_permanent_firewalld_rules()
+
+            # Remove custom service if exists to avoid problems with firewalld
+            try:
+                fileutil.rm_files(*[self.get_service_file_path(), os.path.join(conf.get_lib_dir(), self.BINARY_FILE_NAME)])
+            except Exception as error:
+                logger.info(
+                    "Unable to delete existing service {0}: {1}".format(self._network_setup_service_name, ustr(error)))
             return
 
         logger.info(
@@ -163,12 +175,19 @@ def _setup_network_setup_service(self):
         # the service is always run from the most latest agent.
         self.__setup_binary_file()
 
-        if self.__verify_network_setup_service_enabled():
+        network_service_enabled = self.__verify_network_setup_service_enabled()
+        if network_service_enabled and not self.__unit_file_version_modified():
             logger.info("Service: {0} already enabled. No change needed.".format(self._network_setup_service_name))
             self.__log_network_setup_service_logs()
 
         else:
-            logger.info("Service: {0} not enabled. Adding it now".format(self._network_setup_service_name))
+            if not network_service_enabled:
+                logger.info("Service: {0} not enabled. Adding it now".format(self._network_setup_service_name))
+            else:
+                logger.info(
+                    "Unit file {0} version modified to {1}, setting it up again".format(self.get_service_file_path(),
+                                                                                        self._UNIT_VERSION))
+
             # Create unit file with default values
             self.__set_service_unit_file()
             # Reload systemd configurations when we setup the service for the first time to avoid systemctl warnings
@@ -199,15 +218,17 @@ def __set_service_unit_file(self):
         try:
             fileutil.write_file(service_unit_file,
                                 self.__SERVICE_FILE_CONTENT.format(binary_path=binary_path,
-                                                                   py_path=sys.executable))
+                                                                   py_path=sys.executable,
+                                                                   version=self._UNIT_VERSION))
             fileutil.chmod(service_unit_file, 0o644)
 
             # Finally enable the service. This is needed to ensure the service is started on system boot
             cmd = ["systemctl", "enable", self._network_setup_service_name]
             try:
                 shellutil.run_command(cmd)
             except CommandError as error:
-                msg = "Unable to enable service: {0}; deleting service file: {1}. Command: {2}, Exit-code: {3}.\nstdout: {4}\nstderr: {5}".format(
+                msg = ustr(
+                    "Unable to enable service: {0}; deleting service file: {1}. Command: {2}, Exit-code: {3}.\nstdout: {4}\nstderr: {5}").format(
                     self._network_setup_service_name, service_unit_file, ' '.join(cmd), error.returncode, error.stdout,
                     error.stderr)
                 raise Exception(msg)
@@ -244,7 +265,7 @@ def __verify_network_setup_service_failed(self):
 
     def __log_network_setup_service_logs(self):
         # Get logs from journalctl - https://www.freedesktop.org/software/systemd/man/journalctl.html
-        cmd = ["journalctl", "-u", self._network_setup_service_name, "-b"]
+        cmd = ["journalctl", "-u", self._network_setup_service_name, "-b", "--utc"]
         service_failed = self.__verify_network_setup_service_failed()
         try:
             stdout = shellutil.run_command(cmd)
@@ -273,3 +294,38 @@ def __reload_systemd_conf(self):
             shellutil.run_command(["systemctl", "daemon-reload"])
         except Exception as exception:
             logger.warn("Unable to reload systemctl configurations: {0}".format(ustr(exception)))
+
+    def __get_unit_file_version(self):
+        if not os.path.exists(self.get_service_file_path()):
+            raise OSError("{0} not found".format(self.get_service_file_path()))
+
+        match = fileutil.findre_in_file(self.get_service_file_path(),
+                                        line_re="This unit file \\(Version=([\\d.]+)\\) was created by the Azure VM Agent.")
+        if match is None:
+            raise ValueError("Version tag not found in the unit file")
+
+        return match.group(1).strip()
+
+    def __unit_file_version_modified(self):
+        """
+        Check if the unit file version changed from the expected version
+        :return: True if unit file version changed else False
+        """
+
+        try:
+            unit_file_version = self.__get_unit_file_version()
+        except Exception as error:
+            logger.info("Unable to determine version of unit file: {0}, overwriting unit file".format(ustr(error)))
+            # Since we can't determine the version, marking the file as modified to overwrite the unit file
+            return True
+
+        if unit_file_version != self._UNIT_VERSION:
+            logger.info(
+                "Unit file version: {0} does not match with expected version: {1}, overwriting unit file".format(
+                    unit_file_version, self._UNIT_VERSION))
+            return True
+
+        logger.info(
+            "Unit file version matches with expected version: {0}, not overwriting unit file".format(unit_file_version))
+        return False
+

azurelinuxagent/common/version.py

@@ -196,7 +196,9 @@ def has_logrotate():
 
 AGENT_NAME = "WALinuxAgent"
 AGENT_LONG_NAME = "Azure Linux Agent"
-AGENT_VERSION = '2.2.54.2'
+# Setting the version to 9.9.9.9 to ensure DCR always uses this version and never auto-updates.
+# Replace this with the actual agent version on release.
+AGENT_VERSION = '2.3.0.2'  # (current agent version = 2.3.0.2)
 AGENT_LONG_VERSION = "{0}-{1}".format(AGENT_NAME, AGENT_VERSION)
 AGENT_DESCRIPTION = """
 The Azure Linux Agent supports the provisioning and running of Linux

tests/common/test_persist_firewall_rules.py

@@ -161,6 +161,31 @@ def __mock_network_setup_service_disabled(cmd):
             return True, ["echo", "not enabled"]
         return False, []
 
+    @staticmethod
+    def __mock_firewalld_running_and_not_applied(cmd):
+        if cmd == PersistFirewallRulesHandler._FIREWALLD_RUNNING_CMD:
+            return True, ["echo", "running"]
+        # This is to fail the check if firewalld-rules are already applied
+        cmds_to_fail = ["firewall-cmd", FirewallCmdDirectCommands.QueryPassThrough, "conntrack"]
+        if all(cmd_to_fail in cmd for cmd_to_fail in cmds_to_fail):
+            return True, ["exit", "1"]
+        if "firewall-cmd" in cmd:
+            return True, ["echo", "enabled"]
+        return False, []
+
+    def __setup_and_assert_network_service_setup_scenario(self, handler, mock_popen=None):
+        mock_popen = TestPersistFirewallRulesHandler.__mock_network_setup_service_disabled if mock_popen is None else mock_popen
+        self.__replace_popen_cmd = mock_popen
+        handler.setup()
+
+        self.__assert_systemctl_called(cmd="is-enabled", validate_command_called=True)
+        self.__assert_systemctl_called(cmd="enable", validate_command_called=True)
+        self.__assert_systemctl_reloaded(validate_command_called=True)
+        self.__assert_firewall_cmd_running_called(validate_command_called=True)
+        self.__assert_firewall_called(cmd=FirewallCmdDirectCommands.QueryPassThrough, validate_command_called=False)
+        self.__assert_firewall_called(cmd=FirewallCmdDirectCommands.PassThrough, validate_command_called=False)
+        self.assertTrue(os.path.exists(handler.get_service_file_path()), "Service unit file not found")
+
     def test_it_should_skip_setup_if_firewalld_already_enabled(self):
         self.__replace_popen_cmd = lambda cmd: ("firewall-cmd" in cmd, ["echo", "running"])
         with self._get_persist_firewall_rules_handler() as handler:
@@ -173,19 +198,27 @@ def test_it_should_skip_setup_if_firewalld_already_enabled(self):
         # Assert no commands for systemctl were called
         self.assertFalse(any("systemctl" in cmd for cmd in self.__executed_commands), "Systemctl shouldn't be called")
 
-    def test_it_should_skip_setup_if_agent_network_setup_service_already_enabled(self):
-        self.__replace_popen_cmd = TestPersistFirewallRulesHandler.__mock_network_setup_service_enabled
+    def test_it_should_skip_setup_if_agent_network_setup_service_already_enabled_and_version_same(self):
+
         with self._get_persist_firewall_rules_handler() as handler:
+            # 1st time should setup the service
+            self.__setup_and_assert_network_service_setup_scenario(handler)
+
+            # 2nd time setup should do nothing as service is enabled and no version updated
+            self.__replace_popen_cmd = TestPersistFirewallRulesHandler.__mock_network_setup_service_enabled
+            # Reset state
+            self.__executed_commands = []
             handler.setup()
 
-        self.__assert_systemctl_called(cmd="is-enabled", validate_command_called=True)
-        self.__assert_systemctl_called(cmd="enabled", validate_command_called=False)
-        self.__assert_systemctl_reloaded(validate_command_called=False)
-        self.__assert_firewall_cmd_running_called(validate_command_called=True)
-        self.__assert_firewall_called(cmd=FirewallCmdDirectCommands.QueryPassThrough, validate_command_called=False)
-        self.__assert_firewall_called(cmd=FirewallCmdDirectCommands.PassThrough, validate_command_called=False)
+            self.__assert_systemctl_called(cmd="is-enabled", validate_command_called=True)
+            self.__assert_systemctl_called(cmd="enable", validate_command_called=False)
+            self.__assert_systemctl_reloaded(validate_command_called=False)
+            self.__assert_firewall_cmd_running_called(validate_command_called=True)
+            self.__assert_firewall_called(cmd=FirewallCmdDirectCommands.QueryPassThrough, validate_command_called=False)
+            self.__assert_firewall_called(cmd=FirewallCmdDirectCommands.PassThrough, validate_command_called=False)
+            self.assertTrue(os.path.exists(handler.get_service_file_path()), "Service unit file not found")
 
-    def test_it_should_always_replace_only_drop_in_file_if_using_custom_network_service(self):
+    def test_it_should_always_replace_binary_file_only_if_using_custom_network_service(self):
 
         def _find_in_file(file_name, line_str):
             try:
@@ -207,7 +240,7 @@ def _find_in_file(file_name, line_str):
             handler.setup()
 
         orig_service_file_contents = "ExecStart={py_path} {binary_path}".format(py_path=sys.executable,
-                                                                                 binary_path=self._binary_file)
+                                                                                binary_path=self._binary_file)
         self.__assert_systemctl_called(cmd="is-enabled", validate_command_called=True)
         self.__assert_firewall_called(cmd=FirewallCmdDirectCommands.PassThrough, validate_command_called=False)
         self.assertTrue(os.path.exists(self._binary_file), "Binary file should be there")
@@ -226,7 +259,7 @@ def _find_in_file(file_name, line_str):
         # The service should say its enabled now
         self.__replace_popen_cmd = TestPersistFirewallRulesHandler.__mock_network_setup_service_enabled
         with self._get_persist_firewall_rules_handler() as handler:
-            # The Drop-in file should be available on the 2nd run
+            # The Binary file should be available on the 2nd run
             self.assertTrue(os.path.exists(self._binary_file), "Binary file should be there")
             handler.setup()
 
@@ -241,18 +274,7 @@ def _find_in_file(file_name, line_str):
 
     def test_it_should_use_firewalld_if_available(self):
 
-        def __mock_firewalld_running_and_not_applied(cmd):
-            if cmd == PersistFirewallRulesHandler._FIREWALLD_RUNNING_CMD:
-                return True, ["echo", "running"]
-            # This is to fail the check if firewalld-rules are already applied
-            cmds_to_fail = ["firewall-cmd", FirewallCmdDirectCommands.QueryPassThrough, "conntrack"]
-            if all(cmd_to_fail in cmd for cmd_to_fail in cmds_to_fail):
-                return True, ["exit", "1"]
-            if "firewall-cmd" in cmd:
-                return True, ["echo", "enabled"]
-            return False, []
-
-        self.__replace_popen_cmd = __mock_firewalld_running_and_not_applied
+        self.__replace_popen_cmd = self.__mock_firewalld_running_and_not_applied
         with self._get_persist_firewall_rules_handler() as handler:
             handler.setup()
 
@@ -321,3 +343,37 @@ def test_it_should_not_fail_if_egg_not_found(self):
                 expected_str = "{0} file not found, skipping execution of firewall execution setup for this boot".format(
                     os.path.join(os.getcwd(), test_str))
                 self.assertIn(expected_str, output, "Unexpected output")
+
+    def test_it_should_delete_custom_service_files_if_firewalld_enabled(self):
+        with self._get_persist_firewall_rules_handler() as handler:
+            # 1st run - Setup the Custom Service
+            self.__setup_and_assert_network_service_setup_scenario(handler)
+
+            # 2nd run - Enable Firewalld and ensure the agent sets firewall rules using firewalld and deletes custom service
+            self.__executed_commands = []
+            self.__replace_popen_cmd = self.__mock_firewalld_running_and_not_applied
+            handler.setup()
+
+            self.__assert_firewall_cmd_running_called(validate_command_called=True)
+            self.__assert_firewall_called(cmd=FirewallCmdDirectCommands.QueryPassThrough, validate_command_called=True)
+            self.__assert_firewall_called(cmd=FirewallCmdDirectCommands.PassThrough, validate_command_called=True)
+            self.__assert_systemctl_called(cmd="is-enabled", validate_command_called=False)
+            self.__assert_systemctl_called(cmd="enable", validate_command_called=False)
+            self.__assert_systemctl_reloaded(validate_command_called=False)
+            self.assertFalse(os.path.exists(handler.get_service_file_path()), "Service unit file found")
+            self.assertFalse(os.path.exists(os.path.join(conf.get_lib_dir(), handler.BINARY_FILE_NAME)), "Binary file found")
+
+    def test_it_should_reset_service_unit_files_if_version_changed(self):
+        with self._get_persist_firewall_rules_handler() as handler:
+            # 1st step - Setup the service with old Version
+            test_ver = str(uuid.uuid4())
+            with patch.object(handler, "_UNIT_VERSION", test_ver):
+                self.__setup_and_assert_network_service_setup_scenario(handler)
+                self.assertIn(test_ver, fileutil.read_file(handler.get_service_file_path()), "Test version not found")
+
+            # 2nd step - Re-run the setup and ensure the service file set up again even if service enabled
+            self.__executed_commands = []
+            self.__setup_and_assert_network_service_setup_scenario(handler,
+                                                                   mock_popen=self.__mock_network_setup_service_enabled)
+            self.assertNotIn(test_ver, fileutil.read_file(handler.get_service_file_path()),
+                             "Test version found incorrectly")

tests/ga/test_update.py

@@ -144,8 +144,8 @@ def _get_agent_version():
     def agent_bin(self, version, suffix):
         return "bin/{0}-{1}{2}.egg".format(AGENT_NAME, version, suffix)
 
-    def rename_agent_bin(self, path, src_v, dst_v):
-        src_bin = glob.glob(os.path.join(path, self.agent_bin(src_v, '*')))[0]
+    def rename_agent_bin(self, path, dst_v):
+        src_bin = glob.glob(os.path.join(path, self.agent_bin("*.*.*.*", '*')))[0]
         dst_bin = os.path.join(path, self.agent_bin(dst_v, ''))
         shutil.move(src_bin, dst_bin)
 
@@ -220,7 +220,7 @@ def prepare_agent(self, version):
         if from_path != to_path:
             shutil.move(from_path + ".zip", to_path + ".zip")
             shutil.move(from_path, to_path)
-            self.rename_agent_bin(to_path, src_v, dst_v)
+            self.rename_agent_bin(to_path, dst_v)
         return
 
     def prepare_agents(self,
@@ -267,7 +267,7 @@ def replicate_agents(self,
             to_path = self.agent_dir(dst_v)
             shutil.copyfile(from_path + ".zip", to_path + ".zip")
             shutil.copytree(from_path, to_path)
-            self.rename_agent_bin(to_path, src_v, dst_v)
+            self.rename_agent_bin(to_path, dst_v)
             if not is_available:
                 GuestAgent(to_path).mark_failure(is_fatal=True)
         return dst_v