Repository: Azure/WALinuxAgent
Pull request #3058 (2024-02-14 23:47:28)

reset network service unit file if python version changes
<!-- DO NOT DELETE THIS TEMPLATE -->

## Description

When python version updated, the waagent-network-setup.service still refer to old python version. We don't check the python version in the file vs what version agent used once the unit file is created, so we don't overwrite the unit file. This PR address that probelm

```
before update

2023-11-20T09:36:46.437260Z INFO Daemon Daemon Agent WALinuxAgent-2.3.0.2 forwarding signal 15 to WALinuxAgent-2.10.0.3
2023-11-20T09:36:46.557989Z INFO Daemon Daemon Azure Linux Agent Version:2.3.0.2
2023-11-20T09:36:46.558266Z INFO Daemon Daemon OS: redhat 7.9
2023-11-20T09:36:46.558347Z INFO Daemon Daemon Python: 2.7.5

after update

2023-12-21T19:00:19.225428Z INFO Daemon Daemon Agent WALinuxAgent-2.7.0.6 forwarding signal 15 to WALinuxAgent-2.10.0.3
2023-12-21T19:00:41.863028Z INFO Daemon Daemon Azure Linux Agent Version:2.7.0.6
2023-12-21T19:00:41.863576Z INFO Daemon Daemon OS: redhat 8.9
2023-12-21T19:00:41.863691Z INFO Daemon Daemon Python: 3.6.8

2023-12-21T17:54:53.054401Z INFO ExtHandler ExtHandler Logs from the waagent-network-setup.service since system boot:
 -- Logs begin at Thu 2022-12-15 22:19:36 UTC, end at Thu 2023-12-21 17:54:51 UTC. --
Dec 21 17:54:33 IPZPRFCFADLV002 systemd[1]: waagent-network-setup.service: Main process exited, code=exited, status=203/EXEC
Dec 21 17:54:33 IPZPRFCFADLV002 systemd[1]: waagent-network-setup.service: Failed with result 'exit-code'.
Dec 21 17:54:33 IPZPRFCFADLV002 systemd[1]: Failed to start Setup network rules for WALinuxAgent.
```

The PR also address the issue of `conditionfileexist` problem. Sometimes service does not get started at boot time due to binary file is not present when it runs and eventually failed with start condition failed. I have seen this behavior in redhat7.9. So, we need to specify systemd ordering and run after filesystem is ready by maintaining other existing order in the unit file.



Issue # <!-- if any -->
<!--
Please add an informative description that covers that changes made by the pull request. 
This checklist is used to make sure that common issues in a pull request are addressed.
This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.
-->

---

### PR information
- [ ] The title of the PR is clear and informative.
- [ ] There are a small number of commits, each of which has an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, [see this page](https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/cleaning-up-commits.md).
- [ ] If applicable, the PR references the bug/issue that it fixes in the description.
- [ ] New Unit tests were added for the changes made

### Quality of Code and Contribution Guidelines
- [ ] I have read the [contribution guidelines](https://github.com/Azure/WALinuxAgent/blob/master/.github/CONTRIBUTING.md).

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/Azure/WALinuxAgent/pull/3058?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) Report
Attention: `4 lines` in your changes are missing coverage. Please review.
> :exclamation: No coverage uploaded for pull request base (`develop@49de9b3`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#section-missing-base-commit).

| [Files](https://app.codecov.io/gh/Azure/WALinuxAgent/pull/3058?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) | Patch % | Lines |
|---|---|---|
| [azurelinuxagent/ga/persist\_firewall\_rules.py](https://app.codecov.io/gh/Azure/WALinuxAgent/pull/3058?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2dhL3BlcnNpc3RfZmlyZXdhbGxfcnVsZXMucHk=) | 76.47% | [2 Missing and 2 partials :warning: ](https://app.codecov.io/gh/Azure/WALinuxAgent/pull/3058?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             develop    #3058   +/-   ##
==========================================
  Coverage           ?   71.72%           
==========================================
  Files              ?      109           
  Lines              ?    16405           
  Branches           ?     2346           
==========================================
  Hits               ?    11766           
  Misses             ?     4089           
  Partials           ?      550           
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/Azure/WALinuxAgent/pull/3058?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure).


Patch:
diff --git a/azurelinuxagent/ga/persist_firewall_rules.py b/azurelinuxagent/ga/persist_firewall_rules.py
index 74b878ce..a20e2874 100644
--- a/azurelinuxagent/ga/persist_firewall_rules.py
+++ b/azurelinuxagent/ga/persist_firewall_rules.py
@@ -34,7 +34,8 @@ class PersistFirewallRulesHandler(object):
 # This unit file (Version={version}) was created by the Azure VM Agent.
 # Do not edit.
 [Unit]
-Description=Setup network rules for WALinuxAgent 
+Description=Setup network rules for WALinuxAgent
+After=local-fs.target 
 Before=network-pre.target
 Wants=network-pre.target
 DefaultDependencies=no
@@ -69,7 +70,7 @@ if __name__ == '__main__':
 
     # 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.3"
+    _UNIT_VERSION = "1.4"
 
     @staticmethod
     def get_service_file_path():
@@ -184,7 +185,7 @@ if __name__ == '__main__':
         self.__setup_binary_file()
 
         network_service_enabled = self.__verify_network_setup_service_enabled()
-        if network_service_enabled and not self.__unit_file_version_modified():
+        if network_service_enabled and not self.__should_update_unit_file():
             logger.info("Service: {0} already enabled. No change needed.".format(self._network_setup_service_name))
             self.__log_network_setup_service_logs()
 
@@ -314,17 +315,29 @@ if __name__ == '__main__':
 
         return match.group(1).strip()
 
-    def __unit_file_version_modified(self):
+    def __get_unit_exec_start(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="ExecStart=(.*)")
+        if match is None:
+            raise ValueError("ExecStart tag not found in the unit file")
+
+        return match.group(1).strip()
+
+    def __should_update_unit_file(self):
         """
-        Check if the unit file version changed from the expected version
-        :return: True if unit file version changed else False
+        Check if the unit file version changed from the expected version or if the exec-start changed from the expected exec-start
+        :return: True if unit file need update else False
         """
 
         try:
             unit_file_version = self.__get_unit_file_version()
+            unit_exec_start = self.__get_unit_exec_start()
         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
+            logger.info("Unable to read content of unit file: {0}, overwriting unit file".format(ustr(error)))
+            # Since we can't determine the version or exec start, marking the file as modified to overwrite the unit file
             return True
 
         if unit_file_version != self._UNIT_VERSION:
@@ -332,7 +345,14 @@ if __name__ == '__main__':
                 "Unit file version: {0} does not match with expected version: {1}, overwriting unit file".format(
                     unit_file_version, self._UNIT_VERSION))
             return True
+        binary_path = os.path.join(conf.get_lib_dir(), self.BINARY_FILE_NAME)
+        expected_exec_start = "{0} {1}".format(sys.executable, binary_path)
+        if unit_exec_start != expected_exec_start:
+            logger.info(
+                "Unit file exec-start: {0} does not match with expected exec-start: {1}, overwriting unit file".format(
+                    unit_exec_start, expected_exec_start))
+            return True
 
         logger.info(
-            "Unit file version matches with expected version: {0}, not overwriting unit file".format(unit_file_version))
+            "Unit file matches with expected version: {0} and exec start: {1}, not overwriting unit file".format(unit_file_version, unit_exec_start))
         return False


Test patch:
diff --git a/tests/ga/test_persist_firewall_rules.py b/tests/ga/test_persist_firewall_rules.py
index 5ee397ba..adcf43b7 100644
--- a/tests/ga/test_persist_firewall_rules.py
+++ b/tests/ga/test_persist_firewall_rules.py
@@ -414,3 +414,18 @@ class TestPersistFirewallRulesHandler(AgentTestCase):
                                                                    mock_popen=self.__mock_network_setup_service_enabled)
             self.assertNotIn(test_ver, fileutil.read_file(handler.get_service_file_path()),
                              "Test version found incorrectly")
+
+    def test_it_should_reset_service_unit_file_if_python_version_changes(self):
+        with self._get_persist_firewall_rules_handler() as handler:
+            # 1st step - Setup the service with some python Version
+            python_ver = "test_python"
+            with patch("sys.executable", python_ver):
+                self.__setup_and_assert_network_service_setup_scenario(handler)
+                self.assertIn(python_ver, fileutil.read_file(handler.get_service_file_path()), "Python 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(python_ver, fileutil.read_file(handler.get_service_file_path()),
+                             "Python version found incorrectly")