Description of problem: When expanding PVC to a very large size, it will hit overflow issue. Version-Release number of selected component (if applicable): oc v3.10.1 openshift v3.10.1 kubernetes v1.10.0+b81c8f8 How reproducible: Always Steps to Reproduce: 1. Enable feature gate: ExpandPersistentVolumes 2. Enable addmission controller: PersistentVolumeClaimResize 3. Create a storageclass with allowVolumeExpansion: true 4. Create a PVC using the storageclass. 5. Expand PVC to different very large size Actual results: 1. expand PVC to 100000000000Gi, and get event: Error updating PV spec capacity for volume "tfkb4/pvc-tfkb4" with : PersistentVolume "pvc-58cc18b7-6e22-11e8-b900-fa163e62eb95" is invalid: spec.capacity[storage]: Invalid value: "-8589934591Gi": must be greater than zero 2. expand PVC to 18451247673336922111, and get event: Error expanding volume "piqin/pvc-1" of plugin kubernetes.io/cinder : Expected HTTP response code [202] when accessing [POST https://****.redhat.com:13776/v3/e0fa85b6a06443959d2d3b497174bed6/volumes/f68f3fee-9f5b-45e4-9a96-64faeb807175/action], but got 413 instead {"overLimit": {"message": "VolumeSizeExceedsAvailableQuota: Requested volume or snapshot exceeds allowed gigabytes quota. Requested 4194303G, quota is 10000G and 973G has been consumed.", "code": 413, "retryAfter": "0"}} Expected results: Master Log: Node Log (of failed PODs): PV Dump: PVC Dump: StorageClass Dump (if StorageClass used by PV/PVC): Additional info:
FYI some other values result in positive (incorrect) integers. As well some value seem to update the API object with the bad value, in some cases update to the API object is not performed. I think we need proper validation of all numeric values supplied to the API. It is not clear to me which layer can such a value reach (maybe dependent on underlying storage provider?) and what troubles it could possibly produce there.
This is a known problem with documented workaround - https://github.com/openshift/openshift-docs/pull/8990/ Aleksandar - I am not entirely clear what you mean. Can you provide some examples?
In linked PR I only see information how to recover a PVC. This issue is about > 1. expand PVC to 100000000000Gi, and get event: Error updating PV spec capacity for volume "tfkb4/pvc-tfkb4" with : PersistentVolume "pvc-58cc18b7-6e22-11e8-b900-fa163e62eb95" is invalid: spec.capacity[storage]: Invalid value: "-8589934591Gi": must be greater than zero Apparently there is an integer overflow somewhere. The obvious issue is misleading error message returned to user. But there is more. Providing different huge numeric values results in different kinds of overflows. Sometimes the number is positive. It can also be a small positive number. What I observed is that sometimes the patch command could succeed but no change made to volume. Sometimes it can succeed with bogus change made to volume. The above can confuse user scripting if no indication a problem occurred is returned. Lastly I have no idea where in the system the bogus value reaches. Maybe one storage backend will return error, another one - not. I don't know if this is the case but it would be much safer if we validate parse-ability of the value early during the API call instead of hoping the lower layers will cope with the issue properly. In bug description I don't see `Expected result` field. For me expected result would be, when integer overflow occurs while parsing a value, then operation should be rejected, API object *not* updated and proper error message returned to user.
> Apparently there is an integer overflow somewhere. The obvious issue is misleading error message returned to user. That integer overflow thing does look like something that should have been caught by validation and rejected outright. I will double check and try to see what is going on. I am just curious - what happens when you try to provision such a PVC? Is that accepted by the API and then provisioning fails? I would think resizing behaviour when it comes to integer overflow should be no different than provisioning and anything you notice should be fixed as a bug.
> That integer overflow thing does look like something that should have been caught by validation and rejected outright. Thank you, exactly my thought. > what happens when you try to provision such a PVC? Never tried. But it is a good suggestion. Qin Ping, not sure I will be able to try this tomorrow. If you have some free cycles will appreciate you doing so.
# cat pvc.yaml apiVersion: v1 kind: PersistentVolumeClaim metadata: name: pvc-1 spec: accessModes: - ReadWriteOnce resources: requests: storage: 100000000000Gi # oc get pvc pvc-1 -o yaml apiVersion: v1 kind: PersistentVolumeClaim metadata: annotations: volume.beta.kubernetes.io/storage-provisioner: kubernetes.io/cinder creationTimestamp: 2018-06-22T04:15:40Z finalizers: - kubernetes.io/pvc-protection name: pvc-1 namespace: default resourceVersion: "4589" selfLink: /api/v1/namespaces/default/persistentvolumeclaims/pvc-1 uid: ec9d7903-75d2-11e8-825f-fa163e95a35b spec: accessModes: - ReadWriteOnce resources: requests: storage: "9223372036854775807" storageClassName: standard status: phase: Pending # oc describe pvc pvc-1 Name: pvc-1 Namespace: default StorageClass: standard Status: Pending Volume: Labels: <none> Annotations: volume.beta.kubernetes.io/storage-provisioner=kubernetes.io/cinder Finalizers: [kubernetes.io/pvc-protection] Capacity: Access Modes: Events: Type Reason Age From Message ---- ------ ---- ---- ------- Warning ProvisioningFailed 9s (x7 over 1m) persistentvolume-controller Failed to provision volume with StorageClass "standard": failed to create a -8589934591 GB volume: Invalid request due to incorrect syntax or missing required parameters.
The weirdness observed is due to kubernetes canonicalizing all resource quantities before writing to the API object. e.g. if I create a PVC with size 10E it will be 10E when I `oc get pvc -o yaml` but if I create one with size 10Ei it will be set back down to 2^63-1 (both are > 2^63-1). Integer overflow is nontrivial to detect and implement because of this canonicalization, so I am not convinced it's worth anybody's time to validate for nonsensical values, I would need to see a real example of user script that needs to set a resource quantity beyond 2^63-1, and even then I don't think it is a problem solvable by Storage because resource quantity fields are found throughout kubernetes e.g. compute/memory limits. So I think this problem is cinder specific. If I look at GCE it checks the size is nonnegative before calling the API. Meanwhile, Cinder naively converts the resource quantity into an int of GiB then calls the API (which does not even return a success or failure, explaining why we see "1. expand PVC to 100000000000Gi, and get event: Error updating PV spec capacity for volume "tfkb4/pvc-tfkb4" with : PersistentVolume "pvc-58cc18b7-6e22-11e8-b900-fa163e62eb95" is invalid: spec.capacity[storage]: Invalid value: "-8589934591Gi": must be greater than zero")
Can we modify Kubernetes to return some kind of an error when it hits an overflow converting a value? I'm not concerned about users needing to supply crazy values to the api. My main concern is security of the cluster and any possibility for resource allocation inconsistencies or DoS if values in different places of the system do not match. If we can verify that providing bogus values to the api can't *possibly* do any harm, then fine. But my gut feeling is that this would be much easier if we fail at the first occurrence of invalid values.
I'd like to stress that the overflow does not always result in a negative value. Thus we can't rely on this as a fail safe. Before Qin Ping filed the issue I played with different values and found some values that exhibited other strange behaviors when the integer was not overflowing to a negative value. I don't have the specific values now. IIRC I generated them from binary numbers like "1000...0001000...000" then converted to long decimal number string (no suffix). In case you want to replicate non-negative overflow.
I've opened an upstream PR to fix the internal integer overflow so that cinder will not be passed any int-overflowed value, positive or negative, and an error is returned instead. https://github.com/kubernetes/kubernetes/pull/66464/files#diff-c6603a39b8f63223075a6ac640bd6ef9R415 . I also tried to fix all other plugins. > Can we modify Kubernetes to return some kind of an error when it hits an overflow converting a value? If you mean at object creation time, the earliest: we don't need to worry about int64 value overflow then because as I found, kubernetes will automatically canonicalize them & set them down to int64-max=2^63-1. This is why we can't seem to reproduce the negative overflow. I think this is acceptable and returning an error is unnecessary. And even if we do return an error when we know the value will overflow an int64, that doesn't help if the code that eventually consumes the value is only equipped to deal with int32's, e.g. the cinder code here. The next earliest place to detect overflow is at the volume plugins, some of them can cope with int64s and some not.
I couldn't understand very well what will be the expected behavior after the fixes. > This is why we can't seem to reproduce the negative overflow. Did you mean `non-negative`? I found some of the previous values I tested with. > spec: > accessModes: > - ReadWriteOnce > resources: > requests: > storage: "####################" According to my record: * Using value "18451247673336922111" resulted in: > Error expanding volume "flsqf/pvc-flsqf" of plugin kubernetes.io/cinder : Expected HTTP response code [202] when accessing [POST https://ci-rhos.centralci.eng.rdu2.redhat.com:13776/v3/e0fa85b6a06443959d2d3b497174bed6/volumes/2fab955a-b06b-47ec-9cd7-f996bf0d43dd/action], but got 413 instead {"overLimit": {"message": "VolumeSizeExceedsAvailableQuota: Requested volume or snapshot exceeds allowed gigabytes quota. Requested 4194303G, quota is 10000G and 1188G has been consumed.", "code": 413, "retryAfter": "0"}} * Using value "36893488149566586880" resulted in no change to PVC somehow without error message. * Using value "73786976299133173760" resulted in API object being set to "36893488150640330k" So question is, in the examples above, what behavior should user expect to see after the changes?
@Aleksander, what version of Openshift are you using? When I try the latter two cases on 3.11/master, I get: "36893488149566586880" -> "36893488149566590k" "73786976299133173760" -> "73786976299133170k" which I think are okay. That last case "* Using value "73786976299133173760" resulted in API object being set to "36893488150640330k"" is certainly suspect... In the first case, after the changes, the user will receive an error like "capacity %v is too great, casting results in integer overflow" before any HTTP request is made. https://github.com/openshift/origin/pull/20418 has merged, the new expected behaviour is that users should receive an error like "capacity %v is too great, casting results in integer overflow" in any case the value would be so great that it overflows an int32.
Matthew, it was with the version reported by Qin Ping in description. New behavior sounds great - returning error to user on integer overflow. Thank you!
Verify failed in OCP: oc v3.11.0-0.22.0 openshift v3.11.0-0.22.0 kubernetes v1.11.0+d4cacc0 Expand PVC size from 1Gi to 73786976299133173760 Report expand PVC successfully, but PV size is 4Gi $ oc describe pvc Name: pvc-2 Namespace: mytest StorageClass: gp2 Status: Bound Volume: pvc-bc6651dc-a9b8-11e8-bdcb-0ec5086dbad4 Labels: <none> Annotations: pv.kubernetes.io/bind-completed=yes pv.kubernetes.io/bound-by-controller=yes volume.beta.kubernetes.io/storage-provisioner=kubernetes.io/aws-ebs Finalizers: [kubernetes.io/pvc-protection] Capacity: 1Gi Access Modes: RWO Conditions: Type Status LastProbeTime LastTransitionTime Reason Message ---- ------ ----------------- ------------------ ------ ------- FileSystemResizePending True Mon, 01 Jan 0001 00:00:00 +0000 Mon, 27 Aug 2018 13:28:12 +0800 Waiting for user to (re-)start a pod to finish file system resize of volume on node. Events: Type Reason Age From Message ---- ------ ---- ---- ------- Normal ProvisioningSucceeded 9m persistentvolume-controller Successfully provisioned volume pvc-bc6651dc-a9b8-11e8-bdcb-0ec5086dbad4 using kubernetes.io/aws-ebs $ oc get pvc -oyaml apiVersion: v1 items: - apiVersion: v1 kind: PersistentVolumeClaim metadata: annotations: pv.kubernetes.io/bind-completed: "yes" pv.kubernetes.io/bound-by-controller: "yes" volume.beta.kubernetes.io/storage-provisioner: kubernetes.io/aws-ebs creationTimestamp: 2018-08-27T05:19:13Z finalizers: - kubernetes.io/pvc-protection name: pvc-2 namespace: mytest resourceVersion: "12057" selfLink: /api/v1/namespaces/mytest/persistentvolumeclaims/pvc-2 uid: bc6651dc-a9b8-11e8-bdcb-0ec5086dbad4 spec: accessModes: - ReadWriteOnce resources: requests: storage: 73786976299133170k storageClassName: gp2 volumeName: pvc-bc6651dc-a9b8-11e8-bdcb-0ec5086dbad4 status: accessModes: - ReadWriteOnce capacity: storage: 1Gi conditions: - lastProbeTime: null lastTransitionTime: 2018-08-27T05:30:48Z message: Waiting for user to (re-)start a pod to finish file system resize of volume on node. status: "True" type: FileSystemResizePending phase: Bound kind: List metadata: resourceVersion: "" selfLink: "" $ oc get pv --config=../../admin.kubeconfig NAME CAPACITY ACCESS MODES RECLAIM POLICY STATUS CLAIM STORAGECLASS REASON AGE pvc-bc6651dc-a9b8-11e8-bdcb-0ec5086dbad4 4Gi RWO Retain Bound mytest/pvc-2 gp2 10m
PR #2 opened upstream: https://github.com/kubernetes/kubernetes/pull/67969 Missed many cases that were in fact detailed in the comments here, my bad, plus there are more volume plugins e.g. AWS, azure, that were missed
Although this BZ affects no real customers as far as I know but this is more like a correctness issue of storage system and something I will fix in next sprint.
Expand pvc from 1Gi to 100000000000Gi Warning VolumeResizeFailed 10s volume_expand error expanding volume "aws-ebs-csi-driver-operator/pvc2" of plugin "kubernetes.io/aws-ebs": AWS modifyVolume failed for vol-021dc2e4402421913 with InvalidParameterValue: Invalid integer value 8589934592 Expand pvc to 18451247673336922111 Warning VolumeResizeFailed 1s (x2 over 3s) volume_expand (combined from similar events): error expanding volume "aws-ebs-csi-driver-operator/pvc3" of plugin "kubernetes.io/aws-ebs": AWS modifyVolume failed for vol-0cfa9a027fe3f869d with InvalidParameterValue: Volume of 4194304GiB is too large for volume type gp2; maximum is 16384GiB But If Expand PVC size from 1Gi to 73786976299133173760k, PVC capacity resize is successful and expand to 4Gi pvc1 Bound pvc-d4850a01-e565-467d-9c4d-6e858dc726db 4Gi RWO gp2 23m Tested on 4.5.0-0.nightly-2020-05-17-220731
I tried this on GCP and I think my fix has not landed in build that was tested. I am trying to build nightly on AWS to test and will report once I have verified it.
Verification is passed 4.5.0-0.nightly-2020-05-25-052746 When try to resize pvc to 73786976299133173760k Warning VolumeResizeFailed 17s volume_expand error expanding volume "chaoyang/pvc1" of plugin "kubernetes.io/aws-ebs": quantity {{0 0} {0xc005853aa0} DecimalSI} is too great, overflows int64 Warning VolumeResizeFailed 8s (x3 over 16s) volume_expand (combined from similar events): error expanding volume "chaoyang/pvc1" of plugin "kubernetes.io/aws-ebs": quantity {{0 0} {0xc00c385590} DecimalSI} is too great, overflows int64
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHBA-2020:2409