api: introduce separate VolumeResourceRequirements struct

PVC and containers shared the same ResourceRequirements struct to define their
API. When resource claims were added, that struct got extended, which
accidentally also changed the PVC API. To avoid such a mistake from happening
again, PVC now uses its own VolumeResourceRequirements struct.

The `Claims` field gets removed because risk of breaking someone is low:
theoretically, YAML files which have a claims field for volumes now
get rejected when validating against the OpenAPI. Such files
have never made sense and should be fixed.

Code that uses the struct definitions needs to be updated.
This commit is contained in:
Patrick Ohly
2023-06-14 10:26:56 +02:00
parent bcbceea117
commit 2472291790
48 changed files with 192 additions and 202 deletions

View File

@@ -69,6 +69,13 @@ func getResourceRequirements(requests, limits api.ResourceList) api.ResourceRequ
return res
}
func getVolumeResourceRequirements(requests, limits api.ResourceList) api.VolumeResourceRequirements {
res := api.VolumeResourceRequirements{}
res.Requests = requests
res.Limits = limits
return res
}
// createLimitRange creates a limit range with the specified data
func createLimitRange(limitType api.LimitType, min, max, defaultLimit, defaultRequest, maxLimitRequestRatio api.ResourceList) corev1.LimitRange {
internalLimitRage := api.LimitRange{
@@ -816,7 +823,7 @@ func newHandlerForTest(c clientset.Interface) (*LimitRanger, informers.SharedInf
return handler, f, err
}
func validPersistentVolumeClaim(name string, resources api.ResourceRequirements) api.PersistentVolumeClaim {
func validPersistentVolumeClaim(name string, resources api.VolumeResourceRequirements) api.PersistentVolumeClaim {
pvc := api.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "test"},
Spec: api.PersistentVolumeClaimSpec{
@@ -834,19 +841,19 @@ func TestPersistentVolumeClaimLimitFunc(t *testing.T) {
successCases := []testCase{
{
pvc: validPersistentVolumeClaim("pvc-is-min-storage-request", getResourceRequirements(getStorageResourceList("1Gi"), getStorageResourceList(""))),
pvc: validPersistentVolumeClaim("pvc-is-min-storage-request", getVolumeResourceRequirements(getStorageResourceList("1Gi"), getStorageResourceList(""))),
limitRange: createLimitRange(api.LimitTypePersistentVolumeClaim, getStorageResourceList("1Gi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
},
{
pvc: validPersistentVolumeClaim("pvc-is-max-storage-request", getResourceRequirements(getStorageResourceList("1Gi"), getStorageResourceList(""))),
pvc: validPersistentVolumeClaim("pvc-is-max-storage-request", getVolumeResourceRequirements(getStorageResourceList("1Gi"), getStorageResourceList(""))),
limitRange: createLimitRange(api.LimitTypePersistentVolumeClaim, api.ResourceList{}, getStorageResourceList("1Gi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
},
{
pvc: validPersistentVolumeClaim("pvc-no-minmax-storage-request", getResourceRequirements(getStorageResourceList("100Gi"), getStorageResourceList(""))),
pvc: validPersistentVolumeClaim("pvc-no-minmax-storage-request", getVolumeResourceRequirements(getStorageResourceList("100Gi"), getStorageResourceList(""))),
limitRange: createLimitRange(api.LimitTypePersistentVolumeClaim, getStorageResourceList(""), getStorageResourceList(""), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
},
{
pvc: validPersistentVolumeClaim("pvc-within-minmax-storage-request", getResourceRequirements(getStorageResourceList("5Gi"), getStorageResourceList(""))),
pvc: validPersistentVolumeClaim("pvc-within-minmax-storage-request", getVolumeResourceRequirements(getStorageResourceList("5Gi"), getStorageResourceList(""))),
limitRange: createLimitRange(api.LimitTypePersistentVolumeClaim, getStorageResourceList("1Gi"), getStorageResourceList("10Gi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
},
}
@@ -860,11 +867,11 @@ func TestPersistentVolumeClaimLimitFunc(t *testing.T) {
errorCases := []testCase{
{
pvc: validPersistentVolumeClaim("pvc-below-min-storage-request", getResourceRequirements(getStorageResourceList("500Mi"), getStorageResourceList(""))),
pvc: validPersistentVolumeClaim("pvc-below-min-storage-request", getVolumeResourceRequirements(getStorageResourceList("500Mi"), getStorageResourceList(""))),
limitRange: createLimitRange(api.LimitTypePersistentVolumeClaim, getStorageResourceList("1Gi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
},
{
pvc: validPersistentVolumeClaim("pvc-exceeds-max-storage-request", getResourceRequirements(getStorageResourceList("100Gi"), getStorageResourceList(""))),
pvc: validPersistentVolumeClaim("pvc-exceeds-max-storage-request", getVolumeResourceRequirements(getStorageResourceList("100Gi"), getStorageResourceList(""))),
limitRange: createLimitRange(api.LimitTypePersistentVolumeClaim, getStorageResourceList("1Gi"), getStorageResourceList("1Gi"), api.ResourceList{}, api.ResourceList{}, api.ResourceList{}),
},
}

View File

@@ -1552,7 +1552,7 @@ func makeTestPVC(
pvc := &api.PersistentVolumeClaim{
Spec: api.PersistentVolumeClaimSpec{
VolumeName: "volume1",
Resources: api.ResourceRequirements{
Resources: api.VolumeResourceRequirements{
Requests: api.ResourceList{
api.ResourceStorage: resource.MustParse("10G"),
},

View File

@@ -62,6 +62,13 @@ func getResourceRequirements(requests, limits api.ResourceList) api.ResourceRequ
return res
}
func getVolumeResourceRequirements(requests, limits api.ResourceList) api.VolumeResourceRequirements {
res := api.VolumeResourceRequirements{}
res.Requests = requests
res.Limits = limits
return res
}
func validPod(name string, numContainers int, resources api.ResourceRequirements) *api.Pod {
pod := &api.Pod{
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "test"},
@@ -85,7 +92,7 @@ func validPodWithPriority(name string, numContainers int, resources api.Resource
return pod
}
func validPersistentVolumeClaim(name string, resources api.ResourceRequirements) *api.PersistentVolumeClaim {
func validPersistentVolumeClaim(name string, resources api.VolumeResourceRequirements) *api.PersistentVolumeClaim {
return &api.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "test"},
Spec: api.PersistentVolumeClaimSpec{
@@ -426,14 +433,14 @@ func TestAdmitHandlesNegativePVCUpdates(t *testing.T) {
oldPVC := &api.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{Name: "pvc-to-update", Namespace: "test", ResourceVersion: "1"},
Spec: api.PersistentVolumeClaimSpec{
Resources: getResourceRequirements(api.ResourceList{api.ResourceStorage: resource.MustParse("10Gi")}, api.ResourceList{}),
Resources: getVolumeResourceRequirements(api.ResourceList{api.ResourceStorage: resource.MustParse("10Gi")}, api.ResourceList{}),
},
}
newPVC := &api.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{Name: "pvc-to-update", Namespace: "test"},
Spec: api.PersistentVolumeClaimSpec{
Resources: getResourceRequirements(api.ResourceList{api.ResourceStorage: resource.MustParse("5Gi")}, api.ResourceList{}),
Resources: getVolumeResourceRequirements(api.ResourceList{api.ResourceStorage: resource.MustParse("5Gi")}, api.ResourceList{}),
},
}
@@ -478,14 +485,14 @@ func TestAdmitHandlesPVCUpdates(t *testing.T) {
oldPVC := &api.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{Name: "pvc-to-update", Namespace: "test", ResourceVersion: "1"},
Spec: api.PersistentVolumeClaimSpec{
Resources: getResourceRequirements(api.ResourceList{api.ResourceStorage: resource.MustParse("10Gi")}, api.ResourceList{}),
Resources: getVolumeResourceRequirements(api.ResourceList{api.ResourceStorage: resource.MustParse("10Gi")}, api.ResourceList{}),
},
}
newPVC := &api.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{Name: "pvc-to-update", Namespace: "test"},
Spec: api.PersistentVolumeClaimSpec{
Resources: getResourceRequirements(api.ResourceList{api.ResourceStorage: resource.MustParse("15Gi")}, api.ResourceList{}),
Resources: getVolumeResourceRequirements(api.ResourceList{api.ResourceStorage: resource.MustParse("15Gi")}, api.ResourceList{}),
},
}
@@ -1069,14 +1076,14 @@ func TestAdmitRejectsNegativeUsage(t *testing.T) {
informerFactory.Core().V1().ResourceQuotas().Informer().GetIndexer().Add(resourceQuota)
// verify quota rejects negative pvc storage requests
newPvc := validPersistentVolumeClaim("not-allowed-pvc", getResourceRequirements(api.ResourceList{api.ResourceStorage: resource.MustParse("-1Gi")}, api.ResourceList{}))
newPvc := validPersistentVolumeClaim("not-allowed-pvc", getVolumeResourceRequirements(api.ResourceList{api.ResourceStorage: resource.MustParse("-1Gi")}, api.ResourceList{}))
err = handler.Validate(context.TODO(), admission.NewAttributesRecord(newPvc, nil, api.Kind("PersistentVolumeClaim").WithVersion("version"), newPvc.Namespace, newPvc.Name, corev1.Resource("persistentvolumeclaims").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, nil), nil)
if err == nil {
t.Errorf("Expected an error because the pvc has negative storage usage")
}
// verify quota accepts non-negative pvc storage requests
newPvc = validPersistentVolumeClaim("not-allowed-pvc", getResourceRequirements(api.ResourceList{api.ResourceStorage: resource.MustParse("1Gi")}, api.ResourceList{}))
newPvc = validPersistentVolumeClaim("not-allowed-pvc", getVolumeResourceRequirements(api.ResourceList{api.ResourceStorage: resource.MustParse("1Gi")}, api.ResourceList{}))
err = handler.Validate(context.TODO(), admission.NewAttributesRecord(newPvc, nil, api.Kind("PersistentVolumeClaim").WithVersion("version"), newPvc.Namespace, newPvc.Name, corev1.Resource("persistentvolumeclaims").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, nil), nil)
if err != nil {
t.Errorf("Unexpected error: %v", err)

View File

@@ -88,7 +88,7 @@ func TestPVCResizeAdmission(t *testing.T) {
oldObj: &api.PersistentVolumeClaim{
Spec: api.PersistentVolumeClaimSpec{
VolumeName: "volume1",
Resources: api.ResourceRequirements{
Resources: api.VolumeResourceRequirements{
Requests: getResourceList("1Gi"),
},
StorageClassName: &goldClassName,
@@ -101,7 +101,7 @@ func TestPVCResizeAdmission(t *testing.T) {
newObj: &api.PersistentVolumeClaim{
Spec: api.PersistentVolumeClaimSpec{
VolumeName: "volume1",
Resources: api.ResourceRequirements{
Resources: api.VolumeResourceRequirements{
Requests: getResourceList("2Gi"),
},
StorageClassName: &goldClassName,
@@ -119,7 +119,7 @@ func TestPVCResizeAdmission(t *testing.T) {
oldObj: &api.PersistentVolumeClaim{
Spec: api.PersistentVolumeClaimSpec{
VolumeName: "volume3",
Resources: api.ResourceRequirements{
Resources: api.VolumeResourceRequirements{
Requests: getResourceList("1Gi"),
},
},
@@ -131,7 +131,7 @@ func TestPVCResizeAdmission(t *testing.T) {
newObj: &api.PersistentVolumeClaim{
Spec: api.PersistentVolumeClaimSpec{
VolumeName: "volume3",
Resources: api.ResourceRequirements{
Resources: api.VolumeResourceRequirements{
Requests: getResourceList("2Gi"),
},
},
@@ -148,7 +148,7 @@ func TestPVCResizeAdmission(t *testing.T) {
oldObj: &api.PersistentVolumeClaim{
Spec: api.PersistentVolumeClaimSpec{
VolumeName: "volume4",
Resources: api.ResourceRequirements{
Resources: api.VolumeResourceRequirements{
Requests: getResourceList("1Gi"),
},
StorageClassName: &silverClassName,
@@ -161,7 +161,7 @@ func TestPVCResizeAdmission(t *testing.T) {
newObj: &api.PersistentVolumeClaim{
Spec: api.PersistentVolumeClaimSpec{
VolumeName: "volume4",
Resources: api.ResourceRequirements{
Resources: api.VolumeResourceRequirements{
Requests: getResourceList("2Gi"),
},
StorageClassName: &silverClassName,
@@ -178,7 +178,7 @@ func TestPVCResizeAdmission(t *testing.T) {
resource: api.SchemeGroupVersion.WithResource("persistentvolumeclaims"),
oldObj: &api.PersistentVolumeClaim{
Spec: api.PersistentVolumeClaimSpec{
Resources: api.ResourceRequirements{
Resources: api.VolumeResourceRequirements{
Requests: getResourceList("1Gi"),
},
StorageClassName: &silverClassName,
@@ -191,7 +191,7 @@ func TestPVCResizeAdmission(t *testing.T) {
newObj: &api.PersistentVolumeClaim{
Spec: api.PersistentVolumeClaimSpec{
VolumeName: "volume4",
Resources: api.ResourceRequirements{
Resources: api.VolumeResourceRequirements{
Requests: getResourceList("1Gi"),
},
StorageClassName: &silverClassName,
@@ -208,7 +208,7 @@ func TestPVCResizeAdmission(t *testing.T) {
resource: api.SchemeGroupVersion.WithResource("persistentvolumeclaims"),
oldObj: &api.PersistentVolumeClaim{
Spec: api.PersistentVolumeClaimSpec{
Resources: api.ResourceRequirements{
Resources: api.VolumeResourceRequirements{
Requests: getResourceList("1Gi"),
},
StorageClassName: &silverClassName,
@@ -220,7 +220,7 @@ func TestPVCResizeAdmission(t *testing.T) {
},
newObj: &api.PersistentVolumeClaim{
Spec: api.PersistentVolumeClaimSpec{
Resources: api.ResourceRequirements{
Resources: api.VolumeResourceRequirements{
Requests: getResourceList("2Gi"),
},
StorageClassName: &silverClassName,