api: drop Resources.Claims from PVC and PVC template

PVC and containers share the same ResourceRequirements struct. The Claims field
in it only makes sense when used in containers. When used in a PVC, the field
should have been rejected by validation. This was overlooked when introducing
it, so now persisted objects might have it set and/or people may have started
to rely on it being accepted even when it has no effect.

Therefore we cannot reject it in validation anymore, but we can still strip
it out on create or update.
This commit is contained in:
Patrick Ohly 2023-02-21 15:36:55 +01:00
parent 70f337c0d5
commit f32302e744
6 changed files with 60 additions and 2 deletions

View File

@ -49,6 +49,10 @@ func DropDisabledFields(pvcSpec, oldPVCSpec *core.PersistentVolumeClaimSpec) {
pvcSpec.DataSourceRef = nil
}
}
// Setting VolumeClaimTemplate.Resources.Claims should have been caught by validation when
// extending ResourceRequirements in 1.26. Now we can only accept it and drop the field.
pvcSpec.Resources.Claims = nil
}
// EnforceDataSourceBackwardsCompatibility drops the data source field under certain conditions

View File

@ -263,6 +263,9 @@ func TestDataSourceFilter(t *testing.T) {
anyEnabled: true,
wantRef: xnsVolumeDataSourceRef, // existing field isn't dropped.
},
"clear Resources.Claims": {
spec: core.PersistentVolumeClaimSpec{Resources: core.ResourceRequirements{Claims: []core.ResourceClaim{{Name: "dra"}}}},
},
}
for testName, test := range tests {
@ -278,6 +281,9 @@ func TestDataSourceFilter(t *testing.T) {
t.Errorf("expected condition was not met, test: %s, anyEnabled: %v, xnsEnabled: %v, spec: %+v, expected DataSourceRef: %+v",
testName, test.anyEnabled, test.xnsEnabled, test.spec, test.wantRef)
}
if test.spec.Resources.Claims != nil {
t.Errorf("expected Resources.Claims to be cleared")
}
})
}
}

View File

@ -577,6 +577,16 @@ func dropDisabledDynamicResourceAllocationFields(podSpec, oldPodSpec *api.PodSpe
dropEphemeralResourceClaimRequests(podSpec.EphemeralContainers)
podSpec.ResourceClaims = nil
}
// Setting VolumeClaimTemplate.Resources.Claims should have been
// treated as an error by validation when extending
// ResourceRequirements in 1.26. Now we can only accept it and drop the
// field.
for i := range podSpec.Volumes {
if podSpec.Volumes[i].Ephemeral != nil && podSpec.Volumes[i].Ephemeral.VolumeClaimTemplate != nil {
podSpec.Volumes[i].Ephemeral.VolumeClaimTemplate.Spec.Resources.Claims = nil
}
}
}
func dynamicResourceAllocationInUse(podSpec *api.PodSpec) bool {

View File

@ -2236,3 +2236,41 @@ func TestValidateTopologySpreadConstraintLabelSelectorOption(t *testing.T) {
})
}
}
func TestDropVolumesClaimField(t *testing.T) {
pod := &api.Pod{
Spec: api.PodSpec{
Volumes: []api.Volume{
{},
{
VolumeSource: api.VolumeSource{
Ephemeral: &api.EphemeralVolumeSource{},
},
},
{
VolumeSource: api.VolumeSource{
Ephemeral: &api.EphemeralVolumeSource{
VolumeClaimTemplate: &api.PersistentVolumeClaimTemplate{
Spec: api.PersistentVolumeClaimSpec{
Resources: api.ResourceRequirements{
Claims: []api.ResourceClaim{
{Name: "dra"},
},
},
},
},
},
},
},
},
},
}
DropDisabledPodFields(pod, nil)
for i, volume := range pod.Spec.Volumes {
if volume.Ephemeral != nil && volume.Ephemeral.VolumeClaimTemplate != nil && volume.Ephemeral.VolumeClaimTemplate.Spec.Resources.Claims != nil {
t.Errorf("volume #%d: Resources.Claim should be nil", i)
}
}
}

View File

@ -2190,7 +2190,7 @@ type ResourceRequirements struct {
// This is an alpha field and requires enabling the
// DynamicResourceAllocation feature gate.
//
// This field is immutable.
// This field is immutable. It can only be set for containers.
//
// +featureGate=DynamicResourceAllocation
// +optional

View File

@ -2319,7 +2319,7 @@ type ResourceRequirements struct {
// This is an alpha field and requires enabling the
// DynamicResourceAllocation feature gate.
//
// This field is immutable.
// This field is immutable. It can only be set for containers.
//
// +listType=map
// +listMapKey=name