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.

Sign up for GitHub

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

fix the ignore rule in agent update test #2915

Merged
merged 4 commits into from
Sep 7, 2023
Merged

fix the ignore rule in agent update test #2915

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
96fb92d
ignore the agent installed version
nagworld9 Sep 5, 2023
26fdae1
address comments
nagworld9 Sep 6, 2023
f16540b
address comments
nagworld9 Sep 6, 2023
64b8954
fixes
nagworld9 Sep 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 8 additions & 12 deletions tests_e2e/orchestrator/lib/agent_test_suite.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,8 +467,6 @@ def _execute(self, environment: Environment, variables: Dict[str, Any]):
self.context.lisa_log.info("Executing Test Suite %s", suite.name)
test_suite_success = self._execute_test_suite(suite) and test_suite_success

test_suite_success = self._check_agent_log() and test_suite_success

finally:
collect = self.context.collect_logs
if collect == CollectLogs.Always or collect == CollectLogs.Failed and not test_suite_success:
Expand Down Expand Up @@ -512,6 +510,7 @@ def _execute_test_suite(self, suite: TestSuiteInfo) -> bool:
log.info("")

summary: List[str] = []
ignore_error_rules: List[Dict[str, Any]] = []

for test in suite.tests:
test_full_name = f"{suite_name}-{test.name}"
Expand All @@ -522,9 +521,9 @@ def _execute_test_suite(self, suite: TestSuiteInfo) -> bool:

test_success: bool = True

load_test = test.test_class(self.context)
narrieta marked this conversation as resolved.
Show resolved Hide resolved
try:
test.test_class(self.context).run()

load_test.run()
summary.append(f"[Passed] {test.name}")
log.info("******** [Passed] %s", test.name)
self.context.lisa_log.info("[Passed] %s", test_full_name)
Expand Down Expand Up @@ -587,6 +586,8 @@ def _execute_test_suite(self, suite: TestSuiteInfo) -> bool:
log.warning("%s failed and blocks the suite. Stopping suite execution.", test.name)
break

ignore_error_rules.extend(load_test.get_ignore_error_rules())
Copy link
Member

Choose a reason for hiding this comment

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

i think we need to add the rules even if we hit the break at 587

Copy link
Contributor Author

@nagworld9 nagworld9 Sep 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

you mean for remaining for tests? or every test regardless? I don't see a point if test run failed.

Copy link
Member

Choose a reason for hiding this comment

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

you are already adding the rule if the test failed.

the break is hit when the test failed and it blocks the suite

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you meant for that particular test? Don't care about reaming tests after hit break

Copy link
Member

Choose a reason for hiding this comment

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

for the test, right? that is what you are trying to do. ignore rules are associates to specific tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when we block suite run, do we need to add rules for rest of the tests in suite?Lets' say suite has 1,2,3 test and 2 has failed and has flag to block suite. So ignore rule list should contain 1 and 2 but not 3 or all 1,2,3?

Copy link
Member

Choose a reason for hiding this comment

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

it should not contain the rules for the tests that did not execute

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what I thought, thanks for the clarification


log.info("")
log.info("******** [Test Results]")
log.info("")
Expand All @@ -607,9 +608,11 @@ def _execute_test_suite(self, suite: TestSuiteInfo) -> bool:
if not suite_success:
self._mark_log_as_failed()

suite_success = suite_success and self._check_agent_log(ignore_error_rules)

return suite_success

def _check_agent_log(self) -> bool:
def _check_agent_log(self, ignore_error_rules: List[Dict[str, Any]]) -> bool:
"""
Checks the agent log for errors; returns true on success (no errors int the log)
"""
Expand All @@ -623,13 +626,6 @@ def _check_agent_log(self) -> bool:
errors = json.loads(output, object_hook=AgentLogRecord.from_dictionary)

# Individual tests may have rules to ignore known errors; filter those out
ignore_error_rules = []
# pylint seems to think self.context.test_suites is not iterable. Suppressing warning, since its type is List[AgentTestSuite]
# E1133: Non-iterable value self.context.test_suites is used in an iterating context (not-an-iterable)
for suite in self.context.test_suites: # pylint: disable=E1133
for test in suite.tests:
ignore_error_rules.extend(test.test_class(self.context).get_ignore_error_rules())

if len(ignore_error_rules) > 0:
new = []
for e in errors:
Expand Down
33 changes: 25 additions & 8 deletions tests_e2e/tests/agent_update/rsm_update.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
# For each scenario, we initiate the rsm request with target version and then verify agent updated to that target version.
#
import json
import re
from typing import List, Dict, Any

import requests
Expand All @@ -49,16 +50,19 @@ def __init__(self, context: AgentTestContext):
ip_address=self._context.vm_ip_address,
username=self._context.username,
private_key_file=self._context.private_key_file)
self._installed_agent_version = "9.9.9.9"
self._downgrade_version = "9.9.9.9"

def get_ignore_error_rules(self) -> List[Dict[str, Any]]:
ignore_rules = [
#
# This is expected as we validate the downgrade scenario
#
# WARNING ExtHandler ExtHandler Agent WALinuxAgent-9.9.9.9 is permanently blacklisted
#
# Note: Version varies depending on the pipeline branch the test is running on
{
'message': r"Agent WALinuxAgent-9.9.9.9 is permanently blacklisted"
'message': rf"Agent WALinuxAgent-{self._installed_agent_version} is permanently blacklisted",
'if': lambda r: r.prefix == 'ExtHandler' and self._installed_agent_version > self._downgrade_version
},
# We don't allow downgrades below then daemon version
# 2023-07-11T02:28:21.249836Z WARNING ExtHandler ExtHandler [AgentUpdateError] The Agent received a request to downgrade to version 1.4.0.0, but downgrading to a version less than the Agent installed on the image (1.4.0.1) is not supported. Skipping downgrade.
Expand All @@ -71,20 +75,22 @@ def get_ignore_error_rules(self) -> List[Dict[str, Any]]:
return ignore_rules

def run(self) -> None:
# retrieve the installed agent version in the vm before run the scenario
self._retrieve_installed_agent_version()
# Allow agent to send supported feature flag
self._verify_agent_reported_supported_feature_flag()

log.info("*******Verifying the Agent Downgrade scenario*******")
stdout: str = self._ssh_client.run_command("waagent-version", use_sudo=True)
log.info("Current agent version running on the vm before update is \n%s", stdout)
downgrade_version: str = "1.5.0.0"
log.info("Attempting downgrade version %s", downgrade_version)
self._request_rsm_update(downgrade_version)
self._check_rsm_gs(downgrade_version)
self._downgrade_version: str = "1.5.0.0"
log.info("Attempting downgrade version %s", self._downgrade_version)
self._request_rsm_update(self._downgrade_version)
self._check_rsm_gs(self._downgrade_version)
self._prepare_agent()
# Verify downgrade scenario
self._verify_guest_agent_update(downgrade_version)
self._verify_agent_reported_update_status(downgrade_version)
self._verify_guest_agent_update(self._downgrade_version)
self._verify_agent_reported_update_status(self._downgrade_version)


# Verify upgrade scenario
Expand Down Expand Up @@ -241,6 +247,17 @@ def _verify_agent_reported_update_status(self, version: str):
self._ssh_client.run_command(f"agent_update-verify_agent_reported_update_status.py --version {version}", use_sudo=True)
log.info("Successfully Agent reported update status for version {0}".format(version))

def _retrieve_installed_agent_version(self):
"""
Retrieve the installed agent version
"""
log.info("Retrieving installed agent version")
stdout: str = self._ssh_client.run_command("waagent-version", use_sudo=True)
log.info("Retrieved installed agent version \n {0}".format(stdout))
match = re.search(r'.*Goal state agent: (\S*)', stdout)
if match:
Copy link
Member

@narrieta narrieta Sep 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

no match should be an error instead of defaulting to 9.9.9.9, shouldn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even we raise an error, we still check ignore rules. Need default value either case but that won't change behavior and makes if condition false, so rule won't apply.

But I'll raise error

Copy link
Member

Choose a reason for hiding this comment

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

or at least a warning, rather than silently default to 9.9.9.9

self._installed_agent_version = match.groups()[0]


if __name__ == "__main__":
RsmUpdateBvt.run_from_command_line()
Loading