From 6c12295c998615d34e8b4607107f7e2ea778c3ce Mon Sep 17 00:00:00 2001 From: Rob Bradford Date: Fri, 6 Jan 2017 14:59:16 +0000 Subject: [PATCH 1/3] ciao-storage: Populate BlockDevice with volume size After creating a volume make sure that the BlockDevice size field is populated. Either from the request size or by querying from rbd. Fixes: #994 Signed-off-by: Rob Bradford --- ciao-storage/block.go | 2 +- ciao-storage/ceph.go | 34 +++++++++++++++++++++++++++++++--- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/ciao-storage/block.go b/ciao-storage/block.go index ca6edf4f6..0534702c1 100644 --- a/ciao-storage/block.go +++ b/ciao-storage/block.go @@ -47,5 +47,5 @@ type BlockDevice struct { Local bool // local (ephemeral) or volume service backed Swap bool // linux swap device (attempt swapon via cloudinit) Tag string // arbitrary text identifier - Size int // gigabyte size for autocreation + Size int // size in GiB } diff --git a/ciao-storage/ceph.go b/ciao-storage/ceph.go index d69b49307..6f70d29a9 100644 --- a/ciao-storage/ceph.go +++ b/ciao-storage/ceph.go @@ -32,6 +32,22 @@ type CephDriver struct { ID string } +func (d CephDriver) getBlockDeviceSizeGiB(volumeUUID string) (int, error) { + bytes, err := d.GetBlockDeviceSize(volumeUUID) + + if err != nil { + return 0, err + } + + // When converting to GiB round up unless we've got a multiple of 1GiB + res := bytes / (1024 * 1024 * 1024) + rem := bytes % (1024 * 1024 * 1024) + if rem == 0 { + return int(res), nil + } + return int(res + 1), nil +} + // CreateBlockDevice will create a rbd image in the ceph cluster. func (d CephDriver) CreateBlockDevice(volumeUUID string, imagePath string, size int) (BlockDevice, error) { if volumeUUID == "" { @@ -61,7 +77,7 @@ func (d CephDriver) CreateBlockDevice(volumeUUID string, imagePath string, size return BlockDevice{}, fmt.Errorf("Error when running: %v: %v: %s", cmd.Args, err, out) } - return BlockDevice{ID: volumeUUID}, nil + return BlockDevice{ID: volumeUUID, Size: size}, nil } // CreateBlockDeviceFromSnapshot will create a block device derived from the previously created snapshot. @@ -77,7 +93,13 @@ func (d CephDriver) CreateBlockDeviceFromSnapshot(volumeUUID string, snapshotID return BlockDevice{}, fmt.Errorf("Error when running: %v: %v: %s", cmd.Args, err, out) } - return BlockDevice{ID: ID}, nil + size, err := d.getBlockDeviceSizeGiB(volumeUUID) + if err != nil { + d.DeleteBlockDevice(volumeUUID) + return BlockDevice{}, fmt.Errorf("Error when querying block device size: %v", err) + } + + return BlockDevice{ID: ID, Size: size}, nil } // CreateBlockDeviceSnapshot creates and protects the snapshot with the provided name @@ -113,7 +135,13 @@ func (d CephDriver) CopyBlockDevice(volumeUUID string) (BlockDevice, error) { return BlockDevice{}, fmt.Errorf("Error when running: %v: %v: %s", cmd.Args, err, out) } - return BlockDevice{ID: ID}, nil + size, err := d.getBlockDeviceSizeGiB(volumeUUID) + if err != nil { + d.DeleteBlockDevice(volumeUUID) + return BlockDevice{}, fmt.Errorf("Error when querying block device size: %v", err) + } + + return BlockDevice{ID: ID, Size: size}, nil } // DeleteBlockDevice will remove a rbd image from the ceph cluster. From 776334d284e6c68e0437b5a4438c7485517110dd Mon Sep 17 00:00:00 2001 From: Rob Bradford Date: Fri, 6 Jan 2017 15:16:04 +0000 Subject: [PATCH 2/3] ciao-storage: Populate BlockDevice.Size in the noop backend Now that the Size member is no longer duplicated across BlockData and BlockSize we need to make sure to set it in the noop backend to allow the unit testing to succeed. Signed-off-by: Rob Bradford --- ciao-storage/noop.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ciao-storage/noop.go b/ciao-storage/noop.go index 9098c71bf..02a0f9fb4 100644 --- a/ciao-storage/noop.go +++ b/ciao-storage/noop.go @@ -29,7 +29,7 @@ type NoopDriver struct { // CreateBlockDevice pretends to create a block device. func (d *NoopDriver) CreateBlockDevice(volumeUUID string, image string, size int) (BlockDevice, error) { - return BlockDevice{ID: uuid.Generate().String()}, nil + return BlockDevice{ID: uuid.Generate().String(), Size: size}, nil } // CreateBlockDeviceFromSnapshot pretends to create a block device snapshot From 4ad3fc27e5d0caf3e1d25bc6e5ad49bf74932823 Mon Sep 17 00:00:00 2001 From: Rob Bradford Date: Fri, 6 Jan 2017 15:04:07 +0000 Subject: [PATCH 3/3] ciao-controller: Remove duplicated size from BlockData storage.BlockDevice is embedded in types.BlockData and already contains a Size member. Now that BlockDevice.Size is being populated by the storage backend when the BlockDevice is created this member and its users can be removed. Signed-off-by: Rob Bradford --- ciao-controller/instance.go | 1 - .../internal/datastore/datastore_test.go | 15 --------------- .../internal/datastore/sqlite3db_test.go | 4 ---- ciao-controller/openstack_volume.go | 1 - ciao-controller/types/types.go | 1 - 5 files changed, 22 deletions(-) diff --git a/ciao-controller/instance.go b/ciao-controller/instance.go index 5e6369abf..2ccd46781 100644 --- a/ciao-controller/instance.go +++ b/ciao-controller/instance.go @@ -169,7 +169,6 @@ func addBlockDevice(c *controller, tenant string, instanceID string, device stor // a block device is bootable. data := types.BlockData{ BlockDevice: device, - Size: s.Size, CreateTime: time.Now(), TenantID: tenant, Name: fmt.Sprintf("Storage for instance: %s", instanceID), diff --git a/ciao-controller/internal/datastore/datastore_test.go b/ciao-controller/internal/datastore/datastore_test.go index 14702db01..f10e366f5 100644 --- a/ciao-controller/internal/datastore/datastore_test.go +++ b/ciao-controller/internal/datastore/datastore_test.go @@ -1346,7 +1346,6 @@ func TestAttachVolumeFailure(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: newTenant.ID, CreateTime: time.Now(), @@ -1403,7 +1402,6 @@ func TestDetachVolumeFailure(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: newTenant.ID, CreateTime: time.Now(), @@ -1495,7 +1493,6 @@ func TestAddBlockDevice(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: newTenant.ID, CreateTime: time.Now(), @@ -1537,7 +1534,6 @@ func TestDeleteBlockDevice(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: newTenant.ID, CreateTime: time.Now(), @@ -1585,7 +1581,6 @@ func TestUpdateBlockDevice(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: newTenant.ID, CreateTime: time.Now(), @@ -1649,7 +1644,6 @@ func TestUpdateBlockDeviceErr(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: newTenant.ID, CreateTime: time.Now(), @@ -1674,7 +1668,6 @@ func TestCreateStorageAttachment(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: tenant.ID, CreateTime: time.Now(), @@ -1736,7 +1729,6 @@ func TestUpdateStorageAttachmentExisting(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: tenant.ID, CreateTime: time.Now(), @@ -1804,7 +1796,6 @@ func TestUpdateStorageAttachmentNotExisting(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: tenant.ID, CreateTime: time.Now(), @@ -1848,7 +1839,6 @@ func TestUpdateStorageAttachmentDeleted(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: tenant.ID, CreateTime: time.Now(), @@ -1903,7 +1893,6 @@ func TestGetStorageAttachment(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: tenant.ID, CreateTime: time.Now(), @@ -1960,7 +1949,6 @@ func TestGetStorageAttachmentError(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: tenant.ID, CreateTime: time.Now(), @@ -2003,7 +1991,6 @@ func TestDeleteStorageAttachment(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: tenant.ID, CreateTime: time.Now(), @@ -2070,7 +2057,6 @@ func TestDeleteStorageAttachmentError(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: tenant.ID, CreateTime: time.Now(), @@ -2142,7 +2128,6 @@ func TestGetVolumeAttachments(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: tenant.ID, CreateTime: time.Now(), diff --git a/ciao-controller/internal/datastore/sqlite3db_test.go b/ciao-controller/internal/datastore/sqlite3db_test.go index 875556c23..be64a1389 100644 --- a/ciao-controller/internal/datastore/sqlite3db_test.go +++ b/ciao-controller/internal/datastore/sqlite3db_test.go @@ -68,7 +68,6 @@ func TestSQLiteDBGetTenantDevices(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: uuid.Generate().String(), CreateTime: time.Now(), @@ -114,7 +113,6 @@ func TestSQLiteDBGetTenantWithStorage(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: tenantID, CreateTime: time.Now(), @@ -155,7 +153,6 @@ func TestSQLiteDBGetAllBlockData(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: uuid.Generate().String(), CreateTime: time.Now(), @@ -191,7 +188,6 @@ func TestSQLiteDBDeleteBlockData(t *testing.T) { data := types.BlockData{ BlockDevice: blockDevice, - Size: 0, State: types.Available, TenantID: uuid.Generate().String(), CreateTime: time.Now(), diff --git a/ciao-controller/openstack_volume.go b/ciao-controller/openstack_volume.go index b2bbe865f..6150fe447 100644 --- a/ciao-controller/openstack_volume.go +++ b/ciao-controller/openstack_volume.go @@ -71,7 +71,6 @@ func (c *controller) CreateVolume(tenant string, req block.RequestedVolume) (blo // you should modify BlockData to include a "bootable" flag. data := types.BlockData{ BlockDevice: bd, - Size: req.Size, CreateTime: time.Now(), TenantID: tenant, State: types.Available, diff --git a/ciao-controller/types/types.go b/ciao-controller/types/types.go index 7b666d974..d9a8f3c38 100644 --- a/ciao-controller/types/types.go +++ b/ciao-controller/types/types.go @@ -258,7 +258,6 @@ const ( type BlockData struct { storage.BlockDevice TenantID string // the tenant who owns this volume - Size int // size in GB State BlockState // status of CreateTime time.Time // when we created the volume Name string // a human readable name for this volume