e2e/storage: speed up skipping, simplify APIs and test definition

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 commit is contained in:
Patrick Ohly
2018-12-29 17:08:34 +01:00
parent 1cb121d2a9
commit 05cc31697f
15 changed files with 1278 additions and 1516 deletions

View File

@@ -56,12 +56,11 @@ const (
// hostpathCSI
type hostpathCSIDriver struct {
cleanup func()
driverInfo testsuites.DriverInfo
manifests []string
}
func initHostPathCSIDriver(name string, config testsuites.TestConfig, capabilities map[testsuites.Capability]bool, manifests ...string) testsuites.TestDriver {
func initHostPathCSIDriver(name string, capabilities map[testsuites.Capability]bool, manifests ...string) testsuites.TestDriver {
return &hostpathCSIDriver{
driverInfo: testsuites.DriverInfo{
Name: name,
@@ -71,7 +70,6 @@ func initHostPathCSIDriver(name string, config testsuites.TestConfig, capabiliti
"", // Default fsType
),
Capabilities: capabilities,
Config: config,
},
manifests: manifests,
}
@@ -82,8 +80,8 @@ var _ testsuites.DynamicPVTestDriver = &hostpathCSIDriver{}
var _ testsuites.SnapshottableTestDriver = &hostpathCSIDriver{}
// InitHostPathCSIDriver returns hostpathCSIDriver that implements TestDriver interface
func InitHostPathCSIDriver(config testsuites.TestConfig) testsuites.TestDriver {
return initHostPathCSIDriver("csi-hostpath", config,
func InitHostPathCSIDriver() testsuites.TestDriver {
return initHostPathCSIDriver("csi-hostpath",
map[testsuites.Capability]bool{testsuites.CapPersistence: true, testsuites.CapDataSource: true, testsuites.CapMultiPODs: true},
"test/e2e/testing-manifests/storage-csi/driver-registrar/rbac.yaml",
"test/e2e/testing-manifests/storage-csi/external-attacher/rbac.yaml",
@@ -104,19 +102,19 @@ func (h *hostpathCSIDriver) GetDriverInfo() *testsuites.DriverInfo {
func (h *hostpathCSIDriver) SkipUnsupportedTest(pattern testpatterns.TestPattern) {
}
func (h *hostpathCSIDriver) GetDynamicProvisionStorageClass(fsType string) *storagev1.StorageClass {
provisioner := testsuites.GetUniqueDriverName(h)
func (h *hostpathCSIDriver) GetDynamicProvisionStorageClass(config *testsuites.PerTestConfig, fsType string) *storagev1.StorageClass {
provisioner := config.GetUniqueDriverName()
parameters := map[string]string{}
ns := h.driverInfo.Config.Framework.Namespace.Name
ns := config.Framework.Namespace.Name
suffix := fmt.Sprintf("%s-sc", provisioner)
return testsuites.GetStorageClass(provisioner, parameters, nil, ns, suffix)
}
func (h *hostpathCSIDriver) GetSnapshotClass() *unstructured.Unstructured {
snapshotter := testsuites.GetUniqueDriverName(h)
func (h *hostpathCSIDriver) GetSnapshotClass(config *testsuites.PerTestConfig) *unstructured.Unstructured {
snapshotter := config.GetUniqueDriverName()
parameters := map[string]string{}
ns := h.driverInfo.Config.Framework.Namespace.Name
ns := config.Framework.Namespace.Name
suffix := fmt.Sprintf("%s-vsc", snapshotter)
return testsuites.GetSnapshotClass(snapshotter, parameters, ns, suffix)
@@ -126,57 +124,60 @@ func (h *hostpathCSIDriver) GetClaimSize() string {
return "5Gi"
}
func (h *hostpathCSIDriver) CreateDriver() {
func (h *hostpathCSIDriver) PrepareTest(f *framework.Framework) (*testsuites.PerTestConfig, func()) {
By(fmt.Sprintf("deploying %s driver", h.driverInfo.Name))
f := h.driverInfo.Config.Framework
cancelLogging := testsuites.StartPodLogs(f)
cs := f.ClientSet
// The hostpath CSI driver only works when everything runs on the same node.
nodes := framework.GetReadySchedulableNodesOrDie(cs)
nodeName := nodes.Items[rand.Intn(len(nodes.Items))].Name
h.driverInfo.Config.ClientNodeName = nodeName
config := &testsuites.PerTestConfig{
Driver: h,
Prefix: "hostpath",
Framework: f,
ClientNodeName: nodeName,
}
// TODO (?): the storage.csi.image.version and storage.csi.image.registry
// settings are ignored for this test. We could patch the image definitions.
o := utils.PatchCSIOptions{
OldDriverName: h.driverInfo.Name,
NewDriverName: testsuites.GetUniqueDriverName(h),
NewDriverName: config.GetUniqueDriverName(),
DriverContainerName: "hostpath",
DriverContainerArguments: []string{"--drivername=csi-hostpath-" + f.UniqueName},
DriverContainerArguments: []string{"--drivername=" + config.GetUniqueDriverName()},
ProvisionerContainerName: "csi-provisioner",
SnapshotterContainerName: "csi-snapshotter",
NodeName: nodeName,
}
cleanup, err := h.driverInfo.Config.Framework.CreateFromManifests(func(item interface{}) error {
return utils.PatchCSIDeployment(h.driverInfo.Config.Framework, o, item)
cleanup, err := config.Framework.CreateFromManifests(func(item interface{}) error {
return utils.PatchCSIDeployment(config.Framework, o, item)
},
h.manifests...)
h.cleanup = cleanup
if err != nil {
framework.Failf("deploying %s driver: %v", h.driverInfo.Name, err)
}
}
func (h *hostpathCSIDriver) CleanupDriver() {
if h.cleanup != nil {
return config, func() {
By(fmt.Sprintf("uninstalling %s driver", h.driverInfo.Name))
h.cleanup()
cleanup()
cancelLogging()
}
}
// mockCSI
type mockCSIDriver struct {
cleanup func()
driverInfo testsuites.DriverInfo
manifests []string
podInfoVersion *string
attachable bool
}
var _ testsuites.TestDriver = &mockCSIDriver{}
var _ testsuites.DynamicPVTestDriver = &mockCSIDriver{}
// InitMockCSIDriver returns a mockCSIDriver that implements TestDriver interface
func InitMockCSIDriver(config testsuites.TestConfig, registerDriver, driverAttachable bool, podInfoVersion *string) testsuites.TestDriver {
func InitMockCSIDriver(registerDriver, driverAttachable bool, podInfoVersion *string) testsuites.TestDriver {
driverManifests := []string{
"test/e2e/testing-manifests/storage-csi/cluster-driver-registrar/rbac.yaml",
"test/e2e/testing-manifests/storage-csi/driver-registrar/rbac.yaml",
@@ -187,16 +188,12 @@ func InitMockCSIDriver(config testsuites.TestConfig, registerDriver, driverAttac
"test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver.yaml",
}
config.ServerConfig = &framework.VolumeTestConfig{}
if registerDriver {
driverManifests = append(driverManifests, "test/e2e/testing-manifests/storage-csi/mock/csi-mock-cluster-driver-registrar.yaml")
}
if driverAttachable {
driverManifests = append(driverManifests, "test/e2e/testing-manifests/storage-csi/mock/csi-mock-driver-attacher.yaml")
} else {
config.ServerConfig.ServerArgs = append(config.ServerConfig.ServerArgs, "--disable-attach")
}
return &mockCSIDriver{
@@ -212,10 +209,10 @@ func InitMockCSIDriver(config testsuites.TestConfig, registerDriver, driverAttac
testsuites.CapFsGroup: false,
testsuites.CapExec: false,
},
Config: config,
},
manifests: driverManifests,
podInfoVersion: podInfoVersion,
attachable: driverAttachable,
}
}
@@ -226,10 +223,10 @@ func (m *mockCSIDriver) GetDriverInfo() *testsuites.DriverInfo {
func (m *mockCSIDriver) SkipUnsupportedTest(pattern testpatterns.TestPattern) {
}
func (m *mockCSIDriver) GetDynamicProvisionStorageClass(fsType string) *storagev1.StorageClass {
provisioner := testsuites.GetUniqueDriverName(m)
func (m *mockCSIDriver) GetDynamicProvisionStorageClass(config *testsuites.PerTestConfig, fsType string) *storagev1.StorageClass {
provisioner := config.GetUniqueDriverName()
parameters := map[string]string{}
ns := m.driverInfo.Config.Framework.Namespace.Name
ns := config.Framework.Namespace.Name
suffix := fmt.Sprintf("%s-sc", provisioner)
return testsuites.GetStorageClass(provisioner, parameters, nil, ns, suffix)
@@ -239,20 +236,24 @@ func (m *mockCSIDriver) GetClaimSize() string {
return "5Gi"
}
func (m *mockCSIDriver) CreateDriver() {
func (m *mockCSIDriver) PrepareTest(f *framework.Framework) (*testsuites.PerTestConfig, func()) {
By("deploying csi mock driver")
f := m.driverInfo.Config.Framework
cancelLogging := testsuites.StartPodLogs(f)
cs := f.ClientSet
// pods should be scheduled on the node
nodes := framework.GetReadySchedulableNodesOrDie(cs)
node := nodes.Items[rand.Intn(len(nodes.Items))]
m.driverInfo.Config.ClientNodeName = node.Name
config := &testsuites.PerTestConfig{
Driver: m,
Prefix: "mock",
Framework: f,
ClientNodeName: node.Name,
}
containerArgs := []string{"--name=csi-mock-" + f.UniqueName}
if m.driverInfo.Config.ServerConfig != nil && m.driverInfo.Config.ServerConfig.ServerArgs != nil {
containerArgs = append(containerArgs, m.driverInfo.Config.ServerConfig.ServerArgs...)
if !m.attachable {
containerArgs = append(containerArgs, "--disable-attach")
}
// TODO (?): the storage.csi.image.version and storage.csi.image.registry
@@ -264,29 +265,27 @@ func (m *mockCSIDriver) CreateDriver() {
DriverContainerArguments: containerArgs,
ProvisionerContainerName: "csi-provisioner",
ClusterRegistrarContainerName: "csi-cluster-driver-registrar",
NodeName: m.driverInfo.Config.ClientNodeName,
NodeName: config.ClientNodeName,
PodInfoVersion: m.podInfoVersion,
}
cleanup, err := f.CreateFromManifests(func(item interface{}) error {
return utils.PatchCSIDeployment(f, o, item)
},
m.manifests...)
m.cleanup = cleanup
if err != nil {
framework.Failf("deploying csi mock driver: %v", err)
}
}
func (m *mockCSIDriver) CleanupDriver() {
if m.cleanup != nil {
return config, func() {
By("uninstalling csi mock driver")
m.cleanup()
cleanup()
cancelLogging()
}
}
// InitHostPathV0CSIDriver returns a variant of hostpathCSIDriver with different manifests.
func InitHostPathV0CSIDriver(config testsuites.TestConfig) testsuites.TestDriver {
return initHostPathCSIDriver("csi-hostpath-v0", config,
func InitHostPathV0CSIDriver() testsuites.TestDriver {
return initHostPathCSIDriver("csi-hostpath-v0",
map[testsuites.Capability]bool{testsuites.CapPersistence: true, testsuites.CapMultiPODs: true},
"test/e2e/testing-manifests/storage-csi/driver-registrar/rbac.yaml",
"test/e2e/testing-manifests/storage-csi/external-attacher/rbac.yaml",
@@ -300,16 +299,17 @@ func InitHostPathV0CSIDriver(config testsuites.TestConfig) testsuites.TestDriver
// gce-pd
type gcePDCSIDriver struct {
cleanup func()
driverInfo testsuites.DriverInfo
topologyEnabled bool
driverInfo testsuites.DriverInfo
}
var _ testsuites.TestDriver = &gcePDCSIDriver{}
var _ testsuites.DynamicPVTestDriver = &gcePDCSIDriver{}
// InitGcePDCSIDriver returns gcePDCSIDriver that implements TestDriver interface
func InitGcePDCSIDriver(config testsuites.TestConfig) testsuites.TestDriver {
func InitGcePDCSIDriver(topologyEnabled bool) testsuites.TestDriver {
return &gcePDCSIDriver{
topologyEnabled: topologyEnabled,
driverInfo: testsuites.DriverInfo{
Name: GCEPDCSIProvisionerName,
FeatureTag: "[Serial]",
@@ -327,8 +327,6 @@ func InitGcePDCSIDriver(config testsuites.TestConfig) testsuites.TestDriver {
testsuites.CapExec: true,
testsuites.CapMultiPODs: true,
},
Config: config,
},
}
}
@@ -338,21 +336,14 @@ func (g *gcePDCSIDriver) GetDriverInfo() *testsuites.DriverInfo {
}
func (g *gcePDCSIDriver) SkipUnsupportedTest(pattern testpatterns.TestPattern) {
f := g.driverInfo.Config.Framework
framework.SkipUnlessProviderIs("gce", "gke")
if !g.driverInfo.Config.TopologyEnabled {
// Topology is disabled in external-provisioner, so in a multizone cluster, a pod could be
// scheduled in a different zone from the provisioned volume, causing basic provisioning
// tests to fail.
framework.SkipIfMultizone(f.ClientSet)
}
if pattern.FsType == "xfs" {
framework.SkipUnlessNodeOSDistroIs("ubuntu", "custom")
}
}
func (g *gcePDCSIDriver) GetDynamicProvisionStorageClass(fsType string) *storagev1.StorageClass {
ns := g.driverInfo.Config.Framework.Namespace.Name
func (g *gcePDCSIDriver) GetDynamicProvisionStorageClass(config *testsuites.PerTestConfig, fsType string) *storagev1.StorageClass {
ns := config.Framework.Namespace.Name
provisioner := g.driverInfo.Name
suffix := fmt.Sprintf("%s-sc", g.driverInfo.Name)
@@ -368,8 +359,16 @@ func (g *gcePDCSIDriver) GetClaimSize() string {
return "5Gi"
}
func (g *gcePDCSIDriver) CreateDriver() {
func (g *gcePDCSIDriver) PrepareTest(f *framework.Framework) (*testsuites.PerTestConfig, func()) {
if !g.topologyEnabled {
// Topology is disabled in external-provisioner, so in a multizone cluster, a pod could be
// scheduled in a different zone from the provisioned volume, causing basic provisioning
// tests to fail.
framework.SkipIfMultizone(f.ClientSet)
}
By("deploying csi gce-pd driver")
cancelLogging := testsuites.StartPodLogs(f)
// It would be safer to rename the gcePD driver, but that
// hasn't been done before either and attempts to do so now led to
// errors during driver registration, therefore it is disabled
@@ -382,7 +381,7 @@ func (g *gcePDCSIDriver) CreateDriver() {
// DriverContainerName: "gce-driver",
// ProvisionerContainerName: "csi-external-provisioner",
// }
createGCESecrets(g.driverInfo.Config.Framework.ClientSet, g.driverInfo.Config.Framework.Namespace.Name)
createGCESecrets(f.ClientSet, f.Namespace.Name)
manifests := []string{
"test/e2e/testing-manifests/storage-csi/driver-registrar/rbac.yaml",
@@ -392,23 +391,25 @@ func (g *gcePDCSIDriver) CreateDriver() {
"test/e2e/testing-manifests/storage-csi/gce-pd/node_ds.yaml",
}
if g.driverInfo.Config.TopologyEnabled {
if g.topologyEnabled {
manifests = append(manifests, "test/e2e/testing-manifests/storage-csi/gce-pd/controller_ss_alpha.yaml")
} else {
manifests = append(manifests, "test/e2e/testing-manifests/storage-csi/gce-pd/controller_ss.yaml")
}
cleanup, err := g.driverInfo.Config.Framework.CreateFromManifests(nil, manifests...)
g.cleanup = cleanup
cleanup, err := f.CreateFromManifests(nil, manifests...)
if err != nil {
framework.Failf("deploying csi gce-pd driver: %v", err)
}
}
func (g *gcePDCSIDriver) CleanupDriver() {
By("uninstalling gce-pd driver")
if g.cleanup != nil {
g.cleanup()
}
return &testsuites.PerTestConfig{
Driver: g,
Prefix: "gcepd",
Framework: f,
}, func() {
By("uninstalling gce-pd driver")
cleanup()
cancelLogging()
}
}
// gcePd-external
@@ -420,7 +421,7 @@ var _ testsuites.TestDriver = &gcePDExternalCSIDriver{}
var _ testsuites.DynamicPVTestDriver = &gcePDExternalCSIDriver{}
// InitGcePDExternalCSIDriver returns gcePDExternalCSIDriver that implements TestDriver interface
func InitGcePDExternalCSIDriver(config testsuites.TestConfig) testsuites.TestDriver {
func InitGcePDExternalCSIDriver() testsuites.TestDriver {
return &gcePDExternalCSIDriver{
driverInfo: testsuites.DriverInfo{
Name: GCEPDCSIProvisionerName,
@@ -440,8 +441,6 @@ func InitGcePDExternalCSIDriver(config testsuites.TestConfig) testsuites.TestDri
testsuites.CapExec: true,
testsuites.CapMultiPODs: true,
},
Config: config,
},
}
}
@@ -452,14 +451,13 @@ func (g *gcePDExternalCSIDriver) GetDriverInfo() *testsuites.DriverInfo {
func (g *gcePDExternalCSIDriver) SkipUnsupportedTest(pattern testpatterns.TestPattern) {
framework.SkipUnlessProviderIs("gce", "gke")
framework.SkipIfMultizone(g.driverInfo.Config.Framework.ClientSet)
if pattern.FsType == "xfs" {
framework.SkipUnlessNodeOSDistroIs("ubuntu", "custom")
}
}
func (g *gcePDExternalCSIDriver) GetDynamicProvisionStorageClass(fsType string) *storagev1.StorageClass {
ns := g.driverInfo.Config.Framework.Namespace.Name
func (g *gcePDExternalCSIDriver) GetDynamicProvisionStorageClass(config *testsuites.PerTestConfig, fsType string) *storagev1.StorageClass {
ns := config.Framework.Namespace.Name
provisioner := g.driverInfo.Name
suffix := fmt.Sprintf("%s-sc", g.driverInfo.Name)
@@ -475,8 +473,12 @@ func (g *gcePDExternalCSIDriver) GetClaimSize() string {
return "5Gi"
}
func (g *gcePDExternalCSIDriver) CreateDriver() {
}
func (g *gcePDExternalCSIDriver) PrepareTest(f *framework.Framework) (*testsuites.PerTestConfig, func()) {
framework.SkipIfMultizone(f.ClientSet)
func (g *gcePDExternalCSIDriver) CleanupDriver() {
return &testsuites.PerTestConfig{
Driver: g,
Prefix: "gcepdext",
Framework: f,
}, func() {}
}