Skip to content

Commit

Permalink
Fix GCE CreateVolume allocates in chunks of GiB incorrectly
Browse files Browse the repository at this point in the history
  • Loading branch information
edisonxiang committed Dec 17, 2017
1 parent cab439b commit d80dbe7
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 13 deletions.
2 changes: 1 addition & 1 deletion pkg/volume/gce_pd/gce_pd.go
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ func (c *gcePersistentDiskProvisioner) Provision() (*v1.PersistentVolume, error)
PersistentVolumeReclaimPolicy: c.options.PersistentVolumeReclaimPolicy,
AccessModes: c.options.PVC.Spec.AccessModes,
Capacity: v1.ResourceList{
v1.ResourceName(v1.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)),
v1.ResourceName(v1.ResourceStorage): resource.MustParse(fmt.Sprintf("%dG", sizeGB)),
},
PersistentVolumeSource: v1.PersistentVolumeSource{
GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{
Expand Down
2 changes: 1 addition & 1 deletion pkg/volume/gce_pd/gce_pd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func TestPlugin(t *testing.T) {
}
cap := persistentSpec.Spec.Capacity[v1.ResourceStorage]
size := cap.Value()
if size != 100*1024*1024*1024 {
if size != 100*volume.GB {
t.Errorf("Provision() returned unexpected volume size: %v", size)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/volume/gce_pd/gce_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (strin
name := volume.GenerateVolumeName(c.options.ClusterName, c.options.PVName, 63) // GCE PD name can have up to 63 characters
capacity := c.options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)]
requestBytes := capacity.Value()
// GCE works with gigabytes, convert to GiB with rounding up
requestGB := volume.RoundUpSize(requestBytes, 1024*1024*1024)
// GCE PDs are allocated in chunks of GBs (not GiBs)
requestGB := volume.RoundUpSize(requestBytes, volume.GB)

// Apply Parameters (case-insensitive). We leave validation of
// the values to the cloud provider.
Expand Down
26 changes: 17 additions & 9 deletions test/e2e/storage/volume_provisioning.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,8 @@ var _ = SIGDescribe("Dynamic Provisioning", func() {
"type": "pd-ssd",
"zone": cloudZone,
},
"1.5Gi",
"2Gi",
"1.5G",
"2G",
func(volume *v1.PersistentVolume) error {
return checkGCEPD(volume, "pd-ssd")
},
Expand All @@ -269,8 +269,8 @@ var _ = SIGDescribe("Dynamic Provisioning", func() {
map[string]string{
"type": "pd-standard",
},
"1.5Gi",
"2Gi",
"1.5G",
"2G",
func(volume *v1.PersistentVolume) error {
return checkGCEPD(volume, "pd-standard")
},
Expand Down Expand Up @@ -435,8 +435,8 @@ var _ = SIGDescribe("Dynamic Provisioning", func() {
map[string]string{
"type": "pd-standard",
},
"1Gi",
"1Gi",
"1G",
"1G",
func(volume *v1.PersistentVolume) error {
return checkGCEPD(volume, "pd-standard")
},
Expand Down Expand Up @@ -469,8 +469,8 @@ var _ = SIGDescribe("Dynamic Provisioning", func() {
map[string]string{
"type": "pd-standard",
},
"1Gi",
"1Gi",
"1G",
"1G",
func(volume *v1.PersistentVolume) error {
return checkGCEPD(volume, "pd-standard")
},
Expand Down Expand Up @@ -520,7 +520,7 @@ var _ = SIGDescribe("Dynamic Provisioning", func() {
name: "unmanaged_zone",
provisioner: "kubernetes.io/gce-pd",
parameters: map[string]string{"zone": unmanagedZone},
claimSize: "1Gi",
claimSize: "1G",
}
sc := newStorageClass(test, ns, suffix)
sc, err = c.StorageV1().StorageClasses().Create(sc)
Expand Down Expand Up @@ -640,6 +640,14 @@ var _ = SIGDescribe("Dynamic Provisioning", func() {
claimSize: "2Gi",
expectedSize: "2Gi",
}
// gce or gke
if getDefaultPluginName() == "kubernetes.io/gce-pd" {
// using GB not GiB as e2e test unit since gce-pd returns GB,
// or expectedSize may be greater than claimSize.
test.claimSize = "2G"
test.expectedSize = "2G"
}

claim := newClaim(test, ns, "default")
testDynamicProvisioning(test, c, claim, nil)
})
Expand Down

0 comments on commit d80dbe7

Please sign in to comment.