Fix image size #238
Fix image size #238
Conversation
Madhu-1
force-pushed
the
fix-image-size
branch
2 times, most recently
from
cff46bc
to
0678e1b
Compare
|
looks good, is it still DNM? |
|
@rootfs we need to make a decision on minimum volume size on cephfs #214 (comment) still i need to test this PR. |
|
sure, I'll let @gman0 make the final decision |
I'd go with option (2) because it's the closest to reality (in regards to ceph and csi spec) we can get with current state of external-provisioner. |
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
|
@gman0 reverted back the changes for cephfs PTAL tested for rbd and working fine |
pkg/cephfs/volume.go
Outdated
| if err := setVolumeAttribute(volRootCreating, "ceph.quota.max_bytes", fmt.Sprintf("%d", bytesQuota)); err != nil { | ||
| return err | ||
| } | ||
| if err := setVolumeAttribute(volRootCreating, "ceph.quota.max_bytes", fmt.Sprintf("%d", bytesQuota)); err != nil { |
There was a problem hiding this comment.
I'm not so sure about this. Both CephFS and the CSI spec allows zero-sized requirements - why forbid 0?
Another thing is cephfs doesn't really have "volume sizes" per-se, only quotas. So maybe any CapacityRange.RequiredBytes should be an error and CapacityRange.LimitBytes should be used instead? Don't know whether we can pass LimitBytes through storage class yet, though.
There was a problem hiding this comment.
yeah agreed, but if a user creates a PVC he sees the volume size mismatch in PVC describe and cephfs volume with current setup.
|
@gman0 Mergify is failing can we merge this one manually? |
Fix image size
Syncing latest changes from upstream devel for ceph-csi
Fixes: #214
@rootfs @gman0 PTAL, let me know it changes looks good, i will test it out.