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

Publish test results and logs #2707

Merged
merged 1 commit into from
Dec 1, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions tests_e2e/lisa/runbook/azure.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ variable:
notifier:
- type: html
- type: env_stats
- type: junit
platform:
- type: azure
admin_username: $(user)
Expand Down
35 changes: 14 additions & 21 deletions tests_e2e/lisa/testsuites/agent-bvt.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
from assertpy import assert_that
from pathlib import Path

from tests_e2e.lisa.testsuites.agent_test_suite import AgentTestSuite
from tests_e2e.lisa.tests.agent_bvt import custom_script

from lisa import (
CustomScriptBuilder,
Logger,
Node,
simple_requirement,
TestCaseMetadata,
TestSuite,
TestSuiteMetadata,
)
from lisa.sut_orchestrator.azure.common import get_node_context


@TestSuiteMetadata(
Expand All @@ -22,20 +18,17 @@
""",
requirement=simple_requirement(unsupported_os=[]),
)
class AgentBvt(TestSuite):
Copy link
Member Author

@narrieta narrieta Nov 30, 2022

Choose a reason for hiding this comment

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

Given the limitations on LISA's TestSuite, now I am modeling a DCR scenario as a TestSuite with a single test (main), which invokes each of the steps in the scenario.

I also moved the logic common to all suites to a new class, AgentTestSuite.

class AgentBvt(AgentTestSuite):
@TestCaseMetadata(description="", priority=0)
def check_agent_version(self, node: Node, log: Logger) -> None:
script_path = CustomScriptBuilder(Path(__file__).parent.parent.joinpath("tests", "agent_bvt"), ["check_agent_version.py"])
script = node.tools[script_path]
result = script.run()
log.info(result.stdout)
log.error(result.stderr)
assert_that(result.exit_code).is_equal_to(0)
def main(self, *_, **__) -> None:
self.check_agent_version()
self.custom_script()

def check_agent_version(self) -> None:
exit_code = self._execute_remote_script(self._test_root.joinpath("lisa", "tests", "agent_bvt"), "check_agent_version.py")
assert_that(exit_code).is_equal_to(0)

def custom_script(self) -> None:
custom_script.main(self._subscription_id, self._resource_group_name, self._vm_name)


@TestCaseMetadata(description="", priority=0)
def custom_script(self, node: Node) -> None:
node_context = get_node_context(node)
subscription_id = node.features._platform.subscription_id
resource_group_name = node_context.resource_group_name
vm_name = node_context.vm_name
custom_script.main(subscription_id, resource_group_name, vm_name)
53 changes: 53 additions & 0 deletions tests_e2e/lisa/testsuites/agent_test_suite.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
from pathlib import Path, PurePath
Copy link
Member Author

Choose a reason for hiding this comment

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

Currently just a skeleton, needs many improvements


from lisa import (
CustomScriptBuilder,
TestSuite,
TestSuiteMetadata,
)
from lisa.sut_orchestrator.azure.common import get_node_context


class AgentTestSuite(TestSuite):
def __init__(self, metadata: TestSuiteMetadata):
super().__init__(metadata)
self._log = None
self._node = None
self._test_root = None
self._subscription_id = None
self._resource_group_name = None
self._vm_name = None

def before_case(self, *_, **kwargs) -> None:
node = kwargs['node']
log = kwargs['log']
node_context = get_node_context(node)

self._log = log
self._node = node
self._test_root = Path(__file__).parent.parent.parent
self._subscription_id = node.features._platform.subscription_id
self._resource_group_name = node_context.resource_group_name
self._vm_name = node_context.vm_name

def after_case(self, *_, **__) -> None:
# 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_remote_script(self._test_root.joinpath("scripts"), "collect_logs.sh")

# Copy the tarball to the local logs directory
remote_path = PurePath('/home') / self._node.connection_info['username'] / 'logs.tgz'
local_path = Path.home() / 'logs' / 'vm-logs-{0}.tgz'.format(self._node.name)
self._log.info("Copying %s:%s to %s...", self._node.name, remote_path, local_path)
self._node.shell.copy_back(remote_path, local_path)

def _execute_remote_script(self, path: Path, script: str) -> int:
custom_script_builder = CustomScriptBuilder(path, [script])
custom_script = self._node.tools[custom_script_builder]
self._log.info('Executing %s/%s...', path, script)
result = custom_script.run()
if result.stdout:
self._log.info('%s', result.stdout)
if result.stderr:
self._log.error('%s', result.stderr)
return result.exit_code
17 changes: 17 additions & 0 deletions tests_e2e/scripts/collect_logs.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#!/usr/bin/env bash
Copy link
Member Author

@narrieta narrieta Nov 30, 2022

Choose a reason for hiding this comment

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

executed on the test VM to collect agent and system logs into a compressed tarball


set -euxo pipefail

logs_file_name="$HOME/logs.tgz"

echo "Collecting logs to $logs_file_name ..."

sudo tar --exclude='journal/*' --exclude='omsbundle' --exclude='omsagent' --exclude='mdsd' --exclude='scx*' \
--exclude='*.so' --exclude='*__LinuxDiagnostic__*' --exclude='*.zip' --exclude='*.deb' --exclude='*.rpm' \
-czf "$logs_file_name" \
/var/log \
/var/lib/waagent/ \
/etc/waagent.conf

sudo chmod +r "$logs_file_name"

30 changes: 22 additions & 8 deletions tests_e2e/scripts/execute_tests_container.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,34 @@ az acr login --name waagenttests

docker pull waagenttests.azurecr.io/waagenttests:latest

# Logs will be placed in this location. Make waagent (UID 1000 in the container) the owner.
mkdir "$HOME/logs"
sudo chown 1000 "$HOME/logs"
# Logs will be placed in the staging directory. Make waagent (UID 1000 in the container) the owner so that it can write to that location
sudo chown 1000 "$BUILD_ARTIFACTSTAGINGDIRECTORY"

docker run --rm \
--volume "$BUILD_SOURCESDIRECTORY:/home/waagent/WALinuxAgent" \
--volume "$DOWNLOADSSHKEY_SECUREFILEPATH:/home/waagent/id_rsa" \
--volume "$HOME/logs:/home/waagent/logs" \
--volume "$BUILD_ARTIFACTSTAGINGDIRECTORY:/home/waagent/logs" \
--env SUBSCRIPTION_ID \
--env AZURE_CLIENT_ID \
--env AZURE_CLIENT_SECRET \
--env AZURE_TENANT_ID \
waagenttests.azurecr.io/waagenttests \
bash --login -c '~/WALinuxAgent/tests_e2e/scripts/execute_tests.sh'

ls -lR "$HOME/logs"

bash --login -c '$HOME/WALinuxAgent/tests_e2e/scripts/execute_tests.sh'

# Retake ownership of the staging directory
sudo find "$BUILD_ARTIFACTSTAGINGDIRECTORY" -exec chown "$USER" {} \;

# LISA organizes its logs in a tree similar to
#
# .../20221130
# .../20221130/20221130-160013-749
# .../20221130/20221130-160013-749/environments
# .../20221130/20221130-160013-749/lisa-20221130-160013-749.log
# .../20221130/20221130-160013-749/lisa.junit.xml
# etc
#
# Remove the first 2 levels of the tree (which indicate the time of the test run) to make navigation
# in the Azure Pipelines UI easier.
#
mv "$BUILD_ARTIFACTSTAGINGDIRECTORY"/[0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9]/*/* "$BUILD_ARTIFACTSTAGINGDIRECTORY"
Copy link
Contributor

Choose a reason for hiding this comment

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

good point, I noticed this.

rm -r "$BUILD_ARTIFACTSTAGINGDIRECTORY"/[0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9]
11 changes: 11 additions & 0 deletions tests_e2e/templates/execute-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,14 @@ jobs:
AZURE_CLIENT_SECRET: $(AZURE-CLIENT-SECRET)
AZURE_TENANT_ID: $(AZURE-TENANT-ID)
SUBSCRIPTION_ID: $(SUBSCRIPTION-ID)

- task: PublishTestResults@2
Copy link
Contributor

Choose a reason for hiding this comment

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

I have general question regarding junit test results. Based on the current design when do we say test_case failed, is it if all the steps failed in test case or any of the step failed?

Like dcr today do we show warning/failed status on test case if any of them failed, so that it's easy to navigate to that test case and dig the logs to see the errors.

Copy link
Member Author

Choose a reason for hiding this comment

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

each scenario needs to define what conditions should be treated as an error (i.e. the test case must fail) or as warnings (i.e. the test passes, but there are some warning messages in the logs).

dcr is too limited in that it does not make this distinction and it is hard to know what is an actual failure without checking the logs in detail

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, but how do you pass this information to junit output, now junit output control by lisa and lisa determines the test case result.

Copy link
Member Author

Choose a reason for hiding this comment

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

just pass or fail the test, not sure i get your question

Copy link
Contributor

@nagworld9 nagworld9 Dec 1, 2022

Choose a reason for hiding this comment

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

I understand pass or fail. It's straightforward. I'm curious about warning case If we make test case pass for warnings, then azure pipeline shows green and it's hard to know particular test case has warnings in log without going into each test case 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.

warning would be something that does not prevent a test from executing/succeeding. when a test fails. warnings can help determine why.

not sure if there is a way to report them to the pipeline. but i am not sure we want to since they are only interesting when something fails

Copy link
Contributor

Choose a reason for hiding this comment

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

if we consider one of step failure as warnings to continue execute rest of the steps, in those cases we still need to follow up why particular step as warning to make sure no regressions.

I'm just throwing point; this is something to think about in the grand scheme of things.

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure i understand what you are trying to achieve, but you can always reach out offline

inputs:
testResultsFormat: 'JUnit'
testResultsFiles: '**/*junit.xml'
searchFolder: $(Build.ArtifactStagingDirectory)
testRunTitle: 'Publish test results'

- publish: $(Build.ArtifactStagingDirectory)
artifact: 'test-logs'
displayName: 'Publish test logs'