diff --git a/pkg/controller/topology.go b/pkg/controller/topology.go index 09206e34f4..b6812d28cf 100644 --- a/pkg/controller/topology.go +++ b/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) diff --git a/pkg/controller/topology_test.go b/pkg/controller/topology_test.go index 8f0d1fff8e..aff46147a8 100644 --- a/pkg/controller/topology_test.go +++ b/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"}, diff --git a/pkg/features/features.go b/pkg/features/features.go index 2f4a8fd2fa..195fe7b32b 100644 --- a/pkg/features/features.go +++ b/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}, }