Parse and compare agent manifests #2416

narrieta · 2021-11-18T04:48:13Z

Parse the agent manifests in vmSettings and compare them against extensionsConfig.

In the process, I removed VMAgentManifestUri, VMAgentManifestList, ExtHandlerVersionUri, and ExtHandlerPackageUri and replaced them with simple types (strings and lists)

… and ExtHandlerPackageUri

narrieta · 2021-11-18T04:51:51Z

azurelinuxagent/common/protocol/extensions_goal_state_from_extensions_config.py

 from azurelinuxagent.common.utils.textutil import parse_doc, parse_json, findall, find, findtext, getattrib, gettext, format_exception, \
    is_str_none_or_whitespace, is_str_empty


 class ExtensionsGoalStateFromExtensionsConfig(ExtensionsGoalState):
-    def __init__(self, incarnation, xml_text, wire_client=None):


The default values (None) for wire_client was a left over of a test I was doing during my previous PR.

codecov · 2021-11-18T04:52:17Z

Codecov Report

Merging #2416 (8e6a46a) into develop (eff8187) will increase coverage by 0.00%.
The diff coverage is 85.86%.

@@           Coverage Diff            @@
##           develop    #2416   +/-   ##
========================================
  Coverage    71.64%   71.65%           
========================================
  Files          100      100           
  Lines        14631    14672   +41     
  Branches      2092     2097    +5     
========================================
+ Hits         10483    10513   +30     
- Misses        3684     3692    +8     
- Partials       464      467    +3

Impacted Files	Coverage Δ
...protocol/extensions_goal_state_from_vm_settings.py	`84.09% <75.00%> (-2.03%)`	⬇️
azurelinuxagent/common/protocol/wire.py	`80.53% <77.77%> (-0.05%)`	⬇️
...inuxagent/common/protocol/extensions_goal_state.py	`87.50% <88.23%> (-0.36%)`	⬇️
azurelinuxagent/common/protocol/restapi.py	`97.70% <92.85%> (-0.62%)`	⬇️
...ol/extensions_goal_state_from_extensions_config.py	`92.19% <100.00%> (+0.05%)`	⬆️
azurelinuxagent/ga/exthandlers.py	`85.74% <100.00%> (ø)`
azurelinuxagent/ga/update.py	`89.45% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eff8187...8e6a46a. Read the comment docs.

narrieta · 2021-11-18T04:55:01Z

tests/data/hostgaplugin/ext_conf.xml

-              }
-            }
-          ]
+<Extensions version="1.0.0.0" goalStateIncarnation="10"><GuestAgentExtension xmlns:i="http://www.w3.org/2001/XMLSchema-instance">


I updated the test data from an actual DCR run to get the latest changes in HostGAPlugin

narrieta · 2021-11-18T04:57:22Z

tests/data/hostgaplugin/ext_conf.xml

-          ]
+    }
+  ]
+}</RuntimeSettings>


I added the extensions with protected settings to this data file and removed the -protected-settings. files. This way I do not need to update 2 sets of data files when I want to take an update of the HostGAPlugin schema.

larohra

some minor comments, else LGTM

larohra · 2021-11-18T10:18:38Z

azurelinuxagent/common/protocol/extensions_goal_state.py

@@ -115,6 +119,7 @@ def compare_attribute(attribute):
            compare_attribute("status_upload_blob_type")
        compare_attribute("required_features")
        compare_attribute("on_hold")
+        compare_attribute("agent_manifests")


Is the comparison of ext-handlers going to be done in a separate PR?

yes, all this refactoring is affecting a lot of code and those changes need to be done gradually

azurelinuxagent/common/protocol/extensions_goal_state_from_vm_settings.py

larohra · 2021-11-18T10:23:34Z

azurelinuxagent/common/protocol/restapi.py

-class VMAgentManifestUri(DataContract):
-    def __init__(self, uri=None):
-        self.uri = uri
+class VMAgentManifest(object):


Thanks for the refactoring!

larohra · 2021-11-18T10:31:00Z

tests/data/hostgaplugin/vm_settings.json

    "extensionGoalStatesSource": "Fabric",
-    "StatusUploadBlob": {
+    "statusUploadBlob": {


I know we handle this on our end, but is the casing not guaranteed from HostGA? I remember Rohit mentioning something about fixing this, is that not in yet?

we need to keep doing this until all the nodes in the fleet are updated with the newest HostGAPlugin (which will take a loong time, unfortunately)

totally understand that, was just curious to know if this was being handled by HostGA or not. Like if its supposed to be handled properly by HostGA and still we're seeing discrepancy we should probably let Rohit know about it too as it might be a regression there

At some point we should be ready to declare "version W.X.Y.Z of the HostGAPlugin is ready and all the fleet is updated"... at that point we'll add a single check for that version and remove all those workarounds/checks (case-insensitive dictionary, check for > 112, comparison of goal states, etc). Discrepancies would pop up then. Right now it is not worth checking the case of properties (I haven't seen any during my manual tests, BTW).

nagworld9 · 2021-11-19T14:42:08Z

azurelinuxagent/common/protocol/extensions_goal_state_from_extensions_config.py

-from azurelinuxagent.common.protocol.restapi import Extension, ExtHandler, ExtHandlerVersionUri, VMAgentManifest, \
-    VMAgentManifestUri, ExtensionState, ExtHandlerList, VMAgentManifestList, InVMGoalStateMetaData
+from azurelinuxagent.common.protocol.restapi import Extension, ExtHandler, VMAgentManifest, \
+    ExtensionState, ExtHandlerList, InVMGoalStateMetaData
 from azurelinuxagent.common.utils.textutil import parse_doc, parse_json, findall, find, findtext, getattrib, gettext, format_exception, \
    is_str_none_or_whitespace, is_str_empty


 class ExtensionsGoalStateFromExtensionsConfig(ExtensionsGoalState):


can we test for agent_manifest from extension_config like we have for vm_settings.

not sure i get your comment @nagworld9 ... the extension config stuff is existing code and has existing tests. is that what you meant?

Unfortunately, the existing tests not covering agent manifests checks. I don't mind leaving it now but may be when you touch the test file again it's worthwhile to consider testing.
https://github.com/Azure/WALinuxAgent/blob/develop/tests/protocol/test_extensions_goal_state_from_extensions_config.py#L8

yes, the existing code lacks some tests. retrofitting tests to that code would be costly. do feel free to add tests where you consider it worth the effort

nagworld9

LGTM

narrieta added 5 commits November 17, 2021 13:24

Update test data

9b7717d

Parse agent manifests

8d5b1c5

Remove VMAgentManifestUri, VMAgentManifestList, ExtHandlerVersionUri,…

1a2f724

… and ExtHandlerPackageUri

Compare agent manifests

8dded88

Implement VMAgentManifest.__eq__

64101dd

narrieta requested review from dhivyaganesan, kevinclark19a, larohra, nagworld9 and ZhidongPeng as code owners November 18, 2021 04:48

narrieta commented Nov 18, 2021

View reviewed changes

larohra previously approved these changes Nov 18, 2021

View reviewed changes

nagworld9 reviewed Nov 19, 2021

View reviewed changes

Merge branch 'develop' into agent-manifest

1f83b08

nagworld9 previously approved these changes Nov 19, 2021

View reviewed changes

Merge remote-tracking branch 'upstream/develop' into agent-manifest

8e6a46a

narrieta dismissed stale reviews from nagworld9 and larohra via 8e6a46a November 19, 2021 15:34

larohra approved these changes Nov 19, 2021

View reviewed changes

nagworld9 approved these changes Nov 19, 2021

View reviewed changes

narrieta merged commit a73462e into Azure:develop Nov 19, 2021

narrieta deleted the agent-manifest branch November 19, 2021 16:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parse and compare agent manifests #2416

Parse and compare agent manifests #2416

narrieta commented Nov 18, 2021

narrieta Nov 18, 2021

codecov bot commented Nov 18, 2021 •

edited

Loading

narrieta Nov 18, 2021

narrieta Nov 18, 2021

larohra left a comment

larohra Nov 18, 2021

narrieta Nov 18, 2021

larohra Nov 18, 2021

larohra Nov 18, 2021

narrieta Nov 18, 2021

larohra Nov 18, 2021

narrieta Nov 18, 2021 •

edited

Loading

nagworld9 Nov 19, 2021

narrieta Nov 19, 2021

nagworld9 Nov 19, 2021

narrieta Nov 19, 2021 •

edited

Loading

nagworld9 left a comment

Parse and compare agent manifests #2416

Parse and compare agent manifests #2416

Conversation

narrieta commented Nov 18, 2021

Choose a reason for hiding this comment

codecov bot commented Nov 18, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larohra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

narrieta Nov 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

narrieta Nov 19, 2021 • edited Loading

Choose a reason for hiding this comment

nagworld9 left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 18, 2021 •

edited

Loading

narrieta Nov 18, 2021 •

edited

Loading

narrieta Nov 19, 2021 •

edited

Loading