Skip to content

Commit 91aa277

Browse files
carloryk8s-publishing-bot
carlory
authored and
k8s-publishing-bot
committed
pvc bind pv with vac
Kubernetes-commit: 3a6a4830df003d8efcad7bd8e4f0cce609aee2ca
1 parent f77f7fa commit 91aa277

File tree

2 files changed

+135
-25
lines changed

2 files changed

+135
-25
lines changed

‎storage/volume/pv_helpers.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"k8s.io/client-go/kubernetes/scheme"
2929
storagelisters "k8s.io/client-go/listers/storage/v1"
3030
"k8s.io/client-go/tools/reference"
31+
"k8s.io/utils/ptr"
3132
)
3233

3334
const (
@@ -187,7 +188,15 @@ func FindMatchingVolume(
187188
volumes []*v1.PersistentVolume,
188189
node *v1.Node,
189190
excludedVolumes map[string]*v1.PersistentVolume,
190-
delayBinding bool) (*v1.PersistentVolume, error) {
191+
delayBinding bool,
192+
vacEnabled bool) (*v1.PersistentVolume, error) {
193+
194+
if !vacEnabled {
195+
claimVAC := ptr.Deref(claim.Spec.VolumeAttributesClassName, "")
196+
if claimVAC != "" {
197+
return nil, fmt.Errorf("unsupported volumeAttributesClassName is set on claim %s when the feature-gate VolumeAttributesClass is disabled", claimToClaimKey(claim))
198+
}
199+
}
191200

192201
var smallestVolume *v1.PersistentVolume
193202
var smallestVolumeQty resource.Quantity
@@ -227,6 +236,18 @@ func FindMatchingVolume(
227236
continue
228237
}
229238

239+
claimVAC := ptr.Deref(claim.Spec.VolumeAttributesClassName, "")
240+
volumeVAC := ptr.Deref(volume.Spec.VolumeAttributesClassName, "")
241+
242+
// filter out mismatching volumeAttributesClassName
243+
if vacEnabled && claimVAC != volumeVAC {
244+
continue
245+
}
246+
if !vacEnabled && volumeVAC != "" {
247+
// when the feature gate is disabled, the PV object has VAC set, then we should not bind at all.
248+
continue
249+
}
250+
230251
// check if PV's DeletionTimeStamp is set, if so, skip this volume.
231252
if volume.ObjectMeta.DeletionTimestamp != nil {
232253
continue

‎storage/volume/pv_helpers_test.go

Lines changed: 113 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package volume
1818

1919
import (
20+
"fmt"
2021
"testing"
2122

2223
v1 "k8s.io/api/core/v1"
@@ -33,6 +34,8 @@ var (
3334
classNoMode = "no-mode"
3435
classImmediateMode = "immediate-mode"
3536
classWaitMode = "wait-mode"
37+
classGold = "gold"
38+
classSilver = "silver"
3639

3740
modeImmediate = storagev1.VolumeBindingImmediate
3841
modeWait = storagev1.VolumeBindingWaitForFirstConsumer
@@ -167,6 +170,15 @@ func TestFindMatchVolumeWithNode(t *testing.T) {
167170
}),
168171
}
169172

173+
var volumesWithVAC = func(name string, input []*v1.PersistentVolume) []*v1.PersistentVolume {
174+
output := make([]*v1.PersistentVolume, len(input))
175+
for i, volume := range input {
176+
output[i] = volume.DeepCopy()
177+
output[i].Spec.VolumeAttributesClassName = &name
178+
}
179+
return output
180+
}
181+
170182
node1 := &v1.Node{
171183
ObjectMeta: metav1.ObjectMeta{
172184
Labels: map[string]string{"key1": "value1"},
@@ -190,80 +202,151 @@ func TestFindMatchVolumeWithNode(t *testing.T) {
190202

191203
scenarios := map[string]struct {
192204
expectedMatch string
205+
expectErr bool
193206
claim *v1.PersistentVolumeClaim
194207
node *v1.Node
208+
volumes []*v1.PersistentVolume
195209
excludedVolumes map[string]*v1.PersistentVolume
210+
vacEnabled []bool
196211
}{
197212
"success-match": {
198213
expectedMatch: "affinity-pv",
199-
claim: makeTestPersistentVolumeClaim("claim01", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}),
214+
volumes: volumes,
215+
claim: makeTestPersistentVolumeClaim("claim01", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, nil),
200216
node: node1,
217+
vacEnabled: []bool{true, false},
201218
},
202219
"success-prebound": {
203220
expectedMatch: "affinity-prebound",
204-
claim: makeTestPersistentVolumeClaim("claim02", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}),
221+
volumes: volumes,
222+
claim: makeTestPersistentVolumeClaim("claim02", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, nil),
205223
node: node1,
224+
vacEnabled: []bool{true, false},
206225
},
207226
"success-exclusion": {
208227
expectedMatch: "affinity-pv2",
209-
claim: makeTestPersistentVolumeClaim("claim01", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}),
228+
volumes: volumes,
229+
claim: makeTestPersistentVolumeClaim("claim01", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, nil),
210230
node: node1,
211231
excludedVolumes: map[string]*v1.PersistentVolume{"affinity001": nil},
232+
vacEnabled: []bool{true, false},
212233
},
213234
"fail-exclusion": {
214235
expectedMatch: "",
215-
claim: makeTestPersistentVolumeClaim("claim01", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}),
236+
volumes: volumes,
237+
claim: makeTestPersistentVolumeClaim("claim01", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, nil),
216238
node: node1,
217-
excludedVolumes: map[string]*v1.PersistentVolume{"affinity001": nil, "affinity002": nil},
239+
excludedVolumes: map[string]*v1.PersistentVolume{"affinity001": nil, "affinity002": nil, "affinity002-vac": nil},
240+
vacEnabled: []bool{true, false},
218241
},
219242
"fail-accessmode": {
220243
expectedMatch: "",
221-
claim: makeTestPersistentVolumeClaim("claim01", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteMany}),
244+
volumes: volumes,
245+
claim: makeTestPersistentVolumeClaim("claim01", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteMany}, nil),
222246
node: node1,
247+
vacEnabled: []bool{true, false},
223248
},
224249
"fail-nodeaffinity": {
225250
expectedMatch: "",
226-
claim: makeTestPersistentVolumeClaim("claim01", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}),
251+
volumes: volumes,
252+
claim: makeTestPersistentVolumeClaim("claim01", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, nil),
227253
node: node2,
254+
vacEnabled: []bool{true, false},
228255
},
229256
"fail-prebound-node-affinity": {
230257
expectedMatch: "",
231-
claim: makeTestPersistentVolumeClaim("claim02", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}),
258+
volumes: volumes,
259+
claim: makeTestPersistentVolumeClaim("claim02", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, nil),
232260
node: node3,
261+
vacEnabled: []bool{true, false},
233262
},
234263
"fail-nonavaliable": {
235264
expectedMatch: "",
236-
claim: makeTestPersistentVolumeClaim("claim04", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}),
265+
volumes: volumes,
266+
claim: makeTestPersistentVolumeClaim("claim04", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, nil),
237267
node: node4,
268+
vacEnabled: []bool{true, false},
238269
},
239270
"success-bad-and-good-node-affinity": {
240271
expectedMatch: "affinity-pv3",
241-
claim: makeTestPersistentVolumeClaim("claim03", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}),
272+
volumes: volumes,
273+
claim: makeTestPersistentVolumeClaim("claim03", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, nil),
242274
node: node3,
275+
vacEnabled: []bool{true, false},
276+
},
277+
"success-match-with-vac": {
278+
expectedMatch: "affinity-pv",
279+
volumes: volumesWithVAC(classGold, volumes),
280+
claim: makeTestPersistentVolumeClaim("claim01", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, func(pvc *v1.PersistentVolumeClaim) {
281+
pvc.Spec.VolumeAttributesClassName = &classGold
282+
}),
283+
node: node1,
284+
vacEnabled: []bool{true},
285+
},
286+
"fail-vac": { // claim has a given vac and volumes don't have the same vac.
287+
expectedMatch: "",
288+
volumes: volumes,
289+
claim: makeTestPersistentVolumeClaim("claim01", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, func(pvc *v1.PersistentVolumeClaim) {
290+
pvc.Spec.VolumeAttributesClassName = &classSilver
291+
}),
292+
node: node1,
293+
vacEnabled: []bool{true},
294+
},
295+
"fail-prebound-vac": { // claim has a given vac and volume name but the given volume has a different vac.
296+
expectedMatch: "",
297+
volumes: volumesWithVAC(classGold, volumes),
298+
claim: makeTestPersistentVolumeClaim("claim02", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, func(pvc *v1.PersistentVolumeClaim) {
299+
pvc.Spec.VolumeAttributesClassName = &classSilver
300+
}),
301+
node: node1,
302+
vacEnabled: []bool{true},
303+
},
304+
"fail-on-error": { // claim has a given vac when feature-gate is disabled.
305+
expectedMatch: "",
306+
expectErr: true,
307+
volumes: volumes,
308+
claim: makeTestPersistentVolumeClaim("claim01", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, func(pvc *v1.PersistentVolumeClaim) {
309+
pvc.Spec.VolumeAttributesClassName = &classGold
310+
}),
311+
node: node1,
312+
vacEnabled: []bool{false},
313+
},
314+
"fail-volumes-vac": { // claim has no vac and all volumes have vac when feature-gate is disabled.
315+
expectedMatch: "",
316+
volumes: volumesWithVAC(classGold, volumes),
317+
claim: makeTestPersistentVolumeClaim("claim01", "100G", []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, nil),
318+
node: node1,
319+
vacEnabled: []bool{false},
243320
},
244321
}
245322

246323
for name, scenario := range scenarios {
247-
volume, err := FindMatchingVolume(scenario.claim, volumes, scenario.node, scenario.excludedVolumes, true)
248-
if err != nil {
249-
t.Errorf("Unexpected error matching volume by claim: %v", err)
250-
}
251-
if len(scenario.expectedMatch) != 0 && volume == nil {
252-
t.Errorf("Expected match but received nil volume for scenario: %s", name)
253-
}
254-
if len(scenario.expectedMatch) != 0 && volume != nil && string(volume.UID) != scenario.expectedMatch {
255-
t.Errorf("Expected %s but got volume %s in scenario %s", scenario.expectedMatch, volume.UID, name)
256-
}
257-
if len(scenario.expectedMatch) == 0 && volume != nil {
258-
t.Errorf("Unexpected match for scenario: %s, matched with %s instead", name, volume.UID)
324+
for _, enabled := range scenario.vacEnabled {
325+
name := fmt.Sprintf("[VolumeAttributiesClass: %v] %s", enabled, name)
326+
volume, err := FindMatchingVolume(scenario.claim, scenario.volumes, scenario.node, scenario.excludedVolumes, true, enabled)
327+
if scenario.expectErr && err == nil {
328+
t.Errorf("Expected error for scenario: %s", name)
329+
}
330+
if !scenario.expectErr && err != nil {
331+
t.Errorf("Unexpected error matching volume by claim: %v", err)
332+
}
333+
if len(scenario.expectedMatch) != 0 && volume == nil {
334+
t.Errorf("Expected match but received nil volume for scenario: %s", name)
335+
}
336+
if len(scenario.expectedMatch) != 0 && volume != nil && string(volume.UID) != scenario.expectedMatch {
337+
t.Errorf("Expected %s but got volume %s in scenario %s", scenario.expectedMatch, volume.UID, name)
338+
}
339+
if len(scenario.expectedMatch) == 0 && volume != nil {
340+
t.Errorf("Unexpected match for scenario: %s, matched with %s instead", name, volume.UID)
341+
}
259342
}
260343
}
261344
}
262345

263-
func makeTestPersistentVolumeClaim(name string, size string, accessMode []v1.PersistentVolumeAccessMode) *v1.PersistentVolumeClaim {
346+
func makeTestPersistentVolumeClaim(name string, size string, accessMode []v1.PersistentVolumeAccessMode, modfn func(*v1.PersistentVolumeClaim)) *v1.PersistentVolumeClaim {
264347
fs := v1.PersistentVolumeFilesystem
265348
sc := "wait"
266-
return &v1.PersistentVolumeClaim{
349+
pvc := &v1.PersistentVolumeClaim{
267350
ObjectMeta: metav1.ObjectMeta{
268351
Name: name,
269352
Namespace: "myns",
@@ -279,6 +362,12 @@ func makeTestPersistentVolumeClaim(name string, size string, accessMode []v1.Per
279362
VolumeMode: &fs,
280363
},
281364
}
365+
366+
if modfn != nil {
367+
modfn(pvc)
368+
}
369+
370+
return pvc
282371
}
283372

284373
func makeTestVolume(uid types.UID, name string, capacity string, available bool, modfn func(*v1.PersistentVolume)) *v1.PersistentVolume {

0 commit comments

Comments
 (0)