This repository was archived by the owner on Oct 22, 2024. It is now read-only.
RFC: #420 + Device manager tests in same cluster as e2e tests #474
Conversation
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
DeviceManager.FlushDeviceData() is unused anywhere in the code and could not see any such need going forward.
Defined package level error codes. The implementations are supposed to return these error codes. And also revised the error message formatting and wrapping using go-1.13 fmt package.
Discussion on issue intel#413, raised a requirement that PMEM-CSI should handle the DeleteVolume() call for volumes that the underlined device already deleted manually by the admin. So for such a volume we just ignore and return no error in DeviceManager.DeleteVolume(). This also makes DeleteVolume() idempotent.
…paces While deleting a namespace ndctl might still keeep the namespace inactive and with size zero. And also there might be possible that the unsuccessful deletes via Region.DestroyNamespace() might left the namespace disabled but not deleted, so we should not ignore them too. This change, considers the size of the found namespace and considers only if it's non-zero.
Tests for both device managers: direct and LVM. LVM tests used file backed loop device to mimic an logical volume group, This requires /dev/loop-control file on host where the tests are run. Intentionally disable running these device manager unit tests as it requires sudo/root user permission.
Device manager tests need root/sudo user permission, and loop control device on
host where they run. So to avoid this we run these tests under a new virtual
machine.
Added new make target 'run_dm_tests' which sets up pmem-csi-${CLUSTER}-worker1
node if not exists and runs the device manager unit tests in it.
To make use of existing test cluster scripts, made needed changes to them to
support creating a single pmem node.
As creating a new VM is expensive, Hence running device manager tests in the same cluster used by e2e tests. It uses the first worker of the populated cluster to run tests.
avalluri
force-pushed
the
devicemanager+dmtests-in-e2e
branch
from
9393553 to
8ea3940
Compare
|
@pohly Does this change makes any sense for your concern about the increase in CI time. |
I don't want special cases for individual tests. If |
|
Closing this in favour of #473 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
To avoid extra time consumed to create a new VM to run device manager tests in CI, Now we run them at the time of running e2e tests so that we can reuse the same cluster.