-
Notifications
You must be signed in to change notification settings - Fork 375
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
Conversation
Codecov Report
@@ 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
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 |
There was a problem hiding this comment.
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}" |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 | ||
) | ||
|
There was a problem hiding this comment.
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 }} |
There was a problem hiding this comment.
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: |
There was a problem hiding this comment.
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 \ |
There was a problem hiding this comment.
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 | ||
|
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
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: