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

Fix for issue #513 #581

Merged
merged 2 commits into from
Apr 16, 2019
Merged

Fix for issue #513 #581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c714128
Fixed issue #513
imran-ansari Apr 16, 2019
57024c4
Pep8 fixed
imran-ansari Apr 16, 2019
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
4 changes: 3 additions & 1 deletion hpedockerplugin/hpe_storage_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,9 @@ def volumedriver_create(self, name, opts=None):
contents['Opts']['fsOwner'] != ""):
fs_owner = contents['Opts']['fsOwner']
try:
mode = fs_owner.split(':')
uid, gid = fs_owner.split(':')
int(uid)
int(gid)
except ValueError as ex:
return json.dumps({'Err': "Invalid value '%s' specified "
"for fsOwner. Please "
Expand Down
110 changes: 73 additions & 37 deletions hpedockerplugin/volume_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1299,6 +1299,7 @@ def mount_volume(self, volname, vol_mount, mount_id):
LOG.error(msg)
raise exception.HPEPluginMountException(reason=msg)

undo_steps = []
volid = vol['id']
is_snap = False
if 'is_snap' not in vol:
Expand All @@ -1308,19 +1309,19 @@ def mount_volume(self, volname, vol_mount, mount_id):
is_snap = vol['is_snap']
vol['fsOwner'] = vol['snap_metadata'].get('fsOwner')
vol['fsMode'] = vol['snap_metadata'].get('fsMode')

if 'mount_conflict_delay' not in vol:
m_conf_delay = volume.DEFAULT_MOUNT_CONFLICT_DELAY
vol['mount_conflict_delay'] = m_conf_delay
self._etcd.update_vol(volid, 'mount_conflict_delay',
m_conf_delay)
# Initialize node-mount-info if volume is being mounted
# for the first time
if self._is_vol_not_mounted(vol):
LOG.info("Initializing node_mount_info... adding first "
"mount ID %s" % mount_id)
node_mount_info = {self._node_id: [mount_id]}
vol['node_mount_info'] = node_mount_info

if 'mount_conflict_delay' not in vol:
m_conf_delay = volume.DEFAULT_MOUNT_CONFLICT_DELAY
vol['mount_conflict_delay'] = m_conf_delay
self._etcd.update_vol(volid, 'mount_conflict_delay',
m_conf_delay)
else:
# Volume is in mounted state - Volume fencing logic begins here
node_mount_info = vol['node_mount_info']
Expand Down Expand Up @@ -1373,22 +1374,35 @@ def _mount_volume(driver):
LOG.debug('connection_info: %(connection_info)s, '
'was successfully retrieved',
{'connection_info': json.dumps(connection_info)})

undo_steps.append(
{'undo_func': driver.terminate_connection,
'params': (vol, connector_info, is_snap),
'msg': 'Terminating connection to volume: %s...'
% volname})
except Exception as ex:
msg = (_('Initialize Connection Failed: '
'connection info retrieval failed, error is: '),
six.text_type(ex))
LOG.error(msg)
self._rollback(undo_steps)
raise exception.HPEPluginMountException(reason=msg)

# Call OS Brick to connect volume
try:
LOG.debug("OS Brick Connector Connecting Volume...")
device_info = self._connector.connect_volume(
connection_info['data'])

undo_steps.append(
{'undo_func': self._connector.disconnect_volume,
'params': (connection_info['data'], None),
'msg': 'Undoing connection to volume: %s...' % volname})
except Exception as ex:
msg = (_('OS Brick connect volume failed, error is: '),
six.text_type(ex))
LOG.error(msg)
self._rollback(undo_steps)
raise exception.HPEPluginMountException(reason=msg)
return device_info, connection_info

Expand Down Expand Up @@ -1436,6 +1450,7 @@ def _mount_volume(driver):
if path.exists is False:
msg = (_('path: %s, does not exist'), path)
LOG.error(msg)
self._rollback(undo_steps)
raise exception.HPEPluginMountException(reason=msg)

LOG.debug('path for volume: %(name)s, was successfully created: '
Expand Down Expand Up @@ -1463,11 +1478,21 @@ def _mount_volume(driver):
'%(mount)s',
{'mount_dir': mount_dir, 'mount': device_info['path']})

undo_steps.append(
{'undo_func': fileutil.remove_dir,
'params': mount_dir,
'msg': 'Removing mount directory: %s...' % mount_dir})

# mount the directory
fileutil.mount_dir(path.path, mount_dir)
LOG.debug('Device: %(path)s successfully mounted on %(mount)s',
{'path': path.path, 'mount': mount_dir})

undo_steps.append(
{'undo_func': fileutil.umount_dir,
'params': mount_dir,
'msg': 'Unmounting directory: %s...' % mount_dir})

# TODO: find out how to invoke mkfs so that it creates the
# filesystem without the lost+found directory
# KLUDGE!!!!!
Expand All @@ -1480,37 +1505,42 @@ def _mount_volume(driver):
else:
mount_dir = ''

if 'fsOwner' in vol and vol['fsOwner']:
fs_owner = vol['fsOwner'].split(":")
uid = int(fs_owner[0])
gid = int(fs_owner[1])
os.chown(mount_dir, uid, gid)

if 'fsMode' in vol and vol['fsMode']:
mode = str(vol['fsMode'])
chmod(mode, mount_dir)

path_info = {}
path_info['name'] = volname
path_info['path'] = path.path
path_info['device_info'] = device_info
path_info['connection_info'] = pri_connection_info
path_info['mount_dir'] = mount_dir
if sec_connection_info:
path_info['remote_connection_info'] = sec_connection_info

LOG.info("Updating node_mount_info in etcd with mount_id %s..."
% mount_id)
self._etcd.update_vol(volid,
'node_mount_info',
node_mount_info)
LOG.info("node_mount_info updated successfully in etcd with mount_id "
"%s" % mount_id)
self._etcd.update_vol(volid, 'path_info', json.dumps(path_info))
try:
if 'fsOwner' in vol and vol['fsOwner']:
fs_owner = vol['fsOwner'].split(":")
uid = int(fs_owner[0])
gid = int(fs_owner[1])
os.chown(mount_dir, uid, gid)

if 'fsMode' in vol and vol['fsMode']:
mode = str(vol['fsMode'])
chmod(mode, mount_dir)

path_info = {}
path_info['name'] = volname
path_info['path'] = path.path
path_info['device_info'] = device_info
path_info['connection_info'] = pri_connection_info
path_info['mount_dir'] = mount_dir
if sec_connection_info:
path_info['remote_connection_info'] = sec_connection_info

LOG.info("Updating node_mount_info in etcd with mount_id %s..."
% mount_id)
self._etcd.update_vol(volid,
'node_mount_info',
node_mount_info)
LOG.info("node_mount_info updated successfully in etcd with "
"mount_id %s" % mount_id)
self._etcd.update_vol(volid, 'path_info', json.dumps(path_info))

response = json.dumps({u"Err": '', u"Name": volname,
u"Mountpoint": mount_dir,
u"Devicename": path.path})
except Exception as ex:
self._rollback(undo_steps)
response = json.dumps({"Err": '%s' % six.text_type(ex)})

response = json.dumps({u"Err": '', u"Name": volname,
u"Mountpoint": mount_dir,
u"Devicename": path.path})
return response

def _get_target_driver(self, rcg_info):
Expand Down Expand Up @@ -1837,7 +1867,13 @@ def _rollback(rollback_list):
for undo_action in reversed(rollback_list):
LOG.info(undo_action['msg'])
try:
undo_action['undo_func'](**undo_action['params'])
params = undo_action['params']
if type(params) is dict:
undo_action['undo_func'](**undo_action['params'])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any specific reason to identify the data type of the params and call the undo functions differently.. I'm asking due to the previous code does'nt have this kind of check ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

i got the answer. Tuple is passed explicitly in undo_steps.append(..'params')

undo_steps.append(
                    {'undo_func': driver.terminate_connection,
                     'params': (vol, connector_info, is_snap),

elif type(params) is tuple:
undo_action['undo_func'](*undo_action['params'])
else:
undo_action['undo_func'](undo_action['params'])
except Exception as ex:
# TODO: Implement retry logic
LOG.exception('Ignoring exception: %s' % ex)
Expand Down