Refactor virt module integration tests#57736
Closed
rst0git wants to merge 18 commits intosaltstack:masterfrom
Closed
Refactor virt module integration tests#57736rst0git wants to merge 18 commits intosaltstack:masterfrom
rst0git wants to merge 18 commits intosaltstack:masterfrom
Conversation
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
When get_profiles() fails, it returns a string with an error message. This causes the integration tests to fail as follows: -> integration.modules.test_virt.VirtTest.test_default_esxi_profile Traceback (most recent call last): File "/salt/tests/integration/modules/test_virt.py", line 38, in test_default_esxi_profile nicp = profiles["nic"]["default"] TypeError: string indices must be integers By asserting the return type of get_profiles(), the error message would indicate the actual reason for the failure: -> integration.modules.test_virt.VirtTest.test_default_esxi_profile Traceback (most recent call last): File "/salt/tests/integration/modules/test_virt.py", line 39, in test_default_esxi_profile self.assertIsInstance(profiles, dict) AssertionError: 'ERROR: Sorry, localhost failed to open a connection to the hypervisor software at None' is not an instance of <class 'dict'> Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
The output from libvirt's virConnectGetCapabilities might contain zero or more `guest` elements [1]. This commit introduces an appropriate error message shown when the number of guest elements is 0. [1] https://libvirt.org/formatcaps.html Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
In commit [1] the use of the 'esxi' hypervisor was replaced with 'vmware'. This commit updates the integration test and documentation string to match this change. A number of new arguments have been introduced to _disk_profile(), however, get_profiles() assumes that both _disk_profile() and _nic_profile() take as input the same arguments. In particular, commit [2] introduced the `pool` argument and commit [3] introduced `disks`, `vm_name` and `image`. In commit [4] were removed the default values for the new arguments added to _disk_profile(), which results in get_profiles() to fail with 'invalid arguments' error. The changes introduced in this commit should resolve this error. In addition, the integration tests have been updated to reflect this and other changes. The integration tests require the Python bindings for libvirt to be installed (`pip install libvirt-python`) and a local libvirt daemon instance (`systemctl start libvirtd`). The tests can be executed with: python tests/runtests.py -n integration.modules.test_virt [1] saltstack@34d1b34 [2] saltstack@7555172 [3] saltstack@5479f88 [4] saltstack@8ed54cd Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
The bash autocomplete includes the utility functions check_remote and download_remote. These are functions are not intended to be directly used by users. Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
rst0git
force-pushed
the
virt-integration-tests
branch
7 times, most recently
from
634bb70 to
618a2d9
Compare
The virt minion targets are used for integration testing of the virt module. The targets are based on a container image with preinstalled libvirt and qemu. The salt source repository and configuration files are bind-mounted within the container where a salt-minion instance is started. The host's network stack (--network=host) is used to allow the salt-minion inside the container to connect to the salt-master running on the host. Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
cbosdo
reviewed
Jun 22, 2020
Contributor
cbosdo
left a comment
cbosdo
left a comment
There was a problem hiding this comment.
LGTM, let's see how the CI digests it
Contributor
cbosdo
previously approved these changes
Jun 22, 2020
Contributor
cbosdo
left a comment
cbosdo
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the code factorization.
rst0git
force-pushed
the
virt-integration-tests
branch
from
d3a755a to
b0132e6
Compare
The code used to start virt_minion_0 and virt_minion_1 is very similar and can be abstracted into a function (start_virt_daemon). Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
rst0git
force-pushed
the
virt-integration-tests
branch
from
b0132e6 to
0ddd37a
Compare
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
rst0git
force-pushed
the
virt-integration-tests
branch
2 times, most recently
from
7cf360e to
f18ba22
Compare
rst0git
force-pushed
the
virt-integration-tests
branch
from
f18ba22 to
bcf0b51
Compare
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
rst0git
force-pushed
the
virt-integration-tests
branch
2 times, most recently
from
034e807 to
cf439c8
Compare
Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
`docker run` does not pull the latest version of an image if that image already exists locally. Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
rst0git
force-pushed
the
virt-integration-tests
branch
2 times, most recently
from
eecacdd to
86e1635
Compare
When define_xml_path has caused libvirtError exception the output format is of type string and using only equality assertion will not show the actual error message. Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
ghost
requested review from
This was referenced Jul 15, 2020
Merged
Merged
Contributor
|
Please do not merge this PR, it's blocked on external changes happening first. I will reply once the issues have been resolved to unblock this PR. |
Contributor
Author
|
Closing in favour of #57947 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR is rebased on #57431. It adds two integration test targets --
virt_minion_0andvirt_minion_1. These targets are based on a container image (https://github.com/rst0git/virt-minion) with pre-installed libvirt and qemu packages. This approach allows to run more than one libvirtd+minion instance on a single system to automate the testing of features such as live migration.Previous Behavior
The virt module integration tests require a libvirt daemon to be running on the host system.
New Behavior
The virt module integration tests require a docker daemon to be running on the host system.