Merge pull request #72739 from rajathagasthya/expandpv-72651

Remove ExpandPersistentVolumes feature gate from validation
pull/564/head
Kubernetes Prow Robot 2019-01-10 15:53:47 -08:00 committed by GitHub
commit 94806e6a9a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 96 additions and 12 deletions

View File

@ -1930,10 +1930,6 @@ func ValidatePersistentVolumeClaimStatusUpdate(newPvc, oldPvc *core.PersistentVo
if len(newPvc.Spec.AccessModes) == 0 {
allErrs = append(allErrs, field.Required(field.NewPath("Spec", "accessModes"), ""))
}
if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) && len(newPvc.Status.Conditions) > 0 {
conditionPath := field.NewPath("status", "conditions")
allErrs = append(allErrs, field.Forbidden(conditionPath, "invalid field"))
}
capPath := field.NewPath("status", "capacity")
for r, qty := range newPvc.Status.Capacity {
allErrs = append(allErrs, validateBasicResource(qty, capPath.Key(string(r)))...)

View File

@ -11274,12 +11274,6 @@ func TestValidatePersistentVolumeClaimStatusUpdate(t *testing.T) {
newClaim *core.PersistentVolumeClaim
enableResize bool
}{
"condition-update-with-disabled-feature-gate": {
isExpectedFailure: true,
oldClaim: validClaim,
newClaim: validConditionUpdate,
enableResize: false,
},
"condition-update-with-enabled-feature-gate": {
isExpectedFailure: false,
oldClaim: validClaim,
@ -11290,8 +11284,6 @@ func TestValidatePersistentVolumeClaimStatusUpdate(t *testing.T) {
for name, scenario := range scenarios {
t.Run(name, func(t *testing.T) {
// ensure we have a resource version specified for updates
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, scenario.enableResize)()
scenario.oldClaim.ResourceVersion = "1"
scenario.newClaim.ResourceVersion = "1"
errs := ValidatePersistentVolumeClaimStatusUpdate(scenario.newClaim, scenario.oldClaim)

View File

@ -18,6 +18,7 @@ go_library(
"//pkg/api/persistentvolumeclaim:go_default_library",
"//pkg/apis/core:go_default_library",
"//pkg/apis/core/validation:go_default_library",
"//pkg/features:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
@ -25,6 +26,7 @@ go_library(
"//staging/src/k8s.io/apiserver/pkg/registry/generic:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/storage:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/storage/names:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
],
)
@ -36,6 +38,11 @@ go_test(
"//pkg/api/testapi:go_default_library",
"//pkg/api/testing:go_default_library",
"//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/endpoints/request: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",
],
)

View File

@ -27,10 +27,12 @@ import (
"k8s.io/apiserver/pkg/registry/generic"
"k8s.io/apiserver/pkg/storage"
"k8s.io/apiserver/pkg/storage/names"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/api/legacyscheme"
pvcutil "k8s.io/kubernetes/pkg/api/persistentvolumeclaim"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/apis/core/validation"
"k8s.io/kubernetes/pkg/features"
)
// persistentvolumeclaimStrategy implements behavior for PersistentVolumeClaim objects
@ -97,6 +99,9 @@ func (persistentvolumeclaimStatusStrategy) PrepareForUpdate(ctx context.Context,
newPv := obj.(*api.PersistentVolumeClaim)
oldPv := old.(*api.PersistentVolumeClaim)
newPv.Spec = oldPv.Spec
if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) && oldPv.Status.Conditions == nil {
newPv.Status.Conditions = nil
}
}
func (persistentvolumeclaimStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {

View File

@ -17,10 +17,17 @@ limitations under the License.
package persistentvolumeclaim
import (
"fmt"
"reflect"
"testing"
"k8s.io/apimachinery/pkg/util/diff"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
utilfeature "k8s.io/apiserver/pkg/util/feature"
utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing"
apitesting "k8s.io/kubernetes/pkg/api/testing"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/features"
// install all api groups for testing
_ "k8s.io/kubernetes/pkg/api/testapi"
@ -34,3 +41,80 @@ func TestSelectableFieldLabelConversions(t *testing.T) {
map[string]string{"name": "metadata.name"},
)
}
func TestDropConditions(t *testing.T) {
ctx := genericapirequest.NewDefaultContext()
pvcWithConditions := func() *api.PersistentVolumeClaim {
return &api.PersistentVolumeClaim{
Status: api.PersistentVolumeClaimStatus{
Conditions: []api.PersistentVolumeClaimCondition{
{Type: api.PersistentVolumeClaimResizing, Status: api.ConditionTrue},
},
},
}
}
pvcWithoutConditions := func() *api.PersistentVolumeClaim {
return &api.PersistentVolumeClaim{
Status: api.PersistentVolumeClaimStatus{},
}
}
pvcInfo := []struct {
description string
hasConditions bool
pvc func() *api.PersistentVolumeClaim
}{
{
description: "has Conditions",
hasConditions: true,
pvc: pvcWithConditions,
},
{
description: "does not have Conditions",
hasConditions: false,
pvc: pvcWithoutConditions,
},
}
for _, enabled := range []bool{true, false} {
for _, oldPvcInfo := range pvcInfo {
for _, newPvcInfo := range pvcInfo {
oldPvcHasConditins, oldPvc := oldPvcInfo.hasConditions, oldPvcInfo.pvc()
newPvcHasConditions, newPvc := newPvcInfo.hasConditions, newPvcInfo.pvc()
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.ExpandPersistentVolumes, enabled)()
StatusStrategy.PrepareForUpdate(ctx, newPvc, oldPvc)
// 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 || oldPvcHasConditins:
// new pvc should not be changed if the feature is enabled, or if the old pvc had Conditions
if !reflect.DeepEqual(newPvc, newPvcInfo.pvc()) {
t.Errorf("new pvc changed: %v", diff.ObjectReflectDiff(newPvc, newPvcInfo.pvc()))
}
case newPvcHasConditions:
// new pvc should be changed
if reflect.DeepEqual(newPvc, newPvcInfo.pvc()) {
t.Errorf("new pvc was not changed")
}
// new pvc should not have Conditions
if !reflect.DeepEqual(newPvc, pvcWithoutConditions()) {
t.Errorf("new pvc had Conditions: %v", diff.ObjectReflectDiff(newPvc, pvcWithoutConditions()))
}
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()))
}
}
})
}
}
}
}