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

Conversation

ramonskie
Copy link
Contributor

add native disk resize
as the new agent now supports this see cloudfoundry/bosh-agent#244

Copy link
Contributor

@bgandon bgandon left a comment

Choose a reason for hiding this comment

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

I get an error when running tests, see comments and suggestions.

})

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", ……).


if disk.SizeGb == newsizeGB.SizeGb {
d.logger.Debug(googleDiskServiceLogTag, "Skipping resize Google Disk '%s', becasue current value '%s'is equal to new value '%s'", 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.

@rkoster
Copy link
Contributor

rkoster commented Oct 21, 2021

LGTM, Thanks @ramonskie

@rkoster rkoster merged commit 4da0a3c into cloudfoundry:master Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants