device manager tests + kustomize support for end-users #473
Conversation
The recent commit changed the make rules so that the binaries get unpacked from an archive. But because tar preserves the original modification time stamp, that unpacking is repeated each time the binaries are needed. We can avoid that by bumping the time stamp after unpacking.
pohly
force-pushed
the
dm-tests
branch
2 times, most recently
from
65ca7eb
to
7982fb0
Compare
"kubectl kustomize" cannot be called for all bases in deploy/kustomize because it has no `--load_restrictor none` and the security policy prevents reading some patch files: $ kubectl kustomize deploy/kustomize/kubernetes-1.14-lvm-testing/ Error: AccumulateTarget: AccumulateTarget: AccumulateTarget: security; file '../../patches/controller-role-patch.yaml' is not in or below '/nvme/gopath/src/github.com/intel/pmem-csi/deploy/kustomize/kubernetes-1.14/rbac' We can solve this for users by creating "normal" bases where all patches and resources have already been pulled into a single pmem-csi.yaml. All we need to do is to put them into a separate directory and add a kustomize.yaml. For the sake of preserving current behavior we store those generated files also under their old names. Symlinks are not used because that does not work on Windows or when pulling the files via http.
Fixed. The symlinks in the 1.15 directory were incomplete and tar had to be told to archive the content of the symlinks, not the symlinks themselves. |
This verifies that kustomize works for our base deployments. It also adds some limited testing of the -usefordax and -useforsector parameters because the now unused space will be used for runnning the device manager Go test. If all space was used for LVM, those tests would fail when using direct mode.
Depending on the random number generator, the size of the new volume might have been zero, which isn't valid.
It failed to pass in direct mode when the host already had some other namespaces. We must ignore existing namespaces. The size check leads to better assertions when using BeNumerically.
|
@pohly I try to understand the comments you made in commit 43f3735:
Other than the device manager what other unit tests in your opinion require(benefit from) such an environment.
Currently, running all(registry server, device manager, and state) unit tests taking around 27s in each run. Do you think this argument is still valid once we improve our unit tests? |
| root := os.Getenv("REPO_ROOT") | ||
| var err error | ||
|
|
||
| build := exec.Command("/bin/sh", "-c", os.Getenv("TEST_CMD")+" -c -o _work/test.test "+pkg) |
There was a problem hiding this comment.
How about making a single binary for all unit tests and run in a single stretch.
There was a problem hiding this comment.
We would have to reorganize the test source code. I prefer to keep it decentralized (one test per package).
There was a problem hiding this comment.
This has multiple advantages:
we test in exactly the environment in which the code will runOther than the device manager what other unit tests in your opinion require(benefit from) such an environment.
At this point, none. But even if we all think that this is the case, it's still better to test that opinion.
Because it runs so quickly, it's okay to simply repeat this testing in
each CI step which runs E2E tests.
Currently, running all(registry server, device manager, and state) unit tests taking around 27s in each run. > Do you think this argument is still valid once we improve our unit tests?
Yes.
| root := os.Getenv("REPO_ROOT") | ||
| var err error | ||
|
|
||
| build := exec.Command("/bin/sh", "-c", os.Getenv("TEST_CMD")+" -c -o _work/test.test "+pkg) |
There was a problem hiding this comment.
We would have to reorganize the test source code. I prefer to keep it decentralized (one test per package).
This has multiple advantages: - we test in exactly the environment in which the code will run - it's faster than bringing up a separate VM - no need to remember a separate make target to run all tests Speed is important both for CI and developers. Running the tests in an existing cluster just takes 9 seconds and can be done using the same command as running other tests individually: make test_e2e TEST_E2E_FOCUS=gotests.*pmem-device-manager Because it runs so quickly, it's okay to simply repeat this testing in each CI step which runs E2E tests.
This builds on top of #420 and enables that new test in the CI by running it and all other Go tests in the existing steps and VMs. This makes the CI run a bit faster (3:24min, see #420 (comment)), keeps the Jenkins UI a bit more manageable (the current number of steps already require scrolling), and last but not least, we get better code coverage (all Go tests run on all distros, in exactly the same environment (= container) as the PMEM-CSI driver itself).
To run the device manager inside the normal test cluster, we need some free PMEM. To achieve that, kustomization of the PMEM-CSI driver deployment with
kubectl kustomizegets enabled and used during the cluster setup. This feature should also be useful for admins because it is an easy way to move the driver into its own namespace.Finally, this adds some testing of the
pmem-ns-initparameters, something that we didn't have before.