1. 12 Nov, 2012 1 commit
    • Iustin Pop's avatar
      Change type of program options to 'IO [Options]' · d66aa238
      Iustin Pop authored
      
      
      Some options have defaults that depend on the environment, and we
      could handle these in two ways:
      
      - use a place-holder value (e.g. data X a = Default | Custom a) that
        is later read from the environment
      - move the options list to IO monad, where it can read the
        environment, etc.
      
      The second option allows also displaying the actual defaults in the
      `--help' output, even though it's not as nice, so I went with it.
      
      This patch only changes the option types, without actually changing
      any options yet.
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarHelga Velroyen <helgav@google.com>
      d66aa238
  2. 07 Nov, 2012 1 commit
    • Iustin Pop's avatar
      Fix compatibility with newer Haskell libraries · 1251817b
      Iustin Pop authored
      
      
      This small patch fixes compatibility with a few newer Haskell libraries:
      
      - base 4.6, included with ghc 7.6, removed the deprecated 'catch'
        function from Prelude, so our "import Prelude hiding (catch)" is now
        an error; we workaround by using fully-qualified
        Control.Exception.catch name
      
      - containers 0.5 changed the signature of 'deleteFindMax'; we
        workaround by using separate 'findMax' and 'deleteMax'
      
      - QuickCheck 2.5 removed the 'maxDiscards' test parameter, replacing
        it with a much better 'maxDiscardsRatio'; however, until we can
        depend on that, we workaround by just removing it (we don't control
        anymore the maxDiscards, instead leaving it default; for our default
        test size, this is no change, as the default value is already 500,
        which is our default as well) and not printing it anymore
      
      Tested on Squeeze (+extra libs), Wheezy and experimental, which covers
      all supported GHC versions.
      
      Also, merging this in master will be a pain, but unless we want to
      stop supporting 2.6…
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarGuido Trotter <ultrotter@google.com>
      1251817b
  3. 26 Oct, 2012 3 commits
    • Iustin Pop's avatar
      Convert Luxi results to Ganeti errors · 7adb7dff
      Iustin Pop authored
      
      
      This a bit too complex patch converts the result of Luxi calls
      (submitJob, query*, etc.) from Result to ErrorResult. It then
      immediately revers this in the HTools/Backend/Luxi module, where we
      don't need necessarily the full error type (just a nice error
      message), and does the same in Hbal's job execution functions.
      
      While at first sight this doesn't seem to do much, what we get is
      actual error messages from Ganeti, plus improvements to the result
      parsing: instead of "can't parse char", we now get properly (note,
      wrapped manually):
      
        Executing jobset for instances instance1, …
        Job submission error: Failure: the job queue is marked for drain and
          doesn't accept new requests
      
      Or:
      
        Job submission error: Unhandled exception: LuxiError "parsing job
          id: cannot parse string 'a956101'"
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarMichael Hanselmann <hansmi@google.com>
      7adb7dff
    • Iustin Pop's avatar
      Move htools backends to a separate directory · 879d9290
      Iustin Pop authored
      
      
      Five modules under the HTools/ directories are backend
      implementations, so let's move them to a separate directory, to more
      clearly show the hierarchy. I wanted to do this for a while, but
      merging between branches is always an issue, so let's do it know since
      we have an opportunity.
      
      This patch contains the actual renames, the required changed module
      names, imports, etc., but no other changes.
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarMichael Hanselmann <hansmi@google.com>
      879d9290
    • Iustin Pop's avatar
      Fix a few issues found by newer hlint · 66ad857a
      Iustin Pop authored
      
      
      Testing with a newer hlint found a few minor issues; but all are real,
      valid recommendations:
      
      - don't use "if cond then f x else f y", but "f (if cond then x else y)"
      - "if a then b else True" is equivalent to the simpler "not a || b"
      - and as usual, one more ignore to our "testing basic properties"
        module
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarMichael Hanselmann <hansmi@google.com>
      66ad857a
  4. 22 Oct, 2012 2 commits
  5. 18 Oct, 2012 2 commits
  6. 17 Oct, 2012 2 commits
    • Dato Simó's avatar
      Group.hs: add 'allTags'; adjust loaders and test data for it · 6b6e335b
      Dato Simó authored
      
      
      This commit adds a Group.allTags field to store the tags of node groups,
      and teaches each loader backend in HTools to populate it (additionally, the
      IAllocator class in lib/cmdlib.py now includes tags for groups too). Test
      data is updated to include an empty set of tags for node groups in all
      affected test cases.
      Signed-off-by: default avatarDato Simó <dato@google.com>
      Reviewed-by: default avatarIustin Pop <iustin@google.com>
      6b6e335b
    • Dato Simó's avatar
      Instance.hs: rename 'tags' to 'exclTags', provide 'allTags' · 2f907bad
      Dato Simó authored
      
      
      The mergeData function in Loader.hs included a step to filter an instance's
      tags to include only the exclusion tags (as specified via the commandline,
      or cluster-level tags). Later on, code in Node.hs assumed Instance.tags to
      contain only tags to be used for exclusion.
      
      Because in the future we will need to access the full list of an instance's
      tags (and not only exclusion tags), this commits deprecates the 'tags'
      field, and introduces Instance.exclTags and Instance.allTags.
      
      Instance.allTags is now populated from the different backends (Text, Luxi,
      Rapi, etc.), and Instance.exclTags is only populated from Loader.mergeData,
      as was done previously. This means that loading tags from e.g. Text or Simu
      and assuming that they'll be used as exclusion tags without going through
      Loader.hs will no longer work; but this was already the case with other
      fields, and 'mergeData' or 'loadExternalData' continue to be the only entry
      points to get a consistent view of the cluster. (Additionally, there were
      no tests that made this assumption that I could find.)
      Signed-off-by: default avatarDato Simó <dato@google.com>
      Reviewed-by: default avatarIustin Pop <iustin@google.com>
      2f907bad
  7. 15 Oct, 2012 1 commit
    • Iustin Pop's avatar
      Cleanup HTools.Types/BasicTypes imports · 01e52493
      Iustin Pop authored
      
      
      Before we reorganised the source tree, the 'Result' type was exported
      from HTools/Types.hs. This changed during the reorg, but at that time
      we didn't change the exports; instead, we kept re-exporting it from
      the old module for compatibility.
      
      In light of future changes to the Result type, let's stop this
      re-export and cleanup the imports of the other modules.
      
      One test is slightly rewritten with explicit types declaration (part
      of the values already needed one, let's make it explicit).
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarMichael Hanselmann <hansmi@google.com>
      01e52493
  8. 11 Oct, 2012 2 commits
  9. 08 Oct, 2012 3 commits
  10. 26 Sep, 2012 5 commits
  11. 18 Sep, 2012 1 commit
  12. 07 Sep, 2012 1 commit
    • Iustin Pop's avatar
      Fix bug in non-mirrored instance allocation · 14b5d45f
      Iustin Pop authored
      
      
      The function `allocateOnSingle' has a bug in the calculation of the
      cluster score used for deciding which of the many target nodes to use
      in placing the instance: it uses the original node list for the score
      calculation.
      
      Due to this, since the original node list is the same for all target
      nodes, it means that basically `allocateOnSingle' returns the same
      score, no matter the target node, and hence the choosing of the node
      is arbitrary, instead of being done on the basis of the algorithm.
      
      This has gone uncaught until reported because the unittests only test
      1 allocation at a time on an empty cluster, and do not check the
      consistency of the score. I'll send separate patches on the master
      branch for adding more checks to prevent this in the future.
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarAgata Murawska <agatamurawska@google.com>
      14b5d45f
  13. 05 Sep, 2012 3 commits
    • Iustin Pop's avatar
      Further hlint fixes · 5b11f8db
      Iustin Pop authored
      Commit 2cdaf225, “Re-enable standard hlint warnings”, got it almost
      right. The only problem is that (confusingly) the default set of hints
      is not in HLint.Default, but in HLint.HLint (it includes Default and
      some built-ins).
      
      After changing the lint file to correctly include the defaults, we had
      another 128 suggestions:
      
        - Error: Eta reduce (2)
        - Error: Redundant bracket (4)
        - Error: Redundant do (17)
        - Error: Redundant lambda (7)
        - Error: Redundant return (1)
        - Warning: Avoid lambda (2)
        - Warning: Redundant $ (42)
        - Warning: Redundant bracket (35)
        - Warning: Use : (1)
        - Warning: Use String (4)
        - Warning: Use camelCase (10)
        - Warning: Use section (3)
      
      which are fixed by the current patch. Note that the 10 "Use camelCase"
      were all due to hlint not “knowing” the idiom of ‘case_’ (it does for
      ‘prop_’), for which I filled
      http://code.google.com/p/ndmitchell/issues/detail?id=558
      
      .
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarRené Nussbaumer <rn@google.com>
      5b11f8db
    • Iustin Pop's avatar
      Rework CLI modules and tests · 51000365
      Iustin Pop authored
      While investigating how we could test the Daemon.hs module, I realised
      that we have a very, erm, sub-optimal situation:
      
      - HTools/CLI.hs has a nice IO/pure separation testing in cmdline
        parsing, which allows some basic functionality to be tested, but
        uses direct 'read' in many options, which fails at runtime when
        evaluating the argument, and not when parsing the options
      - Daemon.hs lacks that, but has a much nicer 'reqWithConversion'
        helper that can be used for nicer option parsing, and uses that +
        tryRead instead of plain 'read'
      
      Since this situation is very bad, let's clean it up. We introduce yet
      another module, Common.hs, that holds functionality common to all
      command line programs (daemons or not). We move the parsing to this
      module, and introduce a type class to handle option types which
      support --help/--version. This allows removal of duplicated code from
      CLI.hs and Daemon.hs.
      
      The other part of the patch is cleanup/rework of the tests for this
      code: we introduce some helpers (checkOpt, passFailOpt,
      checkEarlyExit) that can be used from the much-slimmer now tests for
      CLI and Daemon. In the common module, we just test the yes/no helper
      we have. Many new tests for boolean options and numeric options are
      added.
      
      A side change is the removal of the obsolete `--replay-count',
      `--test-size' options (unused since commit 95f6c931
      
      , “Switch Haskell
      test harness to test-framework”).
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarRené Nussbaumer <rn@google.com>
      51000365
    • Iustin Pop's avatar
      Move Version.hs up from under HTools/ · 2997cb0a
      Iustin Pop authored
      
      
      This is another module that is generic, and not htools-specific.
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarRené Nussbaumer <rn@google.com>
      2997cb0a
  14. 04 Sep, 2012 13 commits