Implement storage nfs-on-slave test #341
Conversation
tperard
force-pushed
the
apd-nfs-on-slave
branch
from
6afd538 to
d450bb2
Compare
tperard
force-pushed
the
apd-nfs-on-slave
branch
from
d450bb2 to
81dd04c
Compare
tperard
force-pushed
the
apd-nfs-on-slave
branch
from
81dd04c to
b34c3f5
Compare
tperard
force-pushed
the
apd-nfs-on-slave
branch
from
b34c3f5 to
e498c5b
Compare
|
Just a rebase. |
tests/storage/nfs/test_nfs_sr.py
Outdated
| @pytest.mark.usefixtures('hostA2') | ||
| # Make sure this fixture is called before the parametrized one | ||
| @pytest.mark.usefixtures('vm_ref') | ||
| @pytest.mark.parametrize('dispatch_nfs', ['vm_on_nfs_sr'], indirect=True) |
There was a problem hiding this comment.
- What is the objective behind using a parametrized fixture here?
- A fixture that yields a VM usually has a name that starts with
vm_ - How to actually use the parametrized fixture in our job definitions in
jobs.pyand in Jenkins?
There was a problem hiding this comment.
What is the objective behind using a parametrized fixture here?
Make it work.
I needed a VM with its disk on NFS SR, and every other test in test_nfs_sr.py uses this boilerplate.
Now I've read a bit more about pytest and fixtures and it looks like I can just remove this last line, replace dispatch_nfs by vm_on_nfs_sr, and it still works, like magic. I guess I can remove the vm_ref one as well.
Looks like I still have more to learn about pytest 😅
There was a problem hiding this comment.
I don't think you need to parametrize, here. It looks like you just want the vm_on_nfs_sr fixture. Unless you want the test to run with both NFS3 and NFS4+, in which case you need
@pytest.mark.parametrize('dispatch_nfs', ['vm_on_nfs_sr', 'vm_on_nfs4_sr'], indirect=True)
There was a problem hiding this comment.
CC @Nambrok who, IIRC, implemented this parametrization of the NFS tests.
There was a problem hiding this comment.
@tperard This conversation was not resolved. I discussed with Damien and he said that it would make sense to run the test for both nfs and nfs4, as we do in the rest of the test file.
tperard
force-pushed
the
apd-nfs-on-slave
branch
from
e498c5b to
ef3eaa9
Compare
| vm.start() | ||
| vm.wait_for_os_booted() |
There was a problem hiding this comment.
Shouldn't the start/shutdown be handled by a fixture? Looks like a test failure here would not shutdown that VM we started.
There was a problem hiding this comment.
Looks like this would break test_reboot which reboot the host before starting the guest.
Or do you mean adding a fixture that uses vm_on_nfs_sr?
But vm_on_nfs_sr does a vm.destroy(). Is that enough?
There was a problem hiding this comment.
Or do you mean adding a fixture that uses
vm_on_nfs_sr?
Yes, that one.
But
vm_on_nfs_srdoes avm.destroy(). Is that enough?
Looks so, then it can still be useful to clarify with a comment that the shutdown need not be in cleanup because of this.
tests/storage/nfs/test_nfs_sr.py
Outdated
|
|
||
| @pytest.mark.small_vm | ||
| @pytest.mark.usefixtures('hostA2') | ||
| def test_plugin_nfs_on_on_slave(self, vm_on_nfs_sr: 'VM'): |
There was a problem hiding this comment.
A note on what we expect from this plugin would be nice.
| vm.wait_for_os_booted() | ||
| host = vm.get_residence_host() | ||
|
|
||
| vdi = VDI(vm.vdi_uuids()[0], host=host) |
There was a problem hiding this comment.
Can't it be just vdi = vm.vdis[0]?
There was a problem hiding this comment.
I don't know, vm.vdis looks like brand new code.
Signed-off-by: Anthony PERARD <[email protected]>
.. for functions that are going to be used by test_plugin_nfs_on_on_slave(). Signed-off-by: Anthony PERARD <[email protected]>
This new test ensures that the "nfs-on-slave" host-plugin does check something, and return different result based on the environment. Signed-off-by: Anthony PERARD <[email protected]>
tperard
force-pushed
the
apd-nfs-on-slave
branch
from
ef3eaa9 to
5d89924
Compare
6 participants
This new test ensures that the "nfs-on-slave" host-plugin does
check something, and return different result based on the environment.
the extension for the path. It's currently hard-coded to .vhd.