-
Notifications
You must be signed in to change notification settings - Fork 371
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
Add Flatcar to end-to-end tests, install Pypy on test VMs, etc #2779
Conversation
@@ -44,10 +43,13 @@ | |||
import makepkg | |||
from azurelinuxagent.common.version import AGENT_VERSION | |||
from tests_e2e.orchestrator.lib.agent_test_loader import TestSuiteInfo | |||
from tests_e2e.tests.lib.agent_test import TestSkipped |
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.
Tests can raise TestSkipped to skip a non-supported distro, etc
self.__context.is_vhd = self._get_required_parameter(variables, "c_vhd") != "" | ||
self.__context.image_name = f"{node.os.name}-vhd" if self.__context.is_vhd else self._get_required_parameter(variables, "c_name") | ||
self.__context.is_vhd = self._get_optional_parameter(variables, "c_vhd") != "" | ||
self.__context.image_name = f"{node.os.name}-vhd" if self.__context.is_vhd else self._get_required_parameter(variables, "c_env_name") |
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 added a "vm_name" parameter for existing VMs, so I renamed "c_name" to "c_env_name"
|
||
# Copy the tarball to the local logs directory | ||
remote_path = "/tmp/waagent-logs.tgz" | ||
local_path = self.context.log_path/'{0}.tgz'.format(self.context.image_name) | ||
self._log.info("Copying %s:%s to %s", self.context.node.name, remote_path, local_path) | ||
self.context.node.shell.copy_back(remote_path, local_path) | ||
self.context.ssh_client.copy(remote_path, local_path, remote_source=True) |
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 removed the references to LISA's SSH client and now I am using the Agent's
with _set_thread_name(self.context.image_name): # The thread name is added to self._log | ||
self._log.info( |
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 move this message here from the initialization function. We set the thread name for the log after the initialization, so it was missing from the log for this message
@@ -31,6 +31,8 @@ variable: | |||
# | |||
# These variables identify the existing VM, and the user for SSH connections | |||
# | |||
- name: cloud |
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.
forgot to add "cloud" in my previous PR
@@ -0,0 +1,74 @@ | |||
#!/usr/bin/env bash |
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 script can be used to set the PYTHONPATH for any Python (this PR uses it for Pypy) before invoking 'waagent' or importing any of its modules
@@ -0,0 +1,57 @@ | |||
#!/usr/bin/env bash |
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.
in some distros (e.g. Flatcar) 'waagent' is not in PATH
@@ -0,0 +1,36 @@ | |||
#!/usr/bin/env python3 |
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.
we were using the "unzip" package to do this, but that package may not be available in all tested distros (e.g. Flatcar does not even have a package manager) so now we use this Python script to unzip.
@@ -57,18 +59,19 @@ image-sets: | |||
# '<Publisher>:<Offer>:<Sku>:<Version>' | |||
# | |||
images: | |||
# |
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.
removed this duplicated comment... these distros are not added to 'endorsed' yet, and there is already a TODO comment there
@@ -38,6 +38,9 @@ | |||
|
|||
class VmAccessBvt(AgentTest): | |||
def run(self): | |||
if type(self._context.node.os).__name__ == 'CoreOs' and self._context.node.os.information.full_version.startswith('Flatcar'): |
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.
Should we keep a list of skipped tests by distro? Or add a TODO here so we know to remove this when VMAccess becomes supported on Flatcar.
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.
good point, we already have 3 or so tests/test cases that we skip. i'll create work items for those
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.
Now ssh_client is added to context, can we extend that to tests instead initializing on every test.
exit 1 | ||
fi | ||
|
||
$python -c 'import site; print(":".join(site.getsitepackages()))' |
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.
can you add example what is the path looks like? is this like /usr/bin/pyhton3.6 or something else?
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.
Not sure a sample path helps. getsitepackages() returns the path for the system-wide python modules (as opposed to user-wide modules). The agent uses the system-wide path.
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 is a sample. If you think it helps, I can add it the comment
$ python3
Python 3.10.6 (main, Nov 14 2022, 16:10:14) [GCC 11.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import site
>>> site.getsitepackages()
['/usr/local/lib/python3.10/dist-packages', '/usr/lib/python3/dist-packages', '/usr/lib/python3.10/dist-packages']
>>>
The actual values can change a lot (try this on flatcar, for example)
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.
is this agent module python path or new python version path that we installed?
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.
that sample is for my dev machine
|
||
# Ensure that AutoUpdate is enabled. some distros, e.g. Flatcar, don't have a waagent.conf | ||
# but AutoUpdate defaults to True so there is no need to anything in that case. | ||
if [[ -e /etc/waagent.conf ]]; then |
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.
Don't we need to add in setup of agent for flatcar distro to have conf file? It may break for people who copy agent from source if some config should not be default value for this distro
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 default values are hardcoded in the agent. Specific tests may need to create /etc/waagent.conf, but the overall infrastructure should not create it, since that is not the way actual VMs would look like.
BTW, currently VMAccess breaks on Flatcar because it has a hard dependency on /etc/waagent.conf; the extension needs to fix this dependency, since user-created VMs won't have it. If the AgentTestSuite created the conf file, we would be masking this bug.
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.
But why flatcar vm don't have it? what is special about it? My suspicion is that our agent source setup itself don't have it. In general distro owners copy or look at our agent setup to bootstrap agent right.
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.
That is the way they setup Flatcar and is perfectly valid. Flatcar is supposed to be immutable so I imagine they are not allowing for any configuration changes by default,
# | ||
echo "Verifying agent installation..." | ||
|
||
PYTHONPATH=$(get-agent-pythonpath) | ||
export PYTHONPATH |
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.
After this change, I don't have to look for agent python in agent update test . Which python
should be agent python and also default python right?
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.
If tests need to import agent modules, or invoke 'waagent', they need to do the same as this script
PYTHONPATH=$(get-agent-pythonpath)
export PYTHONPATH
Let me think a little if we should also set PYTHONPATH in .bash_profile, and then tests would not need to worry about it. I can do that on a separate PR.
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 should be fine to set PYTHONPATH in .bas_profile. I'll add that to my next PR
check-version() { | ||
waagent=$(get-waagent-path) |
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 is also helps in agent update, thanks for the change
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.
flatcar forced me to do it :)
Now ssh_client is added to context, can we extend that to tests as well instead initializing on every test. |
Not all tests need the ssh client, but I agree it may be useful. If you feel we need it, please go ahead and send a PR adding to AgentTest. |
Codecov Report
@@ Coverage Diff @@
## develop #2779 +/- ##
===========================================
- Coverage 71.99% 71.99% -0.01%
===========================================
Files 104 104
Lines 15839 15839
Branches 2265 2265
===========================================
- Hits 11404 11403 -1
Misses 3913 3913
- Partials 522 523 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
This PR includes several changes related to adding Flatcar to the test automation: