e2e test "[It] should provision storage with different parameters"
depends on cloud providers as gce/gke, aws, openstack, vsphere and
azure. If the other cloud providers like local, the test is skipped.
However getRandomClusterZone() was called before the above cloud
provider check, and if the zone label is not found the test was failed.
This makes the test call getRandomClusterZone() only if necessary
to avoid such unnecessary failures.
Fixes: #73771
There is a risk that the init function does not reset one of the local
variables that was set by a previous test. To avoid this, all
variables set by init are now in a struct which gets cleaned
completely first.
The recommended approach for not running unsuitable tests is to skip
them at runtime with an explanation. Filtering out unsuitable test
patters and thus not even defining unsuitable tests was done earlier
because it was faster than skipping tests at runtime.
But now these tests can be skipped efficiently, so this special case
can be removed.
CreateDriver (now called SetupTest) is a potentially expensive
operation, depending on the driver. Creating and tearing down a
framework instance also takes time (measured at 6 seconds on a fast
machine) and produces quite a bit of log output.
Both can be avoided for tests that skip based on static
information (like for instance the current OS, vendor, driver and test
pattern) by making the test suite responsible for creating framework
and driver.
The lifecycle of the TestConfig instance was confusing because it was
stored inside the DriverInfo, a struct which conceptually is static,
while the TestConfig is dynamic. It is cleaner to separate the two,
even if that means that an additional pointer must be passed into some
functions. Now CreateDriver is responsible for initializing the
PerTestConfig that is to be used by the test.
To make this approach simpler to implement (= less functions which
need the pointer) and the tests easier to read, the entire setup and
test definition is now contained in a single function. This is how it
is normally done in Ginkgo. This is easier to read because one can see
at a glance where variables are set, instead of having to trace values
though two additional structs (TestResource and TestInput).
Because we are changing the API already, also other changes are made:
- some function prototypes get simplified
- the naming of functions is changed to match their purpose
(tests aren't executed by the test suite, they only get defined
for later execution)
- unused methods get removed (TestSuite.skipUnsupportedTest is redundant)
This increases type safety and makes the code easier to read because
it becomes obvious that the "test resource" passed to some functions
must be the result of a previous CreateVolume.
This makes it possible to remove:
- functions that never did anything (the DeleteVolume methods in
drivers that never create a volume)
- type casts (in the DeleteVolume implementation)
- the unused DeleteVolume parameters
- the stand-alone DeleteVolume function (which would be just a non-nil
check)
GetPersistentVolumeSource and GetVolumeSource could also become
methods on more specific interfaces - they don't actually use anything
from TestDriver instance which provides them.
The main motivation however is to reduce the number of methods which
might need an explicit test config parameter.
Kubernetes Prow Robot