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

agent publish refactor #3091

Merged
merged 8 commits into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
22 changes: 19 additions & 3 deletions tests_e2e/orchestrator/lib/agent_test_suite.py
Original file line number Diff line number Diff line change
Expand Up @@ -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: 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
Expand Down Expand Up @@ -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"]
Copy link
Contributor

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

Copy link
Contributor Author

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

Copy link
Contributor Author

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated


self._cloud = variables["cloud"]
self._subscription_id = variables["subscription_id"]
Expand Down Expand Up @@ -565,6 +568,7 @@ def _execute(self) -> None:

try:
test_context = self._create_test_context()
test_args = self._get_test_args()
Copy link
Member

Choose a reason for hiding this comment

The 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:
Expand All @@ -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)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The 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}")
Expand Down Expand Up @@ -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]:
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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('=')
Copy link
Member

Choose a reason for hiding this comment

The 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():
"""
Expand Down
6 changes: 6 additions & 0 deletions tests_e2e/orchestrator/runbook.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ variable:
- recover_network_interface

#
# Additional arguments pass to the test suites
#
- name: test_args
value: ""
is_case_visible: true

# Parameters used to create test VMs
#
- name: subscription_id
Expand Down
6 changes: 6 additions & 0 deletions tests_e2e/pipeline/pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ parameters:
type: string
default: "-"

- name: test_args
displayName: Test Args (additional arguments pass to the test suites. Comma-separated list of key=value pairs)
type: string
default: "-"

- name: image
displayName: Image (image/image set name, URN, or VHD)
type: string
Expand Down Expand Up @@ -121,6 +126,7 @@ jobs:
KEEP_ENVIRONMENT: ${{ parameters.keep_environment }}
LOCATION: ${{ parameters.location }}
TEST_SUITES: ${{ parameters.test_suites }}
TEST_ARGS: ${{ parameters.test_args }}
VM_SIZE: ${{ parameters.vm_size }}

- bash: $(Build.SourcesDirectory)/tests_e2e/pipeline/scripts/collect_artifacts.sh
Expand Down
4 changes: 4 additions & 0 deletions tests_e2e/pipeline/scripts/execute_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ if [[ $TEST_SUITES == "-" ]]; then
else
TEST_SUITES="-v test_suites:\"$TEST_SUITES\""
fi
if [[ $TEST_ARGS == "-" ]]; then
TEST_ARGS=""
fi
if [[ $IMAGE == "-" ]]; then
IMAGE=""
fi
Expand Down Expand Up @@ -92,4 +95,5 @@ docker run --rm \
-v location:\"$LOCATION\" \
-v vm_size:\"$VM_SIZE\" \
-v allow_ssh:\"$IP_ADDRESS\" \
-v test_args:\"$TEST_ARGS\" \
$TEST_SUITES"
4 changes: 2 additions & 2 deletions tests_e2e/tests/agent_cgroups/agent_cgroups.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ class AgentCgroups(AgentVmTest):
This test verifies that the agent is running in the expected cgroups.
"""

def __init__(self, context: AgentVmTestContext):
super().__init__(context)
def __init__(self, context: AgentVmTestContext, test_args: dict):
super().__init__(context, test_args)
self._ssh_client = self._context.create_ssh_client()

def run(self):
Expand Down
4 changes: 2 additions & 2 deletions tests_e2e/tests/agent_cgroups/agent_cpu_quota.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ class AgentCPUQuota(AgentVmTest):
"""
The test verify that the agent detects when it is throttled for using too much CPU, that it detects processes that do belong to the agent's cgroup, and that resource metrics are generated.
"""
def __init__(self, context: AgentVmTestContext):
super().__init__(context)
def __init__(self, context: AgentVmTestContext, test_args: dict):
super().__init__(context, test_args)
self._ssh_client = self._context.create_ssh_client()

def run(self):
Expand Down
4 changes: 2 additions & 2 deletions tests_e2e/tests/agent_ext_workflow/extension_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ class ExtensionWorkflow(AgentVmTest):
- Match the operation sequence as per the test and make sure they are in the correct chronological order
- Restart the agent and verify if the correct operation sequence is followed
"""
def __init__(self, context: AgentVmTestContext):
super().__init__(context)
def __init__(self, context: AgentVmTestContext, test_args: dict):
super().__init__(context, test_args)
self._ssh_client = context.create_ssh_client()

# This class represents the GuestAgentDcrTestExtension running on the test VM
Expand Down
4 changes: 2 additions & 2 deletions tests_e2e/tests/agent_firewall/agent_firewall.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ class AgentFirewall(AgentVmTest):
This test verifies the agent firewall rules are added properly. It checks each firewall rule is present and working as expected.
"""

def __init__(self, context: AgentVmTestContext):
super().__init__(context)
def __init__(self, context: AgentVmTestContext, test_args: dict):
super().__init__(context, test_args)
self._ssh_client = self._context.create_ssh_client()

def run(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ class AgentPersistFirewallTest(AgentVmTest):
This test verifies agent setup persist firewall rules using custom network setup service or firewalld service. Ensure those rules are added on boot and working as expected.
"""

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()

def run(self):
Expand Down
80 changes: 58 additions & 22 deletions tests_e2e/tests/agent_publish/agent_publish.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
"""
Expand All @@ -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:
Expand All @@ -60,14 +69,55 @@ 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
"""
# First we update the agent to latest version like prod
# Next send RSM update request for new published test version
log.info(
'Updating agent config flags to allow and download test versions')
output: str = self._ssh_client.run_command(
"update-waagent-conf AutoUpdate.Enabled=y AutoUpdate.UpdateToLatestVersion=y", use_sudo=True)
log.info('Successfully updated agent update config \n %s', output)

self._verify_agent_reported_supported_feature_flag()
arch_type = self._ssh_client.get_architecture()
request_rsm_update(self._published_version, self._context.vm, arch_type)
self._check_rsm_gs(self._published_version)

output: str = self._ssh_client.run_command(
"update-waagent-conf Debug.EnableGAVersioning=y AutoUpdate.GAFamily=Test", use_sudo=True)
log.info('Successfully enabled rsm updates \n %s', output)

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)
Copy link
Member

Choose a reason for hiding this comment

The 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:
Expand All @@ -86,20 +136,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]]:
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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()
4 changes: 2 additions & 2 deletions tests_e2e/tests/agent_status/agent_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ class RetryableAgentStatusException(BaseException):


class AgentStatus(AgentVmTest):
def __init__(self, context: AgentVmTestContext):
super().__init__(context)
def __init__(self, context: AgentVmTestContext, test_args: dict):
super().__init__(context, test_args)
self._ssh_client = self._context.create_ssh_client()

def validate_instance_view_vmagent_status(self, instance_view: VirtualMachineInstanceView):
Expand Down
Loading
Loading