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.

Sign up for GitHub

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

Jump to bottom

cs instance root_disk size update resizes the root volume #43817

Merged
merged 4 commits into from
Aug 28, 2018
Merged

cs instance root_disk size update resizes the root volume #43817

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8ce3f57
cloudstack: resize volume on root_disk_size changes
Aug 8, 2018
b8b255a
fixup! cloudstack: resize volume on root_disk_size changes
Aug 8, 2018
d70808c
fixup! fixup! cloudstack: resize volume on root_disk_size changes
Aug 8, 2018
29b7d81
cs_instance: rename flag to allow_root_disk_shrink
Aug 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 34 additions & 0 deletions lib/ansible/modules/cloud/cloudstack/cs_instance.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,12 @@
- Force stop/start the instance if required to apply changes, otherwise a running instance will not be changed.
type: bool
default: no
allow_root_disk_shrink:
description:
- Enables a volume shrinkage when the new size is smaller than the old one.
type: bool
default: no
version_added: '2.7'
tags:
description:
- List of tags. Tags are a list of dictionaries having keys C(key) and C(value).
Expand Down Expand Up @@ -743,11 +749,33 @@ def update_instance(self, instance, start_vm=True):

security_groups_changed = self.security_groups_has_changed()

# Volume data

args_volume_update = {}
root_disk_size = self.module.params.get('root_disk_size')
root_disk_size_changed = False

if root_disk_size is not None:
res = self.query_api('listVolumes', type='ROOT', virtualmachineid=instance['id'])
[volume] = res['volume']
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we catch a just in case missing volume in the response to be cleaner?:

volume = res.get('volume')
if volume:
  ....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

defensive programming doesn't help figuring it out what's wrong, in case something is wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

True


size = volume['size'] >> 30

args_volume_update['id'] = volume['id']
args_volume_update['size'] = root_disk_size

shrinkok = self.module.params.get('allow_root_disk_shrink')
if shrinkok:
args_volume_update['shrinkok'] = shrinkok

root_disk_size_changed = root_disk_size != size
Copy link
Contributor

@resmo resmo Aug 18, 2018 •
edited

Choose a reason for hiding this comment

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

one more question, this would also be "true" for enlargement of the disk, would this work as expected? if yes, allow_root_disk_shrink would be a misleading name because it would not only resize to smaller, but also resize to bigger volume --> allow_root_disk_resize

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's really only for _shrinkagethat a special flag needs to be sent.allow_root_disk_resize` would be misleading as the nothing special is required for enlarging a disk.

http://cloudstack.apache.org/api/apidocs-4.4/user/resizeVolume.html

Copy link
Contributor

Choose a reason for hiding this comment

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

@resmo the shrink option is only for changing the disk size to a smaller partition. It requires more actions from the user if it should success:

Copy link
Contributor

Choose a reason for hiding this comment

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

so, that means, we should change this condition to the following, right?

root_disk_size_changed = root_disk_size < size

Copy link
Contributor

Choose a reason for hiding this comment

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

No, you can resize the volume up or down, but if you want to resize it down you must specify the flag shrink to ensure you know what you are doing since it requires some preparation of the volume.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, got it! 👍


changed = [
service_offering_changed,
instance_changed,
security_groups_changed,
ssh_key_changed,
root_disk_size_changed,
]

if any(changed):
Expand Down Expand Up @@ -787,6 +815,11 @@ def update_instance(self, instance, start_vm=True):
instance = self.poll_job(instance, 'virtualmachine')
self.instance = instance

# Root disk size
if root_disk_size_changed:
async_result = self.query_api('resizeVolume', **args_volume_update)
self.poll_job(async_result, 'volume')

# Start VM again if it was running before
if instance_state == 'running' and start_vm:
instance = self.start_instance()
Expand Down Expand Up @@ -986,6 +1019,7 @@ def main():
tags=dict(type='list', aliases=['tag']),
details=dict(type='dict'),
poll_async=dict(type='bool', default=True),
allow_root_disk_shrink=dict(type='bool', default=False),
))

required_together = cs_required_together()
Expand Down