require CSINode if topology enabled

pkg/controller/topology.go

@@ -29,7 +29,6 @@ import (
 	storage "k8s.io/api/storage/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
-	"k8s.io/klog"
 )
 
 // topologyTerm represents a single term where its topology key value pairs are AND'd together.
@@ -108,7 +107,6 @@ func GenerateAccessibilityRequirements(
 	} else {
 		// selectedNode is set so use topology from that node to populate preferredTerms
 		// TODO (verult) reuse selected node info from aggregateTopologies
-		// TODO (verult) retry
 		nodeInfo, err := kubeClient.StorageV1beta1().CSINodes().Get(selectedNode.Name, metav1.GetOptions{})
 		if err != nil {
 			return nil, fmt.Errorf("error getting node info for selected node: %v", err)
@@ -141,12 +139,10 @@ func aggregateTopologies(
 
 	var topologyKeys []string
 	if selectedNode == nil {
-		// TODO (verult) retry
+		// TODO: use informers
 		nodeInfos, err := kubeClient.StorageV1beta1().CSINodes().List(metav1.ListOptions{})
 		if err != nil {
-			// We must support provisioning if CSINode is missing, for backward compatibility.
-			klog.Warningf("error listing CSINodes: %v; proceeding to provision without topology information", err)
-			return nil, nil
+			return nil, fmt.Errorf("error listing CSINodes: %v", err)
 		}
 
 		rand.Shuffle(len(nodeInfos.Items), func(i, j int) {
@@ -161,12 +157,10 @@ func aggregateTopologies(
 			}
 		}
 	} else {
-		// TODO (verult) retry
+		// TODO: use informers
 		selectedNodeInfo, err := kubeClient.StorageV1beta1().CSINodes().Get(selectedNode.Name, metav1.GetOptions{})
 		if err != nil {
-			// We must support provisioning if CSINode is missing, for backward compatibility.
-			klog.Warningf("error getting CSINode for selected node %q: %v; proceeding to provision without topology information", selectedNode.Name, err)
-			return nil, nil
+			return nil, fmt.Errorf("error getting CSINode for selected node %q: %v", selectedNode.Name, err)
 		}
 		topologyKeys = getTopologyKeys(selectedNodeInfo, driverName)
 	}
@@ -183,6 +177,7 @@ func aggregateTopologies(
 	if err != nil {
 		return nil, err
 	}
+	// TODO: use informers
 	nodes, err := kubeClient.CoreV1().Nodes().List(metav1.ListOptions{LabelSelector: selector})
 	if err != nil {
 		return nil, fmt.Errorf("error listing nodes: %v", err)

pkg/controller/topology_test.go

@@ -918,6 +918,7 @@ func TestTopologyAggregation(t *testing.T) {
 			nodeLabels:        []map[string]string{{}, {}, {}},
 			topologyKeys:      nil,
 			expectedRequisite: nil,
+			expectError:       true,
 		},
 		"random node: missing keys": {
 			nodeLabels: []map[string]string{{}, {}, {}},
@@ -1046,6 +1047,29 @@ func TestPreferredTopologies(t *testing.T) {
 				},
 			},
 		},
+		"allowedTopologies specified: no CSINode": {
+			allowedTopologies: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    "com.example.csi/zone",
+							Values: []string{"zone1", "zone2"},
+						},
+						{
+							Key:    "com.example.csi/rack",
+							Values: []string{"rackA", "rackB"},
+						},
+					},
+				},
+			},
+			nodeLabels: []map[string]string{
+				{"com.example.csi/zone": "zone2", "com.example.csi/rack": "rackA"},
+				{"com.example.csi/zone": "zone1", "com.example.csi/rack": "rackA"},
+				{"com.example.csi/zone": "zone1", "com.example.csi/rack": "rackB"},
+			},
+			topologyKeys: nil,
+			expectError:  true,
+		},
 		"topology aggregation": {
 			nodeLabels: []map[string]string{
 				{"com.example.csi/zone": "zone2", "com.example.csi/rack": "rackA"},

pkg/features/features.go

@@ -32,5 +32,5 @@ func init() {
 // defaultKubernetesFeatureGates consists of all known feature keys specific to external-provisioner.
 // To add a new feature, define a key for it above and add it here.
 var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureSpec{
-	Topology: {Default: true, PreRelease: utilfeature.Beta},
+	Topology: {Default: false, PreRelease: utilfeature.Beta},
 }