Repository: Azure/WALinuxAgent
Pull request #2141 (2021-01-21 20:37:58)

Bring back "Gs fix for no config (#2016)"
<!-- DO NOT DELETE THIS TEMPLATE -->

## Description

This was removed prior to 2.2.53 release due to some concerns around wireserver communication. Bringing the change back for a future release.

<!--
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).
- [ ] Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
- [ ] If applicable, the PR references the bug/issue that it fixes in the description.
- [ ] New Unit tests were added for the changes made and Travis.CI is passing.

### 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://codecov.io/gh/Azure/WALinuxAgent/pull/2141?src=pr&el=h1) Report
> Merging [#2141](https://codecov.io/gh/Azure/WALinuxAgent/pull/2141?src=pr&el=desc) (d5f854e) into [develop](https://codecov.io/gh/Azure/WALinuxAgent/commit/f519183e0b62a5dce87d591f69322c0747371602?el=desc) (f519183) will **decrease** coverage by `0.04%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/Azure/WALinuxAgent/pull/2141/graphs/tree.svg?width=650&height=150&src=pr&token=ae90E2KYCw)](https://codecov.io/gh/Azure/WALinuxAgent/pull/2141?src=pr&el=tree)

```diff
@@             Coverage Diff             @@
##           develop    #2141      +/-   ##
===========================================
- Coverage    70.68%   70.64%   -0.05%     
===========================================
  Files           95       95              
  Lines        12934    12937       +3     
  Branches      1841     1842       +1     
===========================================
- Hits          9143     9139       -4     
- Misses        3386     3390       +4     
- Partials       405      408       +3     
```


| [Impacted Files](https://codecov.io/gh/Azure/WALinuxAgent/pull/2141?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [azurelinuxagent/common/exception.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2141/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9leGNlcHRpb24ucHk=) | `98.87% <100.00%> (+0.01%)` | :arrow_up: |
| [azurelinuxagent/common/protocol/goal\_state.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2141/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9wcm90b2NvbC9nb2FsX3N0YXRlLnB5) | `91.43% <100.00%> (-0.51%)` | :arrow_down: |
| [azurelinuxagent/ga/collect\_telemetry\_events.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2141/diff?src=pr&el=tree#diff-YXp1cmVsaW51eGFnZW50L2dhL2NvbGxlY3RfdGVsZW1ldHJ5X2V2ZW50cy5weQ==) | `89.65% <0.00%> (-1.73%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2141?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2141?src=pr&el=footer). Last update [f519183...d5f854e](https://codecov.io/gh/Azure/WALinuxAgent/pull/2141?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/azurelinuxagent/common/exception.py b/azurelinuxagent/common/exception.py
index 7b89192d..943214af 100644
--- a/azurelinuxagent/common/exception.py
+++ b/azurelinuxagent/common/exception.py
@@ -151,6 +151,12 @@ class ProtocolNotFoundError(ProtocolError):
     """
 
 
+class IncompleteGoalStateError(ProtocolError):
+    """
+    Goal state is returned incomplete.
+    """
+
+
 class HttpError(AgentError):
     """
     Http request failure
diff --git a/azurelinuxagent/common/protocol/goal_state.py b/azurelinuxagent/common/protocol/goal_state.py
index 8b0c851a..214cb863 100644
--- a/azurelinuxagent/common/protocol/goal_state.py
+++ b/azurelinuxagent/common/protocol/goal_state.py
@@ -29,8 +29,9 @@ from azurelinuxagent.common.event import add_event, WALAEventOperation
 from azurelinuxagent.common.exception import ProtocolError, ExtensionConfigError
 from azurelinuxagent.common.future import ustr
 from azurelinuxagent.common.protocol.restapi import Cert, CertList, Extension, ExtHandler, ExtHandlerList, \
-    ExtHandlerVersionUri, RemoteAccessUser, RemoteAccessUsersList, \
-    VMAgentManifest, VMAgentManifestList, VMAgentManifestUri, InVMGoalStateMetaData
+    ExtHandlerVersionUri, RemoteAccessUser, RemoteAccessUsersList, VMAgentManifest, VMAgentManifestList, \
+    VMAgentManifestUri, InVMGoalStateMetaData
+from azurelinuxagent.common.exception import IncompleteGoalStateError
 from azurelinuxagent.common.utils import fileutil
 from azurelinuxagent.common.utils.cryptutil import CryptUtil
 from azurelinuxagent.common.utils.textutil import parse_doc, findall, find, findtext, getattrib, gettext
@@ -43,6 +44,8 @@ PEM_FILE_NAME = "Certificates.pem"
 TRANSPORT_CERT_FILE_NAME = "TransportCert.pem"
 TRANSPORT_PRV_FILE_NAME = "TransportPrivate.pem"
 
+NUM_GS_FETCH_RETRIES = 3
+
 
 class GoalState(object):
     def __init__(self, wire_client, full_goal_state=False, base_incarnation=None):
@@ -59,14 +62,21 @@ class GoalState(object):
         directly.
 
         """
-        try:
-            uri = GOAL_STATE_URI.format(wire_client.get_endpoint())
+        uri = GOAL_STATE_URI.format(wire_client.get_endpoint())
+
+        for _ in range(1, NUM_GS_FETCH_RETRIES + 1):
             self.xml_text = wire_client.fetch_config(uri, wire_client.get_header())
             xml_doc = parse_doc(self.xml_text)
-
             self.incarnation = findtext(xml_doc, "Incarnation")
-            self.expected_state = findtext(xml_doc, "ExpectedState")
+
             role_instance = find(xml_doc, "RoleInstance")
+            if role_instance:
+                break
+        else:
+            raise IncompleteGoalStateError("Fetched goal state without a RoleInstance [incarnation {inc}]".format(inc=self.incarnation))
+
+        try:
+            self.expected_state = findtext(xml_doc, "ExpectedState")
             self.role_instance_id = findtext(role_instance, "InstanceId")
             role_config = find(role_instance, "Configuration")
             self.role_config_name = findtext(role_config, "ConfigName")
@@ -77,15 +87,11 @@ class GoalState(object):
 
             AgentGlobals.update_container_id(self.container_id)
 
-            fetch_full_goal_state = False
             if full_goal_state:
-                fetch_full_goal_state = True
                 reason = 'force update'
-            elif base_incarnation is not None and self.incarnation != base_incarnation:
-                fetch_full_goal_state = True
+            elif base_incarnation not in (None, self.incarnation):
                 reason = 'new incarnation'
-
-            if not fetch_full_goal_state:
+            else:
                 self.hosting_env = None
                 self.shared_conf = None
                 self.certs = None


Test patch:
diff --git a/tests/data/wire/goal_state_noop.xml b/tests/data/wire/goal_state_noop.xml
new file mode 100644
index 00000000..1c456b65
--- /dev/null
+++ b/tests/data/wire/goal_state_noop.xml
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="utf-8"?>
+<GoalState xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="goalstate10.xsd">
+   <Version>2010-12-15</Version>
+   <Incarnation>1</Incarnation>
+   <Machine>
+     <ExpectedState>Started</ExpectedState>
+     <LBProbePorts>
+       <Port>16001</Port>
+     </LBProbePorts>
+   </Machine>
+   <Container>
+     <ContainerId>c6d5526c-5ac2-4200-b6e2-56f2b70c5ab2</ContainerId>
+   </Container>
+ </GoalState>
diff --git a/tests/protocol/mockwiredata.py b/tests/protocol/mockwiredata.py
index c70e9f91..006634f8 100644
--- a/tests/protocol/mockwiredata.py
+++ b/tests/protocol/mockwiredata.py
@@ -50,6 +50,10 @@ DATA_FILE_NO_EXT = DATA_FILE.copy()
 DATA_FILE_NO_EXT["goal_state"] = "wire/goal_state_no_ext.xml"
 DATA_FILE_NO_EXT["ext_conf"] = None
 
+DATA_FILE_NOOP_GS = DATA_FILE.copy()
+DATA_FILE_NOOP_GS["goal_state"] = "wire/goal_state_noop.xml"
+DATA_FILE_NOOP_GS["ext_conf"] = None
+
 DATA_FILE_EXT_NO_SETTINGS = DATA_FILE.copy()
 DATA_FILE_EXT_NO_SETTINGS["ext_conf"] = "wire/ext_conf_no_settings.xml"
 
diff --git a/tests/protocol/test_wire.py b/tests/protocol/test_wire.py
index 648ffa69..86b37069 100644
--- a/tests/protocol/test_wire.py
+++ b/tests/protocol/test_wire.py
@@ -31,7 +31,7 @@ from azurelinuxagent.common.agent_supported_feature import CRPSupportedFeatureNa
     get_agent_supported_features_list_for_crp
 from azurelinuxagent.common.exception import ResourceGoneError, ProtocolError, \
     ExtensionDownloadError, HttpError
-from azurelinuxagent.common.protocol.goal_state import ExtensionsConfig
+from azurelinuxagent.common.protocol.goal_state import ExtensionsConfig, GoalState
 from azurelinuxagent.common.protocol.hostplugin import HostPluginProtocol
 from azurelinuxagent.common.protocol.restapi import VMAgentManifestUri
 from azurelinuxagent.common.protocol.wire import WireProtocol, WireClient, \
@@ -39,6 +39,7 @@ from azurelinuxagent.common.protocol.wire import WireProtocol, WireClient, \
 from azurelinuxagent.common.telemetryevent import GuestAgentExtensionEventsSchema, \
     TelemetryEventParam, TelemetryEvent
 from azurelinuxagent.common.utils import restutil
+from azurelinuxagent.common.exception import IncompleteGoalStateError
 from azurelinuxagent.common.version import CURRENT_VERSION, DISTRO_NAME, DISTRO_VERSION
 from azurelinuxagent.ga.exthandlers import get_exthandlers_handler
 from tests.ga.test_monitor import random_generator
@@ -1197,6 +1198,18 @@ class TryUpdateGoalStateTestCase(HttpRequestPredicates, AgentTestCase):
         with mock_wire_protocol(mockwiredata.DATA_FILE) as protocol:
             self.assertTrue(protocol.client.try_update_goal_state(), "try_update_goal_state should have succeeded")
 
+    def test_incomplete_gs_should_fail(self):
+
+        with mock_wire_protocol(mockwiredata.DATA_FILE) as protocol:
+            GoalState.fetch_full_goal_state(protocol.client)
+
+            protocol.mock_wire_data.data_files = mockwiredata.DATA_FILE_NOOP_GS
+            protocol.mock_wire_data.reload()
+            protocol.mock_wire_data.set_incarnation(2)
+
+            with self.assertRaises(IncompleteGoalStateError):
+                GoalState.fetch_full_goal_state_if_incarnation_different_than(protocol.client, 1)
+
     def test_it_should_return_false_on_failure(self):
         with mock_wire_protocol(mockwiredata.DATA_FILE) as protocol:
             def http_get_handler(url, *_, **__):