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

add native disk resize #325

Merged
merged 4 commits into from
Oct 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/bosh-google-cpi/action/concrete_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ func (f ConcreteFactory) Create(method string, ctx map[string]interface{}, apiVe
vmService,
),
"delete_disk": NewDeleteDisk(diskService),
"resize_disk": NewResizeDisk(diskService),
"attach_disk": f.selectAttachDisk(apiVersion, diskService, vmService, registryClient),
"detach_disk": NewDetachDisk(vmService, registryClient),
"has_disk": NewHasDisk(diskService),
Expand Down
32 changes: 32 additions & 0 deletions src/bosh-google-cpi/action/resize_disk.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package action

import (
bosherr "github.com/cloudfoundry/bosh-utils/errors"

"bosh-google-cpi/api"
disk "bosh-google-cpi/google/disk_service"
"bosh-google-cpi/util"
)

type ResizeDisk struct {
diskService disk.Service
}

func NewResizeDisk(
diskService disk.Service,
) ResizeDisk {
return ResizeDisk{
diskService: diskService,
}
}

func (rd ResizeDisk) Run(diskCID DiskCID, newSize int) (interface{}, error) {
if err := rd.diskService.Resize(string(diskCID), util.ConvertMib2Gib(newSize)); err != nil {
if _, ok := err.(api.CloudError); ok {
return nil, err
}
return nil, bosherr.WrapErrorf(err, "Resizing disk '%s' to '%#v'", diskCID, newSize)
}

return nil, nil
}
44 changes: 44 additions & 0 deletions src/bosh-google-cpi/action/resize_disk_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package action_test

import (
"errors"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

. "bosh-google-cpi/action"

diskfakes "bosh-google-cpi/google/disk_service/fakes"
)

var _ = Describe("ResizeDisk", func() {
var (
err error

diskService *diskfakes.FakeDiskService

resizeDisk ResizeDisk
)

BeforeEach(func() {
diskService = &diskfakes.FakeDiskService{}
resizeDisk = NewResizeDisk(diskService)
})

Describe("Run", func() {
It("resize the disk", func() {
_, err = resizeDisk.Run("fake-disk-id", 123)
Expect(err).NotTo(HaveOccurred())
Expect(diskService.ResizeCalled).To(BeTrue())
})

It("returns an error if diskService resize call returns an error", func() {
diskService.ResizeErr = errors.New("fake-disk-service-error")
Copy link
Contributor

Choose a reason for hiding this comment

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

Would read better if moved into a BeforeEach of a surrounding Context("diskService resize call returns an error", ……).


_, err = resizeDisk.Run("fake-disk-id", 123)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("fake-disk-service-error"))
Expect(diskService.ResizeCalled).To(BeTrue())
})
})
})
1 change: 1 addition & 0 deletions src/bosh-google-cpi/google/disk_service/disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ type Disk struct {
Status string
Zone string
Users []string
SizeGb int64
}
1 change: 1 addition & 0 deletions src/bosh-google-cpi/google/disk_service/disk_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ package disk
type Service interface {
Create(size int, diskType string, zone string) (string, error)
Delete(id string) error
Resize(id string, newSize int) error
Find(id string, zone string) (Disk, bool, error)
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package fakes

import (
"bosh-google-cpi/google/disk_service"
disk "bosh-google-cpi/google/disk_service"
)

type FakeDiskService struct {
Expand All @@ -15,6 +15,9 @@ type FakeDiskService struct {
DeleteCalled bool
DeleteErr error

ResizeCalled bool
ResizeErr error

FindCalled bool
FindFound bool
FindDisk disk.Disk
Expand All @@ -34,6 +37,11 @@ func (d *FakeDiskService) Delete(id string) error {
return d.DeleteErr
}

func (d *FakeDiskService) Resize(id string, newSize int) error {
d.ResizeCalled = true
return d.ResizeErr
}

func (d *FakeDiskService) Find(id string, zone string) (disk.Disk, bool, error) {
d.FindCalled = true
return d.FindDisk, d.FindFound, d.FindErr
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
bosherr "github.com/cloudfoundry/bosh-utils/errors"

"bosh-google-cpi/util"

"google.golang.org/api/googleapi"
)

Expand All @@ -27,6 +28,7 @@ func (d GoogleDiskService) Find(id string, zone string) (Disk, bool, error) {
Status: diskItem.Status,
Zone: diskItem.Zone,
Users: diskItem.Users,
SizeGb: diskItem.SizeGb,
}
return disk, true, nil
}
Expand All @@ -50,6 +52,7 @@ func (d GoogleDiskService) Find(id string, zone string) (Disk, bool, error) {
SelfLink: diskItem.SelfLink,
Status: diskItem.Status,
Zone: diskItem.Zone,
SizeGb: diskItem.SizeGb,
}
return disk, true, nil
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package disk

import (
bosherr "github.com/cloudfoundry/bosh-utils/errors"

"bosh-google-cpi/api"
"bosh-google-cpi/util"

"google.golang.org/api/compute/v1"
)

func (d GoogleDiskService) Resize(id string, newSize int) error {
newsizeGB := &compute.DisksResizeRequest{
SizeGb: int64(newSize),
}

disk, found, err := d.Find(id, "")
if err != nil {
return err
}
if !found {
return api.NewDiskNotFoundError(id, false)
}

if disk.SizeGb == newsizeGB.SizeGb {
d.logger.Debug(googleDiskServiceLogTag, "Skipping resize Google Disk '%s', becasue current value '%#v'is equal to new value '%#v'", id, disk.SizeGb, newsizeGB)
} else if disk.SizeGb > newsizeGB.SizeGb {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to add a test case for disk.SizeGb == newsizeGB.SizeGb and disk.SizeGb > newsizeGB.SizeGb

I can help in possible refactoring that would help mocking d.Find() and disk.

return bosherr.WrapErrorf(err, "Skipping resize Google Disk '%s', cannot resize volume to a smaller size from '%#v' to '%#v'", id, disk.SizeGb, newsizeGB)
}

d.logger.Debug(googleDiskServiceLogTag, "Resizing Google Disk '%s'", id)
operation, err := d.computeService.Disks.Resize(d.project, util.ResourceSplitter(disk.Zone), id, newsizeGB).Do()
if err != nil {
return bosherr.WrapErrorf(err, "Failed to resize Google Disk '%s'", id)
}

if _, err = d.operationService.Waiter(operation, disk.Zone, ""); err != nil {
return bosherr.WrapErrorf(err, "Failed to resize Google Disk '%s'", id)
}

return nil
}
25 changes: 25 additions & 0 deletions src/bosh-google-cpi/integration/disk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,4 +149,29 @@ var _ = Describe("Disk", func() {
}`, diskCID)
assertSucceeds(request)
})

It("can create and resize and delete a disk", func() {
By("creating a disk")
var diskCID string
request := fmt.Sprintf(`{
"method": "create_disk",
"arguments": [32768, {"zone": "%v"}, ""]
}`, zone)
diskCID = assertSucceedsWithResult(request).(string)

By("resizing the disk")
request = fmt.Sprintf(`{
"method": "resize_disk",
"arguments": ["%v", 52768]
}`, diskCID)
assertSucceeds(request)

By("deleting the disk")
request = fmt.Sprintf(`{
"method": "delete_disk",
"arguments": ["%v"]
}`, diskCID)
assertSucceeds(request)
})

})