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

Add BVT for the agent #2719

Merged
merged 16 commits into from
Jan 6, 2023
Merged
Show file tree
Hide file tree
Changes from 15 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
2 changes: 1 addition & 1 deletion .github/workflows/ci_pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ jobs:
PYLINTOPTS: "--rcfile=ci/3.6.pylintrc --ignore=tests_e2e"

- python-version: 3.9
PYLINTOPTS: "--rcfile=ci/3.6.pylintrc --ignore=azure_models.py,BaseExtensionTestClass.py"
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed those files, which were simple prototypes.

PYLINTOPTS: "--rcfile=ci/3.6.pylintrc"
additional-nose-opts: "--with-coverage --cover-erase --cover-inclusive --cover-branches --cover-package=azurelinuxagent"

name: "Python ${{ matrix.python-version }} Unit Tests"
Expand Down
3 changes: 2 additions & 1 deletion makepkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ def run(agent_family, output_directory, log):
os.makedirs(bin_path)
log.info("Created {0} directory".format(target_path))

args = ["python3", "setup.py", "bdist_egg", "--dist-dir={0}".format(bin_path)]
Copy link
Member Author

@narrieta narrieta Jan 3, 2023

Choose a reason for hiding this comment

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

This allows invoking makepkg without cd'ing to the WALinuxAgent directory

setup_path = os.path.join(os.path.dirname(__file__), "setup.py")
args = ["python3", setup_path, "bdist_egg", "--dist-dir={0}".format(bin_path)]

log.info("Creating egg {0}".format(egg_path))
do(*args)
Expand Down
6 changes: 6 additions & 0 deletions test-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,9 @@ wrapt==1.12.0; python_version > '2.6' and python_version < '3.6'
pylint; python_version > '2.6' and python_version < '3.6'
pylint==2.8.3; python_version >= '3.6'

# Requirements to run pylint on the end-to-end tests source code
assertpy
azure-core
azure-identity
azure-mgmt-compute>=22.1.0
azure-mgmt-resource>=15.0.0
239 changes: 160 additions & 79 deletions tests_e2e/orchestrator/lib/agent_test_suite.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,88 +14,109 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
from collections.abc import Callable
from assertpy import assert_that
from pathlib import Path
from shutil import rmtree
from typing import List, Type

import makepkg

# E0401: Unable to import 'lisa' (import-error)
# Disable those warnings, since 'lisa' is an external, non-standard, dependency
# E0401: Unable to import 'lisa' (import-error)
# E0401: Unable to import 'lisa.sut_orchestrator' (import-error)
# E0401: Unable to import 'lisa.sut_orchestrator.azure.common' (import-error)
from lisa import ( # pylint: disable=E0401
CustomScriptBuilder,
Logger,
Node,
TestSuite,
TestSuiteMetadata
)
# E0401: Unable to import 'lisa.sut_orchestrator.azure.common' (import-error)
from lisa.sut_orchestrator.azure.common import get_node_context # pylint: disable=E0401
from lisa.sut_orchestrator import AZURE # pylint: disable=E0401
from lisa.sut_orchestrator.azure.common import get_node_context, AzureNodeSchema # pylint: disable=E0401

import makepkg
from azurelinuxagent.common.version import AGENT_VERSION
from tests_e2e.scenarios.lib.agent_test import AgentTest
from tests_e2e.scenarios.lib.agent_test_context import AgentTestContext
from tests_e2e.scenarios.lib.identifiers import VmIdentifier
from tests_e2e.scenarios.lib.logging import log


class AgentTestScenario(object):
class AgentLisaTestContext(AgentTestContext):
"""
Instances of this class are used to execute Agent test scenarios. It also provides facilities to execute commands over SSH.
Execution context for LISA tests.
"""
def __init__(self, vm: VmIdentifier, node: Node):
super().__init__(
vm=vm,
paths=AgentTestContext.Paths(remote_working_directory=Path('/home')/node.connection_info['username']),
connection=AgentTestContext.Connection(
ip_address=node.connection_info['address'],
username=node.connection_info['username'],
private_key_file=node.connection_info['private_key_file'],
ssh_port=node.connection_info['port'])
)
self._node = node

class Context:
"""
Execution context for test scenarios, this information is passed to test scenarios by AgentTestScenario.execute()
"""
subscription_id: str
resource_group_name: str
vm_name: str
test_source_directory: Path
working_directory: Path
node_home_directory: Path
@property
def node(self) -> Node:
return self._node


class AgentTestSuite(TestSuite):
Copy link
Member Author

@narrieta narrieta Jan 3, 2023

Choose a reason for hiding this comment

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

I derived AgentTestSuite fromLISA's TestSuite again, since it simplifies the calling pattern for tests and allows the use of some metadata. I moved all its data members into the AgentTestContext and replaced self._log with the global Logger.

"""
Base class for Agent test suites. It provides facilities for setup, execution of tests and reporting results. Derived
classes use the execute() method to run the tests in their corresponding suites.
"""
def __init__(self, metadata: TestSuiteMetadata) -> None:
super().__init__(metadata)
# The context is initialized by execute()
self.__context: AgentLisaTestContext = None

def __init__(self, node: Node, log: Logger) -> None:
self._node: Node = node
self._log: Logger = log
@property
def context(self) -> AgentLisaTestContext:
if self.__context is None:
raise Exception("The context for the AgentTestSuite has not been initialized")
return self.__context

def _set_context(self, node: Node):
node_context = get_node_context(node)
self._context: AgentTestScenario.Context = AgentTestScenario.Context()
self._context.subscription_id = node.features._platform.subscription_id
self._context.resource_group_name = node_context.resource_group_name
self._context.vm_name = node_context.vm_name

self._context.test_source_directory = AgentTestScenario._get_test_source_directory()
self._context.working_directory = Path().home()/"waagent-tmp"
self._context.node_home_directory = Path('/home')/node.connection_info['username']
runbook = node.capability.get_extended_runbook(AzureNodeSchema, AZURE)

@staticmethod
def _get_test_source_directory() -> Path:
"""
Returns the root directory of the source code for the end-to-end tests (".../WALinuxAgent/tests_e2e")
"""
path = Path(__file__)
while path.name != '':
if path.name == "tests_e2e":
return path
path = path.parent
raise Exception("Can't find the test root directory (tests_e2e)")
self.__context = AgentLisaTestContext(
VmIdentifier(
location=runbook.location,
subscription=node.features._platform.subscription_id,
resource_group=node_context.resource_group_name,
name=node_context.vm_name
),
node
)

def _setup(self) -> None:
"""
Prepares the test scenario for execution
Prepares the test suite for execution
"""
self._log.info(f"Test Node: {self._context.vm_name}")
self._log.info(f"Resource Group: {self._context.resource_group_name}")
self._log.info(f"Working directory: {self._context.working_directory}...")
log.info("Test Node: %s", self.context.vm.name)
log.info("Resource Group: %s", self.context.vm.resource_group)
log.info("Working directory: %s", self.context.working_directory)

if self._context.working_directory.exists():
self._log.info(f"Removing existing working directory: {self._context.working_directory}...")
if self.context.working_directory.exists():
log.info("Removing existing working directory: %s", self.context.working_directory)
try:
rmtree(self._context.working_directory.as_posix())
rmtree(self.context.working_directory.as_posix())
except Exception as exception:
self._log.warning(f"Failed to remove the working directory: {exception}")
self._context.working_directory.mkdir()
log.warning("Failed to remove the working directory: %s", exception)
self.context.working_directory.mkdir()

def _clean_up(self) -> None:
"""
Cleans up any leftovers from the test scenario run.
Cleans up any leftovers from the test suite run.
"""
self._log.info(f"Removing {self._context.working_directory}...")
rmtree(self._context.working_directory.as_posix(), ignore_errors=True)
try:
log.info("Removing %s", self.context.working_directory)
rmtree(self.context.working_directory.as_posix(), ignore_errors=True)
except: # pylint: disable=bare-except
log.exception("Failed to cleanup the test run")

def _setup_node(self) -> None:
"""
Expand All @@ -108,15 +129,17 @@ def _build_agent_package(self) -> Path:
"""
Builds the agent package and returns the path to the package.
"""
build_path = self._context.working_directory/"build"
build_path = self.context.working_directory/"build"

log.info("Building agent package to %s", build_path)

makepkg.run(agent_family="Test", output_directory=str(build_path), log=log)

self._log.info(f"Building agent package to {build_path}")
makepkg.run(agent_family="Test", output_directory=str(build_path), log=self._log)
package_path = build_path/"eggs"/f"WALinuxAgent-{AGENT_VERSION}.zip"
if not package_path.exists():
raise Exception(f"Can't find the agent package at {package_path}")

self._log.info(f"Agent package: {package_path}")
log.info("Agent package: %s", package_path)

return package_path

Expand All @@ -125,69 +148,127 @@ def _install_agent_on_node(self, agent_package: Path) -> None:
Installs the given agent package on the test node.
"""
# The install script needs to unzip the agent package; ensure unzip is installed on the test node
self._log.info(f"Installing unzip on {self._context.vm_name}...")
self._node.os.install_packages("unzip")
log.info("Installing unzip tool on %s", self.context.node.name)
self.context.node.os.install_packages("unzip")

self._log.info(f"Installing {agent_package} on {self._context.vm_name}...")
agent_package_remote_path = self._context.node_home_directory/agent_package.name
self._log.info(f"Copying {agent_package} to {self._context.vm_name}:{agent_package_remote_path}")
self._node.shell.copy(agent_package, agent_package_remote_path)
log.info("Installing %s on %s", agent_package, self.context.node.name)
agent_package_remote_path = self.context.remote_working_directory / agent_package.name
log.info("Copying %s to %s:%s", agent_package, self.context.node.name, agent_package_remote_path)
self.context.node.shell.copy(agent_package, agent_package_remote_path)
self.execute_script_on_node(
self._context.test_source_directory/"orchestrator"/"scripts"/"install-agent",
self.context.test_source_directory/"orchestrator"/"scripts"/"install-agent",
parameters=f"--package {agent_package_remote_path} --version {AGENT_VERSION}",
sudo=True)

self._log.info("The agent was installed successfully.")
log.info("The agent was installed successfully.")

def _collect_node_logs(self) -> None:
"""
Collects the test logs from the remote machine and copies them to the local machine
"""
try:
# Collect the logs on the test machine into a compressed tarball
self._log.info("Collecting logs on test machine [%s]...", self._node.name)
self.execute_script_on_node(self._context.test_source_directory/"orchestrator"/"scripts"/"collect-logs", sudo=True)
log.info("Collecting logs on test machine [%s]...", self.context.node.name)
self.execute_script_on_node(self.context.test_source_directory/"orchestrator"/"scripts"/"collect-logs", sudo=True)

# Copy the tarball to the local logs directory
remote_path = self._context.node_home_directory/"logs.tgz"
local_path = Path.home()/'logs'/'vm-logs-{0}.tgz'.format(self._node.name)
self._log.info(f"Copying {self._node.name}:{remote_path} to {local_path}")
self._node.shell.copy_back(remote_path, local_path)
except Exception as e:
self._log.warning(f"Failed to collect logs from the test machine: {e}")
remote_path = self.context.remote_working_directory / "logs.tgz"
local_path = Path.home()/'logs'/'vm-logs-{0}.tgz'.format(self.context.node.name)
log.info("Copying %s:%s to %s", self.context.node.name, remote_path, local_path)
self.context.node.shell.copy_back(remote_path, local_path)
except: # pylint: disable=bare-except
log.exception("Failed to collect logs from the test machine")

def execute(self, scenario: Callable[[Context], None]) -> None:
def execute(self, node: Node, test_suite: List[Type[AgentTest]]) -> None:
"""
Executes the given scenario
Executes each of the AgentTests in the given List. Note that 'test_suite' is a list of test classes, rather than
instances of the test class (this method will instantiate each of these test classes).
"""
self._set_context(node)
Copy link
Member Author

Choose a reason for hiding this comment

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

I improved the way we report test results. See the latest test run for an example.


log.info("")
log.info("**************************************** [Setup] ****************************************")
log.info("")

failed: List[str] = []

try:
self._setup()

try:
self._setup_node()
scenario(self._context)

log.info("")
log.info("**************************************** [%s] ****************************************", self._metadata.full_name)
log.info("")

results: List[str] = []

for test in test_suite:
try:
log.info("******************** [%s]", test.__name__)
log.info("")

test(self.context).run()

result = f"[Passed] {test.__name__}"

log.info("")
log.info("******************** %s", result)
log.info("")

results.append(result)
except: # pylint: disable=bare-except
result = f"[Failed] {test.__name__}"

log.info("")
log.exception("******************** %s\n", result)
Copy link
Contributor

Choose a reason for hiding this comment

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

can we log error msg too. I think test name is not enough for debugging.

Copy link
Member Author

Choose a reason for hiding this comment

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

log.exception logs the exception message and traceback

log.info("")

results.append(result)
failed.append(test.__name__)

log.info("**************************************** [Test Results] ****************************************")
log.info("")
for r in results:
log.info("\t%s", r)
log.info("")

finally:
self._collect_node_logs()

finally:
self._clean_up()

# Fail the entire test suite if any test failed
assert_that(failed).described_as("One or more tests failed").is_length(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we do anything with the test names we're appending to the failed list? It looks like we're just checking the length of the list here, should we change failed to a counter?

Copy link
Member Author

Choose a reason for hiding this comment

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

The message in the assertion failure includes the list of list of tests and the count, so we'll have a summary of the tests that failed.

Also, we output them as part of 'results' in the summary (we append failed tests both to 'failed' and 'results')

2023-01-03T00:36:02Z.676 [INFO] **************************************** [Test Results] ****************************************
2023-01-03T00:36:02Z.676 [INFO] 
2023-01-03T00:36:02Z.676 [INFO] 	[Passed] ExtensionOperationsBvt
2023-01-03T00:36:02Z.676 [INFO] 	[Failed] RunCommandBvt
2023-01-03T00:36:02Z.676 [INFO] 	[Passed] VmAccessBvt


def execute_script_on_node(self, script_path: Path, parameters: str = "", sudo: bool = False) -> int:
"""
Executes the given script on the test node; if 'sudo' is True, the script is executed using the sudo command.
"""
custom_script_builder = CustomScriptBuilder(script_path.parent, [script_path.name])
custom_script = self._node.tools[custom_script_builder]
custom_script = self.context.node.tools[custom_script_builder]

if parameters == '':
command_line = f"{script_path}"
else:
command_line = f"{script_path} {parameters}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to check if parameters == ''? Can we get rid of the first case and just have an extra whitespace at the end of command_line?

Copy link
Member Author

Choose a reason for hiding this comment

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

Avoiding the extra space was intentional. A minor detail, but later on we surround the command line with [] and the extra space looks odd.


self._log.info(f"Executing [{command_line}]")
log.info("Executing [%s]", command_line)

result = custom_script.run(parameters=parameters, sudo=sudo)

# LISA appends stderr to stdout so no need to check for stderr
if result.exit_code != 0:
raise Exception(f"Command [{command_line}] failed.\n{result.stdout}")
output = result.stdout if result.stderr == "" else f"{result.stdout}\n{result.stderr}"
raise Exception(f"[{command_line}] failed:\n{output}")

if result.stdout != "":
separator = "\n" if "\n" in result.stdout else " "
log.info("stdout:%s%s", separator, result.stdout)
if result.stderr != "":
separator = "\n" if "\n" in result.stderr else " "
log.error("stderr:%s%s", separator, result.stderr)

return result.exit_code

Expand Down
2 changes: 1 addition & 1 deletion tests_e2e/orchestrator/scripts/run-scenarios
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,4 @@ lisa \
--log_path "$HOME/logs" \
--working_path "$HOME/logs" \
-v subscription_id:"$SUBSCRIPTION_ID" \
-v identity_file:"$HOME/.ssh/id_rsa.pub"
Copy link
Member Author

Choose a reason for hiding this comment

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

This should have been the private key, not the public key

-v identity_file:"$HOME/.ssh/id_rsa"
Loading