Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refact(size): conversion G to Gi #59

Merged
merged 1 commit into from
Dec 6, 2019

Conversation

prateekpandey14
Copy link
Contributor

@prateekpandey14 prateekpandey14 commented Nov 27, 2019

fixes the G to bytes to Gi conversion while trasitioning from PVC to CVC storage capacity the way kubernetes handles, by converting to Gi form.
here is the example link for the same https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/cloud-provider/volume/helpers/rounding.go

Note:
RoundUpSize calculates how many allocation units are needed to accommodate a volume of given size. E.g. when user wants 1500MiB volume, while AWS EBS,GKE etc allocates volumes in gibibyte-sized chunks.
RoundUpSize(1500 * 10241024, 10241024*1024) returns '2'
(2 GiB is the smallest allocatable volume that can hold 1500MiB)

  1. PVC yaml
kind: PersistentVolumeClaim
apiVersion: v1
metadata:
  name: claim-csi-123
spec:
  storageClassName: cstor-sparse-auto
  accessModes:
    - ReadWriteOnce
  resources:
    requests:
      storage: 50G
$ kubectl get pvc
NAME            STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS        AGE
claim-csi-123   Bound    pvc-6ba235b5-2189-4eca-9300-e830c30fc23c   47Gi       RWO            cstor-sparse-auto   30m

$ kubectl get cstorvolume -n openebs
NAME                                       STATUS    AGE   CAPACITY
pvc-6ba235b5-2189-4eca-9300-e830c30fc23c   Healthy   30m   47Gi

Signed-off-by: prateekpandey14 [email protected]

Signed-off-by: prateekpandey14 <[email protected]>
@codecov-io
Copy link

Codecov Report

Merging #59 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #59   +/-   ##
=======================================
  Coverage   53.72%   53.72%           
=======================================
  Files          12       12           
  Lines         631      631           
=======================================
  Hits          339      339           
  Misses        272      272           
  Partials       20       20

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 710be25...cb156fc. Read the comment docs.

@mittachaitu
Copy link

@prateekpandey14 for the requested 1G we are giving 9317314 bytes extra is that correct way to do?

Copy link

@mittachaitu mittachaitu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes are good

Copy link

@vishnuitta vishnuitta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this PR seems to be making size always in GiB.. but, size can be in other units as well like TiB
Need testcases as well

Copy link

@vishnuitta vishnuitta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed that this is how k8s as well is doing same. Hence, approving.
Please add testcases at earliest

@vishnuitta vishnuitta merged commit ca00d4a into openebs-archive:master Dec 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants