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

Python configuration for tests #2793

Merged
merged 7 commits into from
Apr 4, 2023
Merged

Python configuration for tests #2793

merged 7 commits into from
Apr 4, 2023

Conversation

narrieta
Copy link
Member

This PR changes the Python configuration for the end to end tests to an explicit approach: instead of having a default Python, tests need to specify the Python to use (the system's Python or Pypy).

  • Always install Pypy on test VMs
  • Install the Test agent as a module for Pypy
  • Add utilities to find the location of the Agent's executable (waagent) and modules (azurelinuxagent.*), as well as the location of the Python used by the system to execute the Agent.
  • Install tests libraries to ~/lib, instead of ~/bin

The PR also includes some minor cleanup fixes. I'll point them out in review comments.

@@ -277,54 +277,74 @@ def _setup_node(self) -> None:
log.info("Resource Group: %s", self.context.vm.resource_group)
log.info("")

self.context.ssh_client.run_command("mkdir -p ~/bin/tests_e2e/tests; touch ~/bin/agent-env")
Copy link
Member Author

Choose a reason for hiding this comment

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

Now we prepare a tarball on the orchestrator machine that is already structured in the way it needs to be setup on the test VMs, instead of using SSH to create/copy these files. This should make easier to include additional files that need to be copied to the test VMs.

@@ -447,15 +469,15 @@ def _execute_test_suite(self, suite: TestSuiteInfo) -> bool:
test_start_time: datetime.datetime = datetime.datetime.now()

log.info("******** Executing %s", test_name)
self.context.lisa_log.info("******** Executing %s", test_full_name)
self.context.lisa_log.info("Executing test %s", test_full_name)
Copy link
Member Author

Choose a reason for hiding this comment

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

Minor formatting change. We used to output "********" to the LISA log to make those messages stand out. My previous PR created a log for each environment, so now the LISA log is not too crowded and this marker is not needed anymore.

@@ -528,7 +550,7 @@ def _check_agent_log(self) -> bool:
self.context.lisa_log.info("Checking agent log on the test node")
log.info("Checking agent log on the test node")

output = self.context.ssh_client.run_command("check-agent-log.py -j")
output = self.context.ssh_client.run_command("$(get-agent-python) ~/bin/check-agent-log.py -j")
Copy link
Member Author

Choose a reason for hiding this comment

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

get_distro() in version.py is not returning the expected values in some distros when using Pypy. I need to follow up that, and in the meanwhile I am using the system's python to execute this script.

@@ -551,21 +573,17 @@ def _check_agent_log(self) -> bool:
log.info("There are no errors in the agent log")
return True

log_path: Path = self.context.log_path/f"CheckAgentLog-{self.context.environment_name}.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.

My previous PR create a log for each environment, so now we do not need to create this extra CheckAgentLog-*.log, we can simply log those messages to the environment log.

#
set -euo pipefail

# python3 is available on most distros
Copy link
Member Author

Choose a reason for hiding this comment

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

Somehow git captures this as a rename + changes. I actually removed find-python and created a totally unrelated script, get-agent-modules-path

@@ -1,5 +1,3 @@
#!/usr/bin/env python3
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 script runs using whatever Python is installed on the test VM, and that is not necessarily Python 3.

"""
Renames the given log to prefix it with "_".
Adds a message to indicate the log contains 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.

Marking failed logs by renaming them makes developer runs a little harder, since one may be watching the progress in a test log while the file is renamed. Instead, we add the message below and do not change the name.

# Returns the PYTHONPATH on which the azurelinuxagent and associated modules are located.
#
# To do this, the script tries to find the python command used to start the agent and then
# returns the value of site.getsitepackages().
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to change the comment. Now this script returns agent python executable path

# ln -s "$python" ~/bin/python3
# fi
# fi
#fi
Copy link
Contributor

Choose a reason for hiding this comment

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

This is commented, are we still figuring out for flatcar?

@codecov
Copy link

codecov bot commented Mar 31, 2023

Codecov Report

Merging #2793 (ea332fd) into develop (08f86db) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #2793   +/-   ##
========================================
  Coverage    72.01%   72.01%           
========================================
  Files          104      104           
  Lines        15870    15870           
  Branches      2273     2273           
========================================
  Hits         11429    11429           
  Misses        3916     3916           
  Partials       525      525           

see 2 files with indirect coverage changes

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

@@ -86,18 +86,27 @@ sudo find "$LOGS_DIRECTORY" -exec chown "$USER" {} \;
#
# Move the relevant logs to the staging directory
#
# Move the logs for failed tests to a temporary location
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we move failed tests to a temporary location here?

Copy link
Member Author

Choose a reason for hiding this comment

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

we move them back on line 105

the temp location makes it easy to separate them from other logs

@narrieta narrieta merged commit 1601e88 into Azure:develop Apr 4, 2023
@narrieta narrieta deleted the python branch April 4, 2023 19:58
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

3 participants