Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use SVD sequence number instead of incarnation when reporting status #2524

Merged
merged 13 commits into from
Mar 3, 2022

Conversation

narrieta
Copy link
Member

@narrieta narrieta commented Mar 2, 2022

Updated the code to use the SVD sequence number when reporting status, instead of using the incarnation number (which happen to be the same for Fabric goal states).

Also. updated the code to log the goal state id (incarnation or etag) instead of the incarnation while processing extensions.

@codecov
Copy link

codecov bot commented Mar 2, 2022

Codecov Report

Merging #2524 (f62ea29) into develop (415882d) will decrease coverage by 0.00%.
The diff coverage is 69.64%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2524      +/-   ##
===========================================
- Coverage    71.94%   71.93%   -0.01%     
===========================================
  Files          102      102              
  Lines        15145    15166      +21     
  Branches      2400     2400              
===========================================
+ Hits         10896    10910      +14     
- Misses        3768     3775       +7     
  Partials       481      481              
Impacted Files Coverage Δ
azurelinuxagent/common/protocol/goal_state.py 96.81% <0.00%> (ø)
...inuxagent/common/protocol/extensions_goal_state.py 71.65% <50.00%> (-2.04%) ⬇️
...t/common/protocol/extensions_goal_state_factory.py 92.30% <50.00%> (ø)
azurelinuxagent/ga/update.py 88.39% <57.14%> (ø)
...protocol/extensions_goal_state_from_vm_settings.py 81.54% <80.00%> (-0.03%) ⬇️
azurelinuxagent/ga/exthandlers.py 85.69% <83.33%> (ø)
...ol/extensions_goal_state_from_extensions_config.py 91.16% <100.00%> (+0.09%) ⬆️

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 415882d...f62ea29. Read the comment docs.

larohra
larohra previously approved these changes Mar 3, 2022
was created from vmSettings.
"""
raise NotImplementedError()

@property
def svd_sequence_number(self):
raise NotImplementedError()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, what's the difference between this and the id?

Copy link
Member Author

@narrieta narrieta Mar 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The id is either the etag or the incarnation, and is used for logging purposes and to check if the goal state has changed (the etag/incarnation would change).

class ExtensionsGoalStateFromVmSettings(ExtensionsGoalState):
    def __init__(self, etag, json_text, correlation_id):
        self._id = "etag_{0}".format(etag)
        
class ExtensionsGoalStateFromExtensionsConfig(ExtensionsGoalState):
    def __init__(self, incarnation, xml_text, wire_client):
        self._id = "incarnation_{0}".format(incarnation)

The svd_sequence_number is used to report status (the existing code uses the incarnation for this value, but FT goal states don't have an incarnation, instead they expose the svd sequence number)

                "goalStateAggregateStatus": {
                    "formattedMessage": {
                        "lang": "en-US",
                        "message": "GoalState executed successfully"
                    },
                    "timestampUTC": "2022-03-01T19:42:18Z",
                    "inSvdSeqNo": "1",
                    "status": "Success",
                    "code": 0
                }


@property
def incarnation(self):
return self._id
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused, returning id for incarnation and vice versa?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, this is upside down, i'll fix it

@narrieta narrieta merged commit 96a6399 into Azure:develop Mar 3, 2022
@narrieta narrieta deleted the fast-track-3 branch March 3, 2022 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants