The modifications to ConfigState for AddInstanceDisk and AttachInstanceDisk are now done in WConfd daemon instead of using ConfigSync to do the modification from python codebase.
This eliminates _UnlockedAddInstanceDisk, and _UnlockedAttachInstanceDisk, but introduces them in the python tests to simulate the behaviour of AddInstanceDisk and AttachInstanceDisk without making a call to WConfd. Signed-off-by: BSRK Aditya <[email protected]> --- lib/config/__init__.py | 87 +++---------------- src/Ganeti/WConfd/ConfigModifications.hs | 133 ++++++++++++++++++++++++++++-- test/py/ganeti.config_unittest.py | 13 --- test/py/testutils/config_mock.py | 24 ++++++ 4 files changed, 160 insertions(+), 97 deletions(-) diff --git a/lib/config/__init__.py b/lib/config/__init__.py index 5633b54..2f471d9 100644 --- a/lib/config/__init__.py +++ b/lib/config/__init__.py @@ -313,88 +313,21 @@ class ConfigWriter(object): """ return self._UnlockedGetInstanceDisks(inst_uuid) - def _UnlockedAddDisk(self, disk, replace=False): - """Add a disk to the config. - - @type disk: L{objects.Disk} - @param disk: The disk object - - """ + def AddInstanceDisk(self, inst_uuid, disk, idx=None, replace=False): + """Add a disk to the config and attach it to instance.""" if not isinstance(disk, objects.Disk): - raise errors.ProgrammerError("Invalid type passed to _UnlockedAddDisk") - - logging.info("Adding disk %s to configuration", disk.uuid) + raise errors.ProgrammerError("Invalid type passed to AddInstanceDisk") - if replace: - self._CheckUUIDpresent(disk) - else: - self._CheckUniqueUUID(disk, include_temporary=False) - disk.serial_no = 1 - disk.ctime = disk.mtime = time.time() disk.UpgradeConfig() - self._ConfigData().disks[disk.uuid] = disk - self._ConfigData().cluster.serial_no += 1 - self._UnlockedReleaseDRBDMinors(disk.uuid) - - def _UnlockedAttachInstanceDisk(self, inst_uuid, disk_uuid, idx=None): - """Attach a disk to an instance. - - @type inst_uuid: string - @param inst_uuid: The UUID of the instance object - @type disk_uuid: string - @param disk_uuid: The UUID of the disk object - @type idx: int - @param idx: the index of the newly attached disk; if not - passed, the disk will be attached as the last one. - - """ - instance = self._UnlockedGetInstanceInfo(inst_uuid) - if instance is None: - raise errors.ConfigurationError("Instance %s doesn't exist" - % inst_uuid) - if disk_uuid not in self._ConfigData().disks: - raise errors.ConfigurationError("Disk %s doesn't exist" % disk_uuid) - - if idx is None: - idx = len(instance.disks) - else: - if idx < 0: - raise IndexError("Not accepting negative indices other than -1") - elif idx > len(instance.disks): - raise IndexError("Got disk index %s, but there are only %s" % - (idx, len(instance.disks))) - - # Disk must not be attached anywhere else - for inst in self._ConfigData().instances.values(): - if disk_uuid in inst.disks: - raise errors.ReservationError("Disk %s already attached to instance %s" - % (disk_uuid, inst.name)) - - instance.disks.insert(idx, disk_uuid) - instance_disks = self._UnlockedGetInstanceDisks(inst_uuid) - _UpdateIvNames(idx, instance_disks[idx:]) - instance.serial_no += 1 - instance.mtime = time.time() - - @ConfigSync() - def AddInstanceDisk(self, inst_uuid, disk, idx=None, replace=False): - """Add a disk to the config and attach it to instance. - - This is a simple wrapper over L{_UnlockedAddDisk} and - L{_UnlockedAttachInstanceDisk}. - - """ - self._UnlockedAddDisk(disk, replace=replace) - self._UnlockedAttachInstanceDisk(inst_uuid, disk.uuid, idx) + utils.SimpleRetry(True, self._wconfd.AddInstanceDisk, 0.1, 30, + args=[inst_uuid, disk.ToDict(), idx, replace]) + self.OutDate() - @ConfigSync() def AttachInstanceDisk(self, inst_uuid, disk_uuid, idx=None): - """Attach an existing disk to an instance. - - This is a simple wrapper over L{_UnlockedAttachInstanceDisk}. - - """ - self._UnlockedAttachInstanceDisk(inst_uuid, disk_uuid, idx) + """Attach an existing disk to an instance.""" + utils.SimpleRetry(True, self._wconfd.AttachInstanceDisk, 0.1, 30, + args=[inst_uuid, disk_uuid, idx]) + self.OutDate() def _UnlockedDetachInstanceDisk(self, inst_uuid, disk_uuid): """Detach a disk from an instance. diff --git a/src/Ganeti/WConfd/ConfigModifications.hs b/src/Ganeti/WConfd/ConfigModifications.hs index c51b4b1..0141fe7 100644 --- a/src/Ganeti/WConfd/ConfigModifications.hs +++ b/src/Ganeti/WConfd/ConfigModifications.hs @@ -39,28 +39,43 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. module Ganeti.WConfd.ConfigModifications where -import Control.Lens.Setter ((.~)) +import Control.Lens.Setter ((.~), (%~)) import Control.Lens.Traversal (mapMOf) -import Control.Monad (unless) +import Control.Monad (unless, when, forM_) import Control.Monad.IO.Class (liftIO) -import Data.Maybe (isJust, maybeToList) +import Data.Maybe (isJust, maybeToList, fromMaybe) import Language.Haskell.TH (Name) -import System.Time (getClockTime) +import System.Time (getClockTime, ClockTime) import Text.Printf (printf) import qualified Data.Map as M import qualified Data.Set as S -import Ganeti.BasicTypes (GenericResult(..), toError) +import Ganeti.BasicTypes (GenericResult(..), genericResult, toError) import Ganeti.Errors (GanetiException(..)) -import Ganeti.JSON (GenericContainer(..), alterContainerL) +import Ganeti.JSON (Container, GenericContainer(..), alterContainerL + , lookupContainer, MaybeForJSON(..)) import Ganeti.Locking.Locks (ClientId, ciIdentifier) -import Ganeti.Logging.Lifted (logDebug) +import Ganeti.Logging.Lifted (logDebug, logInfo) import Ganeti.Objects import Ganeti.Objects.Lens import Ganeti.WConfd.ConfigState (ConfigState, csConfigData, csConfigDataL) import Ganeti.WConfd.Monad (WConfdMonad, modifyConfigWithLock) import qualified Ganeti.WConfd.TempRes as T +type DiskUUID = String +type InstanceUUID = String + +-- * accessor functions + +getInstanceByUUID :: ConfigState + -> InstanceUUID + -> GenericResult GanetiException Instance +getInstanceByUUID cs uuid = lookupContainer + (Bad . ConfigurationError $ + printf "Could not find instance with UUID %s" uuid) + uuid + (configInstances . csConfigData $ cs) + -- * getters -- | Gets all logical volumes in the cluster @@ -158,6 +173,77 @@ addInstanceChecks inst replace cs = do "Cannot replace %s: UUID %s not present" (show $ instName inst) (instUuid inst) +addDiskChecks :: Disk + -> Bool + -> ConfigState + -> GenericResult GanetiException () +addDiskChecks disk replace cs = + if replace + then + unless (checkUUIDpresent cs disk) . Bad . ConfigurationError $ printf + "Cannot add %s: UUID %s already in use" + (show $ diskName disk) (diskUuid disk) + else + unless (checkUniqueUUID cs disk) . Bad . ConfigurationError $ printf + "Cannot replace %s: UUID %s not present" + (show $ diskName disk) (diskUuid disk) + +attachInstanceDiskChecks :: InstanceUUID + -> DiskUUID + -> MaybeForJSON Int + -> ConfigState + -> GenericResult GanetiException () +attachInstanceDiskChecks uuidInst uuidDisk idx' cs = do + let diskPresent = elem uuidDisk . map diskUuid . M.elems + . fromContainer . configDisks . csConfigData $ cs + unless diskPresent . Bad . ConfigurationError $ printf + "Disk %s doesn't exist" uuidDisk + + inst <- getInstanceByUUID cs uuidInst + let numDisks = length $ instDisks inst + idx = fromMaybe numDisks (unMaybeForJSON idx') + + when (idx < 0) . Bad . GenericError $ + "Not accepting negative indices" + when (idx > numDisks) . Bad . GenericError $ printf + "Got disk index %d, but there are only %d" idx numDisks + + let insts = M.elems . fromContainer . configInstances . csConfigData $ cs + forM_ insts (\inst' -> when (uuidDisk `elem` instDisks inst') . Bad + . ReservationError $ printf "Disk %s already attached to instance %s" + uuidDisk (show $ instName inst)) + +-- * Pure config modifications functions + +attachInstanceDisk' :: InstanceUUID + -> DiskUUID + -> MaybeForJSON Int + -> ClockTime + -> ConfigState + -> ConfigState +attachInstanceDisk' iUuid dUuid idx' ct cs = + let inst = genericResult (error "impossible") id (getInstanceByUUID cs iUuid) + numDisks = length $ instDisks inst + idx = fromMaybe numDisks (unMaybeForJSON idx') + + insert = instDisksL %~ (\ds -> take idx ds ++ [dUuid] ++ drop idx ds) + incr = instSerialL %~ (+ 1) + time = instMtimeL .~ ct + + inst' = time . incr . insert $ inst + disks = updateIvNames idx inst' (configDisks . csConfigData $ cs) + + ri = csConfigDataL . configInstancesL + . alterContainerL iUuid .~ Just inst' + rds = csConfigDataL . configDisksL .~ disks + in rds . ri $ cs + where updateIvNames :: Int -> Instance -> Container Disk -> Container Disk + updateIvNames idx inst (GenericContainer m) = + let dUuids = drop idx (instDisks inst) + upgradeIv m' (idx'', dUuid') = + M.adjust (diskIvNameL .~ "disk/" ++ show idx'') dUuid' m' + in GenericContainer $ foldl upgradeIv m (zip [idx..] dUuids) + -- * RPCs -- | Add a new instance to the configuration, release DRBD minors, @@ -183,8 +269,41 @@ addInstance inst cid replace = do logDebug $ "AddInstance: result of config modification is " ++ show r return $ isJust r +addInstanceDisk :: InstanceUUID + -> Disk + -> MaybeForJSON Int + -> Bool + -> WConfdMonad Bool +addInstanceDisk iUuid disk idx replace = do + logInfo $ printf "Adding disk %s to configuration" (diskUuid disk) + ct <- liftIO getClockTime + let addD = csConfigDataL . configDisksL . alterContainerL (uuidOf disk) + .~ Just disk + incrSerialNo = csConfigDataL . configSerialL %~ (+1) + r <- modifyConfigWithLock (\_ cs -> do + toError $ addDiskChecks disk replace cs + let cs' = incrSerialNo . addD $ cs + toError $ attachInstanceDiskChecks iUuid (diskUuid disk) idx cs' + return $ attachInstanceDisk' iUuid (diskUuid disk) idx ct cs') + . T.releaseDRBDMinors $ uuidOf disk + return $ isJust r + +attachInstanceDisk :: InstanceUUID + -> DiskUUID + -> MaybeForJSON Int + -> WConfdMonad Bool +attachInstanceDisk iUuid dUuid idx = do + ct <- liftIO getClockTime + r <- modifyConfigWithLock (\_ cs -> do + toError $ attachInstanceDiskChecks iUuid dUuid idx cs + return $ attachInstanceDisk' iUuid dUuid idx ct cs) + (return ()) + return $ isJust r + -- * The list of functions exported to RPC. exportedFunctions :: [Name] exportedFunctions = [ 'addInstance + , 'addInstanceDisk + , 'attachInstanceDisk ] diff --git a/test/py/ganeti.config_unittest.py b/test/py/ganeti.config_unittest.py index 47750fe..e56c723 100755 --- a/test/py/ganeti.config_unittest.py +++ b/test/py/ganeti.config_unittest.py @@ -707,24 +707,11 @@ class TestConfigRunner(unittest.TestCase): self.assertRaises(errors.ProgrammerError, cfg.DetachInstanceDisk, "test-uuid", "disk0") - # Attach disk to non-existent instance - self.assertRaises(errors.ConfigurationError, cfg.AttachInstanceDisk, - "1134", "disk0") - - # Attach non-existent disk - self.assertRaises(errors.ConfigurationError, cfg.AttachInstanceDisk, - "test-uuid", "disk1") - # Attach disk cfg.AttachInstanceDisk("test-uuid", "disk0") instance_disks = cfg.GetInstanceDisks("test-uuid") self.assertEqual(instance_disks, [disk]) - # Attach disk again - self.assertRaises(errors.ReservationError, cfg.AttachInstanceDisk, - "test-uuid", "disk0") - - def _IsErrorInList(err_str, err_list): return any(map(lambda e: err_str in e, err_list)) diff --git a/test/py/testutils/config_mock.py b/test/py/testutils/config_mock.py index ed78e63..0cc4eda 100644 --- a/test/py/testutils/config_mock.py +++ b/test/py/testutils/config_mock.py @@ -859,3 +859,27 @@ class ConfigMock(config.ConfigWriter): self._ConfigData().cluster.serial_no += 1 # pylint: disable=E1103 self._UnlockedReleaseDRBDMinors(instance.uuid) self._UnlockedCommitTemporaryIps(ec_id) + + def _UnlockedAddDisk(self, disk): + disk.UpgradeConfig() + self._ConfigData().disks[disk.uuid] = disk + self._ConfigData().cluster.serial_no += 1 + self._UnlockedReleaseDRBDMinors(disk.uuid) + + def _UnlockedAttachInstanceDisk(self, inst_uuid, disk_uuid, idx=None): + instance = self._UnlockedGetInstanceInfo(inst_uuid) + if idx is None: + idx = len(instance.disks) + instance.disks.insert(idx, disk_uuid) + instance_disks = self._UnlockedGetInstanceDisks(inst_uuid) + for (disk_idx, disk) in enumerate(instance_disks[idx:]): + disk.iv_name = "disk/%s" % (idx + disk_idx) + instance.serial_no += 1 + instance.mtime = time.time() + + def AddInstanceDisk(self, inst_uuid, disk, idx=None, replace=False): + self._UnlockedAddDisk(disk) + self._UnlockedAttachInstanceDisk(inst_uuid, disk.uuid, idx) + + def AttachInstanceDisk(self, inst_uuid, disk_uuid, idx=None): + self._UnlockedAttachInstanceDisk(inst_uuid, disk_uuid, idx) -- 1.7.10.4
