Update unit tests and feature name

Update the unit tests to include checks for incorrect APIGroup type in
PVC DataSource and change the name of the feature gate to be more clear:
s/VolumeDataSource/VolumePVCDataSource/
This commit is contained in:
j-griffith 2019-04-22 16:22:19 -06:00
parent 123f1bac35
commit a66bb3c63d
5 changed files with 154 additions and 7 deletions

View File

@ -55,11 +55,15 @@ func dataSourceInUse(oldPVCSpec *core.PersistentVolumeClaimSpec) bool {
func dataSourceIsEnabled(pvcSpec *core.PersistentVolumeClaimSpec) bool {
if pvcSpec.DataSource != nil {
if pvcSpec.DataSource.Kind == "PersistentVolumeClaim" && utilfeature.DefaultFeatureGate.Enabled(features.VolumeDataSource) {
if pvcSpec.DataSource.Kind == "PersistentVolumeClaim" &&
*pvcSpec.DataSource.APIGroup == "" &&
utilfeature.DefaultFeatureGate.Enabled(features.VolumePVCDataSource) {
return true
}
if pvcSpec.DataSource.Kind == "VolumeSnapshot" && utilfeature.DefaultFeatureGate.Enabled(features.VolumeSnapshotDataSource) {
if pvcSpec.DataSource.Kind == "VolumeSnapshot" &&
*pvcSpec.DataSource.APIGroup == "snapshot.storage.k8s.io" &&
utilfeature.DefaultFeatureGate.Enabled(features.VolumeSnapshotDataSource) {
return true
}

View File

@ -118,7 +118,7 @@ func TestDropAlphaPVCVolumeMode(t *testing.T) {
}
}
func TestDropDisabledDataSource(t *testing.T) {
func TestDropDisabledSnapshotDataSource(t *testing.T) {
pvcWithoutDataSource := func() *core.PersistentVolumeClaim {
return &core.PersistentVolumeClaim{
Spec: core.PersistentVolumeClaimSpec{
@ -210,3 +210,139 @@ func TestDropDisabledDataSource(t *testing.T) {
}
}
}
func TestDropDisabledPVCDataSource(t *testing.T) {
pvcWithoutDataSource := func() *core.PersistentVolumeClaim {
return &core.PersistentVolumeClaim{
Spec: core.PersistentVolumeClaimSpec{
DataSource: nil,
},
}
}
apiGroup := ""
pvcWithDataSource := func() *core.PersistentVolumeClaim {
return &core.PersistentVolumeClaim{
Spec: core.PersistentVolumeClaimSpec{
DataSource: &core.TypedLocalObjectReference{
APIGroup: &apiGroup,
Kind: "PersistentVolumeClaim",
Name: "test_clone",
},
},
}
}
pvcInfo := []struct {
description string
hasDataSource bool
pvc func() *core.PersistentVolumeClaim
}{
{
description: "pvc without DataSource",
hasDataSource: false,
pvc: pvcWithoutDataSource,
},
{
description: "pvc with DataSource",
hasDataSource: true,
pvc: pvcWithDataSource,
},
{
description: "is nil",
hasDataSource: false,
pvc: func() *core.PersistentVolumeClaim { return nil },
},
}
for _, enabled := range []bool{true, false} {
for _, oldpvcInfo := range pvcInfo {
for _, newpvcInfo := range pvcInfo {
oldPvcHasDataSource, oldpvc := oldpvcInfo.hasDataSource, oldpvcInfo.pvc()
newPvcHasDataSource, newpvc := newpvcInfo.hasDataSource, 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 utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumePVCDataSource, 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 || oldPvcHasDataSource:
// new pvc should not be changed if the feature is enabled, or if the old pvc had DataSource
if !reflect.DeepEqual(newpvc, newpvcInfo.pvc()) {
t.Errorf("new pvc changed: %v", diff.ObjectReflectDiff(newpvc, newpvcInfo.pvc()))
}
case newPvcHasDataSource:
// new pvc should be changed
if reflect.DeepEqual(newpvc, newpvcInfo.pvc()) {
t.Errorf("new pvc was not changed")
}
// new pvc should not have DataSource
if !reflect.DeepEqual(newpvc, pvcWithoutDataSource()) {
t.Errorf("new pvc had DataSource: %v", diff.ObjectReflectDiff(newpvc, pvcWithoutDataSource()))
}
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()))
}
}
})
}
}
}
}
// TestDropInvalidPVCDataSource checks specifically for invalid APIGroup settings for PVCDataSource
func TestDropInvalidPVCDataSource(t *testing.T) {
apiGroup := ""
pvcWithDataSource := func() *core.PersistentVolumeClaim {
return &core.PersistentVolumeClaim{
Spec: core.PersistentVolumeClaimSpec{
DataSource: &core.TypedLocalObjectReference{
APIGroup: &apiGroup,
Kind: "PersistentVolumeClaim",
Name: "test_clone",
},
},
}
}
invalidAPIGroup := "invalid.pvc.api.group"
pvcWithInvalidDataSource := func() *core.PersistentVolumeClaim {
return &core.PersistentVolumeClaim{
Spec: core.PersistentVolumeClaimSpec{
DataSource: &core.TypedLocalObjectReference{
APIGroup: &invalidAPIGroup,
Kind: "PersistentVolumeClaim",
Name: "test_clone_invalid",
},
},
}
}
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumePVCDataSource, true)()
validPvc := pvcWithDataSource()
originalDS := validPvc.Spec.DataSource
DropDisabledFields(&validPvc.Spec, nil)
if validPvc.Spec.DataSource == nil {
t.Errorf("valid PVC DataSource was dropped: Kind: %s, APIGroup: %s, Name: %s\n", originalDS.Kind, *originalDS.APIGroup, originalDS.Name)
}
invalidPvc := pvcWithInvalidDataSource()
originalDS = invalidPvc.Spec.DataSource
DropDisabledFields(&invalidPvc.Spec, nil)
if invalidPvc.Spec.DataSource != nil {
t.Errorf("invalid PVC DataSource was not dropped: Kind: %s, APIGroup: %s, Name: %s\n", originalDS.Kind, *originalDS.APIGroup, originalDS.Name)
}
}

View File

@ -414,10 +414,10 @@ type PersistentVolumeClaimSpec struct {
// +optional
VolumeMode *PersistentVolumeMode
// This field can be used to specify either:
// * An existing VolumeSnapshot
// * An existing Volume (PVC, Clone operation)
// * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot)
// * An existing PVC (""/PersistentVolumeClaim)
// In order to use either of these DataSource types, the appropriate feature gate
// must be enabled (VolumeSnapshotDataSource, VolumeDataSource)
// must be enabled (VolumeSnapshotDataSource, VolumePVCDataSource)
// If the provisioner can support the specified data source, it will create
// a new volume based on the contents of the specified PVC or Snapshot.
// If the provisioner does not support the specified data source, the volume will

View File

@ -13311,7 +13311,7 @@ func testDataSourceInSpec(name string, kind string, apiGroup string) *core.Persi
return &dataSourceInSpec
}
func TestAlphaVolumeDataSource(t *testing.T) {
func TestAlphaVolumePVCDataSource(t *testing.T) {
successTestCases := []core.PersistentVolumeClaimSpec{
*testDataSourceInSpec("test_snapshot", "VolumeSnapshot", "snapshot.storage.k8s.io"),
*testDataSourceInSpec("test_pvc", "PersistentVolumeClaim", ""),

View File

@ -465,6 +465,12 @@ const (
//
// Enables NonPreempting option for priorityClass and pod.
NonPreemptingPriority featuregate.Feature = "NonPreemptingPriority"
// owner: @j-griffith
// alpha: v1.15
//
// Enable support for specifying an existing PVC as a DataSource
VolumePVCDataSource featuregate.Feature = "VolumePVCDataSource"
)
func init() {
@ -543,6 +549,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
WindowsGMSA: {Default: false, PreRelease: featuregate.Alpha},
LocalStorageCapacityIsolationFSQuotaMonitoring: {Default: false, PreRelease: featuregate.Alpha},
NonPreemptingPriority: {Default: false, PreRelease: featuregate.Alpha},
VolumePVCDataSource: {Default: false, PreRelease: featuregate.Alpha},
// inherited features from generic apiserver, relisted here to get a conflict if it is changed
// unintentionally on either side: