Add checks for: * number of arguments, * instance's compatibility with disk template/nodes
and update existing ones. Signed-off-by: Alex Pyrgiotis <[email protected]> diff --git a/lib/cmdlib/instance_set_params.py b/lib/cmdlib/instance_set_params.py index 9e0009e..db73695 100644 --- a/lib/cmdlib/instance_set_params.py +++ b/lib/cmdlib/instance_set_params.py @@ -89,6 +89,29 @@ class LUInstanceSetParams(LogicalUnit): HTYPE = constants.HTYPE_INSTANCE REQ_BGL = False + def GenericGetDiskInfo(self, uuid=None, name=None): + """Find a disk object using the provided params. + + Accept arguments as keywords and use the GetDiskInfo/GetDiskInfoByName + config functions to retrieve the disk info based on these arguments. + + In case of an error, raise the appropriate exceptions. + """ + if uuid: + disk = self.cfg.GetDiskInfo(uuid) + if disk is None: + raise errors.OpPrereqError("No disk was found with this UUID: %s" % + uuid, errors.ECODE_INVAL) + elif name: + disk = self.cfg.GetDiskInfoByName(name) + if disk is None: + raise errors.OpPrereqError("No disk was found with this name: %s" % + name, errors.ECODE_INVAL) + else: + raise errors.ProgrammerError("No disk UUID or name was given") + + return disk + @staticmethod def _UpgradeDiskNicMods(kind, mods, verify_fn): assert ht.TList(mods) @@ -99,14 +122,15 @@ class LUInstanceSetParams(LogicalUnit): addremove = 0 for op, params in mods: - if op in (constants.DDM_ADD, constants.DDM_REMOVE): + if op in (constants.DDM_ADD, constants.DDM_ATTACH, + constants.DDM_REMOVE, constants.DDM_DETACH): result.append((op, -1, params)) addremove += 1 if addremove > 1: - raise errors.OpPrereqError("Only one %s add or remove operation is" - " supported at a time" % kind, - errors.ECODE_INVAL) + raise errors.OpPrereqError("Only one %s add/attach/remove/detach " + "operation is supported at a time" % + kind, errors.ECODE_INVAL) else: result.append((constants.DDM_MODIFY, op, params)) @@ -120,21 +144,33 @@ class LUInstanceSetParams(LogicalUnit): def _CheckMods(kind, mods, key_types, item_fn): """Ensures requested disk/NIC modifications are valid. + Note that the 'attach' action needs a way to refer to the UUID of the disk, + since the disk name is not unique cluster-wide. However, the UUID of the + disk is not settable but rather generated by Ganeti automatically, + therefore it cannot be passed as an IDISK parameter. For this reason, this + function will override the checks to accept uuid parameters solely for the + attach action. """ + # Create a key_types copy with the 'uuid' as a valid key type. + key_types_attach = key_types.copy() + key_types_attach['uuid'] = 'string' + for (op, _, params) in mods: assert ht.TDict(params) # If 'key_types' is an empty dict, we assume we have an # 'ext' template and thus do not ForceDictType if key_types: - utils.ForceDictType(params, key_types) + utils.ForceDictType(params, (key_types if op != constants.DDM_ATTACH + else key_types_attach)) - if op == constants.DDM_REMOVE: + if op in (constants.DDM_REMOVE, constants.DDM_DETACH): if params: raise errors.OpPrereqError("No settings should be passed when" - " removing a %s" % kind, + " removing or detaching a %s" % kind, errors.ECODE_INVAL) - elif op in (constants.DDM_ADD, constants.DDM_MODIFY): + elif op in (constants.DDM_ADD, constants.DDM_ATTACH, + constants.DDM_MODIFY): item_fn(op, params) else: raise errors.ProgrammerError("Unhandled operation '%s'" % op) @@ -160,6 +196,8 @@ class LUInstanceSetParams(LogicalUnit): if name is not None and name.lower() == constants.VALUE_NONE: params[constants.IDISK_NAME] = None + # This check is necessary both when adding and attaching disks + if op in (constants.DDM_ADD, constants.DDM_ATTACH): CheckSpindlesExclusiveStorage(params, excl_stor, True) # Check disk access param (only for specific disks) @@ -174,6 +212,16 @@ class LUInstanceSetParams(LogicalUnit): (self.instance.hypervisor, access_type), errors.ECODE_STATE) + if op == constants.DDM_ATTACH: + if len(params) != 1 or ('uuid' not in params and + constants.IDISK_NAME not in params): + raise errors.OpPrereqError("Only one argument is permitted in %s op," + " either %s or uuid" % (constants.DDM_ATTACH, + constants.IDISK_NAME, + ), + errors.ECODE_INVAL) + self._CheckAttachDisk(params) + elif op == constants.DDM_MODIFY: if constants.IDISK_SIZE in params: raise errors.OpPrereqError("Disk size change not possible, use" @@ -297,6 +345,29 @@ class LUInstanceSetParams(LogicalUnit): (self.op.pnode_uuid, self.op.pnode) = \ ExpandNodeUuidAndName(self.cfg, self.op.pnode_uuid, self.op.pnode) + def _CheckAttachDisk(self, params): + """Check if disk can be attached to an instance. + + Check if the disk and instance have the same template. Also, check if the + disk nodes are visible from the instance. + """ + uuid = params.get("uuid", None) + name = params.get(constants.IDISK_NAME, None) + + disk = self.GenericGetDiskInfo(uuid, name) + if disk.dev_type != self.instance.disk_template: + raise errors.OpPrereqError("Instance has '%s' template while disk has" + " '%s' template" % + (self.instance.disk_template, disk.dev_type), + errors.ECODE_INVAL) + + instance_nodes = self.cfg.GetInstanceNodes(self.instance.uuid) + if not set(disk.nodes).issubset(set(instance_nodes)): + raise errors.OpPrereqError("Disk nodes are %s while the instance's nodes" + " are %s" % + (disk.nodes, instance_nodes), + errors.ECODE_INVAL) + def ExpandNames(self): self._ExpandAndLockInstance() self.needed_locks[locking.LEVEL_NODEGROUP] = [] @@ -673,12 +744,12 @@ class LUInstanceSetParams(LogicalUnit): if self.instance.disk_template in constants.DT_EXT: for mod in self.diskmod: ext_provider = mod[2].get(constants.IDISK_PROVIDER, None) - if mod[0] == constants.DDM_ADD: + if mod[0] in (constants.DDM_ADD, constants.DDM_ATTACH): if ext_provider is None: raise errors.OpPrereqError("Instance template is '%s' and parameter" - " '%s' missing, during disk add" % + " '%s' missing, during disk %s" % (constants.DT_EXT, - constants.IDISK_PROVIDER), + constants.IDISK_PROVIDER, mod[0]), errors.ECODE_NOENT) elif mod[0] == constants.DDM_MODIFY: if ext_provider: @@ -698,10 +769,10 @@ class LUInstanceSetParams(LogicalUnit): if not self.op.wait_for_sync and not self.instance.disks_active: for mod in self.diskmod: - if mod[0] == constants.DDM_ADD: - raise errors.OpPrereqError("Can't add a disk to an instance with" + if mod[0] in (constants.DDM_ADD, constants.DDM_ATTACH): + raise errors.OpPrereqError("Can't %s a disk to an instance with" " deactivated disks and" - " --no-wait-for-sync given.", + " --no-wait-for-sync given" % mod[0], errors.ECODE_INVAL) def _PrepareDiskMod(_, disk, params, __): -- 1.7.10.4
