Merge pull request #3409 from noahdesu/pvc-binding-size-suffix

ceph: adjust M-to-Mi rounding in PV creation

pkg/daemon/ceph/client/image.go

@@ -25,6 +25,7 @@ import (
 	"regexp"
 
 	"github.com/rook/rook/pkg/clusterd"
+	"github.com/rook/rook/pkg/util/display"
 	"github.com/rook/rook/pkg/util/exec"
 )
 
@@ -68,6 +69,9 @@ func ListImages(context *clusterd.Context, clusterName, poolName string) ([]Ceph
 
 // CreateImage creates a block storage image.
 // If dataPoolName is not empty, the image will use poolName as the metadata pool and the dataPoolname for data.
+// If size is zero an empty image will be created. Otherwise, an image will be
+// created with a size rounded up to the nearest Mi. The adjusted image size is
+// placed in return value CephBlockImage.Size.
 func CreateImage(context *clusterd.Context, clusterName, name, poolName, dataPoolName string, size uint64) (*CephBlockImage, error) {
 	if size > 0 && size < ImageMinSize {
 		// rbd tool uses MB as the smallest unit for size input.  0 is OK but anything else smaller
@@ -100,7 +104,15 @@ func CreateImage(context *clusterd.Context, clusterName, name, poolName, dataPoo
 		}
 	}
 
-	return &CephBlockImage{Name: name, Size: size}, nil
+	// report the adjusted size which will always be >= to the requested size
+	var newSizeBytes uint64
+	if sizeMB > 0 {
+		newSizeBytes = display.MbTob(uint64(sizeMB))
+	} else {
+		newSizeBytes = 0
+	}
+
+	return &CephBlockImage{Name: name, Size: newSizeBytes}, nil
 }
 
 func DeleteImage(context *clusterd.Context, clusterName, name, poolName string) error {

pkg/operator/ceph/provisioner/provisioner.go

@@ -100,8 +100,20 @@ func (p *RookVolumeProvisioner) Provision(options controller.VolumeOptions) (*v1
 		return nil, err
 	}
 
-	// since we can guarantee the size of the volume image generated have to be in `MB` boundary, so we can
-	// convert it to `MB` unit safely here
+	// the size of the PV needs to be at least as large as the size in the PVC
+	// or binding won't be successful. createVolume uses the requestBytes
+	// parameter as a target, and guarantees that the size created as at least
+	// that large. the adjusted value is placed in blockImage.Size and it is
+	// suitable to be converted into Mi.
+	//
+	// note that the rounding error that can occur if the original non-adjusted
+	// request is used in the original formulation here:
+	//
+	//    s := fmt.Sprintf("%dMi", blockImage.Size/sizeMB)
+	//    Size = 500M = 500,000,000 bytes
+	//    500M / 2**20 = 476
+	//    476Mi = 476 * 2**20 = 499122176 < 500M
+	//
 	s := fmt.Sprintf("%dMi", blockImage.Size/sizeMB)
 	quantity, err := resource.ParseQuantity(s)
 	if err != nil {