agent publish refactor #3091

nagworld9 · 2024-03-13T02:21:46Z

Description

Added RSM update verification in agent publish test.

PR includes new pipeline parameter for passing test arguments. Passing all the way from pipeline to each test.
The expected format is comma-separated list of key=value pair like publihsedVersion=2.10.0.8,hostName=Test
This is needed especially for agent publish test to know the published version to validate. This new parameter will be useful for other tests when you want to test something dynamically without needing to change tests locally.

Issue #

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.
If applicable, the PR references the bug/issue that it fixes in the description.
New Unit tests were added for the changes made

Quality of Code and Contribution Guidelines

I have read the contribution guidelines.

codecov · 2024-03-13T02:26:26Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.89%. Comparing base (e72f9e8) to head (fa61288).

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #3091   +/-   ##
========================================
  Coverage    71.89%   71.89%           
========================================
  Files          110      110           
  Lines        16395    16395           
  Branches      2342     2342           
========================================
  Hits         11788    11788           
  Misses        4055     4055           
  Partials       552      552

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

nagworld9 · 2024-03-13T02:24:03Z

tests_e2e/orchestrator/lib/agent_test_suite.py

@@ -842,6 +846,18 @@ def _create_test_context(self,) -> AgentTestContext:
                username=self._user,
                identity_file=self._identity_file)

+    def _get_test_args(self) -> Dict[str, str]:


converting list of key=value to dictionary

nagworld9 · 2024-03-13T02:25:45Z

tests_e2e/tests/lib/agent_test.py

        self._context: AgentTestContext = context
+        self._test_args: Dict[str, str] = test_args


These args exposed to all tests

nagworld9 · 2024-03-13T02:28:32Z

tests_e2e/tests/agent_publish/agent_publish.py

@@ -86,20 +129,6 @@ def _check_cse(self) -> None:
        )
        custom_script_2_1.assert_instance_view(expected_version="2.1", expected_message=message)

-    def get_ignore_error_rules(self) -> List[Dict[str, Any]]:


not needed in this test

nagworld9 · 2024-03-13T02:28:49Z

tests_e2e/tests/agent_update/rsm_update.py

@@ -146,64 +140,6 @@ def _prepare_agent(self) -> None:
                              "update-waagent-conf AutoUpdate.UpdateToLatestVersion=y Debug.EnableGAVersioning=y AutoUpdate.GAFamily=Test", use_sudo=True)
        log.info('Successfully updated agent update config \n %s', output)

-    @staticmethod


moved to common helper

maddieford · 2024-03-13T16:35:01Z

tests_e2e/tests/lib/agent_update_helpers.py

+    if not verify_agent_update_flag_enabled(vm):
+        # enable the flag
+        log.info("Attempting vm update to set the enableVMAgentPlatformUpdates flag")
+        enable_agent_update_flag(vm)


Why is this necessary? How is it used vs. our waagent.conf flag

This is different flag need to set in the vm model for CRP/RSM driving RSM updates

maddieford · 2024-03-13T17:53:57Z

tests_e2e/orchestrator/lib/agent_test_suite.py

@@ -209,6 +211,7 @@ def _initialize(self, environment: Environment, variables: Dict[str, Any], lisa_
        self._environment_name = variables["c_env_name"]

        self._test_suites = variables["c_test_suites"]
+        self._test_args = variables["test_args"]


looks like there may be inconsistency in type here. variables["test_argd"] is str while self._test_args is dict

It's still a string, later we convert to dict. The variables exposed to user in pipeline, runbook or command line is in string in the form of list of comma-separated key=value pair
Example
publihsedVersion=2.10.0.8,hostName=Test

oh, you are right. It should be string. I just noticed

narrieta

Added some comments, could you also describe how you are planning to planning to pass the arguments to the publish test from the pipeline? If the new argument is required, we should probably create a new pipeline.yml for the publish pipeline.

narrieta · 2024-03-19T14:53:44Z

tests_e2e/orchestrator/lib/agent_test_suite.py

+        if self._test_args == "":
+            return test_args
+        for arg in self._test_args.split(','):
+            key, value = arg.split('=')


we should trim spaces after splitting on comma

narrieta · 2024-03-19T15:02:36Z

tests_e2e/orchestrator/lib/agent_test_suite.py

@@ -565,6 +568,7 @@ def _execute(self) -> None:

                    try:
                        test_context = self._create_test_context()
+                        test_args = self._get_test_args()


we should do the conversion in _initialize() and keep the result in self._test_args, to avoid adding more parameters to the execute method

narrieta · 2024-03-19T15:10:14Z

tests_e2e/orchestrator/lib/agent_test_suite.py

@@ -645,7 +649,7 @@ def _execute_test_suite(self, suite: TestSuiteInfo, test_context: AgentTestConte

                        test_success: bool = True

-                        test_instance = test.test_class(test_context)
+                        test_instance = test.test_class(test_context, test_args)


Rather than passing test_args as a new argument to the test, each test_arg should be added to the text_context as a new property (or if the property already exists in the context, the just set its value).

Then AgentPublishTest.run_from_command_line should read the new parameters from the command line and add them to the test context as well. That is, it would parse the publishedVersion argument, instead of the test_args argument

tests_e2e/tests/agent_publish/agent_publish.py

narrieta · 2024-03-19T15:45:39Z

tests_e2e/tests/agent_publish/agent_publish.py

        log.info("Modifying agent update related config flags and renaming the log file")
-        self._run_remote_test(self._ssh_client, "sh -c 'agent-service stop && mv /var/log/waagent.log /var/log/waagent.$(date --iso-8601=seconds).log && update-waagent-conf AutoUpdate.UpdateToLatestVersion=y AutoUpdate.GAFamily=Test AutoUpdate.Enabled=y Extensions.Enabled=y'", use_sudo=True)
-        log.info('Renamed log file and updated agent-update DownloadNewAgents GAFamily config flags')
+        self._run_remote_test(self._ssh_client, "sh -c 'agent-service stop && mv /var/log/waagent.log /var/log/waagent.$(date --iso-8601=seconds).log && rm -rf /var/lib/waagent/WALinuxAgent-* && update-waagent-conf AutoUpdate.UpdateToLatestVersion=y AutoUpdate.GAFamily=Test AutoUpdate.Enabled=y Extensions.Enabled=y Debug.EnableGAVersioning=n'", use_sudo=True)


could you split this in multiple lines to make it more readable? thanks

nagworld9 · 2024-03-19T17:51:47Z

Added some comments, could you also describe how you are planning to planning to pass the arguments to the publish test from the pipeline? If the new argument is required, we should probably create a new pipeline.yml for the publish pipeline.

My idea is to it would benefit for other test_suites(for dev runs etc) if we added to main pipeline because the internal implementation is going to be same regardless.

When we run the pipeline, one need to pass like list of key=value pair in the pipeline arg. This converted to dict and added to agenttestcontext, so that all the tests access to dict. Whichever test needed, they read the keys to get value.

For publish test when I run the pipeline with agent_publish as test_suite, I pass publishedVersion={version}, the publish test reads this key publihsedVersion

narrieta · 2024-03-19T18:07:46Z

My idea is to it would benefit for other test_suites(for dev runs etc) if we added to main pipeline because the internal implementation is going to be same regardless.

Yes, I agree that it is useful and can be added as an optional parameter in the main pipeline

For publish test when I run the pipeline with agent_publish as test_suite, I pass publishedVersion={version}, the publish test reads this key publihsedVersion

If this parameter is required by the publish test, then maybe should have a pipeline just for publish,

nagworld9 · 2024-03-19T18:34:03Z

My idea is to it would benefit for other test_suites(for dev runs etc) if we added to main pipeline because the internal implementation is going to be same regardless.

Yes, I agree that it is useful and can be added as an optional parameter in the main pipeline

For publish test when I run the pipeline with agent_publish as test_suite, I pass publishedVersion={version}, the publish test reads this key publihsedVersion

If this parameter is required by the publish test, then maybe should have a pipeline just for publish,

Not sure I followed here. I wouldn't send this as parameter, rather I send this parameter as value in the new test_arg parameter. That way I don't have deal with separate code path just for agent publish

codecov-commenter · 2024-03-24T05:24:14Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.87%. Comparing base (c125edc) to head (2395615).

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #3091   +/-   ##
========================================
  Coverage    71.87%   71.87%           
========================================
  Files          110      110           
  Lines        16406    16406           
  Branches      2344     2344           
========================================
  Hits         11791    11791           
  Misses        4064     4064           
  Partials       551      551

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

nagworld9 · 2024-03-25T17:51:04Z

@narrieta Updated my pr. I added todo for reading it from cmd line. I couldn't get the flow right, maybe I'm missing something and also, I have different thought like we can directly use argparser for reading test specific arguments in the test itself.

narrieta · 2024-03-27T15:19:12Z

tests_e2e/tests/lib/agent_test.py

@@ -73,6 +73,8 @@ def run_from_command_line(cls):
        """
        Convenience method to execute the test when it is being invoked directly from the command line (as opposed as
        being invoked from a test framework or library.)
+
+        Todo: Need to implement for reading test specific arguments from command line


Let's use capitals: TODO. Then some tools highlight that comment

* agent publish refactor * support arm 64vm * convert dict to str * address comments * pylint * new comments * updated comment

agent publish refactor

2583bce

nagworld9 requested review from narrieta, ZhidongPeng and maddieford as code owners March 13, 2024 02:21

nagworld9 commented Mar 13, 2024

View reviewed changes

maddieford reviewed Mar 13, 2024

View reviewed changes

nagworld9 added 2 commits March 13, 2024 15:25

support arm 64vm

b9acb77

convert dict to str

fa61288

narrieta reviewed Mar 19, 2024

View reviewed changes

address comments

4aa33e5

pylint

f8490b6

narrieta reviewed Mar 27, 2024

View reviewed changes

nagworld9 added 2 commits March 27, 2024 10:00

new comments

528e6e3

updated comment

bad260c

narrieta approved these changes Mar 27, 2024

View reviewed changes

maddieford approved these changes Mar 27, 2024

View reviewed changes

Merge branch 'develop' into agentpublish-test

2395615

nagworld9 merged commit 101cc1e into Azure:develop Mar 28, 2024
11 checks passed

nagworld9 deleted the agentpublish-test branch March 28, 2024 16:25

maddieford pushed a commit to maddieford/WALinuxAgent that referenced this pull request Apr 29, 2024

agent publish refactor (Azure#3091)

4611e52

* agent publish refactor * support arm 64vm * convert dict to str * address comments * pylint * new comments * updated comment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

agent publish refactor #3091

agent publish refactor #3091

nagworld9 commented Mar 13, 2024 •

edited

Loading

codecov bot commented Mar 13, 2024 •

edited

Loading

nagworld9 Mar 13, 2024

nagworld9 Mar 13, 2024

nagworld9 Mar 13, 2024

nagworld9 Mar 13, 2024

maddieford Mar 13, 2024

nagworld9 Mar 13, 2024 •

edited

Loading

maddieford Mar 13, 2024

nagworld9 Mar 13, 2024

nagworld9 Mar 14, 2024

nagworld9 Mar 14, 2024

narrieta left a comment

narrieta Mar 19, 2024

narrieta Mar 19, 2024

narrieta Mar 19, 2024

narrieta Mar 19, 2024

nagworld9 commented Mar 19, 2024

narrieta commented Mar 19, 2024

nagworld9 commented Mar 19, 2024 •

edited

Loading

codecov-commenter commented Mar 24, 2024 •

edited

Loading

nagworld9 commented Mar 25, 2024

narrieta Mar 27, 2024

		self._context: AgentTestContext = context
		self._test_args: Dict[str, str] = test_args

agent publish refactor #3091

agent publish refactor #3091

Conversation

nagworld9 commented Mar 13, 2024 • edited Loading

Description

PR information

Quality of Code and Contribution Guidelines

codecov bot commented Mar 13, 2024 • edited Loading

Codecov Report

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

nagworld9 Mar 13, 2024 • 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

Choose a reason for hiding this comment

narrieta 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

nagworld9 commented Mar 19, 2024

narrieta commented Mar 19, 2024

nagworld9 commented Mar 19, 2024 • edited Loading

codecov-commenter commented Mar 24, 2024 • edited Loading

Codecov Report

nagworld9 commented Mar 25, 2024

Choose a reason for hiding this comment

nagworld9 commented Mar 13, 2024 •

edited

Loading

codecov bot commented Mar 13, 2024 •

edited

Loading

nagworld9 Mar 13, 2024 •

edited

Loading

nagworld9 commented Mar 19, 2024 •

edited

Loading

codecov-commenter commented Mar 24, 2024 •

edited

Loading