Repository: Azure/WALinuxAgent
Pull request #2016 (2020-09-22 18:12:29)

Gs fix for no config
<!-- DO NOT DELETE THIS TEMPLATE -->

## Description

DCR runs are failing when wireserver returns a no-op goal state. This change adds some error handling to skip such goal states.

<!--
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).


Patch:
diff --git a/azurelinuxagent/common/exception.py b/azurelinuxagent/common/exception.py
index 44f0ec31..0f8709f6 100644
--- a/azurelinuxagent/common/exception.py
+++ b/azurelinuxagent/common/exception.py
@@ -153,6 +153,10 @@ class ProtocolNotFoundError(ProtocolError):
     def __init__(self, msg=None, inner=None): # pylint: disable=W0235
         super(ProtocolNotFoundError, self).__init__(msg, inner)
 
+class IncompleteGoalStateError(ProtocolError):
+    """
+    Goal state is returned incomplete.
+    """
 
 class HttpError(AgentError):
     """
diff --git a/azurelinuxagent/common/protocol/goal_state.py b/azurelinuxagent/common/protocol/goal_state.py
index 3bc94722..91ebd849 100644
--- a/azurelinuxagent/common/protocol/goal_state.py
+++ b/azurelinuxagent/common/protocol/goal_state.py
@@ -30,6 +30,7 @@ from azurelinuxagent.common.future import ustr
 from azurelinuxagent.common.protocol.restapi import Cert, CertList, Extension, ExtHandler, ExtHandlerList, \
     ExtHandlerVersionUri, RemoteAccessUser, RemoteAccessUsersList, \
     VMAgentManifest, VMAgentManifestList, VMAgentManifestUri
+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
@@ -42,6 +43,8 @@ PEM_FILE_NAME = "Certificates.pem"
 TRANSPORT_CERT_FILE_NAME = "TransportCert.pem"
 TRANSPORT_PRV_FILE_NAME = "TransportPrivate.pem"
 
+NUM_GS_FETCH_RETRIES = 3
+
 
 # too-many-instance-attributes<R0902> Disabled: The goal state consists of a good number of properties
 class GoalState(object):  # pylint: disable=R0902
@@ -60,14 +63,21 @@ class GoalState(object):  # pylint: disable=R0902
         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")
@@ -78,15 +88,11 @@ class GoalState(object):  # pylint: disable=R0902
 
             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 1510b80f..fb0a5cdf 100644
--- a/tests/protocol/mockwiredata.py
+++ b/tests/protocol/mockwiredata.py
@@ -47,6 +47,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 0c2b2697..bd80a741 100644
--- a/tests/protocol/test_wire.py
+++ b/tests/protocol/test_wire.py
@@ -27,7 +27,7 @@ import uuid
 
 from azurelinuxagent.common.exception import InvalidContainerError, 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, \
@@ -35,6 +35,7 @@ from azurelinuxagent.common.protocol.wire import WireProtocol, WireClient, \
 from azurelinuxagent.common.telemetryevent import TelemetryEventList, 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 tests.ga.test_monitor import random_generator
 from tests.protocol import mockwiredata
@@ -1063,6 +1064,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, *_, **__):