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

Load tests dynamically + Parameterize the test pipeline #2745

Merged
merged 4 commits into from
Jan 30, 2023

Conversation

narrieta
Copy link
Member

@narrieta narrieta commented Jan 26, 2023

Now we load the test suites from JSON files with their description (instead of defining the suites as classes inheriting from LISA's TestSuite). This simplifies triggering test runs with different configurations and makes combining the tests into suites a lot easier.

I also added 3 parameters to the Azure Pipeline:

  • test_suites - lists the test suites to run
  • collect_logs - whether to collect logs from the test VMs
  • skip_setup - whether to skip setup of the test VMs

@codecov
Copy link

codecov bot commented Jan 26, 2023

Codecov Report

Merging #2745 (ff52b40) into develop (8328286) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #2745      +/-   ##
===========================================
- Coverage    72.04%   72.03%   -0.01%     
===========================================
  Files          104      104              
  Lines        15832    15832              
  Branches      2265     2265              
===========================================
- Hits         11406    11405       -1     
  Misses        3912     3912              
- Partials       514      515       +1     
Impacted Files Coverage Δ
azurelinuxagent/common/protocol/hostplugin.py 87.50% <0.00%> (-0.27%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@@ -0,0 +1,107 @@
# Microsoft Azure Linux Agent
Copy link
Member Author

Choose a reason for hiding this comment

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

code to load the tests from their JSON files (you can find them under the "testsuites" directory)

@@ -113,7 +143,23 @@ def _set_context(self, node: Node, log: Logger):

self.__context.log = log
self.__context.node = node
self.__context.suite_name = f"{self._metadata.full_name}_{runbook.marketplace.offer}-{runbook.marketplace.sku}"
self.__context.image_name = f"{runbook.marketplace.offer}-{runbook.marketplace.sku}"
Copy link
Member Author

Choose a reason for hiding this comment

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

Previously we had a single test suite and we were using its name during logging and to create files. Now we have multiple suites and I am using the image name, or the concatenation of test suite name + image name as appropriate.

value = variables.get(name)
if value is None:
raise Exception(f"The runbook is missing required parameter '{name}'")
return value
Copy link
Member Author

Choose a reason for hiding this comment

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

These are the new Pipeline parameters that are then passed to the AgentTestSuite via the LISA runbook.

"""
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, log)
self._set_context(node, variables, log)
Copy link
Member Author

Choose a reason for hiding this comment

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

'variables' are the variables from the runbook

agent_test_logger.set_current_thread_log(suite_log_file)
try:
if not self.context.skip_setup:
self._setup_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.

skipping setup is mainly for developer scenarios... when testing something small, one can skip the setup and execute the runbook faster.

@@ -1,15 +1,17 @@
name: Daily
name: WALinuxAgent
Copy link
Member Author

Choose a reason for hiding this comment

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

When I started working with LISA I thought we would use multiple runbooks for different test runs, scenarios, etc. We have moved away from LISA's concept of a TestSuite and we need only 1 single runbook. I moved the previous "daily" runbook to this file and coalesced it with the ssh settings.

Copy link
Contributor

Choose a reason for hiding this comment

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

with this single runbook, the definition of default distros mentioned in this file will go away eventually when we implement the distro setup from json?

otherwise, the suits run on these defaults.

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, the distros will be generated dynamically as well

echo "Usage: run-scenarios [-t|--test-suites <test suites>] [-l|--collect-logs <always|failed|no>] [-k|--skip-setup <True|False>]"
exit 1
)

Copy link
Member Author

Choose a reason for hiding this comment

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

Added parameters to the script in order to pass them along to the runbook

- name: collect_logs
value: ${{ parameters.collect_logs }}
- name: skip_setup
value: ${{ parameters.skip_setup }}
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 exposes the pipeline parameters as environment variables for the execution tasks

@@ -10,9 +37,50 @@ pr: none
pool:
vmImage: ubuntu-latest

stages:
- stage: "ExecuteTests"
jobs:
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 prototype pipeline was significantly larger than the new pipeline and was using stages and templates to organize the steps. This is not needed in the new pipeline so I am removing the stages and coalescing the templates

@@ -27,7 +27,8 @@ docker run --rm \
--env AZURE_CLIENT_SECRET \
--env AZURE_TENANT_ID \
waagenttests.azurecr.io/waagenttests \
bash --login -c '$HOME/WALinuxAgent/tests_e2e/orchestrator/scripts/run-scenarios' \
bash --login -c \
Copy link
Member Author

Choose a reason for hiding this comment

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

pass along the pipeline parameters to the container executing the tests

Always = 'always' # Always collect logs
Failed = 'failed' # Collect logs only on test failures
No = 'no' # Never collect logs

Copy link
Member Author

Choose a reason for hiding this comment

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

These enum values match the values for LISA's 'keep_environment' runbook variable (which is used to prevent LISA from deleting the test VMs at the end of the runbook)

@@ -1,15 +1,17 @@
name: Daily
name: WALinuxAgent
Copy link
Contributor

Choose a reason for hiding this comment

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

with this single runbook, the definition of default distros mentioned in this file will go away eventually when we implement the distro setup from json?

otherwise, the suits run on these defaults.

@@ -61,5 +85,11 @@ notifier:
- type: env_stats
- type: agent.junit

include:
- path: ../include/ssh_proxy.yml
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better to keep it in separate when we it's same in multiple run books or can be reused if we add new runbook in future.

Copy link
Member Author

Choose a reason for hiding this comment

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

there is no need for a separate file, since we are using a single runbook. the other runbooks (exiting vm and local are samples for the developer scenario)

self.image_name: str = None
self.test_suites: List[str] = None
self.collect_logs: str = None
self.skip_setup: bool = None
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming the single test run from command line will not change with this addition of new parameters.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, don't get your question... Previously the runbook was executed all the tests (methods marked with TestMetadata), now it executes the tests passed as parameters in the 'test_suites' variable

Copy link
Contributor

Choose a reason for hiding this comment

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

My question is this change the behavior of single test run that we execute from command line (developer scenario) like running vmaccess.py etc.

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 see. Without my changes, one cannot pass the test to run as parameter. The runbook executes all the methods marked as tests with metadata. With my changes, one can pass the test suite to run and the runbook will execute only that suite. To execute a single test instead of a test suite, one can pass a piece of JSON describing a test suite comprised of that single test, e.g. { "name": "MyTestRun", "tests": [ "bvts/vmaccess.py"] } (however, the current pipeline has a bug and it cannot take spaces in is parameters, that is already fixed in my next PR. the parameter for the runbook itself can take spaces)

# These variables define parameters for the AgentTestSuite; see the test wiki for details
#
# The test suites to execute
- name: test_suites
Copy link
Contributor

Choose a reason for hiding this comment

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

All the parameters that we pass to lisa as variable should be define here even though we don't use them in this file?

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

@narrieta narrieta merged commit 629a0ec into Azure:develop Jan 30, 2023
@narrieta narrieta deleted the parameters branch January 30, 2023 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants