Use vmSettings for extension processing #2451
Conversation
narrieta
requested review from
dhivyaganesan,
kevinclark19a,
larohra,
nagworld9 and
ZhidongPeng
as code owners
Codecov Report
@@ Coverage Diff @@
## develop #2451 +/- ##
===========================================
+ Coverage 71.84% 71.88% +0.03%
===========================================
Files 101 101
Lines 14944 14964 +20
Branches 2367 2369 +2
===========================================
+ Hits 10737 10757 +20
- Misses 3727 3728 +1
+ Partials 480 479 -1
Continue to review full report at Codecov.
|
| @@ -104,10 +103,8 @@ def compare_goal_states(first, second): | |||
| compare_attributes(first, second, "activity_id") | |||
| compare_attributes(first, second, "correlation_id") | |||
| compare_attributes(first, second, "created_on_timestamp") | |||
| # The status blob was added after version 112 | |||
There was a problem hiding this comment.
now there is a global check for the version so this one is no longer needed
azurelinuxagent/common/protocol/extensions_goal_state_from_extensions_config.py
Show resolved
Hide resolved
| self.ext_handlers = extensions_goal_state.extensions | ||
| etag = self.protocol.client.get_goal_state().incarnation |
There was a problem hiding this comment.
despite the name of this local, this needs to be the incarnation number for status reporting to work - I introduced a regression a couple of PRs ago and was using the 'id' property; now i am fixing this to use the incarnation explicitly
there are a lot renames/refactoring needed in this module, i am planning on doing those gradually
azurelinuxagent/common/protocol/extensions_goal_state_from_extensions_config.py
Show resolved
Hide resolved
azurelinuxagent/common/protocol/extensions_goal_state_from_extensions_config.py
Show resolved
Hide resolved
| if conf.get_enable_fast_track(): | ||
| # Since currently we query vmSettings only to exercise the HostGAPlugin, errors in this section of the code should not | ||
| # prevent the agent from processing the goal state; we simply report a summary of those errors. | ||
| goal_state_updated = True |
There was a problem hiding this comment.
maybe rename this to fabric_goal_state_updated for more clarity?
There was a problem hiding this comment.
the 2 goal states are named _goal_state and _vm_settings_goal_state, the update variables are parallel to those names (goal_state_updated and vm_settings_goal_state updated)
renaming self._goal_state to self.fabric_goal_state would not be appropriate, since in the next iteration there will be only 1 goal state (which can come from fabric or fast track) and self._vm_settings_goal_state will go away)
| if is_first_vm_settings: | ||
| message = "HostGAPlugin version: {0}".format(vm_settings.host_ga_plugin_version) | ||
| logger.info(message) | ||
| add_event(op=WALAEventOperation.HostPlugin, message=message, is_success=True) |
There was a problem hiding this comment.
If the HGAP version < .115, we will keep emitting this event on every call to update_goal_state. We should only log this once irrespective
There was a problem hiding this comment.
Yes, good point. I have another change in the pipeline where if we do not support vmsettings we will stop trying to use it for several hours. I'll add the check for minimum version there, too.
There was a problem hiding this comment.
Cool cool. So this will be taken care in the next PR?
There was a problem hiding this comment.
Yes, I need some refactoring to get accurate telemetry and need to touch the error handling code first. That is the reason I postponed the handling of not supported. Version check is just a variation of not supported.
Now we use vmSettings for extension processing. We still fallback to extensionsConfig if the HostGAPlugin does not support Fast Track, if there are errors fetching the vmSettings, or if they are different to extensionsConfig.