Commit 1cb121d2 authored by Patrick Ohly's avatar Patrick Ohly

e2e/storage: introduce TestVolume interface

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.
parent bf20886f
...@@ -147,7 +147,7 @@ type genericVolumeTestResource struct { ...@@ -147,7 +147,7 @@ type genericVolumeTestResource struct {
pv *v1.PersistentVolume pv *v1.PersistentVolume
sc *storagev1.StorageClass sc *storagev1.StorageClass
driverTestResource interface{} volume TestVolume
} }
var _ TestResource = &genericVolumeTestResource{} var _ TestResource = &genericVolumeTestResource{}
...@@ -162,19 +162,19 @@ func (r *genericVolumeTestResource) setupResource(driver TestDriver, pattern tes ...@@ -162,19 +162,19 @@ func (r *genericVolumeTestResource) setupResource(driver TestDriver, pattern tes
volType := pattern.VolType volType := pattern.VolType
// Create volume for pre-provisioned volume tests // Create volume for pre-provisioned volume tests
r.driverTestResource = CreateVolume(driver, volType) r.volume = CreateVolume(driver, volType)
switch volType { switch volType {
case testpatterns.InlineVolume: case testpatterns.InlineVolume:
framework.Logf("Creating resource for inline volume") framework.Logf("Creating resource for inline volume")
if iDriver, ok := driver.(InlineVolumeTestDriver); ok { if iDriver, ok := driver.(InlineVolumeTestDriver); ok {
r.volSource = iDriver.GetVolumeSource(false, fsType, r.driverTestResource) r.volSource = iDriver.GetVolumeSource(false, fsType, r.volume)
r.volType = dInfo.Name r.volType = dInfo.Name
} }
case testpatterns.PreprovisionedPV: case testpatterns.PreprovisionedPV:
framework.Logf("Creating resource for pre-provisioned PV") framework.Logf("Creating resource for pre-provisioned PV")
if pDriver, ok := driver.(PreprovisionedPVTestDriver); ok { if pDriver, ok := driver.(PreprovisionedPVTestDriver); ok {
pvSource, volumeNodeAffinity := pDriver.GetPersistentVolumeSource(false, fsType, r.driverTestResource) pvSource, volumeNodeAffinity := pDriver.GetPersistentVolumeSource(false, fsType, r.volume)
if pvSource != nil { if pvSource != nil {
r.volSource, r.pv, r.pvc = createVolumeSourceWithPVCPV(f, dInfo.Name, pvSource, volumeNodeAffinity, false) r.volSource, r.pv, r.pvc = createVolumeSourceWithPVCPV(f, dInfo.Name, pvSource, volumeNodeAffinity, false)
} }
...@@ -241,7 +241,9 @@ func (r *genericVolumeTestResource) cleanupResource(driver TestDriver, pattern t ...@@ -241,7 +241,9 @@ func (r *genericVolumeTestResource) cleanupResource(driver TestDriver, pattern t
} }
// Cleanup volume for pre-provisioned volume tests // Cleanup volume for pre-provisioned volume tests
DeleteVolume(driver, volType, r.driverTestResource) if r.volume != nil {
r.volume.DeleteVolume()
}
} }
func createVolumeSourceWithPVCPV( func createVolumeSourceWithPVCPV(
......
...@@ -37,7 +37,7 @@ func GetDriverNameWithFeatureTags(driver TestDriver) string { ...@@ -37,7 +37,7 @@ func GetDriverNameWithFeatureTags(driver TestDriver) string {
} }
// CreateVolume creates volume for test unless dynamicPV test // CreateVolume creates volume for test unless dynamicPV test
func CreateVolume(driver TestDriver, volType testpatterns.TestVolType) interface{} { func CreateVolume(driver TestDriver, volType testpatterns.TestVolType) TestVolume {
switch volType { switch volType {
case testpatterns.InlineVolume: case testpatterns.InlineVolume:
fallthrough fallthrough
...@@ -53,22 +53,6 @@ func CreateVolume(driver TestDriver, volType testpatterns.TestVolType) interface ...@@ -53,22 +53,6 @@ func CreateVolume(driver TestDriver, volType testpatterns.TestVolType) interface
return nil return nil
} }
// DeleteVolume deletes volume for test unless dynamicPV test
func DeleteVolume(driver TestDriver, volType testpatterns.TestVolType, testResource interface{}) {
switch volType {
case testpatterns.InlineVolume:
fallthrough
case testpatterns.PreprovisionedPV:
if pDriver, ok := driver.(PreprovisionedVolumeTestDriver); ok {
pDriver.DeleteVolume(volType, testResource)
}
case testpatterns.DynamicPV:
// No need to delete volume
default:
framework.Failf("Invalid volType specified: %v", volType)
}
}
// GetStorageClass constructs a new StorageClass instance // GetStorageClass constructs a new StorageClass instance
// with a unique name that is based on namespace + suffix. // with a unique name that is based on namespace + suffix.
func GetStorageClass( func GetStorageClass(
......
...@@ -150,7 +150,7 @@ func (s *subPathTestResource) setupResource(driver TestDriver, pattern testpatte ...@@ -150,7 +150,7 @@ func (s *subPathTestResource) setupResource(driver TestDriver, pattern testpatte
switch volType { switch volType {
case testpatterns.InlineVolume: case testpatterns.InlineVolume:
if iDriver, ok := driver.(InlineVolumeTestDriver); ok { if iDriver, ok := driver.(InlineVolumeTestDriver); ok {
s.roVolSource = iDriver.GetVolumeSource(true, fsType, s.genericVolumeTestResource.driverTestResource) s.roVolSource = iDriver.GetVolumeSource(true, fsType, s.genericVolumeTestResource.volume)
} }
case testpatterns.PreprovisionedPV: case testpatterns.PreprovisionedPV:
s.roVolSource = &v1.VolumeSource{ s.roVolSource = &v1.VolumeSource{
......
...@@ -38,22 +38,28 @@ type TestDriver interface { ...@@ -38,22 +38,28 @@ type TestDriver interface {
SkipUnsupportedTest(testpatterns.TestPattern) SkipUnsupportedTest(testpatterns.TestPattern)
} }
// TestVolume is the result of PreprovisionedVolumeTestDriver.CreateVolume.
// The only common functionality is to delete it. Individual driver interfaces
// have additional methods that work with volumes created by them.
type TestVolume interface {
DeleteVolume()
}
// PreprovisionedVolumeTestDriver represents an interface for a TestDriver that has pre-provisioned volume // PreprovisionedVolumeTestDriver represents an interface for a TestDriver that has pre-provisioned volume
type PreprovisionedVolumeTestDriver interface { type PreprovisionedVolumeTestDriver interface {
TestDriver TestDriver
// CreateVolume creates a pre-provisioned volume. // CreateVolume creates a pre-provisioned volume of the desired volume type.
CreateVolume(testpatterns.TestVolType) interface{} CreateVolume(volumeType testpatterns.TestVolType) TestVolume
// DeleteVolume deletes a volume that is created in CreateVolume
DeleteVolume(testpatterns.TestVolType, interface{})
} }
// InlineVolumeTestDriver represents an interface for a TestDriver that supports InlineVolume // InlineVolumeTestDriver represents an interface for a TestDriver that supports InlineVolume
type InlineVolumeTestDriver interface { type InlineVolumeTestDriver interface {
PreprovisionedVolumeTestDriver PreprovisionedVolumeTestDriver
// GetVolumeSource returns a volumeSource for inline volume. // GetVolumeSource returns a volumeSource for inline volume.
// It will set readOnly and fsType to the volumeSource, if TestDriver supports both of them. // It will set readOnly and fsType to the volumeSource, if TestDriver supports both of them.
// It will return nil, if the TestDriver doesn't support either of the parameters. // It will return nil, if the TestDriver doesn't support either of the parameters.
GetVolumeSource(readOnly bool, fsType string, testResource interface{}) *v1.VolumeSource GetVolumeSource(readOnly bool, fsType string, testVolume TestVolume) *v1.VolumeSource
} }
// PreprovisionedPVTestDriver represents an interface for a TestDriver that supports PreprovisionedPV // PreprovisionedPVTestDriver represents an interface for a TestDriver that supports PreprovisionedPV
...@@ -63,7 +69,7 @@ type PreprovisionedPVTestDriver interface { ...@@ -63,7 +69,7 @@ type PreprovisionedPVTestDriver interface {
// It will set readOnly and fsType to the PersistentVolumeSource, if TestDriver supports both of them. // It will set readOnly and fsType to the PersistentVolumeSource, if TestDriver supports both of them.
// It will return nil, if the TestDriver doesn't support either of the parameters. // It will return nil, if the TestDriver doesn't support either of the parameters.
// Volume node affinity is optional, it will be nil for volumes which does not have volume node affinity. // Volume node affinity is optional, it will be nil for volumes which does not have volume node affinity.
GetPersistentVolumeSource(readOnly bool, fsType string, testResource interface{}) (*v1.PersistentVolumeSource, *v1.VolumeNodeAffinity) GetPersistentVolumeSource(readOnly bool, fsType string, testVolume TestVolume) (*v1.PersistentVolumeSource, *v1.VolumeNodeAffinity)
} }
// DynamicPVTestDriver represents an interface for a TestDriver that supports DynamicPV // DynamicPVTestDriver represents an interface for a TestDriver that supports DynamicPV
......
...@@ -146,7 +146,7 @@ type volumeModeTestResource struct { ...@@ -146,7 +146,7 @@ type volumeModeTestResource struct {
pvc *v1.PersistentVolumeClaim pvc *v1.PersistentVolumeClaim
pv *v1.PersistentVolume pv *v1.PersistentVolume
driverTestResource interface{} volume TestVolume
} }
var _ TestResource = &volumeModeTestResource{} var _ TestResource = &volumeModeTestResource{}
...@@ -168,7 +168,7 @@ func (s *volumeModeTestResource) setupResource(driver TestDriver, pattern testpa ...@@ -168,7 +168,7 @@ func (s *volumeModeTestResource) setupResource(driver TestDriver, pattern testpa
) )
// Create volume for pre-provisioned volume tests // Create volume for pre-provisioned volume tests
s.driverTestResource = CreateVolume(driver, volType) s.volume = CreateVolume(driver, volType)
switch volType { switch volType {
case testpatterns.PreprovisionedPV: case testpatterns.PreprovisionedPV:
...@@ -178,7 +178,7 @@ func (s *volumeModeTestResource) setupResource(driver TestDriver, pattern testpa ...@@ -178,7 +178,7 @@ func (s *volumeModeTestResource) setupResource(driver TestDriver, pattern testpa
scName = fmt.Sprintf("%s-%s-sc-for-file", ns.Name, dInfo.Name) scName = fmt.Sprintf("%s-%s-sc-for-file", ns.Name, dInfo.Name)
} }
if pDriver, ok := driver.(PreprovisionedPVTestDriver); ok { if pDriver, ok := driver.(PreprovisionedPVTestDriver); ok {
pvSource, volumeNodeAffinity = pDriver.GetPersistentVolumeSource(false, fsType, s.driverTestResource) pvSource, volumeNodeAffinity = pDriver.GetPersistentVolumeSource(false, fsType, s.volume)
if pvSource == nil { if pvSource == nil {
framework.Skipf("Driver %q does not define PersistentVolumeSource - skipping", dInfo.Name) framework.Skipf("Driver %q does not define PersistentVolumeSource - skipping", dInfo.Name)
} }
...@@ -211,7 +211,6 @@ func (s *volumeModeTestResource) cleanupResource(driver TestDriver, pattern test ...@@ -211,7 +211,6 @@ func (s *volumeModeTestResource) cleanupResource(driver TestDriver, pattern test
f := dInfo.Config.Framework f := dInfo.Config.Framework
cs := f.ClientSet cs := f.ClientSet
ns := f.Namespace ns := f.Namespace
volType := pattern.VolType
By("Deleting pv and pvc") By("Deleting pv and pvc")
errs := framework.PVPVCCleanup(cs, ns.Name, s.pv, s.pvc) errs := framework.PVPVCCleanup(cs, ns.Name, s.pv, s.pvc)
...@@ -224,7 +223,9 @@ func (s *volumeModeTestResource) cleanupResource(driver TestDriver, pattern test ...@@ -224,7 +223,9 @@ func (s *volumeModeTestResource) cleanupResource(driver TestDriver, pattern test
} }
// Cleanup volume for pre-provisioned volume tests // Cleanup volume for pre-provisioned volume tests
DeleteVolume(driver, volType, s.driverTestResource) if s.volume != nil {
s.volume.DeleteVolume()
}
} }
type volumeModeTestInput struct { type volumeModeTestInput struct {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment