e2e: Support any storage class name #1714
Conversation
nirs
changed the title
nirs
force-pushed
the
e2e-config
branch
2 times, most recently
from
c108054 to
0320a45
Compare
|
@nirs the move of the supported calls from deployer to workload hints at a problem in our framework. In the end it is the tuple (workload, deployment) which needs to be inspected for support status. It doesn't matter which entity inspects another, both seem wrong to me. We should have helper functions which need not be methods on any type or even part of a interface which analyze a combination of workload and deployer and tell if it is a supported config. |
| @@ -9,3 +9,5 @@ pvcspecs: | |||
| - name: cephfs | |||
| storageclassname: rook-cephfs | |||
| accessmodes: ReadWriteMany | |||
| unsupportedDeployers: | |||
There was a problem hiding this comment.
I prefer limiting the config file to user preferences. Whether a storageClass is not supported by a deployer is a function of the code not config.
There was a problem hiding this comment.
So in combination with my other comment, the helper function can decide in code if a pvcspec name is incompatible with a workload,deployer config.
There was a problem hiding this comment.
The code does not have any way to tell if something is supported or not. This is pure configuration.
When this is set in the configuration, I can create multiple configurations. For If I work on rbd related stuff, I can run the test only with rbd, without using cryptic -run arguments. This is much more powerful that doing this in code.
We can add helper, but it does not matter much if is it done in one side or in a helper. The natural way is in teh workload since it has the needed state that come from the config. |
We have configuration allowing any storage class name, but the code was hard-coding the names "rook-ceph-block" and "rook-cephfs", limiting the e2e tests to drenv environment. Add a Name key to each PVCSpec entry, eliminating the code to create a suffix. We use the name to crate the name of the test. Part-of: RamenDR#1635 Signed-off-by: Nir Soffer <[email protected]>
We checked if a deployer supports a workload by checking the hardcoded string "Deploy-cephfs". This wrong it 2 ways, assuming that we use the "Deploy" prefix, and assuming that the storage name is "cephfs". Fix by adding "unsupportedDeployers" lists to PVCSpec. The cephfs default configuration include the "disapp" as unsupported deployer. Move the supported check from the deployer to the workload, since the deployer has no state and it cannot tell if a workload is supported. To make it easier to filter deployers, names are now lower case. We anyway use lower case for test names. With this change, we can use any PVCSpec name, and we can keep multiple configurations running all of some the tests. Fixes: RamenDR#1635 Signed-off-by: Nir Soffer <[email protected]>
|
I rebased on main since it should not need the e2e-namesspace branch. Keeping unsupported as configuration is important for upstream. Think about a vendor adapting e2e test for its storage type that we cannot even test (e.g. KubeSAN). We don't want to keep code knowing about all kinds of storage. This should be part of vendor configuration that may be kept upstream for easier maintenance. The way to make this work is to keep the knowledge in the configuration, and keep the code simple. It is also very useful for developers that want run all the tests with specific storage. It is easier to keep multiple configurations than remember to run the tests with the right -run argument. And we need to manage multiple configuration anyway (e.g. drenv, your ocp test cluster). This is same as storageClassName - we can use any value from the config. We can run the tests with 1 storageClassName or 4, without changing any code. |
Fix the way we configure storage classes so we can create configuration that works with any clsuter.
Fixes #1635