github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #72251 from sbezverk/VolumeMode

Browse Source 
VolumeMode - Update DropDisabled[Alpha]Fields behaviour
This commit is contained in:
Kubernetes Prow Robot
2018-12-26 16:20:36 -08:00
committed by GitHub
parent 52b6b4086f a5d5a174af
commit d61286987d
10 changed files with 192 additions and 175 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
pkg/api/persistentvolume/util.go
View File

@@ -25,13 +25,8 @@ import (
// DropDisabledFields removes disabled fields from the pv spec.
// This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a pv spec.
func DropDisabledFields(pvSpec *api.PersistentVolumeSpec, oldPVSpec *api.PersistentVolumeSpec) {
if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) {
// TODO(liggitt): change this to only drop pvSpec.VolumeMode if (oldPVSpec == nil || oldPVSpec.VolumeMode == nil)
// Requires more coordinated changes to validation
if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) && !volumeModeInUse(oldPVSpec) {
pvSpec.VolumeMode = nil
if oldPVSpec != nil {
oldPVSpec.VolumeMode = nil
}
}
if !utilfeature.DefaultFeatureGate.Enabled(features.CSIPersistentVolume) {
@@ -41,3 +36,13 @@ func DropDisabledFields(pvSpec *api.PersistentVolumeSpec, oldPVSpec *api.Persist
}
}
}
func volumeModeInUse(oldPVSpec *api.PersistentVolumeSpec) bool {
if oldPVSpec == nil {
return false
}
if oldPVSpec.VolumeMode != nil {
return true
}
return false
}

7
pkg/api/persistentvolume/util_test.go
View File

@@ -106,13 +106,12 @@ func TestDropDisabledFields(t *testing.T) {
oldSpec: specWithMode(nil),
expectOldSpec: specWithMode(nil),
},
// TODO: consider changing this case to preserve
"disabled block clears old and new on update when old pv did use block": {
"disabled block does not clear new on update when old pv did use block": {
blockEnabled: false,
newSpec: specWithMode(&modeBlock),
expectNewSpec: specWithMode(nil),
expectNewSpec: specWithMode(&modeBlock),
oldSpec: specWithMode(&modeBlock),
expectOldSpec: specWithMode(nil),
expectOldSpec: specWithMode(&modeBlock),
},
"enabled block preserves new": {

1
pkg/api/persistentvolumeclaim/BUILD
View File

@@ -37,6 +37,7 @@ go_test(
deps = [
"//pkg/apis/core:go_default_library",
"//pkg/features:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/util/feature/testing:go_default_library",
],

15
pkg/api/persistentvolumeclaim/util.go
View File

@@ -25,10 +25,17 @@ import (
// DropDisabledFields removes disabled fields from the pvc spec.
// This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a pvc spec.
func DropDisabledFields(pvcSpec, oldPVCSpec *core.PersistentVolumeClaimSpec) {
if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) {
if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) && !volumeModeInUse(oldPVCSpec) {
pvcSpec.VolumeMode = nil
if oldPVCSpec != nil {
oldPVCSpec.VolumeMode = nil
}
}
}
func volumeModeInUse(oldPVCSpec *core.PersistentVolumeClaimSpec) bool {
if oldPVCSpec == nil {
return false
}
if oldPVCSpec.VolumeMode != nil {
return true
}
return false
}

103
pkg/api/persistentvolumeclaim/util_test.go
View File

@@ -17,8 +17,11 @@ limitations under the License.
package persistentvolumeclaim
import (
"fmt"
"reflect"
"testing"
"k8s.io/apimachinery/pkg/util/diff"
utilfeature "k8s.io/apiserver/pkg/util/feature"
utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing"
"k8s.io/kubernetes/pkg/apis/core"
@@ -28,33 +31,89 @@ import (
func TestDropAlphaPVCVolumeMode(t *testing.T) {
vmode := core.PersistentVolumeFilesystem
// PersistentVolume with VolumeMode set
pvc := core.PersistentVolumeClaim{
Spec: core.PersistentVolumeClaimSpec{
AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce},
VolumeMode: &vmode,
pvcWithoutVolumeMode := func() *core.PersistentVolumeClaim {
return &core.PersistentVolumeClaim{
Spec: core.PersistentVolumeClaimSpec{
VolumeMode: nil,
},
}
}
pvcWithVolumeMode := func() *core.PersistentVolumeClaim {
return &core.PersistentVolumeClaim{
Spec: core.PersistentVolumeClaimSpec{
VolumeMode: &vmode,
},
}
}
pvcInfo := []struct {
description string
hasVolumeMode bool
pvc func() *core.PersistentVolumeClaim
}{
{
description: "pvc without VolumeMode",
hasVolumeMode: false,
pvc: pvcWithoutVolumeMode,
},
{
description: "pvc with Filesystem VolumeMode",
hasVolumeMode: true,
pvc: pvcWithVolumeMode,
},
{
description: "is nil",
hasVolumeMode: false,
pvc: func() *core.PersistentVolumeClaim { return nil },
},
}
// Enable alpha feature BlockVolume
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)()
// now test dropping the fields - should not be dropped
DropDisabledFields(&pvc.Spec, nil)
for _, enabled := range []bool{true, false} {
for _, oldpvcInfo := range pvcInfo {
for _, newpvcInfo := range pvcInfo {
oldpvcHasVolumeMode, oldpvc := oldpvcInfo.hasVolumeMode, oldpvcInfo.pvc()
newpvcHasVolumeMode, newpvc := newpvcInfo.hasVolumeMode, newpvcInfo.pvc()
if newpvc == nil {
continue
}
// check to make sure VolumeDevices is still present
// if featureset is set to true
if pvc.Spec.VolumeMode == nil {
t.Error("VolumeMode in pvc.Spec should not have been dropped based on feature-gate")
}
t.Run(fmt.Sprintf("feature enabled=%v, old pvc %v, new pvc %v", enabled, oldpvcInfo.description, newpvcInfo.description), func(t *testing.T) {
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, enabled)()
// Disable alpha feature BlockVolume
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, false)()
// now test dropping the fields
DropDisabledFields(&pvc.Spec, nil)
var oldpvcSpec *core.PersistentVolumeClaimSpec
if oldpvc != nil {
oldpvcSpec = &oldpvc.Spec
}
DropDisabledFields(&newpvc.Spec, oldpvcSpec)
// check to make sure VolumeDevices is nil
// if featureset is set to false
if pvc.Spec.VolumeMode != nil {
t.Error("DropDisabledFields VolumeMode for pvc.Spec failed")
// old pvc should never be changed
if !reflect.DeepEqual(oldpvc, oldpvcInfo.pvc()) {
t.Errorf("old pvc changed: %v", diff.ObjectReflectDiff(oldpvc, oldpvcInfo.pvc()))
}
switch {
case enabled || oldpvcHasVolumeMode:
// new pvc should not be changed if the feature is enabled, or if the old pvc had BlockVolume
if !reflect.DeepEqual(newpvc, newpvcInfo.pvc()) {
t.Errorf("new pvc changed: %v", diff.ObjectReflectDiff(newpvc, newpvcInfo.pvc()))
}
case newpvcHasVolumeMode:
// new pvc should be changed
if reflect.DeepEqual(newpvc, newpvcInfo.pvc()) {
t.Errorf("new pvc was not changed")
}
// new pvc should not have BlockVolume
if !reflect.DeepEqual(newpvc, pvcWithoutVolumeMode()) {
t.Errorf("new pvc had pvcBlockVolume: %v", diff.ObjectReflectDiff(newpvc, pvcWithoutVolumeMode()))
}
default:
// new pvc should not need to be changed
if !reflect.DeepEqual(newpvc, newpvcInfo.pvc()) {
t.Errorf("new pvc changed: %v", diff.ObjectReflectDiff(newpvc, newpvcInfo.pvc()))
}
}
})
}
}
}
}