Add tests for corruption recovery on LINSTOR SR#360
Add tests for corruption recovery on LINSTOR SR#360Millefeuille42 wants to merge 2 commits intomasterfrom
Conversation
tests/storage/linstor/conftest.py
Outdated
|
|
||
| @pytest.fixture(scope='function') | ||
| def host_and_corrupted_vdi_on_linstor_sr(host, linstor_sr, vm_ref): | ||
| vm: VM = host.import_vm(vm_ref, sr_uuid=linstor_sr.uuid) |
There was a problem hiding this comment.
I think this should be created in a fixture
That would avoid the big try/catch at the end
There was a problem hiding this comment.
I need the VM lifetime to be directly tied to the fixture's lifetime and since we already have a vm_on_linstor_sr fixture with the "innapropriate" scope we would have double the fixture with different scopes.
There was a problem hiding this comment.
This will break when vm_ref is a UUID referring to an existing VM. This also totally bypasses local cache for imported VMs.
If it's best to import a VM and you don't care which VM, that there's no advantage in , then you should import one of the mini VMs without depending on vm_ref, as we already do in some storage tests:
vm = host.import_vm(vm_image('mini-linux-x86_64-bios'), sr_uuid=sr.uuid)
On the contrary, if you want the test to run with any VM we can test (as part of a "multi" job), then maybe you should still rely on the imported_vm fixture, but then copy it to the SR where you need it.
And if what you want is actually an empty VM with just a disk that you can break, then maybe it's time we implemented creating a VM from scratch, without relying on an XVA.
Worth a discussion.
There was a problem hiding this comment.
I think the last option would be the best in that case. I just want an empty VM with a disk indeed.
There was a problem hiding this comment.
done in 2580566
Finally making a function scoped fixture instead.
Millefeuille42
force-pushed
the
mlr/629-xostor-drbdadm-verify
branch
from
e65c96c to
5af35f6
Compare
Millefeuille42
force-pushed
the
mlr/629-xostor-drbdadm-verify
branch
from
5af35f6 to
76186f2
Compare
Millefeuille42
force-pushed
the
mlr/629-xostor-drbdadm-verify
branch
from
70925ed to
d1fafa5
Compare
Tests DRBD's corruption recovery using drbdadm verify and invalidate-remote commands as well as basic VM operations (startup, snapshot, shutdown) Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
Millefeuille42
force-pushed
the
mlr/629-xostor-drbdadm-verify
branch
from
6ebf72d to
d6b53e9
Compare
tests/storage/linstor/conftest.py
Outdated
| pool: Pool = host.pool | ||
| master: Host = pool.master | ||
|
|
||
| def get_vdi_volume_name_from_linstor() -> str: |
There was a problem hiding this comment.
Why are the helper functions local to the fixture definition. There's no chance we could reuse them in other tests? This makes the fixture definition rather big.
There was a problem hiding this comment.
I'll move them out of the fixture. Indeed this could be useful for other linstor related tests.
tests/storage/linstor/conftest.py
Outdated
| vm.destroy(verify=True) | ||
|
|
||
| @pytest.fixture(scope='function') | ||
| def host_and_corrupted_vdi_on_linstor_sr(host: Host, linstor_sr: SR, vm_ref: str): |
There was a problem hiding this comment.
May this function-scope fixture be reused in future tests? If yes, I think conftest.py is the right location for it. If not, it would be better defined close to the only function that currently requires it.
There was a problem hiding this comment.
I don't see any other usage than the test requiring it. I will move it next to it
tests/storage/linstor/conftest.py
Outdated
|
|
||
| @pytest.fixture(scope='function') | ||
| def host_and_corrupted_vdi_on_linstor_sr(host, linstor_sr, vm_ref): | ||
| vm: VM = host.import_vm(vm_ref, sr_uuid=linstor_sr.uuid) |
There was a problem hiding this comment.
This will break when vm_ref is a UUID referring to an existing VM. This also totally bypasses local cache for imported VMs.
If it's best to import a VM and you don't care which VM, that there's no advantage in , then you should import one of the mini VMs without depending on vm_ref, as we already do in some storage tests:
vm = host.import_vm(vm_image('mini-linux-x86_64-bios'), sr_uuid=sr.uuid)
On the contrary, if you want the test to run with any VM we can test (as part of a "multi" job), then maybe you should still rely on the imported_vm fixture, but then copy it to the SR where you need it.
And if what you want is actually an empty VM with just a disk that you can break, then maybe it's time we implemented creating a VM from scratch, without relying on an XVA.
Worth a discussion.
Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
|
I have yet to run the tests to test the latest edits. |
7 participants
Tests DRBD's corruption recovery using drbdadm verify and invalidate-remote commands as well as basic VM operations (startup, snapshot, shutdown)