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 support for OpenStack instances that require boot volumes #485

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
13 changes: 13 additions & 0 deletions cloud_resources.conf
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,19 @@
#
#cloud_type: OpenStack

# Boot Volume:
# Option for OpenStackNative cloud type
# Should be set to True if the cloud interface requires the creation of a boot
# volume that the root filesystem of the instance resides on
#
#boot_volume = True

# Boot volume size
# To control the boot volume size set a value in GByte per core. If
# not set the volumes will default to 20GByte
#
#boot_volume_gb_per_core = 20

# Virtual Machine Slots:
# The Maximum Number of virtual machines that can be
# run on a cluster at a time
Expand Down
2 changes: 2 additions & 0 deletions cloudscheduler/cloud_management.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,8 @@ def _cluster_from_config(cconfig, cluster):
keep_alive=keep_alive,
user_domain_name=get_or_none(cconfig, cluster, "user_domain_name"),
project_domain_name=get_or_none(cconfig, cluster, "project_domain_name")
Copy link
Contributor

Choose a reason for hiding this comment

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

comma missing in line 384 ;)

Copy link
Author

Choose a reason for hiding this comment

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

indeed , added

boot_volume = get_or_none(cconfig, cluster, "boot_volume"),
boot_volume_gb_per_core = get_or_none(cconfig, cluster, "boot_volume_gb_per_core"),
)
elif cloud_type.lower() == "azure" and cloudconfig.verify_cloud_conf_azure(cconfig, cluster):
return azurecluster.AzureCluster(name = cluster.lower(),
Expand Down
69 changes: 63 additions & 6 deletions cloudscheduler/openstackcluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ def __init__(self, name="Dummy Cluster", cloud_type="Dummy",
key_name=None, boot_timeout=None, secure_connection="",
regions="", reverse_dns_lookup=False,placement_zone=None,
enabled=True, priority=0, cacert=None,keep_alive=0, user_domain_name=None,
project_domain_name=None):
project_domain_name=None, boot_volume=False,
boot_volume_gb_per_core=20):

# Call super class's init
cluster_tools.ICluster.__init__(self,name=name, host=auth_url, cloud_type=cloud_type,
Expand Down Expand Up @@ -115,6 +116,8 @@ def vm_create(self, vm_name, vm_type, vm_user, vm_networkassoc,
import novaclient.exceptions
use_cloud_init = use_cloud_init or config.use_cloud_init
nova = self._get_creds_nova_updated()
if boot_volume:
cinder = self._get_creds_cinder()
if len(securitygroup) != 0:
sec_group = []
for group in securitygroup:
Expand Down Expand Up @@ -238,14 +241,52 @@ def vm_create(self, vm_name, vm_type, vm_user, vm_networkassoc,
netid = []
else:
netid = []
# Need to get the rotating hostname from the google code to use for here.
# Need to get the rotating hostname from the google code to use for here.
name = self._generate_next_name()
instance = None

if name:
log.info("Trying to create VM on %s: " % self.name)
try:
instance = nova.servers.create(name=name, image=imageobj, flavor=flavor, key_name=key_name,
availability_zone=self.placement_zone, nics =netid, userdata=user_data, security_groups=sec_group)
if not boot_volume:
instance = nova.servers.create(name=name,
image=imageobj,
flavor=flavor,
key_name=key_name,
availability_zone=self.placement_zone,
nics =netid,
userdata=user_data,
security_groups=sec_group)
else:
bdm = None
log.debug("creating boot volume")
try:
Copy link
Contributor

Choose a reason for hiding this comment

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

Memo for a similar implementation in v2:
during my testing I got the impression that nested try except won't work, this should be checked and reviewed before we move this into V2

Copy link
Author

Choose a reason for hiding this comment

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

Yeah I had trouble with nested try except blocks in the past. Let me try to find the type of exceptions cinder throws ... maybe can be smarter here.

bv_name = "vol-{}".format(name)
if boot_volume_gb_per_core:
bv_size = boot_volume_gb_per_core * cpu_cores
else:
bv_size = 20
cv = cinder.volumes.create(name=bv_name,
size=bv_size,
imageRef=imageobj.id)
while (cv.status != 'available'):
time.sleep(1)
cv = cinder.volumes.get(cv.id)
berghaus marked this conversation as resolved.
Show resolved Hide resolved
cinder.volumes.set_bootable(cv, True)
bdm = {'vda': str(cv.id) + ':::1'}
except Exception as e:
log.error("failed to create boot volume: {}".format(e))
raise e
Copy link
Contributor

Choose a reason for hiding this comment

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

For V2 we should also think about possible failures, e.g. during testing volumes were created fine, but creation of VMs failed e.g. due to wrong networks. This left the volume lying around and needed to be removed by hand. There's several ways to do it, IMHO but at this point in time nothing needed for V1.

Copy link
Author

Choose a reason for hiding this comment

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

So I have added something to the exception handling in vm_create, but to add some cleanup to the vm deletion means we have to change the database model for the VMs. Note sure how significant an edit that is in the end.

log.debug("boot volume creation successful")
instance = nova.servers.create(name=name,
image=imageobj,
flavor=flavor,
key_name=key_name,
block_device_mapping=bdm,
availability_zone=self.placement_zone,
nics=netid,
userdata=user_data,
security_groups=sec_group)
#print instance.__dict__
except novaclient.exceptions.OverLimit as e:
log.info("Unable to create VM without exceeded quota on %s: %s" % (self.name, e.message))
Expand Down Expand Up @@ -293,8 +334,8 @@ def vm_destroy(self, vm, return_resources=True, reason=""):
except novaclient.exceptions.NotFound as e:
log.error("VM %s not found on %s: removing from CS" % (vm.id, self.name))
except Exception as e:
try:
log.error("Unhandled exception while destroying VM on %s : %s" % (self.name,e))
try:
Copy link
Contributor

Choose a reason for hiding this comment

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

These two lines got accidentally additional spaces it seems

Copy link
Author

Choose a reason for hiding this comment

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

how did those get there, hmmm

log.error("Unhandled exception while destroying VM on %s : %s" % (self.name,e))
return 1
except:
log.error("Failed to log exception properly?")
Expand Down Expand Up @@ -400,6 +441,22 @@ def _get_keystone_session_v3(self):
log.debug("Session object for %s created" % self.name)
return sess

def _get_creds_cinder(self):
try:
from cinderclient import client as cclient
except Exception as e:
print("Unable to import cinderclient - cannot create boot volumes")
print(e)
sys.exit(1)
try:
cinder = cclient.Client("3",
session=self.session,
region_name=self.regions[0],
timeout=10)
except Exception as e:
log.error("Cannot use cinder on {}: {}".format(self.name, e))
raise e
return cinder

def _find_network(self, name):
nova = self._get_creds_nova_updated()
Expand Down