Check real spindles in ipolicies

When exclusive storage is enabled, the spindles in instance disks are used
to check the instance policies (as outlined in design-partitioned.rst).

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Klaus Aehlig <aehlig@google.com>

diff --git a/lib/cmdlib/common.py b/lib/cmdlib/common.py
index 4759736..571a69f 100644 (file)
--- a/lib/cmdlib/common.py
+++ b/lib/cmdlib/common.py
@@ -544,17 +544,30 @@ def ComputeIPolicyInstanceViolation(ipolicy, instance, cfg,
   @see: L{ComputeIPolicySpecViolation}
 
   """
+  ret = []
   be_full = cfg.GetClusterInfo().FillBE(instance)
   mem_size = be_full[constants.BE_MAXMEM]
   cpu_count = be_full[constants.BE_VCPUS]
-  spindle_use = be_full[constants.BE_SPINDLE_USE]
+  es_flags = rpc.GetExclusiveStorageForNodeNames(cfg, instance.all_nodes)
+  if any(es_flags.values()):
+    # With exclusive storage use the actual spindles
+    try:
+      spindle_use = sum([disk.spindles for disk in instance.disks])
+    except TypeError:
+      ret.append("Number of spindles not configured for disks of instance %s"
+                 " while exclusive storage is enabled, try running gnt-cluster"
+                 " repair-disk-sizes" % instance.name)
+      # _ComputeMinMaxSpec ignores 'None's
+      spindle_use = None
+  else:
+    spindle_use = be_full[constants.BE_SPINDLE_USE]
   disk_count = len(instance.disks)
   disk_sizes = [disk.size for disk in instance.disks]
   nic_count = len(instance.nics)
   disk_template = instance.disk_template
 
-  return _compute_fn(ipolicy, mem_size, cpu_count, disk_count, nic_count,
-                     disk_sizes, spindle_use, disk_template)
+  return ret + _compute_fn(ipolicy, mem_size, cpu_count, disk_count, nic_count,
+                           disk_sizes, spindle_use, disk_template)
 
 
 def _ComputeViolatingInstances(ipolicy, instances, cfg):

diff --git a/src/Ganeti/HTools/Cluster.hs b/src/Ganeti/HTools/Cluster.hs
index 7b4a67c..582530b 100644 (file)
--- a/src/Ganeti/HTools/Cluster.hs
+++ b/src/Ganeti/HTools/Cluster.hs
@@ -489,7 +489,7 @@ allocateOnSingle nl inst new_pdx =
   let p = Container.find new_pdx nl
       new_inst = Instance.setBoth inst new_pdx Node.noSecondary
   in do
-    Instance.instMatchesPolicy inst (Node.iPolicy p)
+    Instance.instMatchesPolicy inst (Node.iPolicy p) (Node.exclStorage p)
     new_p <- Node.addPri p inst
     let new_nl = Container.add new_pdx new_p nl
         new_score = compCV new_nl
@@ -503,6 +503,7 @@ allocateOnPair nl inst new_pdx new_sdx =
       tgt_s = Container.find new_sdx nl
   in do
     Instance.instMatchesPolicy inst (Node.iPolicy tgt_p)
+      (Node.exclStorage tgt_p)
     new_p <- Node.addPri tgt_p inst
     new_s <- Node.addSec tgt_s inst new_pdx
     let new_inst = Instance.setBoth inst new_pdx new_sdx

diff --git a/src/Ganeti/HTools/Instance.hs b/src/Ganeti/HTools/Instance.hs
index 44c5afd..69c563a 100644 (file)
--- a/src/Ganeti/HTools/Instance.hs
+++ b/src/Ganeti/HTools/Instance.hs
@@ -59,6 +59,8 @@ module Ganeti.HTools.Instance
   , mirrorType
   ) where
 
+import Control.Monad (liftM2)
+
 import Ganeti.BasicTypes
 import qualified Ganeti.HTools.Types as T
 import qualified Ganeti.HTools.Container as Container
@@ -278,13 +280,18 @@ specOf Instance { mem = m, dsk = d, vcpus = c } =
 -- | Checks if an instance is smaller/bigger than a given spec. Returns
 -- OpGood for a correct spec, otherwise Bad one of the possible
 -- failure modes.
-instCompareISpec :: Ordering -> Instance-> T.ISpec -> T.OpResult ()
-instCompareISpec which inst ispec
+instCompareISpec :: Ordering -> Instance-> T.ISpec -> Bool -> T.OpResult ()
+instCompareISpec which inst ispec exclstor
   | which == mem inst `compare` T.iSpecMemorySize ispec = Bad T.FailMem
   | which `elem` map ((`compare` T.iSpecDiskSize ispec) . dskSize)
     (disks inst) = Bad T.FailDisk
   | which == vcpus inst `compare` T.iSpecCpuCount ispec = Bad T.FailCPU
-  | which == spindleUse inst `compare` T.iSpecSpindleUse ispec
+  | exclstor &&
+    case getTotalSpindles inst of
+      Nothing -> True
+      Just sp_sum -> which == sp_sum `compare` T.iSpecSpindleUse ispec
+    = Bad T.FailSpindles
+  | not exclstor && which == spindleUse inst `compare` T.iSpecSpindleUse ispec
     = Bad T.FailSpindles
   | diskTemplate inst /= T.DTDiskless &&
     which == length (disks inst) `compare` T.iSpecDiskCount ispec
@@ -292,33 +299,33 @@ instCompareISpec which inst ispec
   | otherwise = Ok ()
 
 -- | Checks if an instance is smaller than a given spec.
-instBelowISpec :: Instance -> T.ISpec -> T.OpResult ()
+instBelowISpec :: Instance -> T.ISpec -> Bool -> T.OpResult ()
 instBelowISpec = instCompareISpec GT
 
 -- | Checks if an instance is bigger than a given spec.
-instAboveISpec :: Instance -> T.ISpec -> T.OpResult ()
+instAboveISpec :: Instance -> T.ISpec -> Bool -> T.OpResult ()
 instAboveISpec = instCompareISpec LT
 
 -- | Checks if an instance matches a min/max specs pair
-instMatchesMinMaxSpecs :: Instance -> T.MinMaxISpecs -> T.OpResult ()
-instMatchesMinMaxSpecs inst minmax = do
-  instAboveISpec inst (T.minMaxISpecsMinSpec minmax)
-  instBelowISpec inst (T.minMaxISpecsMaxSpec minmax)
+instMatchesMinMaxSpecs :: Instance -> T.MinMaxISpecs -> Bool -> T.OpResult ()
+instMatchesMinMaxSpecs inst minmax exclstor = do
+  instAboveISpec inst (T.minMaxISpecsMinSpec minmax) exclstor
+  instBelowISpec inst (T.minMaxISpecsMaxSpec minmax) exclstor
 
 -- | Checks if an instance matches any specs of a policy
-instMatchesSpecs :: Instance -> [T.MinMaxISpecs] -> T.OpResult ()
+instMatchesSpecs :: Instance -> [T.MinMaxISpecs] -> Bool -> T.OpResult ()
  -- Return Ok for no constraints, though this should never happen
-instMatchesSpecs _ [] = Ok ()
-instMatchesSpecs inst minmaxes =
+instMatchesSpecs _ [] _ = Ok ()
+instMatchesSpecs inst minmaxes exclstor =
   -- The initial "Bad" should be always replaced by a real result
   foldr eithermatch (Bad T.FailInternal) minmaxes
-  where eithermatch mm (Bad _) = instMatchesMinMaxSpecs inst mm
+  where eithermatch mm (Bad _) = instMatchesMinMaxSpecs inst mm exclstor
         eithermatch _ y@(Ok ()) = y
 
 -- | Checks if an instance matches a policy.
-instMatchesPolicy :: Instance -> T.IPolicy -> T.OpResult ()
-instMatchesPolicy inst ipol = do
-  instMatchesSpecs inst $ T.iPolicyMinMaxISpecs ipol
+instMatchesPolicy :: Instance -> T.IPolicy -> Bool -> T.OpResult ()
+instMatchesPolicy inst ipol exclstor = do
+  instMatchesSpecs inst (T.iPolicyMinMaxISpecs ipol) exclstor
   if diskTemplate inst `elem` T.iPolicyDiskTemplates ipol
     then Ok ()
     else Bad T.FailDisk

diff --git a/test/hs/Test/Ganeti/HTools/Cluster.hs b/test/hs/Test/Ganeti/HTools/Cluster.hs
index 75525b4..bb11ac9 100644 (file)
--- a/test/hs/Test/Ganeti/HTools/Cluster.hs
+++ b/test/hs/Test/Ganeti/HTools/Cluster.hs
@@ -394,8 +394,9 @@ prop_AllocPolicy =
   forAll genOnlineNode $ \node ->
   forAll (choose (5, 20)) $ \count ->
   forAll (genInstanceSmallerThanNode node) $ \inst ->
-  forAll (arbitrary `suchThat` (isBad .
-                                Instance.instMatchesPolicy inst)) $ \ipol ->
+  forAll (arbitrary `suchThat`
+          (isBad . flip (Instance.instMatchesPolicy inst)
+           (Node.exclStorage node))) $ \ipol ->
   let rqn = Instance.requiredNodes $ Instance.diskTemplate inst
       node' = Node.setPolicy ipol node
       nl = makeSmallCluster node' count

diff --git a/test/py/ganeti.cmdlib_unittest.py b/test/py/ganeti.cmdlib_unittest.py
index 21b2c26..0ee3742 100755 (executable)
--- a/test/py/ganeti.cmdlib_unittest.py
+++ b/test/py/ganeti.cmdlib_unittest.py
@@ -827,12 +827,21 @@ class _StubComputeIPolicySpecViolation:
 
 
 class _FakeConfigForComputeIPolicyInstanceViolation:
-  def __init__(self, be):
+  def __init__(self, be, excl_stor):
     self.cluster = objects.Cluster(beparams={"default": be})
+    self.excl_stor = excl_stor
 
   def GetClusterInfo(self):
     return self.cluster
 
+  def GetNodeInfo(self, _):
+    return {}
+
+  def GetNdParams(self, _):
+    return {
+      constants.ND_EXCLUSIVE_STORAGE: self.excl_stor,
+      }
+
 
 class TestComputeIPolicyInstanceViolation(unittest.TestCase):
   def test(self):
@@ -841,8 +850,8 @@ class TestComputeIPolicyInstanceViolation(unittest.TestCase):
       constants.BE_VCPUS: 2,
       constants.BE_SPINDLE_USE: 4,
       }
-    disks = [objects.Disk(size=512)]
-    cfg = _FakeConfigForComputeIPolicyInstanceViolation(beparams)
+    disks = [objects.Disk(size=512, spindles=13)]
+    cfg = _FakeConfigForComputeIPolicyInstanceViolation(beparams, False)
     instance = objects.Instance(beparams=beparams, disks=disks, nics=[],
                                 disk_template=constants.DT_PLAIN)
     stub = _StubComputeIPolicySpecViolation(2048, 2, 1, 0, [512], 4,
@@ -855,6 +864,15 @@ class TestComputeIPolicyInstanceViolation(unittest.TestCase):
     ret = common.ComputeIPolicyInstanceViolation(NotImplemented, instance2,
                                                  cfg, _compute_fn=stub)
     self.assertEqual(ret, [])
+    cfg_es = _FakeConfigForComputeIPolicyInstanceViolation(beparams, True)
+    stub_es = _StubComputeIPolicySpecViolation(2048, 2, 1, 0, [512], 13,
+                                               constants.DT_PLAIN)
+    ret = common.ComputeIPolicyInstanceViolation(NotImplemented, instance,
+                                                 cfg_es, _compute_fn=stub_es)
+    self.assertEqual(ret, [])
+    ret = common.ComputeIPolicyInstanceViolation(NotImplemented, instance2,
+                                                 cfg_es, _compute_fn=stub_es)
+    self.assertEqual(ret, [])
 
 
 class TestComputeIPolicyInstanceSpecViolation(unittest.TestCase):