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

Promote block volume features to GA

Browse Source
This commit is contained in:
Jan Safranek
2020-02-28 20:48:38 +01:00
parent a9c7529f99
commit 2c1b743766
32 changed files with 117 additions and 843 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -25,10 +25,6 @@ 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) && !volumeModeInUse(oldPVSpec) {
pvSpec.VolumeMode = nil
}
if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandCSIVolumes) && !hasExpansionSecrets(oldPVSpec) {
if pvSpec.CSI != nil {
pvSpec.CSI.ControllerExpandSecretRef = nil
@@ -46,13 +42,3 @@ func hasExpansionSecrets(oldPVSpec *api.PersistentVolumeSpec) bool {
}
return false
}
func volumeModeInUse(oldPVSpec *api.PersistentVolumeSpec) bool {
if oldPVSpec == nil {
return false
}
if oldPVSpec.VolumeMode != nil {
return true
}
return false
}

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

@@ -28,68 +28,18 @@ import (
)
func TestDropDisabledFields(t *testing.T) {
specWithMode := func(mode *api.PersistentVolumeMode) *api.PersistentVolumeSpec {
return &api.PersistentVolumeSpec{VolumeMode: mode}
}
secretRef := &api.SecretReference{
Name: "expansion-secret",
Namespace: "default",
}
modeBlock := api.PersistentVolumeBlock
tests := map[string]struct {
oldSpec *api.PersistentVolumeSpec
newSpec *api.PersistentVolumeSpec
expectOldSpec *api.PersistentVolumeSpec
expectNewSpec *api.PersistentVolumeSpec
blockEnabled bool
csiExpansionEnabled bool
}{
"disabled block clears new": {
blockEnabled: false,
newSpec: specWithMode(&modeBlock),
expectNewSpec: specWithMode(nil),
oldSpec: nil,
expectOldSpec: nil,
},
"disabled block clears update when old pv did not use block": {
blockEnabled: false,
newSpec: specWithMode(&modeBlock),
expectNewSpec: specWithMode(nil),
oldSpec: specWithMode(nil),
expectOldSpec: specWithMode(nil),
},
"disabled block does not clear new on update when old pv did use block": {
blockEnabled: false,
newSpec: specWithMode(&modeBlock),
expectNewSpec: specWithMode(&modeBlock),
oldSpec: specWithMode(&modeBlock),
expectOldSpec: specWithMode(&modeBlock),
},
"enabled block preserves new": {
blockEnabled: true,
newSpec: specWithMode(&modeBlock),
expectNewSpec: specWithMode(&modeBlock),
oldSpec: nil,
expectOldSpec: nil,
},
"enabled block preserves update when old pv did not use block": {
blockEnabled: true,
newSpec: specWithMode(&modeBlock),
expectNewSpec: specWithMode(&modeBlock),
oldSpec: specWithMode(nil),
expectOldSpec: specWithMode(nil),
},
"enabled block preserves update when old pv did use block": {
blockEnabled: true,
newSpec: specWithMode(&modeBlock),
expectNewSpec: specWithMode(&modeBlock),
oldSpec: specWithMode(&modeBlock),
expectOldSpec: specWithMode(&modeBlock),
},
"disabled csi expansion clears secrets": {
csiExpansionEnabled: false,
newSpec: specWithCSISecrets(secretRef),
@@ -129,7 +79,6 @@ func TestDropDisabledFields(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, tc.blockEnabled)()
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandCSIVolumes, tc.csiExpansionEnabled)()
DropDisabledFields(tc.newSpec, tc.oldSpec)

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

@@ -30,24 +30,11 @@ const (
// 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) && !volumeModeInUse(oldPVCSpec) {
pvcSpec.VolumeMode = nil
}
if !dataSourceIsEnabled(pvcSpec) && !dataSourceInUse(oldPVCSpec) {
pvcSpec.DataSource = nil
}
}
func volumeModeInUse(oldPVCSpec *core.PersistentVolumeClaimSpec) bool {
if oldPVCSpec == nil {
return false
}
if oldPVCSpec.VolumeMode != nil {
return true
}
return false
}
func dataSourceInUse(oldPVCSpec *core.PersistentVolumeClaimSpec) bool {
if oldPVCSpec == nil {
return false

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

@@ -28,96 +28,6 @@ import (
"k8s.io/kubernetes/pkg/features"
)
func TestDropAlphaPVCVolumeMode(t *testing.T) {
vmode := core.PersistentVolumeFilesystem
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 },
},
}
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
}
t.Run(fmt.Sprintf("feature enabled=%v, old pvc %v, new pvc %v", enabled, oldpvcInfo.description, newpvcInfo.description), func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, enabled)()
var oldpvcSpec *core.PersistentVolumeClaimSpec
if oldpvc != nil {
oldpvcSpec = &oldpvc.Spec
}
DropDisabledFields(&newpvc.Spec, oldpvcSpec)
// 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()))
}
}
})
}
}
}
}
func TestDropDisabledSnapshotDataSource(t *testing.T) {
pvcWithoutDataSource := func() *core.PersistentVolumeClaim {
return &core.PersistentVolumeClaim{

31
pkg/api/pod/util.go
View File

@@ -385,8 +385,6 @@ func dropDisabledFields(
})
}
dropDisabledVolumeDevicesFields(podSpec, oldPodSpec)
dropDisabledRunAsGroupField(podSpec, oldPodSpec)
dropDisabledGMSAFields(podSpec, oldPodSpec)
@@ -484,17 +482,6 @@ func dropDisabledProcMountField(podSpec, oldPodSpec *api.PodSpec) {
}
}
// dropDisabledVolumeDevicesFields removes disabled fields from []VolumeDevice if it has not been already populated.
// This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a VolumeDevice
func dropDisabledVolumeDevicesFields(podSpec, oldPodSpec *api.PodSpec) {
if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) && !volumeDevicesInUse(oldPodSpec) {
VisitContainers(podSpec, func(c *api.Container) bool {
c.VolumeDevices = nil
return true
})
}
}
// dropDisabledCSIVolumeSourceAlphaFields removes disabled alpha fields from []CSIVolumeSource.
// This should be called from PrepareForCreate/PrepareForUpdate for all pod specs resources containing a CSIVolumeSource
func dropDisabledCSIVolumeSourceAlphaFields(podSpec, oldPodSpec *api.PodSpec) {
@@ -646,24 +633,6 @@ func emptyDirSizeLimitInUse(podSpec *api.PodSpec) bool {
return false
}
// volumeDevicesInUse returns true if the pod spec is non-nil and has VolumeDevices set.
func volumeDevicesInUse(podSpec *api.PodSpec) bool {
if podSpec == nil {
return false
}
var inUse bool
VisitContainers(podSpec, func(c *api.Container) bool {
if c.VolumeDevices != nil {
inUse = true
return false
}
return true
})
return inUse
}
// runAsGroupInUse returns true if the pod spec is non-nil and has a SecurityContext's RunAsGroup field set
func runAsGroupInUse(podSpec *api.PodSpec) bool {
if podSpec == nil {

144
pkg/api/pod/util_test.go
View File

@@ -435,150 +435,6 @@ func TestPodConfigmaps(t *testing.T) {
}
}
func TestDropAlphaVolumeDevices(t *testing.T) {
podWithVolumeDevices := func() *api.Pod {
return &api.Pod{
Spec: api.PodSpec{
RestartPolicy: api.RestartPolicyNever,
Containers: []api.Container{
{
Name: "container1",
Image: "testimage",
VolumeDevices: []api.VolumeDevice{
{
Name: "myvolume",
DevicePath: "/usr/test",
},
},
},
},
InitContainers: []api.Container{
{
Name: "container1",
Image: "testimage",
VolumeDevices: []api.VolumeDevice{
{
Name: "myvolume",
DevicePath: "/usr/test",
},
},
},
},
Volumes: []api.Volume{
{
Name: "myvolume",
VolumeSource: api.VolumeSource{
HostPath: &api.HostPathVolumeSource{
Path: "/dev/xvdc",
},
},
},
},
},
}
}
podWithoutVolumeDevices := func() *api.Pod {
return &api.Pod{
Spec: api.PodSpec{
RestartPolicy: api.RestartPolicyNever,
Containers: []api.Container{
{
Name: "container1",
Image: "testimage",
},
},
InitContainers: []api.Container{
{
Name: "container1",
Image: "testimage",
},
},
Volumes: []api.Volume{
{
Name: "myvolume",
VolumeSource: api.VolumeSource{
HostPath: &api.HostPathVolumeSource{
Path: "/dev/xvdc",
},
},
},
},
},
}
}
podInfo := []struct {
description string
hasVolumeDevices bool
pod func() *api.Pod
}{
{
description: "has VolumeDevices",
hasVolumeDevices: true,
pod: podWithVolumeDevices,
},
{
description: "does not have VolumeDevices",
hasVolumeDevices: false,
pod: podWithoutVolumeDevices,
},
{
description: "is nil",
hasVolumeDevices: false,
pod: func() *api.Pod { return nil },
},
}
for _, enabled := range []bool{true, false} {
for _, oldPodInfo := range podInfo {
for _, newPodInfo := range podInfo {
oldPodHasVolumeDevices, oldPod := oldPodInfo.hasVolumeDevices, oldPodInfo.pod()
newPodHasVolumeDevices, newPod := newPodInfo.hasVolumeDevices, newPodInfo.pod()
if newPod == nil {
continue
}
t.Run(fmt.Sprintf("feature enabled=%v, old pod %v, new pod %v", enabled, oldPodInfo.description, newPodInfo.description), func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, enabled)()
var oldPodSpec *api.PodSpec
if oldPod != nil {
oldPodSpec = &oldPod.Spec
}
dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil)
// old pod should never be changed
if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) {
t.Errorf("old pod changed: %v", diff.ObjectReflectDiff(oldPod, oldPodInfo.pod()))
}
switch {
case enabled || oldPodHasVolumeDevices:
// new pod should not be changed if the feature is enabled, or if the old pod had VolumeDevices
if !reflect.DeepEqual(newPod, newPodInfo.pod()) {
t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod()))
}
case newPodHasVolumeDevices:
// new pod should be changed
if reflect.DeepEqual(newPod, newPodInfo.pod()) {
t.Errorf("new pod was not changed")
}
// new pod should not have VolumeDevices
if !reflect.DeepEqual(newPod, podWithoutVolumeDevices()) {
t.Errorf("new pod had VolumeDevices: %v", diff.ObjectReflectDiff(newPod, podWithoutVolumeDevices()))
}
default:
// new pod should not need to be changed
if !reflect.DeepEqual(newPod, newPodInfo.pod()) {
t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod()))
}
}
})
}
}
}
}
func TestDropSubPath(t *testing.T) {
podWithSubpaths := func() *api.Pod {
return &api.Pod{