Repository: Azure/WALinuxAgent
Pull request #2649 (2022-08-15 22:32:08)

Save sharedconfig to disk
As part of the changes for Fast Track, the agent does not shave SharedConfig.xml under /var/lib/waagent.

It turned out that HPC has a dependency on this file to obtain some network configuration information. This changelist creates the file again, while the dependency from HPC is removed.

codecov[bot]: # [Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2649?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) Report
> Merging [#2649](https://codecov.io/gh/Azure/WALinuxAgent/pull/2649?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) (207bef7) into [release-2.8.0.0](https://codecov.io/gh/Azure/WALinuxAgent/commit/3d2e605de475edf200a089d7a3c828fd7ef1693a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) (3d2e605) will **increase** coverage by `0.02%`.
> The diff coverage is `83.33%`.

```diff
@@                 Coverage Diff                 @@
##           release-2.8.0.0    #2649      +/-   ##
===================================================
+ Coverage            72.07%   72.10%   +0.02%     
===================================================
  Files                  102      102              
  Lines                15434    15439       +5     
  Branches              2454     2454              
===================================================
+ Hits                 11124    11132       +8     
  Misses                3817     3817              
+ Partials               493      490       -3     
```


| [Impacted Files](https://codecov.io/gh/Azure/WALinuxAgent/pull/2649?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) | Coverage Δ | |
|---|---|---|
| [azurelinuxagent/common/protocol/goal\_state.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2649/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9wcm90b2NvbC9nb2FsX3N0YXRlLnB5) | `95.31% <75.00%> (-0.50%)` | :arrow_down: |
| [azurelinuxagent/common/utils/archive.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2649/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi91dGlscy9hcmNoaXZlLnB5) | `82.28% <100.00%> (ø)` | |
| [azurelinuxagent/ga/update.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2649/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2dhL3VwZGF0ZS5weQ==) | `88.30% <100.00%> (ø)` | |
| [azurelinuxagent/ga/collect\_telemetry\_events.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2649/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2dhL2NvbGxlY3RfdGVsZW1ldHJ5X2V2ZW50cy5weQ==) | `91.43% <0.00%> (+1.71%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure)


Patch:
diff --git a/azurelinuxagent/common/protocol/goal_state.py b/azurelinuxagent/common/protocol/goal_state.py
index 4d354e56..97ae270f 100644
--- a/azurelinuxagent/common/protocol/goal_state.py
+++ b/azurelinuxagent/common/protocol/goal_state.py
@@ -33,7 +33,7 @@ from azurelinuxagent.common.protocol.extensions_goal_state import VmSettingsPars
 from azurelinuxagent.common.protocol.hostplugin import VmSettingsNotSupported, VmSettingsSupportStopped
 from azurelinuxagent.common.protocol.restapi import Cert, CertList, RemoteAccessUser, RemoteAccessUsersList
 from azurelinuxagent.common.utils import fileutil
-from azurelinuxagent.common.utils.archive import GoalStateHistory
+from azurelinuxagent.common.utils.archive import GoalStateHistory, SHARED_CONF_FILE_NAME
 from azurelinuxagent.common.utils.cryptutil import CryptUtil
 from azurelinuxagent.common.utils.textutil import parse_doc, findall, find, findtext, getattrib
 
@@ -345,8 +345,14 @@ class GoalState(object):
 
             shared_conf_uri = findtext(xml_doc, "SharedConfig")
             xml_text = self._wire_client.fetch_config(shared_conf_uri, self._wire_client.get_header())
-            shared_conf = SharedConfig(xml_text)
+            shared_config = SharedConfig(xml_text)
             self._history.save_shared_conf(xml_text)
+            # SharedConfig.xml is used by other components (Azsec and Singularity/HPC Infiniband), so save it to the agent's root directory as well
+            shared_config_file = os.path.join(conf.get_lib_dir(), SHARED_CONF_FILE_NAME)
+            try:
+                fileutil.write_file(shared_config_file, xml_text)
+            except Exception as e:
+                logger.warn("Failed to save {0}: {1}".format(shared_config, e))
 
             certs = EmptyCertificates()
             certs_uri = findtext(xml_doc, "Certificates")
@@ -372,7 +378,7 @@ class GoalState(object):
             self._role_config_name = role_config_name
             self._container_id = container_id
             self._hosting_env = hosting_env
-            self._shared_conf = shared_conf
+            self._shared_conf = shared_config
             self._certs = certs
             self._remote_access = remote_access
 
diff --git a/azurelinuxagent/common/utils/archive.py b/azurelinuxagent/common/utils/archive.py
index b624d174..1f8fdd93 100644
--- a/azurelinuxagent/common/utils/archive.py
+++ b/azurelinuxagent/common/utils/archive.py
@@ -46,11 +46,13 @@ _PLACEHOLDER_FILE_NAME = 'GoalState.1.xml'
 _MAX_ARCHIVED_STATES = 50
 
 _CACHE_PATTERNS = [
+    #
+    # Note that SharedConfig.xml is not included here; this file is used by other components (Azsec and Singularity/HPC Infiniband)
+    #
     re.compile(r"^VmSettings\.\d+\.json$"),
     re.compile(r"^(.*)\.(\d+)\.(agentsManifest)$", re.IGNORECASE),
     re.compile(r"^(.*)\.(\d+)\.(manifest\.xml)$", re.IGNORECASE),
     re.compile(r"^(.*)\.(\d+)\.(xml)$", re.IGNORECASE),
-    re.compile(r"^SharedConfig\.xml$", re.IGNORECASE),
     re.compile(r"^HostingEnvironmentConfig\.xml$", re.IGNORECASE),
     re.compile(r"^RemoteAccess\.xml$", re.IGNORECASE),
     re.compile(r"^waagent_status\.\d+\.json$"),
@@ -78,12 +80,12 @@ _GOAL_STATE_FILE_NAME = "GoalState.xml"
 _VM_SETTINGS_FILE_NAME = "VmSettings.json"
 _CERTIFICATES_FILE_NAME = "Certificates.json"
 _HOSTING_ENV_FILE_NAME = "HostingEnvironmentConfig.xml"
-_SHARED_CONF_FILE_NAME = "SharedConfig.xml"
 _REMOTE_ACCESS_FILE_NAME = "RemoteAccess.xml"
 _EXT_CONF_FILE_NAME = "ExtensionsConfig.xml"
 _MANIFEST_FILE_NAME = "{0}.manifest.xml"
 
 AGENT_STATUS_FILE = "waagent_status.json"
+SHARED_CONF_FILE_NAME = "SharedConfig.xml"
 
 # TODO: use @total_ordering once RHEL/CentOS and SLES 11 are EOL.
 # @total_ordering first appeared in Python 2.7 and 3.2
@@ -166,9 +168,10 @@ class StateArchiver(object):
     def purge_legacy_goal_state_history():
         lib_dir = conf.get_lib_dir()
         for current_file in os.listdir(lib_dir):
+            # Don't remove the placeholder goal state file.
             # TODO: See comment in GoalStateHistory._save_placeholder and remove this code when no longer needed
             if current_file == _PLACEHOLDER_FILE_NAME:
-                return
+                continue
             # END TODO
             full_path = os.path.join(lib_dir, current_file)
             for pattern in _CACHE_PATTERNS:
@@ -302,4 +305,4 @@ class GoalStateHistory(object):
         self.save(text, _HOSTING_ENV_FILE_NAME)
 
     def save_shared_conf(self, text):
-        self.save(text, _SHARED_CONF_FILE_NAME)
+        self.save(text, SHARED_CONF_FILE_NAME)
diff --git a/azurelinuxagent/ga/update.py b/azurelinuxagent/ga/update.py
index 66b0de5d..d17fff6a 100644
--- a/azurelinuxagent/ga/update.py
+++ b/azurelinuxagent/ga/update.py
@@ -378,6 +378,7 @@ class UpdateHandler(object):
             self._ensure_firewall_rules_persisted(dst_ip=protocol.get_endpoint())
             self._add_accept_tcp_firewall_rule_if_not_enabled(dst_ip=protocol.get_endpoint())
             self._reset_legacy_blacklisted_agents()
+            self._cleanup_legacy_goal_state_history()
 
             # Get all thread handlers
             telemetry_handler = get_send_telemetry_events_handler(self.protocol_util)
@@ -396,8 +397,6 @@ class UpdateHandler(object):
 
             logger.info("Goal State Period: {0} sec. This indicates how often the agent checks for new goal states and reports status.", self._goal_state_period)
 
-            self._cleanup_legacy_goal_state_history()
-
             while self.is_running:
                 self._check_daemon_running(debug)
                 self._check_threads_running(all_thread_handlers)


Test patch:
diff --git a/tests/protocol/test_goal_state.py b/tests/protocol/test_goal_state.py
index c7741715..87a1db50 100644
--- a/tests/protocol/test_goal_state.py
+++ b/tests/protocol/test_goal_state.py
@@ -8,6 +8,7 @@ import os
 import re
 import time
 
+from azurelinuxagent.common import conf
 from azurelinuxagent.common.future import httpclient
 from azurelinuxagent.common.protocol.extensions_goal_state import GoalStateSource, GoalStateChannel
 from azurelinuxagent.common.protocol.extensions_goal_state_from_extensions_config import ExtensionsGoalStateFromExtensionsConfig
@@ -96,7 +97,15 @@ class GoalStateTestCase(AgentTestCase, HttpRequestPredicates):
                     GoalState(protocol.client)
                 self.assertEqual(_GET_GOAL_STATE_MAX_ATTEMPTS, mock_sleep.call_count, "Unexpected number of retries")
 
-    def test_instantiating_goal_state_should_save_the_goal_state_to_the_history_directory(self):
+    def test_fetching_the_goal_state_should_save_the_shared_config(self):
+        # SharedConfig.xml is used by other components (Azsec and Singularity/HPC Infiniband); verify that we do not delete it
+        with mock_wire_protocol(mockwiredata.DATA_FILE_VM_SETTINGS) as protocol:
+            _ = GoalState(protocol.client)
+
+            shared_config = os.path.join(conf.get_lib_dir(), 'SharedConfig.xml')
+            self.assertTrue(os.path.exists(shared_config), "{0} should have been created".format(shared_config))
+
+    def test_fetching_the_goal_state_should_save_the_goal_state_to_the_history_directory(self):
         with mock_wire_protocol(mockwiredata.DATA_FILE_VM_SETTINGS) as protocol:
             protocol.mock_wire_data.set_incarnation(999)
             protocol.mock_wire_data.set_etag(888)
diff --git a/tests/utils/test_archive.py b/tests/utils/test_archive.py
index ce97d65f..54766862 100644
--- a/tests/utils/test_archive.py
+++ b/tests/utils/test_archive.py
@@ -133,27 +133,32 @@ class TestArchive(AgentTestCase):
 
     def test_purge_legacy_goal_state_history(self):
         with patch("azurelinuxagent.common.conf.get_lib_dir", return_value=self.tmp_dir):
+            # SharedConfig.xml is used by other components (Azsec and Singularity/HPC Infiniband); verify that we do not delete it
+            shared_config = os.path.join(self.tmp_dir, 'SharedConfig.xml')
+
             legacy_files = [
                 'GoalState.2.xml',
                 'VmSettings.2.json',
                 'Prod.2.manifest.xml',
                 'ExtensionsConfig.2.xml',
                 'Microsoft.Azure.Extensions.CustomScript.1.xml',
-                'SharedConfig.xml',
                 'HostingEnvironmentConfig.xml',
                 'RemoteAccess.xml',
                 'waagent_status.1.json'
             ]
             legacy_files = [os.path.join(self.tmp_dir, f) for f in legacy_files]
+
+            self._write_file(shared_config)
             for f in legacy_files:
                 self._write_file(f)
 
             StateArchiver.purge_legacy_goal_state_history()
 
+            self.assertTrue(os.path.exists(shared_config), "{0} should not have been removed".format(shared_config))
+
             for f in legacy_files:
                 self.assertFalse(os.path.exists(f), "Legacy file {0} was not removed".format(f))
 
-
     @staticmethod
     def parse_isoformat(timestamp_str):
         return datetime.strptime(timestamp_str, '%Y-%m-%dT%H:%M:%S.%f')