-
Notifications
You must be signed in to change notification settings - Fork 375
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
agent publish refactor #3091
agent publish refactor #3091
Changes from 1 commit
2583bce
b9acb77
fa61288
4aa33e5
f8490b6
528e6e3
bad260c
2395615
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -149,6 +149,8 @@ def __init__(self, metadata: TestSuiteMetadata) -> None: | |
|
||
self._test_suites: List[AgentTestSuite] # Test suites to execute in the environment | ||
|
||
self._test_args: Dict[str, str] # Additional arguments pass to the test suite | ||
|
||
self._cloud: str # Azure cloud where test VMs are located | ||
self._subscription_id: str # Azure subscription where test VMs are located | ||
self._location: str # Azure location (region) where test VMs are located | ||
|
@@ -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"] | ||
|
||
self._cloud = variables["cloud"] | ||
self._subscription_id = variables["subscription_id"] | ||
|
@@ -565,6 +568,7 @@ def _execute(self) -> None: | |
|
||
try: | ||
test_context = self._create_test_context() | ||
test_args = self._get_test_args() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should do the conversion in _initialize() and keep the result in self._test_args, to avoid adding more parameters to the execute method |
||
|
||
if not self._skip_setup: | ||
try: | ||
|
@@ -578,7 +582,7 @@ def _execute(self) -> None: | |
for suite in self._test_suites: | ||
log.info("Executing test suite %s", suite.name) | ||
self._lisa_log.info("Executing Test Suite %s", suite.name) | ||
case_success, check_log_start_time = self._execute_test_suite(suite, test_context, check_log_start_time) | ||
case_success, check_log_start_time = self._execute_test_suite(suite, test_context, test_args, check_log_start_time) | ||
test_suite_success = case_success and test_suite_success | ||
if not case_success: | ||
failed_cases.append(suite.name) | ||
|
@@ -613,7 +617,7 @@ def _execute(self) -> None: | |
if not test_suite_success or unexpected_error: | ||
raise TestFailedException(self._environment_name, failed_cases) | ||
|
||
def _execute_test_suite(self, suite: TestSuiteInfo, test_context: AgentTestContext, check_log_start_time: datetime.datetime) -> Tuple[bool, datetime.datetime]: | ||
def _execute_test_suite(self, suite: TestSuiteInfo, test_context: AgentTestContext, test_args: Dict[str, str], check_log_start_time: datetime.datetime) -> Tuple[bool, datetime.datetime]: | ||
""" | ||
Executes the given test suite and returns a tuple of a bool indicating whether all the tests in the suite succeeded, and the timestamp that should be used | ||
for the next check of the agent log. | ||
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
try: | ||
test_instance.run() | ||
summary.append(f"[Passed] {test.name}") | ||
|
@@ -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]: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. converting list of key=value to dictionary |
||
""" | ||
Returns the arguments to be passed to the test classes | ||
""" | ||
test_args: Dict[str, str] = {} | ||
if self._test_args == "": | ||
return test_args | ||
for arg in self._test_args.split(','): | ||
key, value = arg.split('=') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should trim spaces after splitting on comma |
||
test_args[key] = value | ||
return test_args | ||
|
||
@staticmethod | ||
def _mark_log_as_failed(): | ||
""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,10 +18,10 @@ | |
# | ||
import uuid | ||
from datetime import datetime | ||
from typing import Any, Dict, List | ||
|
||
from tests_e2e.tests.lib.agent_test import AgentVmTest | ||
from tests_e2e.tests.lib.agent_test_context import AgentVmTestContext | ||
from tests_e2e.tests.lib.agent_update_helpers import request_rsm_update | ||
from tests_e2e.tests.lib.vm_extension_identifier import VmExtensionIds, VmExtensionIdentifier | ||
from tests_e2e.tests.lib.logging import log | ||
from tests_e2e.tests.lib.ssh_client import SshClient | ||
|
@@ -33,9 +33,10 @@ class AgentPublishTest(AgentVmTest): | |
This script verifies if the agent update performed in the vm. | ||
""" | ||
|
||
def __init__(self, context: AgentVmTestContext): | ||
super().__init__(context) | ||
def __init__(self, context: AgentVmTestContext, test_args: dict): | ||
super().__init__(context, test_args) | ||
self._ssh_client: SshClient = self._context.create_ssh_client() | ||
self._published_version = self._test_args.get('publishedVersion', '9.9.9.9') | ||
|
||
def run(self): | ||
""" | ||
|
@@ -47,9 +48,17 @@ def run(self): | |
5. Ensure CSE is working | ||
""" | ||
self._get_agent_info() | ||
self._prepare_agent() | ||
|
||
narrieta marked this conversation as resolved.
Show resolved
Hide resolved
|
||
log.info("Testing rsm update flow....") | ||
self._prepare_agent_for_rsm_update() | ||
self._check_update() | ||
self._get_agent_info() | ||
|
||
log.info("Testing self update flow....") | ||
self._prepare_agent_for_self_update() | ||
self._check_update() | ||
self._get_agent_info() | ||
|
||
self._check_cse() | ||
|
||
def get_ignore_errors_before_timestamp(self) -> datetime: | ||
|
@@ -60,14 +69,48 @@ def _get_agent_info(self) -> None: | |
stdout: str = self._ssh_client.run_command("waagent-version", use_sudo=True) | ||
log.info('Agent info \n%s', stdout) | ||
|
||
def _prepare_agent(self) -> None: | ||
def _verify_agent_reported_supported_feature_flag(self): | ||
""" | ||
RSM update rely on supported feature flag that agent sends to CRP.So, checking if GA reports feature flag from reported status | ||
""" | ||
log.info( | ||
"Executing verify_versioning_supported_feature.py remote script to verify agent reported supported feature flag, so that CRP can send RSM update request") | ||
self._run_remote_test(self._ssh_client, "agent_update-verify_versioning_supported_feature.py", use_sudo=True) | ||
log.info("Successfully verified that Agent reported VersioningGovernance supported feature flag") | ||
|
||
def _check_rsm_gs(self, requested_version: str) -> None: | ||
# This checks if RSM GS available to the agent after we send the rsm update request | ||
log.info( | ||
'Executing wait_for_rsm_gs.py remote script to verify latest GS contain requested version after rsm update requested') | ||
self._run_remote_test(self._ssh_client, f"agent_update-wait_for_rsm_gs.py --version {requested_version}", | ||
use_sudo=True) | ||
log.info('Verified latest GS contain requested version after rsm update requested') | ||
|
||
def _prepare_agent_for_rsm_update(self) -> None: | ||
""" | ||
This method prepares the agent for the RSM update | ||
""" | ||
log.info( | ||
'Updating agent config flags to allow and download test versions') | ||
output: str = self._ssh_client.run_command( | ||
"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) | ||
|
||
self._verify_agent_reported_supported_feature_flag() | ||
request_rsm_update(self._published_version, self._context.vm) | ||
self._check_rsm_gs(self._published_version) | ||
|
||
def _prepare_agent_for_self_update(self) -> None: | ||
""" | ||
This method prepares the agent for the self update | ||
""" | ||
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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you split this in multiple lines to make it more readable? thanks |
||
log.info('Renamed log file and updated self-update config flags') | ||
|
||
def _check_update(self) -> None: | ||
log.info("Verifying for agent update status") | ||
self._run_remote_test(self._ssh_client, "agent_publish-check_update.py") | ||
self._run_remote_test(self._ssh_client, f"agent_publish-check_update.py --published-version {self._published_version}") | ||
log.info('Successfully checked the agent update') | ||
|
||
def _check_cse(self) -> None: | ||
|
@@ -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]]: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not needed in this test |
||
ignore_rules = [ | ||
# | ||
# This is expected as latest version can be the less than test version | ||
# | ||
# WARNING ExtHandler ExtHandler Agent WALinuxAgent-9.9.9.9 is permanently blacklisted | ||
# | ||
{ | ||
'message': r"Agent WALinuxAgent-9.9.9.9 is permanently blacklisted" | ||
} | ||
|
||
] | ||
return ignore_rules | ||
|
||
|
||
if __name__ == "__main__": | ||
AgentPublishTest.run_from_command_line() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like there may be inconsistency in type here. variables["test_argd"] is str while self._test_args is dict
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, you are right. It should be string. I just noticed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated