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

Jump to bottom

Add distros and location to the test suites definition files #2756

Merged
merged 8 commits into from
Feb 13, 2023
Merged

Add distros and location to the test suites definition files #2756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5b71336
Add distro, location, and VM size configuration to test suites
Feb 8, 2023
df74387
typo
Feb 8, 2023
9e09a37
support for multiple test suites
Feb 9, 2023
e35f5f6
disable centos 6.10, debian 8
Feb 9, 2023
224d47f
test requirements
Feb 9, 2023
4974f23
disable centos 6.10, debian 8
Feb 9, 2023
b7982a3
pylint
Feb 9, 2023
e3c18b7
Merge branch 'develop' into test-suites
narrieta Feb 13, 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
234 changes: 188 additions & 46 deletions tests_e2e/orchestrator/lib/agent_test_loader.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,77 +15,178 @@
# limitations under the License.
#
import importlib.util
import json
# E0401: Unable to import 'yaml' (import-error)
import yaml # pylint: disable=E0401

from pathlib import Path
from typing import Any, Dict, List, Type

import tests_e2e
from tests_e2e.tests.lib.agent_test import AgentTest


class TestSuiteDescription(object):
class TestSuiteInfo(object):
"""
Description of the test suite loaded from its JSON file.
Description of a test suite
"""
# The name of the test suite
name: str
# The tests that comprise the suite
tests: List[Type[AgentTest]]
# An image or image set (as defined in images.yml) specifying the images the suite must run on.
images: str
Copy link
Contributor

@nagworld9 nagworld9 Feb 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

what if I want to run on multiple images, this is just handling either one image-set or single image. I feel like this is very common case where we want to run particular tets_suite on few distros and need this feature too.

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 solution is the same as in dcr: create a new image set that includes all the images that the suite needs to run on and use that set

Copy link
Contributor

Choose a reason for hiding this comment

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

In DCR, we create the new image set in specific test_suite, now we need to define all those in image.yml. If we end of doing it in one file, it becomes too much in single file. May be worth considering allowing new image set at test_suite level. For now, ok and I guess when we port more tests, we get the idea whether we need to implement this or not.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, start simple and add functionality only if actually needed. Using an array instead of a single item is pretty simple, actually, but for the moment keep things simple.

# The location (region) on which the suite must run; if empty, the suite can run on any location
location: str
# Whether this suite must run on its own test VM
owns_vm: bool

def __str__(self):
return self.name


class VmImageInfo(object):
# The URN of the image (publisher, offer, version separated by spaces)
urn: str
# Indicates that the image is available only on those locations. If empty, the image should be available in all locations
locations: List[str]
# Indicates that the image is available only for those VM sizes. If empty, the image should be available for all VM sizes
vm_sizes: List[str]

def __str__(self):
return self.urn


class AgentTestLoader(object):
"""
Loads the description of a set of test suites
Loads a given set of test suites from the YAML configuration files.
"""
def __init__(self, test_source_directory: Path):
def __init__(self, test_suites: str):
"""
The test_source_directory parameter must be the root directory of the end-to-end tests (".../WALinuxAgent/tests_e2e")
"""
self._root: Path = test_source_directory
Loads the specified 'test_suites', which are given as a string of comma-separated suite names or a YAML description
of a single test_suite.

When given as a comma-separated list, each item must correspond to the name of the YAML files describing s suite (those
files are located under the .../WALinuxAgent/tests_e2e/test_suites directory). For example, if test_suites == "agent_bvt, fast_track"
then this method will load files agent_bvt.yml and fast_track.yml.

When given as a YAML string, the value must correspond to the description a single test suite, for example

def load(self, test_suites: str) -> List[TestSuiteDescription]:
name: "AgentBvt"
tests:
- "bvts/extension_operations.py"
- "bvts/run_command.py"
- "bvts/vm_access.py"
"""
Loads the specified 'test_suites', which are given as a string of comma-separated suite names or a JSON description
of a single test_suite.
self.__test_suites: List[TestSuiteInfo] = self._load_test_suites(test_suites)
self.__images: Dict[str, List[VmImageInfo]] = self._load_images()
self._validate()

When given as a comma-separated list, each item must correspond to the name of the JSON files describing s suite (those
files are located under the .../WALinuxAgent/tests_e2e/test_suites directory). For example, if test_suites == "agent_bvt, fast-track"
then this method will load files agent_bvt.json and fast-track.json.
_SOURCE_CODE_ROOT: Path = Path(tests_e2e.__path__[0])

When given as a JSON string, the value must correspond to the description a single test suite, for example
@property
def test_suites(self) -> List[TestSuiteInfo]:
return self.__test_suites

{
"name": "AgentBvt",
@property
def images(self) -> Dict[str, List[VmImageInfo]]:
"""
A dictionary where, for each item, the key is the name of an image or image set and the value is a list of VmImageInfos for
the corresponding images.
"""
return self.__images

"tests": [
"bvts/extension_operations.py",
"bvts/run_command.py",
"bvts/vm_access.py"
]
}
def _validate(self):
"""
# Attempt to parse 'test_suites' as the JSON description for a single suite
try:
return [self._load_test_suite(json.loads(test_suites))]
except json.decoder.JSONDecodeError:
pass
Performs some basic validations on the data loaded from the YAML description files
"""
for suite in self.test_suites:
# Validate that the images the suite must run on are in images.yml
if suite.images not in self.images:
raise Exception(f"Invalid image reference in test suite {suite.name}: Can't find {suite.images} in images.yml")

# If the suite specifies a location, validate that the images are available in that location
if suite.location != '':
if not any(suite.location in i.locations for i in self.images[suite.images]):
raise Exception(f"Test suite {suite.name} must be executed in {suite.location}, but no images in {suite.images} are available in that location")

# Else, it should be a comma-separated list of description files
description_files: List[Path] = [self._root/"test_suites"/f"{t.strip()}.json" for t in test_suites.split(',')]
return [self._load_test_suite(AgentTestLoader._load_file(s)) for s in description_files]
@staticmethod
def _load_test_suites(test_suites: str) -> List[TestSuiteInfo]:
#
# Attempt to parse 'test_suites' as the YML description of a single suite
#
parsed = yaml.safe_load(test_suites)

#
# A comma-separated list (e.g. "foo", "foo, bar", etc.) is valid YAML, but it is parsed as a string. An actual test suite would
# be parsed as a dictionary. If it is a dict, take is as the YML description of a single test suite
#
if isinstance(parsed, dict):
return [AgentTestLoader._load_test_suite(parsed)]

#
# If test_suites is not YML, then it should be a comma-separated list of description files
#
description_files: List[Path] = [AgentTestLoader._SOURCE_CODE_ROOT/"test_suites"/f"{t.strip()}.yml" for t in test_suites.split(',')]
return [AgentTestLoader._load_test_suite(f) for f in description_files]

def _load_test_suite(self, test_suite: Dict[str, Any]) -> TestSuiteDescription:
@staticmethod
def _load_test_suite(description_file: Path) -> TestSuiteInfo:
"""
Creates a TestSuiteDescription from its JSON representation, which has been loaded by JSON.loads and is passed
to this method as a dictionary
Loads the description of a TestSuite from its YAML file.

A test suite has 5 properties: name, tests, images, location, and owns-vm. For example:

name: "AgentBvt"
tests:
- "bvts/extension_operations.py"
- "bvts/run_command.py"
- "bvts/vm_access.py"
images: "endorsed"
location: "eastuseaup"
owns-vm: true

* name - A string used to identify the test suite
* tests - A list of the tests in the suite. Each test is specified by the path for its source code relative to
WALinuxAgent/tests_e2e/tests.
* images - A string specifying the images on which the test suite must be executed. The value can be the name
of a single image (e.g."ubuntu_2004"), or the name of an image set (e.g. "endorsed"). The names for
images and image sets are defined in WALinuxAgent/tests_e2e/tests_suites/images.yml.
* location - [Optional; string] If given, the test suite must be executed on that location. If not specified,
or set to an empty string, the test suite will be executed in the default location. This is useful
for test suites that exercise a feature that is enabled only in certain regions.
* owns-vm - [Optional; boolean] By default all suites in a test run are executed on the same test VMs; if this
value is set to True, new test VMs will be created and will be used exclusively for this test suite.
This is useful for suites that modify the test VMs in such a way that the setup may cause problems
in other test suites (for example, some tests targeted to the HGAP block internet access in order to
force the agent to use the HGAP).

"""
suite = TestSuiteDescription()
suite.name = test_suite["name"]
suite.tests = []
for source_file in [self._root/"tests"/t for t in test_suite["tests"]]:
suite.tests.extend(AgentTestLoader._load_tests(source_file))
return suite
test_suite: Dict[str, Any] = AgentTestLoader._load_file(description_file)

if any([test_suite.get(p) is None for p in ["name", "tests", "images"]]):
raise Exception(f"Invalid test suite: {description_file}. 'name', 'tests', and 'images' are required properties")

test_suite_info = TestSuiteInfo()

test_suite_info.name = test_suite["name"]

test_suite_info.tests = []
source_files = [AgentTestLoader._SOURCE_CODE_ROOT/"tests"/t for t in test_suite["tests"]]
for f in source_files:
test_suite_info.tests.extend(AgentTestLoader._load_test_classes(f))

test_suite_info.images = test_suite["images"]

test_suite_info.location = test_suite.get("location")
if test_suite_info.location is None:
test_suite_info.location = ""

test_suite_info.owns_vm = "owns-vm" in test_suite and test_suite["owns-vm"]

return test_suite_info

@staticmethod
def _load_tests(source_file: Path) -> List[Type[AgentTest]]:
def _load_test_classes(source_file: Path) -> List[Type[AgentTest]]:
"""
Takes a 'source_file', which must be a Python module, and returns a list of all the classes derived from AgentTest.
"""
Expand All @@ -96,12 +197,53 @@ def _load_tests(source_file: Path) -> List[Type[AgentTest]]:
return [v for v in module.__dict__.values() if isinstance(v, type) and issubclass(v, AgentTest) and v != AgentTest]

@staticmethod
def _load_file(file: Path):
"""Helper to load a JSON file"""
def _load_images() -> Dict[str, List[VmImageInfo]]:
"""
Loads images.yml into a dictionary where, for each item, the key is an image or image set and the value is a list of VmImageInfos
for the corresponding images.

See the comments in image.yml for a description of the structure of each item.
"""
image_descriptions = AgentTestLoader._load_file(AgentTestLoader._SOURCE_CODE_ROOT/"test_suites"/"images.yml")
if "images" not in image_descriptions:
raise Exception("images.yml is missing the 'images' item")

images = {}

# first load the images as 1-item lists
for name, description in image_descriptions["images"].items():
i = VmImageInfo()
if isinstance(description, str):
i.urn = description
i.locations = []
i.vm_sizes = []
else:
if "urn" not in description:
raise Exception(f"Image {name} is missing the 'urn' property: {description}")
i.urn = description["urn"]
i.locations = description["locations"] if "locations" in description else []
i.vm_sizes = description["vm_sizes"] if "vm_sizes" in description else []
images[name] = [i]

# now load the image-sets, mapping them to the images that we just computed
for image_set_name, image_list in image_descriptions["image-sets"].items():
# the same name cannot denote an image and an image-set
if image_set_name in images:
Copy link
Contributor

Choose a reason for hiding this comment

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

We currently only have one image_set which is 'endorsed', is that correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

for now, yes. the bvts use 'endorsed'. dcr has many more images and image sets, but i don't want to migrate them in bulk. we'll migrate them as we migrate the tests that actually used them.

raise Exception(f"Invalid image-set in images.yml: {image_set_name}. The name is used by an existing image")
images_in_set = []
for i in image_list:
if i not in images:
raise Exception(f"Can't find image {i} (referenced by image-set {image_set_name}) in images.yml")
images_in_set.extend(images[i])
images[image_set_name] = images_in_set

return images

@staticmethod
def _load_file(file: Path) -> Dict[str, Any]:
"""Helper to load a YML file"""
try:
with file.open() as f:
return json.load(f)
return yaml.safe_load(f)
except Exception as e:
raise Exception(f"Can't load {file}: {e}")


35 changes: 17 additions & 18 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 @@ -35,7 +35,7 @@
Node,
notifier,
TestCaseMetadata,
TestSuite,
TestSuite as LisaTestSuite,
TestSuiteMetadata,
)
from lisa.messages import TestStatus, TestResultMessage # pylint: disable=E0401
Expand All @@ -44,7 +44,7 @@

import makepkg
from azurelinuxagent.common.version import AGENT_VERSION
from tests_e2e.orchestrator.lib.agent_test_loader import AgentTestLoader, TestSuiteDescription
from tests_e2e.orchestrator.lib.agent_test_loader import TestSuiteInfo
from tests_e2e.tests.lib.agent_test_context import AgentTestContext
from tests_e2e.tests.lib.identifiers import VmIdentifier
from tests_e2e.tests.lib.logging import log as agent_test_logger # Logger used by the tests
Expand Down Expand Up @@ -98,7 +98,7 @@ class CollectLogs(Enum):


@TestSuiteMetadata(area="waagent", category="", description="")
class AgentTestSuite(TestSuite):
class AgentTestSuite(LisaTestSuite):
"""
Manages the setup of test VMs and execution of Agent test suites. This class acts as the interface with the LISA framework, which
will invoke the execute() method when a runbook is executed.
Expand All @@ -112,7 +112,7 @@ def __init__(self, vm: VmIdentifier, paths: AgentTestContext.Paths, connection:
self.node: Node = None
self.runbook_name: str = None
self.image_name: str = None
self.test_suites: List[str] = None
self.test_suites: List[AgentTestSuite] = None
self.collect_logs: str = None
self.skip_setup: bool = None

Expand All @@ -128,7 +128,7 @@ def _set_context(self, node: Node, variables: Dict[str, Any], log: Logger):
# Remove the resource group and node suffix, e.g. "e1-n0" in "lisa-20230110-162242-963-e1-n0"
runbook_name = re.sub(r"-\w+-\w+$", "", runbook.name)

self.__context = AgentTestSuite._Context(
self.__context = self._Context(
vm=VmIdentifier(
location=runbook.location,
subscription=node.features._platform.subscription_id,
Expand All @@ -147,15 +147,15 @@ def _set_context(self, node: Node, variables: Dict[str, Any], log: Logger):
self.__context.log = log
self.__context.node = node
self.__context.image_name = f"{runbook.marketplace.offer}-{runbook.marketplace.sku}"
self.__context.test_suites = AgentTestSuite._get_required_parameter(variables, "test_suites")
self.__context.collect_logs = AgentTestSuite._get_required_parameter(variables, "collect_logs")
self.__context.skip_setup = AgentTestSuite._get_required_parameter(variables, "skip_setup")
self.__context.test_suites = self._get_required_parameter(variables, "test_suites_info")
self.__context.collect_logs = self._get_required_parameter(variables, "collect_logs")
self.__context.skip_setup = self._get_required_parameter(variables, "skip_setup")

self._log.info(
"Test suite parameters: [skip_setup: %s] [collect_logs: %s] [test_suites: %s]",
self.context.skip_setup,
self.context.collect_logs,
self.context.test_suites)
[t.name for t in self.context.test_suites])

@staticmethod
def _get_required_parameter(variables: Dict[str, Any], name: str) -> Any:
Expand Down Expand Up @@ -191,7 +191,7 @@ def _setup(self) -> None:

Returns the path to the agent package.
"""
AgentTestSuite._setup_lock.acquire()
self._setup_lock.acquire()

try:
self._log.info("")
Expand All @@ -211,7 +211,7 @@ def _setup(self) -> None:
completed.touch()

finally:
AgentTestSuite._setup_lock.release()
self._setup_lock.release()

def _build_agent_package(self) -> None:
"""
Expand Down Expand Up @@ -290,10 +290,9 @@ def _collect_node_logs(self) -> None:
self._log.exception("Failed to collect logs from the test machine")

@TestCaseMetadata(description="", priority=0)
def execute(self, node: Node, variables: Dict[str, Any], log: Logger) -> None:
def agent_test_suite(self, node: Node, variables: Dict[str, Any], log: Logger) -> None:
"""
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).
Executes each of the AgentTests included in "test_suites_info" variable (which is generated by the AgentTestSuitesCombinator).
"""
self._set_context(node, variables, log)

Expand All @@ -308,9 +307,9 @@ def execute(self, node: Node, variables: Dict[str, Any], log: Logger) -> None:
if not self.context.skip_setup:
self._setup_node()

test_suites: List[TestSuiteDescription] = AgentTestLoader(self.context.test_source_directory).load(self.context.test_suites)

for suite in test_suites:
# 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
test_suite_success = self._execute_test_suite(suite) and test_suite_success
Copy link
Contributor

@nagworld9 nagworld9 Feb 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

how this suite type as TestSuiteInfo. I see self.context.test_suites get assigned from the variable we pass to Lisa run. That would be list of commas separated or dictionary type of single test.

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 string we pass to the runbook is actually an argument for the combinator:

  #
  # These variables parameters for the AgentTestSuitesCombinator combinator
  #
  # The test suites to execute
  - name: test_suites
    value: "agent_bvt"
    is_case_visible: true

Then the combinator produces an array of TestSuiteInfos that is passed to the AgentTestSuite in the "test_suites_info" variable:

  #
  # These variables are set by the AgentTestSuitesCombinator combinator
  #
...
  - name: test_suites_info
    value: []
    is_case_visible: true

My next PR already includes extra comments and renames the variables produced by the combinator so that they are prefixed by "c_", which should help.


finally:
Expand All @@ -329,7 +328,7 @@ def execute(self, node: Node, variables: Dict[str, Any], log: Logger) -> None:
finally:
self._clean_up()

def _execute_test_suite(self, suite: TestSuiteDescription) -> bool:
def _execute_test_suite(self, suite: TestSuiteInfo) -> bool:
"""
Executes the given test suite and returns True if all the tests in the suite succeeded.
"""
Expand Down
Loading