From d7a086ddd82d7d1b287ac4da4c35c5c96b98da0d Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick.ohly@intel.com>
Date: Wed, 24 Mar 2021 22:31:43 +0100
Subject: [PATCH] storage e2e: simplify CSIStorageCapacity test

We can support topology detection for all drivers with a single
topology key by allowing different nodes to have the same topology
value. This makes the test a bit more generic and its configuration
simpler.
---
 test/e2e/storage/framework/testdriver.go |  7 -------
 test/e2e/storage/testsuites/capacity.go  | 26 ++++++++++++++----------
 2 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/test/e2e/storage/framework/testdriver.go b/test/e2e/storage/framework/testdriver.go
index e2844cdca00..d4e36dc4132 100644
--- a/test/e2e/storage/framework/testdriver.go
+++ b/test/e2e/storage/framework/testdriver.go
@@ -171,13 +171,6 @@ const (
 	// for dynamic provisioning exists, the driver is expected to provide
 	// capacity information for it.
 	CapCapacity Capability = "capacity"
-
-	// The driver manages storage locally through CSI. TopologyKeys must be
-	// set and contain exactly one entry for distinguishing nodes.
-	// When the driver supports CapCapacity and the storage class
-	// for dynamic provisioning exists, the driver is expected to
-	// provider capacity information for it for each node.
-	CapCSILocalStorage Capability = "CSILocalStorage"
 )
 
 // DriverInfo represents static information about a TestDriver.
diff --git a/test/e2e/storage/testsuites/capacity.go b/test/e2e/storage/testsuites/capacity.go
index 06a6f31f683..ac223be8e59 100644
--- a/test/e2e/storage/testsuites/capacity.go
+++ b/test/e2e/storage/testsuites/capacity.go
@@ -125,12 +125,17 @@ func (p *capacityTestSuite) DefineTests(driver storageframework.TestDriver, patt
 		// is that it provides some arbitrary capacity for the
 		// storage class.
 		matcher := matchSC
-		if dInfo.Capabilities[storageframework.CapCSILocalStorage] {
-			// Local storage. We expect one CSIStorageCapacity object per
-			// node for the storage class.
-			if len(dInfo.TopologyKeys) != 1 {
-				framework.Failf("need exactly one topology key for local storage, DriverInfo.TopologyKeys has: %v", dInfo.TopologyKeys)
-			}
+		if len(dInfo.TopologyKeys) == 1 {
+			// We can construct topology segments by
+			// collecting all values for this one key and
+			// then expect one CSIStorageCapacity object
+			// per value for the storage class.
+			//
+			// Local storage on a node will be covered by
+			// this checking. A more complex approach for
+			// drivers with multiple keys might be
+			// possible, too, but is not currently
+			// implemented.
 			matcher = HaveCapacitiesForClassAndNodes(f.ClientSet, sc.Provisioner, sc.Name, dInfo.TopologyKeys[0])
 		}
 
@@ -259,7 +264,6 @@ type haveLocalStorageCapacities struct {
 	topologyKey string
 
 	matchSuccess          bool
-	topologyValues        []string
 	expectedCapacities    []storagev1beta1.CSIStorageCapacity
 	unexpectedCapacities  []storagev1beta1.CSIStorageCapacity
 	missingTopologyValues []string
@@ -268,7 +272,6 @@ type haveLocalStorageCapacities struct {
 var _ CapacityMatcher = &haveLocalStorageCapacities{}
 
 func (h *haveLocalStorageCapacities) Match(actual interface{}) (success bool, err error) {
-	h.topologyValues = nil
 	h.expectedCapacities = nil
 	h.unexpectedCapacities = nil
 	h.missingTopologyValues = nil
@@ -285,6 +288,7 @@ func (h *haveLocalStorageCapacities) Match(actual interface{}) (success bool, er
 	if err != nil {
 		return false, err
 	}
+	topologyValues := map[string]bool{}
 	for _, csiNode := range csiNodes.Items {
 		for _, driver := range csiNode.Spec.Drivers {
 			if driver.Name != h.driverName {
@@ -301,17 +305,17 @@ func (h *haveLocalStorageCapacities) Match(actual interface{}) (success bool, er
 					node.Name,
 					h.topologyKey)
 			}
-			h.topologyValues = append(h.topologyValues, value)
+			topologyValues[value] = true
 			break
 		}
 	}
-	if len(h.topologyValues) == 0 {
+	if len(topologyValues) == 0 {
 		return false, fmt.Errorf("driver %q not running on any node", h.driverName)
 	}
 
 	// Now check that for each topology value there is exactly one CSIStorageCapacity object.
 	remainingTopologyValues := map[string]bool{}
-	for _, value := range h.topologyValues {
+	for value := range topologyValues {
 		remainingTopologyValues[value] = true
 	}
 	capacities := h.match.MatchedCapacities()