Enable volume limit feature by default

Also add tests for it.
pull/58/head
Hemant Kumar 2018-09-28 11:13:59 -04:00
parent c79ad5a65b
commit 575f79e03e
4 changed files with 125 additions and 29 deletions

View File

@ -408,7 +408,7 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS
QOSReserved: {Default: false, PreRelease: utilfeature.Alpha},
ExpandPersistentVolumes: {Default: true, PreRelease: utilfeature.Beta},
ExpandInUsePersistentVolumes: {Default: false, PreRelease: utilfeature.Alpha},
AttachVolumeLimit: {Default: false, PreRelease: utilfeature.Beta},
AttachVolumeLimit: {Default: true, PreRelease: utilfeature.Beta},
CPUManager: {Default: true, PreRelease: utilfeature.Beta},
CPUCFSQuotaPeriod: {Default: false, PreRelease: utilfeature.Alpha},
ServiceNodeExclusion: {Default: false, PreRelease: utilfeature.Alpha},

View File

@ -43,10 +43,13 @@ go_test(
embed = [":go_default_library"],
deps = [
"//pkg/apis/core/helper:go_default_library",
"//pkg/apis/core/v1/helper:go_default_library",
"//pkg/features:go_default_library",
"//pkg/volume/testing:go_default_library",
"//pkg/volume/util:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",

View File

@ -89,11 +89,16 @@ func (nim *nodeInfoManager) AddNodeInfo(driverName string, driverNodeID string,
nodeUpdateFuncs := []nodeUpdateFunc{
updateNodeIDInNode(driverName, driverNodeID),
updateMaxAttachLimit(driverName, maxAttachLimit),
}
if utilfeature.DefaultFeatureGate.Enabled(features.CSINodeInfo) {
nodeUpdateFuncs = append(nodeUpdateFuncs, updateTopologyLabels(topology))
}
if utilfeature.DefaultFeatureGate.Enabled(features.AttachVolumeLimit) {
nodeUpdateFuncs = append(nodeUpdateFuncs, updateMaxAttachLimit(driverName, maxAttachLimit))
}
err := nim.updateNode(nodeUpdateFuncs...)
if err != nil {
return fmt.Errorf("error updating Node object with CSI driver node info: %v", err)

View File

@ -18,9 +18,12 @@ package nodeinfomanager
import (
"encoding/json"
"testing"
"github.com/container-storage-interface/spec/lib/go/csi/v0"
"k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
@ -31,9 +34,10 @@ import (
csiv1alpha1 "k8s.io/csi-api/pkg/apis/csi/v1alpha1"
csifake "k8s.io/csi-api/pkg/client/clientset/versioned/fake"
"k8s.io/kubernetes/pkg/apis/core/helper"
v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
"k8s.io/kubernetes/pkg/features"
volumetest "k8s.io/kubernetes/pkg/volume/testing"
"testing"
"k8s.io/kubernetes/pkg/volume/util"
)
type testcase struct {
@ -43,10 +47,12 @@ type testcase struct {
existingNodeInfo *csiv1alpha1.CSINodeInfo
inputNodeID string
inputTopology *csi.Topology
inputVolumeLimit int64
expectedNodeIDMap map[string]string
expectedTopologyMap map[string]sets.String
expectedLabels map[string]string
expectNoNodeInfo bool
expectedVolumeLimit int64
expectFail bool
}
@ -61,7 +67,7 @@ func TestAddNodeInfo(t *testing.T) {
{
name: "empty node",
driverName: "com.example.csi/driver1",
existingNode: generateNode(nil /* nodeIDs */, nil /* labels */),
existingNode: generateNode(nil /* nodeIDs */, nil /* labels */, nil /*capacity*/),
inputNodeID: "com.example.csi/csi-node1",
inputTopology: &csi.Topology{
Segments: map[string]string{
@ -85,7 +91,8 @@ func TestAddNodeInfo(t *testing.T) {
},
labelMap{
"com.example.csi/zone": "zoneA",
}),
},
nil /*capacity*/),
existingNodeInfo: generateNodeInfo(
nodeIDMap{
"com.example.csi/driver1": "com.example.csi/csi-node1",
@ -117,7 +124,7 @@ func TestAddNodeInfo(t *testing.T) {
nodeIDMap{
"com.example.csi/driver1": "com.example.csi/csi-node1",
},
nil /* labels */),
nil /* labels */, nil /*capacity*/),
existingNodeInfo: generateNodeInfo(
nodeIDMap{
"com.example.csi/driver1": "com.example.csi/csi-node1",
@ -149,7 +156,7 @@ func TestAddNodeInfo(t *testing.T) {
},
labelMap{
"net.example.storage/rack": "rack1",
}),
}, nil /*capacity*/),
existingNodeInfo: generateNodeInfo(
nodeIDMap{
"net.example.storage/other-driver": "net.example.storage/test-node",
@ -186,7 +193,7 @@ func TestAddNodeInfo(t *testing.T) {
},
labelMap{
"com.example.csi/zone": "zoneA",
}),
}, nil /*capacity*/),
existingNodeInfo: generateNodeInfo(
nodeIDMap{
"com.example.csi/driver1": "com.example.csi/csi-node1",
@ -212,7 +219,7 @@ func TestAddNodeInfo(t *testing.T) {
},
labelMap{
"com.example.csi/zone": "zoneA",
}),
}, nil /*capacity*/),
existingNodeInfo: generateNodeInfo(
nodeIDMap{
"com.example.csi/driver1": "com.example.csi/csi-node1",
@ -241,7 +248,7 @@ func TestAddNodeInfo(t *testing.T) {
{
name: "nil topology, empty node",
driverName: "com.example.csi/driver1",
existingNode: generateNode(nil /* nodeIDs */, nil /* labels */),
existingNode: generateNode(nil /* nodeIDs */, nil /* labels */, nil /*capacity*/),
inputNodeID: "com.example.csi/csi-node1",
inputTopology: nil,
expectedNodeIDMap: map[string]string{
@ -261,7 +268,7 @@ func TestAddNodeInfo(t *testing.T) {
},
labelMap{
"com.example.csi/zone": "zoneA",
}),
}, nil /*capacity*/),
existingNodeInfo: generateNodeInfo(
nodeIDMap{
"com.example.csi/driver1": "com.example.csi/csi-node1",
@ -291,7 +298,7 @@ func TestAddNodeInfo(t *testing.T) {
},
labelMap{
"net.example.storage/rack": "rack1",
}),
}, nil /*capacity*/),
existingNodeInfo: generateNodeInfo(
nodeIDMap{
"net.example.storage/other-driver": "net.example.storage/test-node",
@ -317,10 +324,48 @@ func TestAddNodeInfo(t *testing.T) {
{
name: "empty node ID",
driverName: "com.example.csi/driver1",
existingNode: generateNode(nil /* nodeIDs */, nil /* labels */),
existingNode: generateNode(nil /* nodeIDs */, nil /* labels */, nil /*capacity*/),
inputNodeID: "",
expectFail: true,
},
{
name: "new node with valid max limit",
driverName: "com.example.csi/driver1",
existingNode: generateNode(nil /*nodeIDs*/, nil /*labels*/, nil /*capacity*/),
inputVolumeLimit: 10,
inputTopology: nil,
inputNodeID: "com.example.csi/csi-node1",
expectedVolumeLimit: 10,
expectedNodeIDMap: map[string]string{
"com.example.csi/driver1": "com.example.csi/csi-node1",
},
expectedTopologyMap: map[string]sets.String{
"com.example.csi/driver1": nil,
},
expectedLabels: nil,
},
{
name: "node with existing valid max limit",
driverName: "com.example.csi/driver1",
existingNode: generateNode(
nil, /*nodeIDs*/
nil, /*labels*/
map[v1.ResourceName]resource.Quantity{
v1.ResourceCPU: *resource.NewScaledQuantity(4, -3),
v1.ResourceName(util.GetCSIAttachLimitKey("com.example.csi/driver1")): *resource.NewQuantity(10, resource.DecimalSI),
}),
inputVolumeLimit: 20,
inputTopology: nil,
inputNodeID: "com.example.csi/csi-node1",
expectedVolumeLimit: 20,
expectedNodeIDMap: map[string]string{
"com.example.csi/driver1": "com.example.csi/csi-node1",
},
expectedTopologyMap: map[string]sets.String{
"com.example.csi/driver1": nil,
},
expectedLabels: nil,
},
}
test(t, true /* addNodeInfo */, true /* csiNodeInfoEnabled */, testcases)
@ -333,7 +378,7 @@ func TestAddNodeInfo_CSINodeInfoDisabled(t *testing.T) {
{
name: "empty node",
driverName: "com.example.csi/driver1",
existingNode: generateNode(nil /* nodeIDs */, nil /* labels */),
existingNode: generateNode(nil /* nodeIDs */, nil /* labels */, nil /*capacity*/),
inputNodeID: "com.example.csi/csi-node1",
expectedNodeIDMap: map[string]string{
"com.example.csi/driver1": "com.example.csi/csi-node1",
@ -346,7 +391,7 @@ func TestAddNodeInfo_CSINodeInfoDisabled(t *testing.T) {
nodeIDMap{
"com.example.csi/driver1": "com.example.csi/csi-node1",
},
nil /* labels */),
nil /* labels */, nil /*capacity*/),
inputNodeID: "com.example.csi/csi-node1",
expectedNodeIDMap: map[string]string{
"com.example.csi/driver1": "com.example.csi/csi-node1",
@ -359,7 +404,7 @@ func TestAddNodeInfo_CSINodeInfoDisabled(t *testing.T) {
nodeIDMap{
"net.example.storage/other-driver": "net.example.storage/test-node",
},
nil /* labels */),
nil /* labels */, nil /*capacity*/),
inputNodeID: "com.example.csi/csi-node1",
expectedNodeIDMap: map[string]string{
"com.example.csi/driver1": "com.example.csi/csi-node1",
@ -377,7 +422,7 @@ func TestRemoveNodeInfo(t *testing.T) {
{
name: "empty node and no CSINodeInfo",
driverName: "com.example.csi/driver1",
existingNode: generateNode(nil /* nodeIDs */, nil /* labels */),
existingNode: generateNode(nil /* nodeIDs */, nil /* labels */, nil /*capacity*/),
expectedNodeIDMap: nil,
expectedLabels: nil,
expectNoNodeInfo: true,
@ -391,7 +436,7 @@ func TestRemoveNodeInfo(t *testing.T) {
},
labelMap{
"com.example.csi/zone": "zoneA",
}),
}, nil /*capacity*/),
existingNodeInfo: generateNodeInfo(
nodeIDMap{
"com.example.csi/driver1": "com.example.csi/csi-node1",
@ -413,7 +458,7 @@ func TestRemoveNodeInfo(t *testing.T) {
},
labelMap{
"net.example.storage/zone": "zoneA",
}),
}, nil /*capacity*/),
existingNodeInfo: generateNodeInfo(
nodeIDMap{
"net.example.storage/other-driver": "net.example.storage/csi-node1",
@ -437,7 +482,7 @@ func TestRemoveNodeInfo(t *testing.T) {
nodeIDMap{
"com.example.csi/driver1": "com.example.csi/csi-node1",
},
nil /* labels */),
nil /* labels */, nil /*capacity*/),
expectedNodeIDMap: nil,
expectedLabels: nil,
expectNoNodeInfo: true,
@ -448,13 +493,29 @@ func TestRemoveNodeInfo(t *testing.T) {
nodeIDMap{
"net.example.storage/other-driver": "net.example.storage/csi-node1",
},
nil /* labels */),
nil /* labels */, nil /*capacity*/),
expectedNodeIDMap: map[string]string{
"net.example.storage/other-driver": "net.example.storage/csi-node1",
},
expectedLabels: nil,
expectNoNodeInfo: true,
},
{
name: "new node with valid max limit",
driverName: "com.example.csi/driver1",
existingNode: generateNode(
nil, /*nodeIDs*/
nil, /*labels*/
map[v1.ResourceName]resource.Quantity{
v1.ResourceCPU: *resource.NewScaledQuantity(4, -3),
v1.ResourceName(util.GetCSIAttachLimitKey("com.example.csi/driver1")): *resource.NewQuantity(10, resource.DecimalSI),
},
),
inputTopology: nil,
inputNodeID: "com.example.csi/csi-node1",
expectNoNodeInfo: true,
expectedVolumeLimit: 0,
},
}
test(t, false /* addNodeInfo */, true /* csiNodeInfoEnabled */, testcases)
@ -467,7 +528,7 @@ func TestRemoveNodeInfo_CSINodeInfoDisabled(t *testing.T) {
{
name: "empty node",
driverName: "com.example.csi/driver1",
existingNode: generateNode(nil /* nodeIDs */, nil /* labels */),
existingNode: generateNode(nil /* nodeIDs */, nil /* labels */, nil /*capacity*/),
expectedNodeIDMap: nil,
},
{
@ -477,7 +538,7 @@ func TestRemoveNodeInfo_CSINodeInfoDisabled(t *testing.T) {
nodeIDMap{
"com.example.csi/driver1": "com.example.csi/csi-node1",
},
nil /* labels */),
nil /* labels */, nil /*capacity*/),
expectedNodeIDMap: nil,
},
{
@ -487,7 +548,7 @@ func TestRemoveNodeInfo_CSINodeInfoDisabled(t *testing.T) {
nodeIDMap{
"net.example.storage/other-driver": "net.example.storage/csi-node1",
},
nil /* labels */),
nil /* labels */, nil /*capacity*/),
expectedNodeIDMap: map[string]string{
"net.example.storage/other-driver": "net.example.storage/csi-node1",
},
@ -513,7 +574,7 @@ func TestAddNodeInfoExistingAnnotation(t *testing.T) {
nodeIDMap{
"com.example.csi/driver1": "com.example.csi/csi-node1",
},
nil /* labels */),
nil /* labels */, nil /*capacity*/),
},
{
name: "pre-existing info about a different driver in node, but no CSINodeInfo",
@ -521,7 +582,7 @@ func TestAddNodeInfoExistingAnnotation(t *testing.T) {
nodeIDMap{
"net.example.storage/other-driver": "net.example.storage/test-node",
},
nil /* labels */),
nil /* labels */, nil /*capacity*/),
},
}
@ -575,6 +636,7 @@ func TestAddNodeInfoExistingAnnotation(t *testing.T) {
func test(t *testing.T, addNodeInfo bool, csiNodeInfoEnabled bool, testcases []testcase) {
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSINodeInfo, csiNodeInfoEnabled)()
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AttachVolumeLimit, true)()
for _, tc := range testcases {
t.Logf("test case: %q", tc.name)
@ -604,7 +666,7 @@ func test(t *testing.T, addNodeInfo bool, csiNodeInfoEnabled bool, testcases []t
//// Act
if addNodeInfo {
err = nim.AddNodeInfo(tc.driverName, tc.inputNodeID, 0 /* maxVolumeLimit */, tc.inputTopology) // TODO test maxVolumeLimit
err = nim.AddNodeInfo(tc.driverName, tc.inputNodeID, tc.inputVolumeLimit, tc.inputTopology)
} else {
err = nim.RemoveNodeInfo(tc.driverName)
}
@ -627,6 +689,13 @@ func test(t *testing.T, addNodeInfo bool, csiNodeInfoEnabled bool, testcases []t
continue
}
// We are testing max volume limits
attachLimit := getVolumeLimit(node, tc.driverName)
if attachLimit != tc.expectedVolumeLimit {
t.Errorf("expected volume limit to be %d got %d", tc.expectedVolumeLimit, attachLimit)
continue
}
// Node ID annotation
annNodeID, ok := node.Annotations[annotationKeyNodeID]
if ok {
@ -685,19 +754,38 @@ func test(t *testing.T, addNodeInfo bool, csiNodeInfoEnabled bool, testcases []t
}
}
func generateNode(nodeIDs, labels map[string]string) *v1.Node {
func getVolumeLimit(node *v1.Node, driverName string) int64 {
volumeLimits := map[v1.ResourceName]int64{}
nodeAllocatables := node.Status.Allocatable
for k, v := range nodeAllocatables {
if v1helper.IsAttachableVolumeResourceName(k) {
volumeLimits[k] = v.Value()
}
}
attachKey := v1.ResourceName(util.GetCSIAttachLimitKey(driverName))
attachLimit := volumeLimits[attachKey]
return attachLimit
}
func generateNode(nodeIDs, labels map[string]string, capacity map[v1.ResourceName]resource.Quantity) *v1.Node {
var annotations map[string]string
if len(nodeIDs) > 0 {
b, _ := json.Marshal(nodeIDs)
annotations = map[string]string{annotationKeyNodeID: string(b)}
}
return &v1.Node{
node := &v1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "node1",
Annotations: annotations,
Labels: labels,
},
}
if len(capacity) > 0 {
node.Status.Capacity = v1.ResourceList(capacity)
node.Status.Allocatable = v1.ResourceList(capacity)
}
return node
}
func generateNodeInfo(nodeIDs map[string]string, topologyKeys map[string][]string) *csiv1alpha1.CSINodeInfo {