github
/
k3s
mirror of https://github.com/k3s-io/k3s
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #72375 from sbezverk/containers_volumedevices 

VolumeDevices validation and tests
pull/564/head
Kubernetes Prow Robot 2018-12-27 17:39:05 -08:00 committed by GitHub
parent 66bf481114 5bf84db713
commit 456ffa0453
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 147 additions and 82 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -264,7 +264,7 @@ func DropDisabledFields(podSpec, oldPodSpec *api.PodSpec) {
}
}
dropDisabledVolumeDevicesAlphaFields(podSpec, oldPodSpec)
dropDisabledVolumeDevicesFields(podSpec, oldPodSpec)
dropDisabledRunAsGroupField(podSpec, oldPodSpec)
@ -330,25 +330,16 @@ func dropDisabledProcMountField(podSpec, oldPodSpec *api.PodSpec) {
}
}
// dropDisabledVolumeDevicesAlphaFields removes disabled fields from []VolumeDevice.
// 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 dropDisabledVolumeDevicesAlphaFields(podSpec, oldPodSpec *api.PodSpec) {
if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) {
func dropDisabledVolumeDevicesFields(podSpec, oldPodSpec *api.PodSpec) {
if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) && !volumeDevicesInUse(oldPodSpec) {
for i := range podSpec.Containers {
podSpec.Containers[i].VolumeDevices = nil
}
for i := range podSpec.InitContainers {
podSpec.InitContainers[i].VolumeDevices = nil
}
if oldPodSpec != nil {
for i := range oldPodSpec.Containers {
oldPodSpec.Containers[i].VolumeDevices = nil
}
for i := range oldPodSpec.InitContainers {
oldPodSpec.InitContainers[i].VolumeDevices = nil
}
}
}
}
@ -436,3 +427,21 @@ 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
}
for i := range podSpec.Containers {
if podSpec.Containers[i].VolumeDevices != nil {
return true
}
}
for i := range podSpec.InitContainers {
if podSpec.InitContainers[i].VolumeDevices != nil {
return true
}
}
return false
}

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

@ -266,74 +266,146 @@ func TestPodConfigmaps(t *testing.T) {
}
func TestDropAlphaVolumeDevices(t *testing.T) {
testPod := api.Pod{
Spec: api.PodSpec{
RestartPolicy: api.RestartPolicyNever,
Containers: []api.Container{
{
Name: "container1",
Image: "testimage",
VolumeDevices: []api.VolumeDevice{
{
Name: "myvolume",
DevicePath: "/usr/test",
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",
},
},
},
},
},
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 },
},
}
// Enable alpha feature BlockVolume
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)()
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
}
// now test dropping the fields - should not be dropped
DropDisabledFields(&testPod.Spec, nil)
t.Run(fmt.Sprintf("feature enabled=%v, old pod %v, new pod %v", enabled, oldPodInfo.description, newPodInfo.description), func(t *testing.T) {
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, enabled)()
// check to make sure VolumeDevices is still present
// if featureset is set to true
if testPod.Spec.Containers[0].VolumeDevices == nil {
t.Error("VolumeDevices in Container should not have been dropped based on feature-gate")
}
if testPod.Spec.InitContainers[0].VolumeDevices == nil {
t.Error("VolumeDevices in InitContainers should not have been dropped based on feature-gate")
}
var oldPodSpec *api.PodSpec
if oldPod != nil {
oldPodSpec = &oldPod.Spec
}
DropDisabledFields(&newPod.Spec, oldPodSpec)
// Disable alpha feature BlockVolume
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, false)()
// old pod should never be changed
if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) {
t.Errorf("old pod changed: %v", diff.ObjectReflectDiff(oldPod, oldPodInfo.pod()))
}
// now test dropping the fields
DropDisabledFields(&testPod.Spec, nil)
// check to make sure VolumeDevices is nil
// if featureset is set to false
if testPod.Spec.Containers[0].VolumeDevices != nil {
t.Error("DropDisabledFields for Containers failed")
}
if testPod.Spec.InitContainers[0].VolumeDevices != nil {
t.Error("DropDisabledFields for InitContainers failed")
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()))
}
}
})
}
}
}
}

4
pkg/apis/core/validation/validation.go
View File

@ -2296,10 +2296,6 @@ func ValidateVolumeDevices(devices []core.VolumeDevice, volmounts map[string]str
devicepath := sets.NewString()
devicename := sets.NewString()
if devices != nil && !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("volumeDevices"), "Container volumeDevices is disabled by feature-gate"))
return allErrs
}
if devices != nil {
for i, dev := range devices {
idxPath := fldPath.Index(i)

12
pkg/apis/core/validation/validation_test.go
View File

@ -4961,10 +4961,6 @@ func TestAlphaValidateVolumeDevices(t *testing.T) {
return
}
disabledAlphaVolDevice := []core.VolumeDevice{
{Name: "abc", DevicePath: "/foo"},
}
successCase := []core.VolumeDevice{
{Name: "abc", DevicePath: "/foo"},
{Name: "abc-123", DevicePath: "/usr/share/test"},
@ -4992,8 +4988,6 @@ func TestAlphaValidateVolumeDevices(t *testing.T) {
{Name: "abc-123", MountPath: "/this/path/exists"},
}
// enable BlockVolume
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)()
// Success Cases:
// Validate normal success cases - only PVC volumeSource
if errs := ValidateVolumeDevices(successCase, GetVolumeMountMap(goodVolumeMounts), vols, field.NewPath("field")); len(errs) != 0 {
@ -5007,12 +5001,6 @@ func TestAlphaValidateVolumeDevices(t *testing.T) {
t.Errorf("expected failure for %s", k)
}
}
// disable BlockVolume
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, false)()
if errs := ValidateVolumeDevices(disabledAlphaVolDevice, GetVolumeMountMap(goodVolumeMounts), vols, field.NewPath("field")); len(errs) == 0 {
t.Errorf("expected failure: %v", errs)
}
}
func TestValidateProbe(t *testing.T) {