Re: Issue 1196 in ganeti: Htools location tag mechanismus for non-DRBD instances

2016-12-05 Thread ganeti


Comment #1 on issue 1196 by gannef...@googlemail.com: Htools location tag  
mechanismus for non-DRBD instances

https://code.google.com/p/ganeti/issues/detail?id=1196

And as I tripped myself rereading it, some more:

VM1/3 CAN migrate to Node B (same tag).
VM2/4 CAN migrate to Node D (same tag).

Add two more nodes:

node E is tagged enclosure:11
node F is tagged enclosure:11

One of VM1/3 CAN migrate to one of Node E/F
One of VM2/4 CAN migrate to one of Node E/F

(And in an hbal run with nodes E/F being fresh and empty, they SHOULD  
migrate)


Now, i think order matters: Those xlocation checks should come before other  
checks, ie. iextags are checked after xlocation and may rule out a  
migration.


Joerg

--
You received this message because this project is configured to send all  
issue notifications to this address.

You may adjust your notification preferences at:
https://code.google.com/hosting/settings


Re: [PATCH stable-2.15 00/37] Cleanup for pylint 1.6.4

2016-12-05 Thread 'Brian Foley' via ganeti-devel
On Mon, Dec 05, 2016 at 03:21:32PM +, Federico Pareschi wrote:
>I'm replying here for all the 37 patches, after addressing all the
>comments,
>I'd say this is an LGTM for me :)
>Thanks Brian for the good work.
>Cheers,
>Morg.

Committed, thanks. Now, onto the next batch... :)

Cheers,
Brian.


Re: [PATCH master 0/6] Clear/remove public/private OS params in gnt-instance modify/reinstall (version 2)

2016-12-05 Thread 'Brian Foley' via ganeti-devel
On Mon, Dec 05, 2016 at 04:41:33PM +0200, Yiannis Tsiouris wrote:
> This patchset extends 'gnt-instance modify' and 'gnt-instance reinstall'
> functionality by allowing partial/total removal of current OS parameters of an
> instance. This is specifically useful for preparing or performing an instance
> reinstall to a different OS provider or the same OS provider that changed its
> OS parameter policy/supported list.
> 
> Nikos Skalkotos (1):
>   Add cmdlib tests for removing instance OS params
> 
> Yiannis Tsiouris (5):
>   Add clear OS params options in gnt-instance modify
>   Support OS params removal in gnt-instance modify
>   Test new options of gnt-instance modify in RAPI
>   Support clear OS params in gnt-instance reinstall
>   Add OS params removal in gnt-instance reinstall

LGTM. Committed with a minor tweak to make pep8 happy.

Cheers,
Brian.


Re: [PATCH stable-2.15 00/37] Cleanup for pylint 1.6.4

2016-12-05 Thread 'Federico Pareschi' via ganeti-devel
I'm replying here for all the 37 patches, after addressing all the comments,
I'd say this is an LGTM for me :)

Thanks Brian for the good work.

Cheers,
Morg.

On 5 December 2016 at 12:37, 'Iustin Pop' via ganeti-devel <
ganeti-devel@googlegroups.com> wrote:

> On 5 December 2016 at 13:04, 'Brian Foley' via ganeti-devel <
> ganeti-devel@googlegroups.com> wrote:
>
>> On Mon, Dec 05, 2016 at 12:01:14PM +0100, Iustin Pop wrote:
>> >Quick question: is there a reason to keep that compat, as opposed to
>> >switching the "blessed" (and required) pylint version to 1.6.4?
>> >(If this is addressed in the patch series, sorry, didn't read the
>> >patches)
>> >iustin
>>
>> Hi Iustin,
>>
>> no, there isn't any very good reason, I was just playing it safe, and
>> kinda
>> assumed that make dist or the debian build process might run pylint, but
>> it
>> appears tht neither do.
>>
>
> As long as we require a fixed version, it's not really feasible to do so.
> Similarly how -Werror is only enabled in developer builds, I guess.
>
> In that case, I'll move us to requiring 1.6.4, and remove a couple of the
>> compatibility workarounds.
>>
>
> Yes please, that sounds cleaner.
>
> thanks,
> iustin
>


Re: [PATCH stable-2.15 08/37] Cleanup: Remove unused format key

2016-12-05 Thread 'Federico Pareschi' via ganeti-devel
Ah yes, you're correct.

Looks good then, thanks.

On 5 December 2016 at 15:18, Brian Foley  wrote:

> On Mon, Dec 05, 2016 at 12:01:08PM +, Federico Pareschi wrote:
> >This is an LGTM for me, however it made me wonder...
> >We are adamant in not storing the secret parameters anywhere on the
> >cluster,
> >at least according to our design docs. Wouldn't this be going against
> >such policy
> >by having them printed out in an error (and, I assume, logged to
> disk)?
> >This is probably not the correct patchset to debate this, but it'd be
> >useful to maybe
> >investigate this just to make sure we are not accidentally leaking
> data
> >that we
> >promised, in our design docs, we wouldn't.
>
> I think this should be OK, because it's only showing secret OS parameter
> names,
> not the values, which, presumably are the bit.
>
> Cheers,
> Brian.
>


Re: [PATCH stable-2.15 08/37] Cleanup: Remove unused format key

2016-12-05 Thread 'Brian Foley' via ganeti-devel
On Mon, Dec 05, 2016 at 12:01:08PM +, Federico Pareschi wrote:
>This is an LGTM for me, however it made me wonder...
>We are adamant in not storing the secret parameters anywhere on the
>cluster,
>at least according to our design docs. Wouldn't this be going against
>such policy
>by having them printed out in an error (and, I assume, logged to disk)?
>This is probably not the correct patchset to debate this, but it'd be
>useful to maybe
>investigate this just to make sure we are not accidentally leaking data
>that we
>promised, in our design docs, we wouldn't.

I think this should be OK, because it's only showing secret OS parameter names,
not the values, which, presumably are the bit.

Cheers,
Brian.


[PATCH stable-2.15 15/36] Replace deprecated pylint >=0.27 pragma with new form

2016-12-05 Thread 'Brian Foley' via ganeti-devel
The disable-all pragma was deprecated in favour of the new skip-file
pragma in pylint >= 0.27. Since we're moving to 1.6.4, use this instead.

Signed-off-by: Brian Foley 
---
 src/Ganeti/THH/PyRPC.hs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Ganeti/THH/PyRPC.hs b/src/Ganeti/THH/PyRPC.hs
index eee1554..d25c811 100644
--- a/src/Ganeti/THH/PyRPC.hs
+++ b/src/Ganeti/THH/PyRPC.hs
@@ -181,7 +181,7 @@ genPyUDSRpcStub className constName = liftM (header $+$) .
   namesToClass className stubCode
   where
 header = text "# This file is automatically generated, do not edit!" $+$
- text "# pylint: disable-all"
+ text "# pylint: skip-file"
 stubCode =
   abstrMethod genericInvokeName [ text "method", text "*args"] $+$
   method socketPathName [] (
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 14/36] Delete old warning disables removed from pylint 1.6

2016-12-05 Thread 'Brian Foley' via ganeti-devel
These include star-args and r0924: badly implemented immutable.

Signed-off-by: Brian Foley 
---
 autotools/build-bash-completion   | 4 
 lib/build/sphinx_ext.py   | 3 +--
 lib/cli.py| 4 ++--
 lib/client/gnt_debug.py   | 1 -
 lib/client/gnt_instance.py| 3 +--
 lib/client/gnt_network.py | 1 -
 lib/cmdlib/cluster/verify.py  | 2 +-
 lib/cmdlib/instance_storage.py| 1 -
 lib/cmdlib/instance_utils.py  | 2 +-
 lib/cmdlib/network.py | 4 ++--
 lib/cmdlib/query.py   | 2 --
 lib/compat.py | 2 +-
 lib/config/__init__.py| 2 +-
 lib/errors.py | 1 -
 lib/hypervisor/hv_lxc.py  | 2 +-
 lib/objects.py| 2 +-
 lib/rapi/baserlib.py  | 2 +-
 lib/rapi/testutils.py | 2 +-
 lib/rpc/node.py   | 2 +-
 lib/server/noded.py   | 5 +
 lib/server/rapi.py| 2 +-
 lib/storage/container.py  | 2 +-
 lib/tools/burnin.py   | 4 ++--
 lib/utils/algo.py | 2 --
 lib/utils/retry.py| 3 ---
 lib/utils/text.py | 3 +--
 lib/workerpool.py | 2 +-
 test/py/cmdlib/testsupport/cmdlib_testcase.py | 2 --
 test/py/testutils/config_mock.py  | 2 +-
 tools/confd-client| 3 ---
 tools/ganeti-listrunner   | 2 +-
 31 files changed, 24 insertions(+), 50 deletions(-)

diff --git a/autotools/build-bash-completion b/autotools/build-bash-completion
index 3e35ee1..4a064ba 100755
--- a/autotools/build-bash-completion
+++ b/autotools/build-bash-completion
@@ -703,8 +703,6 @@ def HaskellOptToOptParse(opts, kind):
   kept in sync
 
   """
-  # pylint: disable=W0142
-  # since we pass *opts in a number of places
   opts = opts.split(",")
   if kind == "none":
 return cli.cli_option(*opts, action="store_true")
@@ -761,8 +759,6 @@ def HaskellArgToCliArg(kind, min_cnt, max_cnt):
 max_cnt = None
   else:
 max_cnt = int(max_cnt)
-  # pylint: disable=W0142
-  # since we pass **kwargs
   kwargs = {"min": min_cnt, "max": max_cnt}
 
   if kind.startswith("choices=") or kind.startswith("suggest="):
diff --git a/lib/build/sphinx_ext.py b/lib/build/sphinx_ext.py
index 2150288..6b30894 100644
--- a/lib/build/sphinx_ext.py
+++ b/lib/build/sphinx_ext.py
@@ -281,9 +281,8 @@ def PythonEvalRole(role, rawtext, text, lineno, inliner,
   The expression's result is included as a literal.
 
   """
-  # pylint: disable=W0102,W0613,W0142
+  # pylint: disable=W0102,W0613
   # W0102: Dangerous default value as argument
-  # W0142: Used * or ** magic
   # W0613: Unused argument
 
   code = docutils.utils.unescape(text, restore_backslashes=True)
diff --git a/lib/cli.py b/lib/cli.py
index e29bc02..3c3241d 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -1732,8 +1732,8 @@ def GenerateTable(headers, fields, separator, data,
   if unitfields is None:
 unitfields = []
 
-  numfields = utils.FieldSet(*numfields)   # pylint: disable=W0142
-  unitfields = utils.FieldSet(*unitfields) # pylint: disable=W0142
+  numfields = utils.FieldSet(*numfields)
+  unitfields = utils.FieldSet(*unitfields)
 
   format_fields = []
   for field in fields:
diff --git a/lib/client/gnt_debug.py b/lib/client/gnt_debug.py
index d05fbc2..e79ae8d 100644
--- a/lib/client/gnt_debug.py
+++ b/lib/client/gnt_debug.py
@@ -103,7 +103,6 @@ def GenericOpCodes(opts, args):
 ToStdout("Loading...")
   for job_idx in range(opts.rep_job):
 for fname in args:
-  # pylint: disable=W0142
   op_data = simplejson.loads(utils.ReadFile(fname))
   op_list = [opcodes.OpCode.LoadOpCode(val) for val in op_data]
   op_list = op_list * opts.rep_op
diff --git a/lib/client/gnt_instance.py b/lib/client/gnt_instance.py
index 52da28e..6280b83 100644
--- a/lib/client/gnt_instance.py
+++ b/lib/client/gnt_instance.py
@@ -101,7 +101,6 @@ def _ExpandMultiNames(mode, names, client=None):
   @raise errors.OpPrereqError: for invalid input parameters
 
   """
-  # pylint: disable=W0142
 
   if client is None:
 client = GetClient()
@@ -300,7 +299,7 @@ def BatchCreate(opts, args):
  (idx, utils.CommaJoin(unknown)),
  errors.ECODE_INVAL)
 
-op = opcodes.OpInstanceCreate(**inst) # pylint: disable=W0142
+op = opcodes.OpInstanceCreate(**inst)
 op.Validate(False)
 instances.append(op)
 
diff --git a/lib/client/gnt_network.py b/lib/client/gnt_network.py
index 09c1121..41348c4 100644
--- a/lib/client/gnt_network.py
+++ 

Re: [PATCH stable-2.15 14/37] Disable pylint warning about unrecognised --disable

2016-12-05 Thread 'Brian Foley' via ganeti-devel
On Mon, Dec 05, 2016 at 10:36:04AM +, Brian Foley wrote:
> Allows us to disable warnings that are present in some versions of
> pylint but not others.
> 
> Signed-off-by: Brian Foley 
> ---
>  Makefile.am | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 8b859c1..abaa325 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -2686,7 +2686,9 @@ PEP8_EXCLUDE = $(subst $(space),$(comma),$(strip 
> $(notdir $(built_python_sources
>  
>  # A space-separated list of pylint warnings to completely ignore:
>  # I0013 = disable warnings for ignoring whole files
> -LINT_DISABLE = I0013
> +# E0012 = disable bad option vals: some warnings are removed from newer 
> pylints,
> +# and older pylints don't understand newer pylint's options
> +LINT_DISABLE = I0013 E0012
>  # Additional pylint options
>  LINT_OPTS =
>  # The combined set of pylint options
> -- 
> 2.8.0.rc3.226.g39d4020
> 

I'm dropping this patch as we're not going to bother supporting pylint <1.6.4


[PATCH stable-2.15 13/36] Fix pylint >1.4 pycurl no-member warnings

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Add pycurl to the whitelist of C extensions that can be loaded for the
purposes of attribute checking. This works around a security feature
introduced in pylint 1.4 to avoid arbitrary code injection.

Signed-off-by: Brian Foley 
---
 Makefile.am | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 0b30033..108cbba 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2693,6 +2693,9 @@ LINT_OPTS =
 LINT_OPTS_ALL = $(LINT_OPTS) \
   $(addprefix --disable=,$(LINT_DISABLE))
 
+# Whitelist loading pycurl C extension for attribute checking
+LINT_OPTS_ALL += --extension-pkg-whitelist=pycurl
+
 LINT_TARGETS = pylint pylint-qa pylint-test
 if HAS_PEP8
 LINT_TARGETS += pep8
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 02/36] Cleanup: Simplify boolean assignment

2016-12-05 Thread 'Brian Foley' via ganeti-devel
No functional change. Found by pylint's simplifiable-if-statement.

Signed-off-by: Brian Foley 
---
 lib/client/gnt_cluster.py | 5 +
 tools/cluster-merge   | 5 +
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
index 40792e2..d63310e 100644
--- a/lib/client/gnt_cluster.py
+++ b/lib/client/gnt_cluster.py
@@ -294,10 +294,7 @@ def InitCluster(opts, args):
 
   default_ialloc_params = opts.default_iallocator_params
 
-  if opts.enabled_user_shutdown:
-enabled_user_shutdown = True
-  else:
-enabled_user_shutdown = False
+  enabled_user_shutdown = bool(opts.enabled_user_shutdown)
 
   bootstrap.InitCluster(cluster_name=args[0],
 secondary_ip=opts.secondary_ip,
diff --git a/tools/cluster-merge b/tools/cluster-merge
index 926b705..c83e350 100755
--- a/tools/cluster-merge
+++ b/tools/cluster-merge
@@ -448,10 +448,7 @@ class Merger(object):
   check_params_strict.append("shared_file_storage_dir")
 check_params.extend(check_params_strict)
 
-if self.params == _PARAMS_STRICT:
-  params_strict = True
-else:
-  params_strict = False
+params_strict = (self.params == _PARAMS_STRICT)
 
 for param_name in check_params:
   my_param = getattr(my_cluster, param_name)
-- 
2.8.0.rc3.226.g39d4020



[PATCH master 6/6] Add OS params removal in gnt-instance reinstall

2016-12-05 Thread Yiannis Tsiouris
This patch adds '--remove-os-parameters' and
'--remove-os-parameters-private' options to 'gnt-instance reinstall'.
Similarly to 'gnt-instance modify', the new options can be used to
perform reinstalls to OS providers that support different parameters.
E.g.:

$ gnt-instance reinstall --remove-os-parameters param1,param2 \
  -o  -O param3=val3 

or

$ gnt-instance reinstall --remove-os-parameters param1,param2 \
  -o  -O param3=val3 

Signed-off-by: Yiannis Tsiouris 
---
 lib/client/gnt_instance.py   | 18 --
 lib/cmdlib/instance_operation.py | 16 
 lib/rapi/rlib2.py|  9 -
 man/gnt-instance.rst |  6 +-
 src/Ganeti/HTools/Repair.hs  |  4 
 src/Ganeti/OpCodes.hs|  2 ++
 src/Ganeti/OpParams.hs   | 12 
 test/hs/Test/Ganeti/OpCodes.hs   |  2 ++
 8 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/lib/client/gnt_instance.py b/lib/client/gnt_instance.py
index 3a6686f..22d2cdc 100644
--- a/lib/client/gnt_instance.py
+++ b/lib/client/gnt_instance.py
@@ -398,6 +398,17 @@ def ReinstallInstance(opts, args):
   if not AskUser(usertext):
 return 1
 
+  if opts.clear_osparams and opts.remove_osparams is not None:
+raise errors.OpPrereqError("Using --remove-os-parameters with "
+  "--clear-os-parameters is not possible", errors.ECODE_INVAL)
+
+  if opts.clear_osparams_private and opts.remove_osparams_private is not None:
+raise errors.OpPrereqError("Using --remove-os-parameters-private with "
+  "--clear-os-parameters-private is not possible", errors.ECODE_INVAL)
+
+  remove_osparams = opts.remove_osparams or []
+  remove_osparams_private = opts.remove_osparams_private or []
+
   jex = JobExecutor(verbose=multi_on, opts=opts)
   for instance_name in inames:
 op = opcodes.OpInstanceReinstall(
@@ -408,7 +419,9 @@ def ReinstallInstance(opts, args):
   osparams_private=opts.osparams_private,
   osparams_secret=opts.osparams_secret,
   clear_osparams=opts.clear_osparams,
-  clear_osparams_private=opts.clear_osparams_private
+  clear_osparams_private=opts.clear_osparams_private,
+  remove_osparams=remove_osparams,
+  remove_osparams_private=remove_osparams_private
 )
 jex.QueueJob(instance_name, op)
 
@@ -1663,7 +1676,8 @@ commands = {
  m_pri_node_tags_opt, m_sec_node_tags_opt, m_inst_tags_opt, SELECT_OS_OPT]
 + SUBMIT_OPTS + [DRY_RUN_OPT, PRIORITY_OPT, OSPARAMS_OPT,
  OSPARAMS_PRIVATE_OPT, OSPARAMS_SECRET_OPT,
- CLEAR_OSPARAMS_OPT, CLEAR_OSPARAMS_PRIVATE_OPT],
+ CLEAR_OSPARAMS_OPT, CLEAR_OSPARAMS_PRIVATE_OPT,
+ REMOVE_OSPARAMS_OPT, REMOVE_OSPARAMS_PRIVATE_OPT],
 "[-f] ", "Reinstall a stopped instance"),
   "remove": (
 RemoveInstance, ARGS_ONE_INSTANCE,
diff --git a/lib/cmdlib/instance_operation.py b/lib/cmdlib/instance_operation.py
index e784a1a..be74a41 100644
--- a/lib/cmdlib/instance_operation.py
+++ b/lib/cmdlib/instance_operation.py
@@ -370,6 +370,8 @@ class LUInstanceReinstall(LogicalUnit):
 self.op.osparams = self.op.osparams or {}
 self.op.osparams_private = self.op.osparams_private or {}
 self.op.osparams_secret = self.op.osparams_secret or {}
+self.op.remove_osparams = self.op.remove_osparams or []
+self.op.remove_osparams_private = self.op.remove_osparams_private or []
 
 # Handle the use of 'default' values.
 if self.op.clear_osparams:
@@ -383,6 +385,20 @@ class LUInstanceReinstall(LogicalUnit):
 self.op.osparams_private)
 params_secret = self.op.osparams_secret
 
+for osp in self.op.remove_osparams:
+  if osp in params_public:
+del params_public[osp]
+  else:
+self.LogWarning("Trying to remove OS parameter %s but parameter"
+" does not exist" % osp)
+
+for osp in self.op.remove_osparams_private:
+  if osp in params_private:
+del params_private[osp]
+  else:
+self.LogWarning("Trying to remove private OS parameter %s but"
+" parameter does not exist" % osp)
+
 # Handle OS parameters
 if self.op.os_type is not None:
   instance_os = self.op.os_type
diff --git a/lib/rapi/rlib2.py b/lib/rapi/rlib2.py
index 92156d1..4f76d44 100644
--- a/lib/rapi/rlib2.py
+++ b/lib/rapi/rlib2.py
@@ -1307,13 +1307,20 @@ def _ParseInstanceReinstallRequest(name, data):
   clear_osparams_private = baserlib.CheckParameter(data,
"clear_osparams_private",
default=False)
+  remove_osparams = baserlib.CheckParameter(data, "remove_osparams",
+default=None)
+  remove_osparams_priv = baserlib.CheckParameter(data,
+ "remove_osparams_private",
+ 

[PATCH master 0/6] Clear/remove public/private OS params in gnt-instance modify/reinstall (version 2)

2016-12-05 Thread Yiannis Tsiouris
This patchset extends 'gnt-instance modify' and 'gnt-instance reinstall'
functionality by allowing partial/total removal of current OS parameters of an
instance. This is specifically useful for preparing or performing an instance
reinstall to a different OS provider or the same OS provider that changed its
OS parameter policy/supported list.

Nikos Skalkotos (1):
  Add cmdlib tests for removing instance OS params

Yiannis Tsiouris (5):
  Add clear OS params options in gnt-instance modify
  Support OS params removal in gnt-instance modify
  Test new options of gnt-instance modify in RAPI
  Support clear OS params in gnt-instance reinstall
  Add OS params removal in gnt-instance reinstall

 lib/cli_opts.py   |  32 ++
 lib/client/gnt_instance.py| 106 +++---
 lib/cmdlib/instance_operation.py  |  22 +++
 lib/cmdlib/instance_set_params.py |  52 -
 lib/rapi/rlib2.py |  16 -
 man/gnt-instance.rst  |  21 +++
 src/Ganeti/HTools/Repair.hs   |   8 +++
 src/Ganeti/OpCodes.hs |   8 +++
 src/Ganeti/OpParams.hs|  48 +++
 test/hs/Test/Ganeti/OpCodes.hs|   7 +++
 test/py/cmdlib/instance_unittest.py   |  37 
 test/py/ganeti.rapi.rlib2_unittest.py |  73 ++-
 12 files changed, 367 insertions(+), 63 deletions(-)

-- 
2.10.2



[PATCH master 1/6] Add clear OS params options in gnt-instance modify

2016-12-05 Thread Yiannis Tsiouris
This patch adds two new options in 'gnt-instance modify' which allow the
user to clear all current public/private OS parameters of an instance.
This might be useful for OS providers that consider no parameters as
valid or, more commonly, for changing OS provider (and parameters)
before performing a reinstall. E.g.:

$ gnt-instance modify --clear-os-parameters 

or

$ gnt-instance modify --clear-os-parameters-private 

Signed-off-by: Yiannis Tsiouris 
---
 lib/cli_opts.py   | 15 ++
 lib/client/gnt_instance.py| 58 +--
 lib/cmdlib/instance_set_params.py | 24 +++-
 man/gnt-instance.rst  |  6 
 src/Ganeti/OpCodes.hs |  2 ++
 src/Ganeti/OpParams.hs| 12 
 test/hs/Test/Ganeti/OpCodes.hs|  2 ++
 7 files changed, 92 insertions(+), 27 deletions(-)

diff --git a/lib/cli_opts.py b/lib/cli_opts.py
index 334c961..6f850e1 100644
--- a/lib/cli_opts.py
+++ b/lib/cli_opts.py
@@ -58,6 +58,8 @@ __all__ = [
   "CAPAB_MASTER_OPT",
   "CAPAB_VM_OPT",
   "CLEANUP_OPT",
+  "CLEAR_OSPARAMS_OPT",
+  "CLEAR_OSPARAMS_PRIVATE_OPT",
   "cli_option",
   "CLUSTER_DOMAIN_SECRET_OPT",
   "COMMIT_OPT",
@@ -729,6 +731,19 @@ OSPARAMS_SECRET_OPT = cli_option("--os-parameters-secret",
   " saved; you must supply these for every"
   " operation.)")
 
+CLEAR_OSPARAMS_OPT = cli_option("--clear-os-parameters",
+dest="clear_osparams",
+action="store_true",
+default=False,
+help="Clear current OS parameters")
+
+CLEAR_OSPARAMS_PRIVATE_OPT = cli_option("--clear-os-parameters-private",
+dest="clear_osparams_private",
+action="store_true",
+default=False,
+help="Clear current private OS"
+ " parameters")
+
 FORCE_VARIANT_OPT = cli_option("--force-variant", dest="force_variant",
action="store_true", default=False,
help="Force an unknown variant")
diff --git a/lib/client/gnt_instance.py b/lib/client/gnt_instance.py
index dd1013f..6bc59d3 100644
--- a/lib/client/gnt_instance.py
+++ b/lib/client/gnt_instance.py
@@ -1371,6 +1371,7 @@ def SetInstanceParams(opts, args):
   """
   if not (opts.nics or opts.disks or opts.disk_template or opts.hvparams or
   opts.beparams or opts.os or opts.osparams or opts.osparams_private
+  or opts.clear_osparams or opts.clear_osparams_private
   or opts.offline_inst or opts.online_inst or opts.runtime_mem or
   opts.new_primary_node or opts.instance_communication is not None):
 ToStderr("Please give at least one of the parameters.")
@@ -1435,31 +1436,35 @@ def SetInstanceParams(opts, args):
 
   instance_comm = opts.instance_communication
 
-  op = opcodes.OpInstanceSetParams(instance_name=args[0],
-   nics=nics,
-   disks=disks,
-   hotplug=opts.hotplug,
-   
hotplug_if_possible=opts.hotplug_if_possible,
-   disk_template=opts.disk_template,
-   ext_params=ext_params,
-   file_driver=opts.file_driver,
-   file_storage_dir=opts.file_storage_dir,
-   remote_node=opts.node,
-   iallocator=opts.iallocator,
-   pnode=opts.new_primary_node,
-   hvparams=opts.hvparams,
-   beparams=opts.beparams,
-   runtime_mem=opts.runtime_mem,
-   os_name=opts.os,
-   osparams=opts.osparams,
-   osparams_private=opts.osparams_private,
-   force_variant=opts.force_variant,
-   force=opts.force,
-   wait_for_sync=opts.wait_for_sync,
-   offline=offline,
-   conflicts_check=opts.conflicts_check,
-   ignore_ipolicy=opts.ignore_ipolicy,
-   instance_communication=instance_comm)
+  op = opcodes.OpInstanceSetParams(
+instance_name=args[0],
+nics=nics,
+disks=disks,
+hotplug=opts.hotplug,
+hotplug_if_possible=opts.hotplug_if_possible,
+disk_template=opts.disk_template,
+ext_params=ext_params,
+file_driver=opts.file_driver,
+

[PATCH master 5/6] Support clear OS params in gnt-instance reinstall

2016-12-05 Thread Yiannis Tsiouris
This extends 'gnt-instance reinstall' to support
'--clear-os-parameters' and '--clear-os-parameters-private' options from
'gnt-instance modify'. E.g.:

$ gnt-instance reinstall --clear-os-parameters -o  \
  -O param1=val1,param2=val2 

or

$ gnt-instance reinstall --clear-os-parameters-private \
  -o  -O param1=val1,param2=val2 

Signed-off-by: Yiannis Tsiouris 
---
 lib/client/gnt_instance.py   | 19 ---
 lib/cmdlib/instance_operation.py |  6 ++
 lib/rapi/rlib2.py|  9 -
 man/gnt-instance.rst |  6 ++
 src/Ganeti/HTools/Repair.hs  |  4 
 src/Ganeti/OpCodes.hs|  2 ++
 src/Ganeti/OpParams.hs   | 12 
 test/hs/Test/Ganeti/OpCodes.hs   |  1 +
 8 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/lib/client/gnt_instance.py b/lib/client/gnt_instance.py
index 3516f22..3a6686f 100644
--- a/lib/client/gnt_instance.py
+++ b/lib/client/gnt_instance.py
@@ -400,12 +400,16 @@ def ReinstallInstance(opts, args):
 
   jex = JobExecutor(verbose=multi_on, opts=opts)
   for instance_name in inames:
-op = opcodes.OpInstanceReinstall(instance_name=instance_name,
- os_type=os_name,
- force_variant=opts.force_variant,
- osparams=opts.osparams,
- osparams_private=opts.osparams_private,
- osparams_secret=opts.osparams_secret)
+op = opcodes.OpInstanceReinstall(
+  instance_name=instance_name,
+  os_type=os_name,
+  force_variant=opts.force_variant,
+  osparams=opts.osparams,
+  osparams_private=opts.osparams_private,
+  osparams_secret=opts.osparams_secret,
+  clear_osparams=opts.clear_osparams,
+  clear_osparams_private=opts.clear_osparams_private
+)
 jex.QueueJob(instance_name, op)
 
   results = jex.WaitOrShow(not opts.submit_only)
@@ -1658,7 +1662,8 @@ commands = {
  m_pri_node_opt, m_sec_node_opt, m_clust_opt, m_inst_opt, m_node_tags_opt,
  m_pri_node_tags_opt, m_sec_node_tags_opt, m_inst_tags_opt, SELECT_OS_OPT]
 + SUBMIT_OPTS + [DRY_RUN_OPT, PRIORITY_OPT, OSPARAMS_OPT,
- OSPARAMS_PRIVATE_OPT, OSPARAMS_SECRET_OPT],
+ OSPARAMS_PRIVATE_OPT, OSPARAMS_SECRET_OPT,
+ CLEAR_OSPARAMS_OPT, CLEAR_OSPARAMS_PRIVATE_OPT],
 "[-f] ", "Reinstall a stopped instance"),
   "remove": (
 RemoveInstance, ARGS_ONE_INSTANCE,
diff --git a/lib/cmdlib/instance_operation.py b/lib/cmdlib/instance_operation.py
index 048b1e4..e784a1a 100644
--- a/lib/cmdlib/instance_operation.py
+++ b/lib/cmdlib/instance_operation.py
@@ -372,6 +372,12 @@ class LUInstanceReinstall(LogicalUnit):
 self.op.osparams_secret = self.op.osparams_secret or {}
 
 # Handle the use of 'default' values.
+if self.op.clear_osparams:
+  instance.osparams.clear()
+
+if self.op.clear_osparams_private:
+  instance.osparams_private.clear()
+
 params_public = GetUpdatedParams(instance.osparams, self.op.osparams)
 params_private = GetUpdatedParams(instance.osparams_private,
 self.op.osparams_private)
diff --git a/lib/rapi/rlib2.py b/lib/rapi/rlib2.py
index 7b14e81..92156d1 100644
--- a/lib/rapi/rlib2.py
+++ b/lib/rapi/rlib2.py
@@ -1302,11 +1302,18 @@ def _ParseInstanceReinstallRequest(name, data):
   start = baserlib.CheckParameter(data, "start", exptype=bool,
   default=True)
   osparams = baserlib.CheckParameter(data, "osparams", default=None)
+  clear_osparams = baserlib.CheckParameter(data, "clear_osparams",
+   default=False)
+  clear_osparams_private = baserlib.CheckParameter(data,
+   "clear_osparams_private",
+   default=False)
 
   ops = [
 opcodes.OpInstanceShutdown(instance_name=name),
 opcodes.OpInstanceReinstall(instance_name=name, os_type=ostype,
-osparams=osparams),
+osparams=osparams,
+clear_osparams=clear_osparams,
+clear_osparams_private=clear_osparams_private),
 ]
 
   if start:
diff --git a/man/gnt-instance.rst b/man/gnt-instance.rst
index f1e2640..9a082e4 100644
--- a/man/gnt-instance.rst
+++ b/man/gnt-instance.rst
@@ -1526,6 +1526,8 @@ REINSTALL
 | [{-O|\--os-parameters} *OS\_PARAMETERS*]
 | [--os-parameters-private} *OS\_PARAMETERS*]
 | [--os-parameters-secret} *OS\_PARAMETERS*]
+| [--clear-os-parameters]
+| [--clear-os-parameters-private]
 | [\--submit] [\--print-jobid]
 | {*instance*...}
 
@@ -1539,6 +1541,10 @@ available OS templates. OS parameters can be overridden 
using ``-O
 (--os-parameters)`` (more documentation for this option under the
 **add** command).
 
+The 

[PATCH master 3/6] Add cmdlib tests for removing instance OS params

2016-12-05 Thread Yiannis Tsiouris
From: Nikos Skalkotos 

Add LUInstanceSetParams tests for removing individual public & private
parameters, as well as, for clearing out public & private parameters.

Signed-off-by: Nikos Skalkotos 
Signed-off-by: Yiannis Tsiouris 
---
 test/py/cmdlib/instance_unittest.py | 37 +
 1 file changed, 37 insertions(+)

diff --git a/test/py/cmdlib/instance_unittest.py 
b/test/py/cmdlib/instance_unittest.py
index d725015..0ce755b 100644
--- a/test/py/cmdlib/instance_unittest.py
+++ b/test/py/cmdlib/instance_unittest.py
@@ -2237,6 +2237,43 @@ class TestLUInstanceSetParams(CmdlibTestCase):
  })
 self.ExecOpCode(op)
 
+  def testRemoveOsParam(self):
+os_param = self.os.supported_parameters[0]
+self.inst.osparams[os_param] = "test_param_val"
+op = self.CopyOpCode(self.op, remove_osparams=[os_param])
+self.ExecOpCode(op)
+# FIXME: use assertNotIn when py 2.7 is minimum supported version
+self.assertFalse(os_param in self.inst.osparams)
+
+  def testClearOsParams(self):
+os = self.cfg.CreateOs(supported_parameters=["parm1", "parm2"])
+inst = self.cfg.AddNewInstance(osparams={"parm1": "val1", "parm2": "val1"})
+
+op = self.CopyOpCode(self.op, clear_osparams=True, instance_name=inst.name)
+self.ExecOpCode(op)
+self.assertEqual(len(inst.osparams), 0)
+
+  def testRemovePrivateOsParam(self):
+os_param = self.os.supported_parameters[0]
+self.inst.osparams_private[os_param] = "test_param_val"
+op = self.CopyOpCode(self.op, remove_osparams_private=[os_param])
+self.ExecOpCode(op)
+# FIXME: use assertNotIn when py 2.7 is minimum supported version
+self.assertFalse(os_param in self.inst.osparams_private)
+
+  def testClearPrivateOsParams(self):
+os = self.cfg.CreateOs(supported_parameters=["parm1", "parm2"])
+inst = self.cfg.AddNewInstance(osparams_private={
+ "parm1": "val1",
+ "parm2": "val2"
+   })
+
+op = self.CopyOpCode(self.op,
+ clear_osparams_private=True,
+ instance_name=inst.name)
+self.ExecOpCode(op)
+self.assertEqual(len(inst.osparams), 0)
+
   def testIncreaseMemoryTooMuch(self):
 op = self.CopyOpCode(self.running_op,
  beparams={
-- 
2.10.2



[PATCH master 4/6] Test new options of gnt-instance modify in RAPI

2016-12-05 Thread Yiannis Tsiouris
This patch adds tests to RAPI for gnt-instance modify options:
- '--clear-os-parameters'/'--clear-os-parameters-private'
- '--remove-os-parameters'/'--remove-os-parameters-private'

Signed-off-by: Yiannis Tsiouris 
---
 test/py/ganeti.rapi.rlib2_unittest.py | 73 +--
 1 file changed, 45 insertions(+), 28 deletions(-)

diff --git a/test/py/ganeti.rapi.rlib2_unittest.py 
b/test/py/ganeti.rapi.rlib2_unittest.py
index fe67538..1f52ec1 100755
--- a/test/py/ganeti.rapi.rlib2_unittest.py
+++ b/test/py/ganeti.rapi.rlib2_unittest.py
@@ -1077,33 +1077,48 @@ class TestParseModifyInstanceRequest(RAPITestCase):
 for nics in [[], [(0, { constants.INIC_IP: "192.0.2.1", })]]:
   for disks in test_disks:
 for disk_template in constants.DISK_TEMPLATES:
-  data = {
-"osparams": osparams,
-"hvparams": hvparams,
-"beparams": beparams,
-"nics": nics,
-"disks": disks,
-"force": force,
-"disk_template": disk_template,
-}
-
-  op = self.getSubmittedOpcode(
-rlib2.R_2_instances_name_modify, [name], {}, data, "PUT",
-opcodes.OpInstanceSetParams
-  )
-
-  self.assertEqual(op.instance_name, name)
-  self.assertEqual(op.hvparams, hvparams)
-  self.assertEqual(op.beparams, beparams)
-  self.assertEqual(op.osparams, osparams)
-  self.assertEqual(op.force, force)
-  self.assertEqual(op.nics, nics)
-  self.assertEqual(op.disks, disks)
-  self.assertEqual(op.disk_template, disk_template)
-  self.assertTrue(op.remote_node is None)
-  self.assertTrue(op.os_name is None)
-  self.assertFalse(op.force_variant)
-  self.assertFalse(op.dry_run)
+  for clear_osparams in [False, True]:
+for clear_osparams_private in [False, True]:
+  for remove_osparams in [[], ["osp1", "osp2"]]:
+for remove_osparams_private in [[], ["priv_osp1",
+ "priv_osp2"]]:
+  data = {
+"osparams": osparams,
+"hvparams": hvparams,
+"beparams": beparams,
+"nics": nics,
+"disks": disks,
+"force": force,
+"disk_template": disk_template,
+"clear_osparams": clear_osparams,
+"clear_osparams_private": clear_osparams_private,
+"remove_osparams": remove_osparams,
+"remove_osparams_private": remove_osparams_private,
+  }
+
+  op = self.getSubmittedOpcode(
+rlib2.R_2_instances_name_modify, [name], {}, data,
+"PUT", opcodes.OpInstanceSetParams
+  )
+
+  self.assertEqual(op.instance_name, name)
+  self.assertEqual(op.hvparams, hvparams)
+  self.assertEqual(op.beparams, beparams)
+  self.assertEqual(op.osparams, osparams)
+  self.assertEqual(op.force, force)
+  self.assertEqual(op.nics, nics)
+  self.assertEqual(op.disks, disks)
+  self.assertEqual(op.disk_template, disk_template)
+  self.assertEqual(op.clear_osparams, clear_osparams)
+  self.assertEqual(op.clear_osparams_private,
+   clear_osparams_private)
+  self.assertEqual(op.remove_osparams, remove_osparams)
+  self.assertEqual(op.remove_osparams_private,
+   remove_osparams_private)
+  self.assertTrue(op.remote_node is None)
+  self.assertTrue(op.os_name is None)
+  self.assertFalse(op.force_variant)
+  self.assertFalse(op.dry_run)
 
   def testDefaults(self):
 name = "instir8aish31"
@@ -1112,7 +1127,9 @@ class TestParseModifyInstanceRequest(RAPITestCase):
  {}, "PUT", opcodes.OpInstanceSetParams)
 
 for i in ["hvparams", "beparams", "osparams", "force", "nics", "disks",
-  "disk_template", "remote_node", "os_name", "force_variant"]:
+  "disk_template", "remote_node", "os_name", "force_variant",
+  

[PATCH master 2/6] Support OS params removal in gnt-instance modify

2016-12-05 Thread Yiannis Tsiouris
This patch extends 'gnt-instance modify' by allowing a user to remove a
list of public/private OS parameters from an instance. This can be
useful before performing a reinstall to a new OS provider. Example
usage:

$ gnt-instance modify --remove-os-parameters parm1,parm2 

or

$ gnt-instance modify --remove-os-parameters-private parm3 

Signed-off-by: Yiannis Tsiouris 
---
 lib/cli_opts.py   | 17 +
 lib/client/gnt_instance.py| 17 -
 lib/cmdlib/instance_set_params.py | 34 +++---
 man/gnt-instance.rst  |  7 ++-
 src/Ganeti/OpCodes.hs |  2 ++
 src/Ganeti/OpParams.hs| 12 
 test/hs/Test/Ganeti/OpCodes.hs|  2 ++
 7 files changed, 86 insertions(+), 5 deletions(-)

diff --git a/lib/cli_opts.py b/lib/cli_opts.py
index 6f850e1..445e53f 100644
--- a/lib/cli_opts.py
+++ b/lib/cli_opts.py
@@ -221,6 +221,8 @@ __all__ = [
   "REASON_OPT",
   "REBOOT_TYPE_OPT",
   "REMOVE_INSTANCE_OPT",
+  "REMOVE_OSPARAMS_OPT",
+  "REMOVE_OSPARAMS_PRIVATE_OPT",
   "REMOVE_RESERVED_IPS_OPT",
   "REMOVE_UIDS_OPT",
   "RESERVED_LVS_OPT",
@@ -744,6 +746,21 @@ CLEAR_OSPARAMS_PRIVATE_OPT = 
cli_option("--clear-os-parameters-private",
 help="Clear current private OS"
  " parameters")
 
+REMOVE_OSPARAMS_OPT = cli_option("--remove-os-parameters",
+ dest="remove_osparams",
+ type="list",
+ default=None,
+ help="Comma-separated list of OS parameters"
+  " that should be removed")
+
+REMOVE_OSPARAMS_PRIVATE_OPT = cli_option("--remove-os-parameters-private",
+ dest="remove_osparams_private",
+ type="list",
+ default=None,
+ help="Comma-separated list of private"
+  " OS parameters that should be"
+  " removed")
+
 FORCE_VARIANT_OPT = cli_option("--force-variant", dest="force_variant",
action="store_true", default=False,
help="Force an unknown variant")
diff --git a/lib/client/gnt_instance.py b/lib/client/gnt_instance.py
index 6bc59d3..3516f22 100644
--- a/lib/client/gnt_instance.py
+++ b/lib/client/gnt_instance.py
@@ -1372,6 +1372,7 @@ def SetInstanceParams(opts, args):
   if not (opts.nics or opts.disks or opts.disk_template or opts.hvparams or
   opts.beparams or opts.os or opts.osparams or opts.osparams_private
   or opts.clear_osparams or opts.clear_osparams_private
+  or opts.remove_osparams or opts.remove_osparams_private
   or opts.offline_inst or opts.online_inst or opts.runtime_mem or
   opts.new_primary_node or opts.instance_communication is not None):
 ToStderr("Please give at least one of the parameters.")
@@ -1436,6 +1437,17 @@ def SetInstanceParams(opts, args):
 
   instance_comm = opts.instance_communication
 
+  if opts.clear_osparams and opts.remove_osparams is not None:
+raise errors.OpPrereqError("Using --remove-os-parameters with "
+  "--clear-os-parameters is not possible", errors.ECODE_INVAL)
+
+  if opts.clear_osparams_private and opts.remove_osparams_private is not None:
+raise errors.OpPrereqError("Using --remove-os-parameters-private with "
+  "--clear-os-parameters-private is not possible", errors.ECODE_INVAL)
+
+  remove_osparams = opts.remove_osparams or []
+  remove_osparams_private = opts.remove_osparams_private or []
+
   op = opcodes.OpInstanceSetParams(
 instance_name=args[0],
 nics=nics,
@@ -1457,6 +1469,8 @@ def SetInstanceParams(opts, args):
 osparams_private=opts.osparams_private,
 clear_osparams=opts.clear_osparams,
 clear_osparams_private=opts.clear_osparams_private,
+remove_osparams=remove_osparams,
+remove_osparams_private=remove_osparams_private,
 force_variant=opts.force_variant,
 force=opts.force,
 wait_for_sync=opts.wait_for_sync,
@@ -1674,7 +1688,8 @@ commands = {
  NOCONFLICTSCHECK_OPT, NEW_PRIMARY_OPT, HOTPLUG_OPT,
  HOTPLUG_IF_POSSIBLE_OPT, INSTANCE_COMMUNICATION_OPT,
  EXT_PARAMS_OPT, FILESTORE_DRIVER_OPT, FILESTORE_DIR_OPT,
- CLEAR_OSPARAMS_OPT, CLEAR_OSPARAMS_PRIVATE_OPT],
+ CLEAR_OSPARAMS_OPT, CLEAR_OSPARAMS_PRIVATE_OPT,
+ REMOVE_OSPARAMS_OPT, REMOVE_OSPARAMS_PRIVATE_OPT],
 "", "Alters the parameters of an instance"),
   "shutdown": (
 GenericManyOps("shutdown", _ShutdownInstance), [ArgInstance()],
diff --git a/lib/cmdlib/instance_set_params.py 
b/lib/cmdlib/instance_set_params.py
index b9f74a4..1b4f4c5 100644
--- a/lib/cmdlib/instance_set_params.py
+++ 

Re: [PATCH stable-2.15 37/37] Quell pylint socket.timeout warning

2016-12-05 Thread 'Brian Foley' via ganeti-devel
On Mon, Dec 05, 2016 at 02:29:05PM +, Federico Pareschi wrote:
>LGTM but little nit.
>s/doensn't/doesn't/ in the commit message :)

Thanks. :)


Re: [PATCH stable-2.15 37/37] Quell pylint socket.timeout warning

2016-12-05 Thread 'Federico Pareschi' via ganeti-devel
LGTM but little nit.
s/doensn't/doesn't/ in the commit message :)

On 5 December 2016 at 10:37, 'Brian Foley' via ganeti-devel <
ganeti-devel@googlegroups.com> wrote:

> socket.timeout doensn't have string or errno attributes.
>
> Signed-off-by: Brian Foley 
> ---
>  lib/hypervisor/hv_kvm/monitor.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/hypervisor/hv_kvm/monitor.py b/lib/hypervisor/hv_kvm/
> monitor.py
> index 98c8a6f..f7ca814 100644
> --- a/lib/hypervisor/hv_kvm/monitor.py
> +++ b/lib/hypervisor/hv_kvm/monitor.py
> @@ -385,7 +385,7 @@ class QmpConnection(MonitorSocket):
>self.sock.sendall(message_str)
>  except socket.timeout, err:
>raise errors.HypervisorError("Timeout while sending a QMP message:
> "
> -   "%s (%s)" % (err.string, err.errno))
> +   "%s" % err)
>  except socket.error, err:
>raise errors.HypervisorError("Unable to send data from KVM using
> the"
> " QMP protocol: %s" % err)
> --
> 2.8.0.rc3.226.g39d4020
>
>


Re: [PATCH stable-2.15 13/37] Fix pylint >1.4 pycurl no-member warnings

2016-12-05 Thread 'Iustin Pop' via ganeti-devel
On 5 December 2016 at 14:56, 'Brian Foley' via ganeti-devel <
ganeti-devel@googlegroups.com> wrote:

> On Mon, Dec 05, 2016 at 01:43:57PM +, Federico Pareschi wrote:
> >  to avoid arbitrary code injection.
> >
> >Is this safe? Should we be looking more into this or is it something
> >that does not affect us?
>
> The attack vector is someone writing their own pycurl.so with malicious
> code
> in it, puting that on the python search path, and using that to perform
> arbitrary actions as the user running pylint when pylint loads pycurl.
>
> I think this is a legitimate concern for developers running pylint as
> themselves and potentially accepting arbitary patches from the internet
> without
> checking what's in them, but it shouldn't be much of a problem for a
> sandboxed
> buildbot.
>

Even for developers, random patches can't easily ship a .so, so while this
is a valid concern for some cases, I think it's less likely for accepting
source diffs (that show no binary files) and linting them.

I wonder if it's possible to write a git config that rejects binary deltas…

The only alternatives I can think of are to explicitly disable the warning
> for
> any ganeti modules that use pycurl directly, or to explicitly add disables
> to
> each use of pycurl.foo but I don't much like those.
>

I tend to agree, this seems a worthwhile tradeoff.

iustin


Re: Issue 1185 in ganeti: hbal: IntMap.!: key -1 is not an element of the map

2016-12-05 Thread ganeti


Comment #19 on issue 1185 by gannef...@googlemail.com: hbal: IntMap.!: key  
-1 is not an element of the map

https://code.google.com/p/ganeti/issues/detail?id=1185

For future reference, the new feature goes into Issue 1196

--
You received this message because this project is configured to send all  
issue notifications to this address.

You may adjust your notification preferences at:
https://code.google.com/hosting/settings


Issue 1196 in ganeti: Htools location tag mechanismus for non-DRBD instances

2016-12-05 Thread ganeti

Status: New
Owner: 

New issue 1196 by gannef...@googlemail.com: Htools location tag mechanismus  
for non-DRBD instances

https://code.google.com/p/ganeti/issues/detail?id=1196

Hi

This came out of issue 1185, and the subject basically says it: Htools need  
a "virtual node group" location tag mechanismus similar to the  
htools:nlocation: way that exists for DRBD instances.


Currently non-DRBD instances can be guarded to not end up on the same node  
using node exclusion tags (htools:iextags:), but a common cause of error  
can be something that affects multiple nodes, e.g. a blade center hosting  
multiple blades.


Obviously this needs a kind of multi-tag: I need a way of saying "Node X/Y  
have the same common error cause" and a way of saying "VMs X/Y may not end  
up on same common error cause", which may may make it interesting to  
implement. Also, going from the current htools:nlocation: tagset for DRBD  
instances, I could imagine something like:



cluster tag htools:xlocation:enclosure

node A is tagged enclosure:35
node B is tagged enclosure:35
node C is tagged enclosure:15
node D is tagged enclosure:15

VM1 is tagged enclosure:dns
VM2 is tagged enclosure:dns
VM3 is tagged enclosure:www
VM4 is tagged enclosure:www

VM1/3 run on node A
VM2/4 run on node C

VM1/3 can NOT migrate to node D (or C)
VM2/4 can NOT migrate to node B (or A)

So unlike the other Htools tags this would be a 3-legged tag, actually, but  
IMO easy enough. The xlocation tag applied to Nodes would define the common  
causes, the xlocation tag applied to VMs would work the same as iextags,  
except it wouldn't look at nodes, but at the same-named xlocation tags on  
all nodes.


(I'm sure there are ways to describe this even better...)

Greetings
 Joerg

--
You received this message because this project is configured to send all  
issue notifications to this address.

You may adjust your notification preferences at:
https://code.google.com/hosting/settings


Re: [PATCH stable-2.15 13/37] Fix pylint >1.4 pycurl no-member warnings

2016-12-05 Thread 'Brian Foley' via ganeti-devel
On Mon, Dec 05, 2016 at 01:43:57PM +, Federico Pareschi wrote:
>  to avoid arbitrary code injection.
> 
>Is this safe? Should we be looking more into this or is it something
>that does not affect us?

The attack vector is someone writing their own pycurl.so with malicious code
in it, puting that on the python search path, and using that to perform
arbitrary actions as the user running pylint when pylint loads pycurl.

I think this is a legitimate concern for developers running pylint as
themselves and potentially accepting arbitary patches from the internet without
checking what's in them, but it shouldn't be much of a problem for a sandboxed
buildbot.

The only alternatives I can think of are to explicitly disable the warning for
any ganeti modules that use pycurl directly, or to explicitly add disables to
each use of pycurl.foo but I don't much like those.

Cheers,
Brian.


Re: [PATCH stable-2.15 13/37] Fix pylint >1.4 pycurl no-member warnings

2016-12-05 Thread 'Federico Pareschi' via ganeti-devel
>
> to avoid arbitrary code injection.
>

Is this safe? Should we be looking more into this or is it something that
does not affect us?


Re: [PATCH master 2/6] Support OS params removal in gnt-instance modify

2016-12-05 Thread 'Brian Foley' via ganeti-devel
On Mon, Dec 05, 2016 at 02:17:32PM +0200, Yiannis Tsiouris wrote:
> > OK, that's fine by me. I don't have strong feelings about it one way or the
> > other, but I thought I'd suggest the alternative.
> 
> That was actually a very good suggestion! I don't have any strong objections
> against it (i.e. using "*" as a "special" parameter); We discussed this with
> Alex and we think that maybe the
> --clear-os-parameters(-private)/--remove-os-parameters(-private) options are
> "closer" to what the user might expect while the "*" approach might be
> non-intuisive as this isn't used anywhere else
> 
> In regards of using "*"-expansion to OS parameters, I mostly agree with Morg: 
> I
> don't think it deserves the effort and it also messes with how bash and other
> shells handle expansion.
> 
> Should I resend the whole patchset addressing all the other comments and leave
> this as-is for now?

Hi Yiannis,

that sounds good, and thanks for the work on this.

Cheers,
Brian.


Re: [PATCH stable-2.15 00/37] Cleanup for pylint 1.6.4

2016-12-05 Thread 'Iustin Pop' via ganeti-devel
On 5 December 2016 at 13:04, 'Brian Foley' via ganeti-devel <
ganeti-devel@googlegroups.com> wrote:

> On Mon, Dec 05, 2016 at 12:01:14PM +0100, Iustin Pop wrote:
> >Quick question: is there a reason to keep that compat, as opposed to
> >switching the "blessed" (and required) pylint version to 1.6.4?
> >(If this is addressed in the patch series, sorry, didn't read the
> >patches)
> >iustin
>
> Hi Iustin,
>
> no, there isn't any very good reason, I was just playing it safe, and kinda
> assumed that make dist or the debian build process might run pylint, but it
> appears tht neither do.
>

As long as we require a fixed version, it's not really feasible to do so.
Similarly how -Werror is only enabled in developer builds, I guess.

In that case, I'll move us to requiring 1.6.4, and remove a couple of the
> compatibility workarounds.
>

Yes please, that sounds cleaner.

thanks,
iustin


Re: [PATCH master 2/6] Support OS params removal in gnt-instance modify

2016-12-05 Thread Yiannis Tsiouris
Hi Brian,

On 12/05, Brian Foley wrote:
> On Mon, Dec 05, 2016 at 01:31:15PM +0200, Alex Kiousis wrote:
> > Hello (I work with Yiannis at grnet),
> > the main reason for this patchset is that a command like
> > clear-os-params is needed if you are reinstalling between 2 os
> > providers with different parameters. Without a mechanism to remove a
> > defined osparam the reinstall will fail when ganeti tries to validate
> > the params against the new provider.
> > 
> > The remove-os-params was added when we figured that there should be a
> > way to bring the Instance osparams to the desired state without
> > respecifying everything.
> > 
> > Given the above, i believe that --clear-os-params is much more
> > intuitive than --remove-os-params "\*". We can add both to the patch
> > but unless there is considerable overhead for the extra option,  we'd
> > like for clean-os-params to remain seperate.
> 
> OK, that's fine by me. I don't have strong feelings about it one way or the
> other, but I thought I'd suggest the alternative.

That was actually a very good suggestion! I don't have any strong objections
against it (i.e. using "*" as a "special" parameter); We discussed this with
Alex and we think that maybe the
--clear-os-parameters(-private)/--remove-os-parameters(-private) options are
"closer" to what the user might expect while the "*" approach might be
non-intuisive as this isn't used anywhere else

In regards of using "*"-expansion to OS parameters, I mostly agree with Morg: I
don't think it deserves the effort and it also messes with how bash and other
shells handle expansion.

Should I resend the whole patchset addressing all the other comments and leave
this as-is for now?

Regards,
Yiannis


Re: [PATCH stable-2.15 02/37] Cleanup: Simplify boolean assignment

2016-12-05 Thread 'Brian Foley' via ganeti-devel
On Mon, Dec 05, 2016 at 11:13:18AM +, Federico Pareschi wrote:
>  diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
>  index 40792e2..9ad82d8 100644
>  --- a/lib/client/gnt_cluster.py
>  +++ b/lib/client/gnt_cluster.py
>  @@ -294,11 +294,6 @@ def InitCluster(opts, args):
> default_ialloc_params = opts.default_iallocator_params
>  -  if opts.enabled_user_shutdown:
>  -enabled_user_shutdown = True
>  -  else:
>  -enabled_user_shutdown = False
>  -
> bootstrap.InitCluster(cluster_name=args[0],
>   secondary_ip=opts.secondary_ip,
>   vg_name=vg_name,
>  @@ -332,7 +327,7 @@ def InitCluster(opts, args):
>   install_image=install_image,
>   zeroing_image=zeroing_image,
>   compression_tools=compression_tools,
>  -enabled_user_shutdown=enabled_
>  user_shutdown,
>  +enabled_user_shutdown=opts.
>  enabled_user_shutdown,
>   )
> op = opcodes.OpClusterPostInit()
> SubmitOpCode(op, opts=opts)
> 
>Can't this cause some issues if opts.enabled_user_shutdown is None?
>Pre-patch this would set enabled_user_shutdown in the function as
>False,
>post-patch, it would pass None down the chain call.
>The code calls into bootstrap.py's InitCluster which defaults the value
>of
>enabled_user_shutdown to False. Passing None to it would completely
>change the dynamics of the function and would likely trigger some
>errors.

Good point. I'll cast this to bool instead.

Thanks,
Brian.


Re: [PATCH stable-2.15 00/37] Cleanup for pylint 1.6.4

2016-12-05 Thread 'Brian Foley' via ganeti-devel
On Mon, Dec 05, 2016 at 12:01:14PM +0100, Iustin Pop wrote:
>Quick question: is there a reason to keep that compat, as opposed to
>switching the "blessed" (and required) pylint version to 1.6.4?
>(If this is addressed in the patch series, sorry, didn't read the
>patches)
>iustin

Hi Iustin,

no, there isn't any very good reason, I was just playing it safe, and kinda
assumed that make dist or the debian build process might run pylint, but it
appears tht neither do.

In that case, I'll move us to requiring 1.6.4, and remove a couple of the
compatibility workarounds.

Cheers,
Brian.


Re: [PATCH stable-2.15 08/37] Cleanup: Remove unused format key

2016-12-05 Thread 'Federico Pareschi' via ganeti-devel
This is an LGTM for me, however it made me wonder...
We are adamant in not storing the secret parameters anywhere on the cluster,
at least according to our design docs. Wouldn't this be going against such
policy
by having them printed out in an error (and, I assume, logged to disk)?

This is probably not the correct patchset to debate this, but it'd be
useful to maybe
investigate this just to make sure we are not accidentally leaking data
that we
promised, in our design docs, we wouldn't.


On 5 December 2016 at 10:35, 'Brian Foley' via ganeti-devel <
ganeti-devel@googlegroups.com> wrote:

> No functional change, but quietens pylint unused-format-string-argument
> warning.
>
> Signed-off-by: Brian Foley 
> ---
>  lib/objects.py | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/lib/objects.py b/lib/objects.py
> index 96e7092..29dbbbc 100644
> --- a/lib/objects.py
> +++ b/lib/objects.py
> @@ -2061,8 +2061,7 @@ class Cluster(TaggableObject):
>- at public visibility:  {public}
>- at private visibility: {private}
>- at secret visibility:  {secret}
> -  """.format(dupes=formatter(duplicate_keys),
> - public=formatter(params_public & duplicate_keys),
> +  """.format(public=formatter(params_public & duplicate_keys),
>   private=formatter(params_private & duplicate_keys),
>   secret=formatter(params_secret & duplicate_keys))
>raise errors.OpPrereqError(msg)
> --
> 2.8.0.rc3.226.g39d4020
>
>


Re: [PATCH master 2/6] Support OS params removal in gnt-instance modify

2016-12-05 Thread 'Brian Foley' via ganeti-devel
On Mon, Dec 05, 2016 at 01:31:15PM +0200, Alex Kiousis wrote:
> Hello (I work with Yiannis at grnet),
> the main reason for this patchset is that a command like
> clear-os-params is needed if you are reinstalling between 2 os
> providers with different parameters. Without a mechanism to remove a
> defined osparam the reinstall will fail when ganeti tries to validate
> the params against the new provider.
> 
> The remove-os-params was added when we figured that there should be a
> way to bring the Instance osparams to the desired state without
> respecifying everything.
> 
> Given the above, i believe that --clear-os-params is much more
> intuitive than --remove-os-params "\*". We can add both to the patch
> but unless there is considerable overhead for the extra option,  we'd
> like for clean-os-params to remain seperate.

OK, that's fine by me. I don't have strong feelings about it one way or the
other, but I thought I'd suggest the alternative.

Cheers,
Brian.


Re: [PATCH stable-2.15 02/37] Cleanup: Simplify boolean assignment

2016-12-05 Thread 'Federico Pareschi' via ganeti-devel
>
> diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
> index 40792e2..9ad82d8 100644
> --- a/lib/client/gnt_cluster.py
> +++ b/lib/client/gnt_cluster.py
> @@ -294,11 +294,6 @@ def InitCluster(opts, args):
>
>default_ialloc_params = opts.default_iallocator_params
>
> -  if opts.enabled_user_shutdown:
> -enabled_user_shutdown = True
> -  else:
> -enabled_user_shutdown = False
> -
>bootstrap.InitCluster(cluster_name=args[0],
>  secondary_ip=opts.secondary_ip,
>  vg_name=vg_name,
> @@ -332,7 +327,7 @@ def InitCluster(opts, args):
>  install_image=install_image,
>  zeroing_image=zeroing_image,
>  compression_tools=compression_tools,
> -enabled_user_shutdown=enabled_user_shutdown,
> +enabled_user_shutdown=opts.enabled_user_shutdown,
>  )
>op = opcodes.OpClusterPostInit()
>SubmitOpCode(op, opts=opts)
>

Can't this cause some issues if opts.enabled_user_shutdown is None?
Pre-patch this would set enabled_user_shutdown in the function as False,
post-patch, it would pass None down the chain call.

The code calls into bootstrap.py's InitCluster which defaults the value of
enabled_user_shutdown to False. Passing None to it would completely
change the dynamics of the function and would likely trigger some errors.


Re: Issue 1185 in ganeti: hbal: IntMap.!: key -1 is not an element of the map

2016-12-05 Thread ganeti


Comment #18 on issue 1185 by ius...@google.com: hbal: IntMap.!: key -1 is  
not an element of the map

https://code.google.com/p/ganeti/issues/detail?id=1185

Yes, it will get into 2.15 patch fix and 2.16 releases.

As to location for non-DRBD instances, based on how I understand the design  
docs, there are no options for node virtual groups (like your enclosure  
group):


- there is the exclusion tag mechanism, which applies to the same node only
- there is the nlocation tag, which applies for DRBD instances

Your idea makes a lot of sense though, I'd suggest opening a new bug to  
discuss that feature.


--
You received this message because this project is configured to send all  
issue notifications to this address.

You may adjust your notification preferences at:
https://code.google.com/hosting/settings


Re: [PATCH stable-2.15] Fix coexistence of location tags and non-DRBD instances

2016-12-05 Thread 'Iustin Pop' via ganeti-devel
On 4 December 2016 at 18:44, Brian Foley  wrote:

> On Fri, Dec 02, 2016 at 11:03:55PM +0100, Iustin Pop wrote:
> > From: Iustin Pop 
> >
> > This addresses issue 1185, “hbal: IntMap.!: key -1 is not an element of
> > the map”. The issue is that the location tags code presumed all
> > instances have a primary and a secondary (i.e., they are DRBD).
> >
> > The fix is to set the location score for non-DRBD instances to zero,
> > since this is what I understand the correct value: such an instance
> > cannot be optimized by moving, so its score is "perfect" (zero).
> >
> > Signed-off-by: Iustin Pop 
>
> LGTM, and thank you. I'm going to do a merge forward of the recent (and a
> batch of upcoming) 2.15 patches in the next few days, and this is a nice
> fix
> to include.
>

Note:  someone with edit rights will need to update the issue once this is
committed (or released). Thanks for the review!

iustin


Re: [PATCH stable-2.15 00/37] Cleanup for pylint 1.6.4

2016-12-05 Thread 'Iustin Pop' via ganeti-devel
On 5 December 2016 at 11:35, 'Brian Foley' via ganeti-devel <
ganeti-devel@googlegroups.com> wrote:

> Ganeti's python code passes the pylint checks in the version of pylint
> included with Debian Jessie. Unfortunately this is a really old pylint
> (0.26 from 2012) and the latest stable pylint (1.6.4 from 2016) has a
> lot of additional checks. Ganeti produces many many warnings and errors
> when run against 1.6.4.
>
> This patchset is a first cut at fixing these, while still maintaining
>  compatibility with pylint 0.26.
>

Quick question: is there a reason to keep that compat, as opposed to
switching the "blessed" (and required) pylint version to 1.6.4?

(If this is addressed in the patch series, sorry, didn't read the patches)

iustin


[PATCH stable-2.15 28/37] Disable pylint unsubscriptable-object warning

2016-12-05 Thread 'Brian Foley' via ganeti-devel
In these cases result is subscriptable.

Signed-off-by: Brian Foley 
---
 lib/cmdlib/instance_create.py| 8 
 lib/cmdlib/instance_migration.py | 3 ++-
 lib/cmdlib/instance_storage.py   | 2 +-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/lib/cmdlib/instance_create.py b/lib/cmdlib/instance_create.py
index 1b1abf8..d69ec50 100644
--- a/lib/cmdlib/instance_create.py
+++ b/lib/cmdlib/instance_create.py
@@ -472,8 +472,8 @@ class LUInstanceCreate(LogicalUnit):
  (self.op.iallocator, ial.info),
  ecode)
 
-(self.op.pnode_uuid, self.op.pnode) = \
-  ExpandNodeUuidAndName(self.cfg, None, ial.result[0])
+(self.op.pnode_uuid, self.op.pnode) = ExpandNodeUuidAndName(
+self.cfg, None, ial.result[0]) # pylint: disable=E1136
 self.LogInfo("Selected nodes for instance %s via iallocator %s: %s",
  self.op.instance_name, self.op.iallocator,
  utils.CommaJoin(ial.result))
@@ -481,8 +481,8 @@ class LUInstanceCreate(LogicalUnit):
 assert req.RequiredNodes() in (1, 2), "Wrong node count from iallocator"
 
 if req.RequiredNodes() == 2:
-  (self.op.snode_uuid, self.op.snode) = \
-ExpandNodeUuidAndName(self.cfg, None, ial.result[1])
+  (self.op.snode_uuid, self.op.snode) = ExpandNodeUuidAndName(
+  self.cfg, None, ial.result[1]) # pylint: disable=E1136
 
   def BuildHooksEnv(self):
 """Build hooks env.
diff --git a/lib/cmdlib/instance_migration.py b/lib/cmdlib/instance_migration.py
index ad6f9c3..b1321fa 100644
--- a/lib/cmdlib/instance_migration.py
+++ b/lib/cmdlib/instance_migration.py
@@ -499,7 +499,8 @@ class TLMigrateInstance(Tasklet):
  " iallocator '%s': %s" %
  (self.lu.op.iallocator, ial.info),
  errors.ECODE_NORES)
-self.target_node_uuid = self.cfg.GetNodeInfoByName(ial.result[0]).uuid
+self.target_node_uuid = self.cfg.GetNodeInfoByName(
+ial.result[0]).uuid # pylint: disable=E1136
 self.lu.LogInfo("Selected nodes for instance %s via iallocator %s: %s",
 self.instance_name, self.lu.op.iallocator,
 utils.CommaJoin(ial.result))
diff --git a/lib/cmdlib/instance_storage.py b/lib/cmdlib/instance_storage.py
index 4c1c964..0093edc 100644
--- a/lib/cmdlib/instance_storage.py
+++ b/lib/cmdlib/instance_storage.py
@@ -2246,7 +2246,7 @@ class TLReplaceDisks(Tasklet):
  " %s" % (iallocator_name, ial.info),
  errors.ECODE_NORES)
 
-remote_node_name = ial.result[0]
+remote_node_name = ial.result[0] # pylint: disable=E1136
 remote_node = lu.cfg.GetNodeInfoByName(remote_node_name)
 
 if remote_node is None:
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 31/37] Quell trailing newline

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Signed-off-by: Brian Foley 
---
 autotools/build-rpc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/autotools/build-rpc b/autotools/build-rpc
index d3ccad6..1d59aae 100755
--- a/autotools/build-rpc
+++ b/autotools/build-rpc
@@ -212,7 +212,7 @@ def main():
 for (clsname, calls) in sorted(module.CALLS.items()):
   _WriteBaseClass(sw, clsname, calls.values())
 
-  print buf.getvalue()
+  print buf.getvalue().rstrip()
 
 
 if __name__ == "__main__":
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 35/37] Quell cell-var-from-loop warning

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Signed-off-by: Brian Foley 
---
 autotools/build-bash-completion | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/autotools/build-bash-completion b/autotools/build-bash-completion
index 4a064ba..e0ad9b2 100755
--- a/autotools/build-bash-completion
+++ b/autotools/build-bash-completion
@@ -779,18 +779,18 @@ def ParseHaskellOptsArgs(script, output):
   cli_args = []
   for line in output.splitlines():
 v = line.split(None)
-exc = lambda msg: Exception("Invalid %s output from %s: %s" %
-(msg, script, v))
+exc = lambda msg, v: Exception("Invalid %s output from %s: %s" %
+   (msg, script, v))
 if len(v) < 2:
-  raise exc("help completion")
+  raise exc("help completion", v)
 if v[0].startswith("-"):
   if len(v) != 2:
-raise exc("option format")
+raise exc("option format", v)
   (opts, kind) = v
   cli_opts.append(HaskellOptToOptParse(opts, kind))
 else:
   if len(v) != 3:
-raise exc("argument format")
+raise exc("argument format", v)
   (kind, min_cnt, max_cnt) = v
   cli_args.append(HaskellArgToCliArg(kind, min_cnt, max_cnt))
   return (cli_opts, cli_args)
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 23/37] Disable pylint unpacking-non-sequence warning

2016-12-05 Thread 'Brian Foley' via ganeti-devel
For these cases, self.ia_result actually will have a tuple assigned to
it, it's just hard for static analysis tools to see it.

Signed-off-by: Brian Foley 
---
 lib/cmdlib/instance.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
index a8335dc..d20aa81 100644
--- a/lib/cmdlib/instance.py
+++ b/lib/cmdlib/instance.py
@@ -658,7 +658,7 @@ class LUInstanceMultiAlloc(NoHooksLU):
 
 """
 if self.op.iallocator:
-  (allocatable, failed_insts) = self.ia_result
+  (allocatable, failed_insts) = self.ia_result # pylint: disable=W0633
   allocatable_insts = map(compat.fst, allocatable)
 else:
   allocatable_insts = [op.instance_name for op in self.op.instances]
@@ -676,7 +676,7 @@ class LUInstanceMultiAlloc(NoHooksLU):
 jobs = []
 if self.op.iallocator:
   op2inst = dict((op.instance_name, op) for op in self.op.instances)
-  (allocatable, failed) = self.ia_result
+  (allocatable, failed) = self.ia_result # pylint: disable=W0633
 
   for (name, node_names) in allocatable:
 op = op2inst.pop(name)
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 22/37] Disable pylint misplaced-comparison-constant warning

2016-12-05 Thread 'Brian Foley' via ganeti-devel
It's correct, and easier to read a < x < b in this case.

Signed-off-by: Brian Foley 
---
 lib/utils/__init__.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/utils/__init__.py b/lib/utils/__init__.py
index bdd9761..d3a71d5 100644
--- a/lib/utils/__init__.py
+++ b/lib/utils/__init__.py
@@ -532,7 +532,7 @@ def SplitTime(value):
 
   """
   (seconds, microseconds) = divmod(int(value * 100), 100)
-
+  # pylint: disable=C0122
   assert 0 <= seconds, \
 "Seconds must be larger than or equal to 0, but are %s" % seconds
   assert 0 <= microseconds <= 99, \
@@ -550,7 +550,7 @@ def MergeTime(timetuple):
 
   """
   (seconds, microseconds) = timetuple
-
+  # pylint: disable=C0122
   assert 0 <= seconds, \
 "Seconds must be larger than or equal to 0, but are %s" % seconds
   assert 0 <= microseconds <= 99, \
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 33/37] Quell too-many-boolean-expressions

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Signed-off-by: Brian Foley 
---
 tools/move-instance | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/move-instance b/tools/move-instance
index 4ca6327..84b5972 100755
--- a/tools/move-instance
+++ b/tools/move-instance
@@ -943,9 +943,9 @@ def _CheckInstanceOptions(parser, options, instance_names):
   options.nics = cli.ParseNicOption(options.nics)
   else:
 # Moving more than one instance
-if (options.dest_instance_name or options.dest_primary_node or
-options.dest_secondary_node or options.hvparams or
-options.beparams or options.osparams or options.nics):
+if compat.any(options.dest_instance_name, options.dest_primary_node,
+  options.dest_secondary_node, options.hvparams,
+  options.beparams, options.osparams, options.nics):
   parser.error("The options --dest-instance-name, --dest-primary-node,"
" --dest-secondary-node, --hypervisor-parameters,"
" --backend-parameters, --os-parameters and --net can"
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 24/37] Disable incorrect pylint not-callable warning

2016-12-05 Thread 'Brian Foley' via ganeti-devel
REQ_REQUEST will be a parameter validating function from ht, but pylint
can't statically determine this.

Signed-off-by: Brian Foley 
---
 lib/masterd/iallocator.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/masterd/iallocator.py b/lib/masterd/iallocator.py
index a94d880..6d6ed48 100644
--- a/lib/masterd/iallocator.py
+++ b/lib/masterd/iallocator.py
@@ -156,7 +156,7 @@ class IARequestBase(outils.ValidatedSlots):
 @raises ResultValidationError: If validation fails
 
 """
-if ia.success and not self.REQ_RESULT(result):
+if ia.success and not self.REQ_RESULT(result): # pylint: disable=E1102
   raise errors.ResultValidationError("iallocator returned invalid result,"
  " expected %s, got %s" %
  (self.REQ_RESULT, result))
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 29/37] Quell consider-using-enumerate warning

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Signed-off-by: Brian Foley 
---
 lib/config/verify.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/config/verify.py b/lib/config/verify.py
index e53b384..2d8b99e 100644
--- a/lib/config/verify.py
+++ b/lib/config/verify.py
@@ -107,8 +107,8 @@ def VerifyIpolicy(owner, ipolicy, iscluster, callback):
 callback("%s has invalid instance policy: %s" % (owner, err))
   for key, value in ipolicy.items():
 if key == constants.ISPECS_MINMAX:
-  for k in range(len(value)):
-VerifyIspecs(owner, "ipolicy/%s[%s]" % (key, k), value[k], callback)
+  for i, val in enumerate(value):
+VerifyIspecs(owner, "ipolicy/%s[%s]" % (key, i), val, callback)
 elif key == constants.ISPECS_STD:
   VerifyType(owner, "ipolicy/" + key, value,
  constants.ISPECS_PARAMETER_TYPES, callback)
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 30/37] Quell bad-whitespace warning

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Signed-off-by: Brian Foley 
---
 lib/objects.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/objects.py b/lib/objects.py
index 6235947..13e5ee2 100644
--- a/lib/objects.py
+++ b/lib/objects.py
@@ -1254,7 +1254,7 @@ class Instance(TaggableObject):
 if _with_private:
   bo["osparams_private"] = self.osparams_private.Unprivate()
 
-for attr in "nics", :
+for attr in ("nics",):
   alist = bo.get(attr, None)
   if alist:
 nlist = outils.ContainerToDicts(alist)
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 26/37] Disable unwanted pylint wrong-import-position warnings

2016-12-05 Thread 'Brian Foley' via ganeti-devel
For these, we want to import some modules in a particular order, so
disable the pylint warnings.

Signed-off-by: Brian Foley 
---
 autotools/check-imports | 3 ++-
 lib/build/sphinx_ext.py | 3 +++
 lib/ovf.py  | 3 ++-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/autotools/check-imports b/autotools/check-imports
index c90ea70..27790cf 100755
--- a/autotools/check-imports
+++ b/autotools/check-imports
@@ -32,8 +32,9 @@
 
 """
 
-# pylint: disable=C0103
+# pylint: disable=C0103, C0413
 # C0103: Invalid name
+# C0413: Wrong import position
 
 import sys
 
diff --git a/lib/build/sphinx_ext.py b/lib/build/sphinx_ext.py
index 37f1574..4ab7b29 100644
--- a/lib/build/sphinx_ext.py
+++ b/lib/build/sphinx_ext.py
@@ -32,6 +32,9 @@
 
 """
 
+# pylint: disable=C0413
+# C0413: Wrong import position
+
 import re
 from cStringIO import StringIO
 
diff --git a/lib/ovf.py b/lib/ovf.py
index 2662ca7..66dbf83 100644
--- a/lib/ovf.py
+++ b/lib/ovf.py
@@ -32,10 +32,11 @@
 
 """
 
-# pylint: disable=F0401, E1101
+# pylint: disable=F0401, E1101, C0413
 
 # F0401 because ElementTree is not default for python 2.4
 # E1101 makes no sense - pylint assumes that ElementTree object is a tuple
+# C0413 Wrong import position
 
 
 import ConfigParser
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 20/37] Disable pylint eval-used warning

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Signed-off-by: Brian Foley 
---
 lib/build/sphinx_ext.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/build/sphinx_ext.py b/lib/build/sphinx_ext.py
index 6b30894..37f1574 100644
--- a/lib/build/sphinx_ext.py
+++ b/lib/build/sphinx_ext.py
@@ -288,7 +288,7 @@ def PythonEvalRole(role, rawtext, text, lineno, inliner,
   code = docutils.utils.unescape(text, restore_backslashes=True)
 
   try:
-result = eval(code, EVAL_NS)
+result = eval(code, EVAL_NS) # pylint: disable=W0123
   except Exception, err: # pylint: disable=W0703
 msg = inliner.reporter.error("Failed to evaluate %r: %s" % (code, err),
  line=lineno)
@@ -321,7 +321,7 @@ class PythonAssert(s_compat.Directive):
 code = "\n".join(self.content)
 
 try:
-  result = eval(code, EVAL_NS)
+  result = eval(code, EVAL_NS) # pylint: disable=W0123
 except Exception, err:
   raise self.error("Failed to evaluate %r: %s" % (code, err))
 
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 37/37] Quell pylint socket.timeout warning

2016-12-05 Thread 'Brian Foley' via ganeti-devel
socket.timeout doensn't have string or errno attributes.

Signed-off-by: Brian Foley 
---
 lib/hypervisor/hv_kvm/monitor.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/hypervisor/hv_kvm/monitor.py b/lib/hypervisor/hv_kvm/monitor.py
index 98c8a6f..f7ca814 100644
--- a/lib/hypervisor/hv_kvm/monitor.py
+++ b/lib/hypervisor/hv_kvm/monitor.py
@@ -385,7 +385,7 @@ class QmpConnection(MonitorSocket):
   self.sock.sendall(message_str)
 except socket.timeout, err:
   raise errors.HypervisorError("Timeout while sending a QMP message: "
-   "%s (%s)" % (err.string, err.errno))
+   "%s" % err)
 except socket.error, err:
   raise errors.HypervisorError("Unable to send data from KVM using the"
" QMP protocol: %s" % err)
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 25/37] Disable pylint unused-wildcard-import warning

2016-12-05 Thread 'Brian Foley' via ganeti-devel
We're deliberately importing all the variables defined in cli_opt into
cli's namespace.

Signed-off-by: Brian Foley 
---
 lib/cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/cli.py b/lib/cli.py
index 3c3241d..8622738 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -56,7 +56,7 @@ from ganeti import pathutils
 from ganeti import serializer
 import ganeti.cli_opts
 # Import constants
-from ganeti.cli_opts import *  # pylint: disable=W0401
+from ganeti.cli_opts import *  # pylint: disable=W0401,W0614
 
 from ganeti.runtime import (GetClient)
 
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 27/37] Disable pylint bare-except warning

2016-12-05 Thread 'Brian Foley' via ganeti-devel
This is actually dead code which wasn't removed until Ganeti 2.16

Signed-off-by: Brian Foley 
---
 lib/server/masterd.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/server/masterd.py b/lib/server/masterd.py
index be98f69..f8787d4 100644
--- a/lib/server/masterd.py
+++ b/lib/server/masterd.py
@@ -107,7 +107,7 @@ class ClientRequestWorker(workerpool.BaseWorker):
   logging.exception("Unexpected exception")
   success = False
   result = errors.EncodeException(err)
-except:
+except: # pylint: disable=W0702
   logging.exception("Unexpected exception")
   err = sys.exc_info()
   result = "Caught exception: %s" % str(err[1])
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 21/37] Disable incorect pylint simplify-if-statement warning

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Signed-off-by: Brian Foley 
---
 lib/cmdlib/instance_create.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/cmdlib/instance_create.py b/lib/cmdlib/instance_create.py
index aec9d9f..1b1abf8 100644
--- a/lib/cmdlib/instance_create.py
+++ b/lib/cmdlib/instance_create.py
@@ -113,7 +113,7 @@ class LUInstanceCreate(LogicalUnit):
 for disk in self.op.disks:
   if self.op.disk_template != constants.DT_EXT:
 utils.ForceDictType(disk, constants.IDISK_PARAMS_TYPES)
-  if constants.IDISK_ADOPT in disk:
+  if constants.IDISK_ADOPT in disk: # pylint: disable=R0102
 has_adopt = True
   else:
 has_no_adopt = True
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 32/37] Remove pylint tests removed in pylint 2.0

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Signed-off-by: Brian Foley 
---
 pylintrc  | 2 --
 pylintrc-test | 2 --
 2 files changed, 4 deletions(-)

diff --git a/pylintrc b/pylintrc
index 47be2b8..342eeff 100644
--- a/pylintrc
+++ b/pylintrc
@@ -20,7 +20,6 @@ evaluation = 10.0 - ((float(5 * error + warning + refactor + 
convention) / state
 comment = yes
 
 [BASIC]
-required-attributes =
 # disabling docstring checks since we have way too many without (complex
 # inheritance hierarchies)
 #no-docstring-rgx = __.*__
@@ -53,7 +52,6 @@ dummy-variables-rgx = _
 additional-builtins =
 
 [CLASSES]
-ignore-iface-methods =
 defining-attr-methods = __init__,__new__,setUp
 valid-classmethod-first-arg = cls,mcs
 
diff --git a/pylintrc-test b/pylintrc-test
index d61735a..686c5ef 100644
--- a/pylintrc-test
+++ b/pylintrc-test
@@ -19,7 +19,6 @@ evaluation = 10.0 - ((float(5 * error + warning + refactor + 
convention) / state
 comment = yes
 
 [BASIC]
-required-attributes =
 # disabling docstring checks since we have way too many without (complex
 # inheritance hierarchies)
 #no-docstring-rgx = __.*__
@@ -51,7 +50,6 @@ dummy-variables-rgx = _
 additional-builtins =
 
 [CLASSES]
-ignore-iface-methods =
 defining-attr-methods = __init__,__new__,setUp
 
 [DESIGN]
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 34/37] Use default value lambda param to avoid cell-var-from-loop

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Signed-off-by: Brian Foley 
---
 lib/tools/burnin.py | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/tools/burnin.py b/lib/tools/burnin.py
index 3054d37..a32e4db 100755
--- a/lib/tools/burnin.py
+++ b/lib/tools/burnin.py
@@ -677,8 +677,10 @@ class Burner(JobHandler):
 hypervisor=self.hypervisor,
 osparams=self.opts.osparams,
 )
-  remove_instance = lambda name: lambda: self.to_rem.append(name)
-  self.ExecOrQueue(instance, [op], post_process=remove_instance(instance))
+  # NB the i=instance default param is needed here so the lambda captures
+  # the variable. See https://docs.python.org/2/faq/programming.html#id11
+  rm_inst = lambda i=instance: self.to_rem.append(i) # pylint: 
disable=C0322
+  self.ExecOrQueue(instance, [op], post_process=rm_inst)
 
   @_DoBatch(False)
   def BurnModifyRuntimeMemory(self):
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 36/37] Quell the pylint wrong-import-order warnings

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Signed-off-by: Brian Foley 
---
 lib/backend.py| 7 ---
 lib/cli.py| 4 ++--
 lib/cli_opts.py   | 5 +++--
 lib/client/gnt_cluster.py | 8 +---
 lib/client/gnt_debug.py   | 7 ---
 lib/client/gnt_instance.py| 3 ++-
 lib/cmdlib/backup.py  | 3 ++-
 lib/cmdlib/instance_create.py | 2 +-
 lib/http/__init__.py  | 5 +++--
 lib/http/auth.py  | 4 ++--
 lib/http/client.py| 4 +++-
 lib/masterd/iallocator.py | 6 +++---
 lib/objects.py| 3 +--
 lib/rapi/client.py| 7 ---
 lib/rapi/testutils.py | 6 --
 lib/rpc/node.py   | 9 +
 lib/storage/drbd_info.py  | 3 ++-
 lib/tools/common.py   | 4 +++-
 lib/utils/security.py | 5 +++--
 lib/utils/x509.py | 9 +
 20 files changed, 61 insertions(+), 43 deletions(-)

diff --git a/lib/backend.py b/lib/backend.py
index 3cd07d1..634f2e5 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -47,11 +47,12 @@
 
 
 import base64
+import contextlib
+import collections
 import errno
 import logging
 import os
 import os.path
-import pycurl
 import random
 import re
 import shutil
@@ -60,8 +61,8 @@ import stat
 import tempfile
 import time
 import zlib
-import contextlib
-import collections
+
+import pycurl
 
 from ganeti import errors
 from ganeti import http
diff --git a/lib/cli.py b/lib/cli.py
index 8622738..f01d8d9 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -39,7 +39,9 @@ import logging
 import errno
 import itertools
 import shlex
+
 from cStringIO import StringIO
+from optparse import (OptionParser, TitledHelpFormatter)
 
 from ganeti import utils
 from ganeti import errors
@@ -60,8 +62,6 @@ from ganeti.cli_opts import *  # pylint: disable=W0401,W0614
 
 from ganeti.runtime import (GetClient)
 
-from optparse import (OptionParser, TitledHelpFormatter)
-
 
 __all__ = [
   # Generic functions for CLI programs
diff --git a/lib/cli_opts.py b/lib/cli_opts.py
index 6a7a33a..0b1bb0c 100644
--- a/lib/cli_opts.py
+++ b/lib/cli_opts.py
@@ -31,6 +31,9 @@
 """Module containing Ganeti's command line parsing options"""
 
 import re
+
+from optparse import (Option, OptionValueError)
+
 import simplejson
 
 from ganeti import utils
@@ -40,8 +43,6 @@ from ganeti import compat
 from ganeti import pathutils
 from ganeti import serializer
 
-from optparse import (Option, OptionValueError)
-
 
 __all__ = [
   "ABSOLUTE_OPT",
diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
index 9ad82d8..924a3f3 100644
--- a/lib/client/gnt_cluster.py
+++ b/lib/client/gnt_cluster.py
@@ -35,12 +35,14 @@
 # W0614: Unused import %s from wildcard import (since we need cli)
 # C0103: Invalid name gnt-cluster
 
-from cStringIO import StringIO
+import itertools
 import os
 import time
-import OpenSSL
 import tempfile
-import itertools
+
+from cStringIO import StringIO
+
+import OpenSSL
 
 from ganeti.cli import *
 from ganeti import bootstrap
diff --git a/lib/client/gnt_debug.py b/lib/client/gnt_debug.py
index e79ae8d..2393691 100644
--- a/lib/client/gnt_debug.py
+++ b/lib/client/gnt_debug.py
@@ -34,10 +34,11 @@
 # W0614: Unused import %s from wildcard import (since we need cli)
 # C0103: Invalid name gnt-backup
 
-import simplejson
-import time
-import socket
 import logging
+import socket
+import time
+
+import simplejson
 
 from ganeti.cli import *
 from ganeti import cli
diff --git a/lib/client/gnt_instance.py b/lib/client/gnt_instance.py
index 6280b83..1804c39 100644
--- a/lib/client/gnt_instance.py
+++ b/lib/client/gnt_instance.py
@@ -36,9 +36,10 @@
 
 import copy
 import itertools
-import simplejson
 import logging
 
+import simplejson
+
 from ganeti.cli import *
 from ganeti import opcodes
 from ganeti import constants
diff --git a/lib/cmdlib/backup.py b/lib/cmdlib/backup.py
index 84ceecf..88e0f0e 100644
--- a/lib/cmdlib/backup.py
+++ b/lib/cmdlib/backup.py
@@ -30,9 +30,10 @@
 
 """Logical units dealing with backup operations."""
 
-import OpenSSL
 import logging
 
+import OpenSSL
+
 from ganeti import compat
 from ganeti import constants
 from ganeti import errors
diff --git a/lib/cmdlib/instance_create.py b/lib/cmdlib/instance_create.py
index d69ec50..462522c 100644
--- a/lib/cmdlib/instance_create.py
+++ b/lib/cmdlib/instance_create.py
@@ -29,10 +29,10 @@
 
 """Logical unit for creating a single instance."""
 
-import OpenSSL
 import logging
 import os
 
+import OpenSSL
 
 from ganeti import compat
 from ganeti import constants
diff --git a/lib/http/__init__.py b/lib/http/__init__.py
index 596dd3d..1b04c2b 100644
--- a/lib/http/__init__.py
+++ b/lib/http/__init__.py
@@ -31,15 +31,16 @@
 
 """
 
+import errno
 import logging
 import mimetools
-import OpenSSL
 import select
 import socket
-import errno
 
 from cStringIO import StringIO
 
+import OpenSSL
+
 from ganeti import constants
 from ganeti import utils
 
diff --git a/lib/http/auth.py b/lib/http/auth.py
index 

[PATCH stable-2.15 19/37] Disable pylint invalid-name warning

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Signed-off-by: Brian Foley 
---
 lib/cli_opts.py | 2 +-
 lib/compat.py   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/cli_opts.py b/lib/cli_opts.py
index ae58ede..6a7a33a 100644
--- a/lib/cli_opts.py
+++ b/lib/cli_opts.py
@@ -551,7 +551,7 @@ class CliOption(Option):
 
 
 # optparse.py sets make_option, so we do it for our own option class, too
-cli_option = CliOption
+cli_option = CliOption # pylint: disable=C0103
 
 
 _YORNO = "yes|no"
diff --git a/lib/compat.py b/lib/compat.py
index c1f9b83..7aac59e 100644
--- a/lib/compat.py
+++ b/lib/compat.py
@@ -59,12 +59,12 @@ try:
   from hashlib import md5 as md5_hash # pylint: disable=W0611,E0611,F0401
   from hashlib import sha1 as sha1_hash # pylint: disable=W0611,E0611,F0401
   # this additional version is needed for compatibility with the hmac module
-  sha1 = sha1_hash
+  sha1 = sha1_hash # pylint: disable=C0103
 except ImportError:
   from md5 import new as md5_hash # pylint: disable=W0611
   import sha
   sha1 = sha
-  sha1_hash = sha.new
+  sha1_hash = sha.new # pylint: disable=C0103
 
 
 def _all(seq):
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 16/37] Disable pylint warning about use of deprecated pragmas

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Autogenerated RPC code like that in metad.py uses pylint: disable-all
to disable unimportant warnings. This option has been deprecated in
favour of the new skip-file option in pylint >= 0.27, but we want to
keep the old option for backward compatibility with earlier pylints.

Signed-off-by: Brian Foley 
---
 Makefile.am | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index d139d8b..3174c2c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2686,10 +2686,11 @@ PEP8_EXCLUDE = $(subst $(space),$(comma),$(strip 
$(notdir $(built_python_sources
 
 # A space-separated list of pylint warnings to completely ignore:
 # I0013 = disable warnings for ignoring whole files
+# I0022 = disable warning on use of deprecated pragma (disable-all)
 # W0142 = disable star-args warning. pylint 1.6 removed this warn as a bad idea
 # E0012 = disable bad option vals: some warnings are removed from newer 
pylints,
 # and older pylints don't understand newer pylint's options
-LINT_DISABLE = I0013 W0142 E0012
+LINT_DISABLE = I0013 I0022 W0142 E0012
 # Additional pylint options
 LINT_OPTS =
 # The combined set of pylint options
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 18/37] Disable pylint import-self warning

2016-12-05 Thread 'Brian Foley' via ganeti-devel
In this case it's a deliberate test to make sure that the contents
of lib/ have been correctly installed in a dir called ganeti.

Signed-off-by: Brian Foley 
---
 lib/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/__init__.py b/lib/__init__.py
index 1bb4f1a..d754172 100644
--- a/lib/__init__.py
+++ b/lib/__init__.py
@@ -33,7 +33,7 @@
 """Ganeti python modules"""
 
 try:
-  from ganeti import ganeti
+  from ganeti import ganeti # pylint: disable=W0406
 except ImportError:
   pass
 else:
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 17/37] Disable some pylint unused-import warnings

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Technically the imorts are unused in the module, but they're being
imported to make them available to users of the module.

Signed-off-by: Brian Foley 
---
 lib/compat.py | 2 +-
 lib/rapi/testutils.py | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/compat.py b/lib/compat.py
index e04d68d..c1f9b83 100644
--- a/lib/compat.py
+++ b/lib/compat.py
@@ -61,7 +61,7 @@ try:
   # this additional version is needed for compatibility with the hmac module
   sha1 = sha1_hash
 except ImportError:
-  from md5 import new as md5_hash
+  from md5 import new as md5_hash # pylint: disable=W0611
   import sha
   sha1 = sha
   sha1_hash = sha.new
diff --git a/lib/rapi/testutils.py b/lib/rapi/testutils.py
index 3c09db2..40989dd 100644
--- a/lib/rapi/testutils.py
+++ b/lib/rapi/testutils.py
@@ -49,8 +49,8 @@ import ganeti.rpc.client as rpccl
 from ganeti import rapi
 
 import ganeti.http.server # pylint: disable=W0611
-import ganeti.server.rapi
-import ganeti.rapi.client
+import ganeti.server.rapi # pylint: disable=W0611
+import ganeti.rapi.client # pylint: disable=W0611
 
 
 _URI_RE = re.compile(r"https://(?P.*):(?P\d+)(?P/.*)")
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 15/37] Remove pylint's star-args warning

2016-12-05 Thread 'Brian Foley' via ganeti-devel
pylint 1.6 removed this warning as it's perfectly legitimate Python.
Unconditionally switch off the warning when running pylint and
remove the pylint disable=W0142 pragmas.

Signed-off-by: Brian Foley 
---
 Makefile.am   | 3 ++-
 autotools/build-bash-completion   | 4 
 lib/build/sphinx_ext.py   | 3 +--
 lib/cli.py| 4 ++--
 lib/client/gnt_debug.py   | 1 -
 lib/client/gnt_instance.py| 3 +--
 lib/client/gnt_network.py | 1 -
 lib/cmdlib/cluster/verify.py  | 2 +-
 lib/cmdlib/instance_storage.py| 1 -
 lib/cmdlib/instance_utils.py  | 2 +-
 lib/cmdlib/network.py | 4 ++--
 lib/cmdlib/query.py   | 2 --
 lib/compat.py | 2 +-
 lib/config/__init__.py| 2 +-
 lib/errors.py | 1 -
 lib/objects.py| 2 +-
 lib/rapi/baserlib.py  | 2 +-
 lib/rapi/testutils.py | 2 +-
 lib/rpc/node.py   | 2 +-
 lib/server/noded.py   | 5 +
 lib/server/rapi.py| 2 +-
 lib/storage/container.py  | 2 +-
 lib/tools/burnin.py   | 4 ++--
 lib/utils/algo.py | 2 --
 lib/utils/retry.py| 3 ---
 lib/utils/text.py | 3 +--
 lib/workerpool.py | 2 +-
 test/py/cmdlib/testsupport/cmdlib_testcase.py | 2 --
 test/py/testutils/config_mock.py  | 2 +-
 tools/confd-client| 3 ---
 tools/ganeti-listrunner   | 2 +-
 31 files changed, 25 insertions(+), 50 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index abaa325..d139d8b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2686,9 +2686,10 @@ PEP8_EXCLUDE = $(subst $(space),$(comma),$(strip 
$(notdir $(built_python_sources
 
 # A space-separated list of pylint warnings to completely ignore:
 # I0013 = disable warnings for ignoring whole files
+# W0142 = disable star-args warning. pylint 1.6 removed this warn as a bad idea
 # E0012 = disable bad option vals: some warnings are removed from newer 
pylints,
 # and older pylints don't understand newer pylint's options
-LINT_DISABLE = I0013 E0012
+LINT_DISABLE = I0013 W0142 E0012
 # Additional pylint options
 LINT_OPTS =
 # The combined set of pylint options
diff --git a/autotools/build-bash-completion b/autotools/build-bash-completion
index 3e35ee1..4a064ba 100755
--- a/autotools/build-bash-completion
+++ b/autotools/build-bash-completion
@@ -703,8 +703,6 @@ def HaskellOptToOptParse(opts, kind):
   kept in sync
 
   """
-  # pylint: disable=W0142
-  # since we pass *opts in a number of places
   opts = opts.split(",")
   if kind == "none":
 return cli.cli_option(*opts, action="store_true")
@@ -761,8 +759,6 @@ def HaskellArgToCliArg(kind, min_cnt, max_cnt):
 max_cnt = None
   else:
 max_cnt = int(max_cnt)
-  # pylint: disable=W0142
-  # since we pass **kwargs
   kwargs = {"min": min_cnt, "max": max_cnt}
 
   if kind.startswith("choices=") or kind.startswith("suggest="):
diff --git a/lib/build/sphinx_ext.py b/lib/build/sphinx_ext.py
index 2150288..6b30894 100644
--- a/lib/build/sphinx_ext.py
+++ b/lib/build/sphinx_ext.py
@@ -281,9 +281,8 @@ def PythonEvalRole(role, rawtext, text, lineno, inliner,
   The expression's result is included as a literal.
 
   """
-  # pylint: disable=W0102,W0613,W0142
+  # pylint: disable=W0102,W0613
   # W0102: Dangerous default value as argument
-  # W0142: Used * or ** magic
   # W0613: Unused argument
 
   code = docutils.utils.unescape(text, restore_backslashes=True)
diff --git a/lib/cli.py b/lib/cli.py
index e29bc02..3c3241d 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -1732,8 +1732,8 @@ def GenerateTable(headers, fields, separator, data,
   if unitfields is None:
 unitfields = []
 
-  numfields = utils.FieldSet(*numfields)   # pylint: disable=W0142
-  unitfields = utils.FieldSet(*unitfields) # pylint: disable=W0142
+  numfields = utils.FieldSet(*numfields)
+  unitfields = utils.FieldSet(*unitfields)
 
   format_fields = []
   for field in fields:
diff --git a/lib/client/gnt_debug.py b/lib/client/gnt_debug.py
index d05fbc2..e79ae8d 100644
--- a/lib/client/gnt_debug.py
+++ b/lib/client/gnt_debug.py
@@ -103,7 +103,6 @@ def GenericOpCodes(opts, args):
 ToStdout("Loading...")
   for job_idx in range(opts.rep_job):
 for fname in args:
-  # pylint: disable=W0142
   op_data = simplejson.loads(utils.ReadFile(fname))
   op_list = [opcodes.OpCode.LoadOpCode(val) for val in op_data]
   op_list = op_list * opts.rep_op
diff --git a/lib/client/gnt_instance.py 

[PATCH stable-2.15 12/37] Cleanup: Remove unused/duplicate module/fn import

2016-12-05 Thread 'Brian Foley' via ganeti-devel
No functional change. Found by pylint's reimported warning.

Signed-off-by: Brian Foley 
---
 lib/cmdlib/cluster/__init__.py | 2 +-
 lib/cmdlib/instance_utils.py   | 2 +-
 lib/metad.py   | 4 ++--
 lib/server/rapi.py | 1 -
 tools/move-instance| 2 --
 5 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/lib/cmdlib/cluster/__init__.py b/lib/cmdlib/cluster/__init__.py
index cfe5feb..06dcb84 100644
--- a/lib/cmdlib/cluster/__init__.py
+++ b/lib/cmdlib/cluster/__init__.py
@@ -67,7 +67,7 @@ from ganeti.cmdlib.common import ShareAll, RunPostHook, \
   CheckIpolicyVsDiskTemplates, CheckDiskAccessModeValidity, \
   CheckDiskAccessModeConsistency, GetClientCertDigest, \
   AddInstanceCommunicationNetworkOp, ConnectInstanceCommunicationNetworkOp, \
-  CheckImageValidity, CheckDiskAccessModeConsistency, EnsureKvmdOnNodes
+  CheckImageValidity, EnsureKvmdOnNodes
 
 import ganeti.masterd.instance
 
diff --git a/lib/cmdlib/instance_utils.py b/lib/cmdlib/instance_utils.py
index 20cc3d4..3037d9e 100644
--- a/lib/cmdlib/instance_utils.py
+++ b/lib/cmdlib/instance_utils.py
@@ -44,7 +44,7 @@ from ganeti import pathutils
 from ganeti import utils
 from ganeti.cmdlib.common import AnnotateDiskParams, \
   ComputeIPolicyInstanceViolation, CheckDiskTemplateEnabled, \
-  CheckDiskTemplateEnabled, ComputeIPolicySpecViolation
+  ComputeIPolicySpecViolation
 
 
 #: Type description for changes as returned by L{ApplyContainerMods}'s
diff --git a/lib/metad.py b/lib/metad.py
index 48f543e..fbd24f1 100644
--- a/lib/metad.py
+++ b/lib/metad.py
@@ -40,7 +40,7 @@ from ganeti import constants
 from ganeti import errors
 import ganeti.rpc.client as cl
 from ganeti.rpc.transport import Transport
-from ganeti.rpc import errors
+from ganeti.rpc.errors import TimeoutError
 
 
 # If the metadata daemon is disabled, there is no stub generated for it.
@@ -70,7 +70,7 @@ if constants.ENABLE_METAD:
 try:
   self._InitTransport()
   return
-except errors.TimeoutError:
+except TimeoutError:
   logging.debug("Timout trying to connect to MetaD")
   if try_no == retries - 1:
 raise
diff --git a/lib/server/rapi.py b/lib/server/rapi.py
index 9782ada..0626b32 100644
--- a/lib/server/rapi.py
+++ b/lib/server/rapi.py
@@ -61,7 +61,6 @@ from ganeti.rapi import connector
 from ganeti.rapi import baserlib
 
 import ganeti.http.auth   # pylint: disable=W0611
-import ganeti.http.server
 
 
 class RemoteApiRequestContext(object):
diff --git a/tools/move-instance b/tools/move-instance
index 8913f62..4ca6327 100755
--- a/tools/move-instance
+++ b/tools/move-instance
@@ -51,8 +51,6 @@ from ganeti import compat
 from ganeti import rapi
 from ganeti import errors
 
-import ganeti.rapi.client # pylint: disable=W0611
-import ganeti.rapi.client_utils
 from ganeti.rapi.client import UsesRapiClient
 
 
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 14/37] Disable pylint warning about unrecognised --disable

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Allows us to disable warnings that are present in some versions of
pylint but not others.

Signed-off-by: Brian Foley 
---
 Makefile.am | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 8b859c1..abaa325 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2686,7 +2686,9 @@ PEP8_EXCLUDE = $(subst $(space),$(comma),$(strip $(notdir 
$(built_python_sources
 
 # A space-separated list of pylint warnings to completely ignore:
 # I0013 = disable warnings for ignoring whole files
-LINT_DISABLE = I0013
+# E0012 = disable bad option vals: some warnings are removed from newer 
pylints,
+# and older pylints don't understand newer pylint's options
+LINT_DISABLE = I0013 E0012
 # Additional pylint options
 LINT_OPTS =
 # The combined set of pylint options
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 13/37] Fix pylint >1.4 pycurl no-member warnings

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Add pycurl to the whitelist of C extensions that can be loaded for the
purposes of attribute checking. This works around a security feature
introduced in pylint 1.4 to avoid arbitrary code injection.

Signed-off-by: Brian Foley 
---
 Makefile.am  | 4 
 configure.ac | 4 
 2 files changed, 8 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 0b30033..8b859c1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2692,6 +2692,10 @@ LINT_OPTS =
 # The combined set of pylint options
 LINT_OPTS_ALL = $(LINT_OPTS) \
   $(addprefix --disable=,$(LINT_DISABLE))
+if !PYLINT_LT_14
+# Whitelist loading pycurl C extension for attribute checking
+LINT_OPTS_ALL += --extension-pkg-whitelist=pycurl
+endif
 
 LINT_TARGETS = pylint pylint-qa pylint-test
 if HAS_PEP8
diff --git a/configure.ac b/configure.ac
index 9b5d06f..f57b98c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -576,6 +576,10 @@ then
   AC_MSG_WARN([pylint not found, checking code will not be possible])
 fi
 
+# Note: Character classes ([...]) need to be double quoted due to autoconf
+# using m4
+AM_CONDITIONAL([PYLINT_LT_14], [$PYLINT --version | grep -q '^pylint 
\(0\.[[0-9]]\|1\.[[0-3]]\.'])
+
 # Check for pep8
 AC_ARG_VAR(PEP8, [pep8 path])
 AC_PATH_PROG(PEP8, [pep8], [])
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 09/37] Cleanup: Iterate dict rather than key list

2016-12-05 Thread 'Brian Foley' via ganeti-devel
The later explicitly constructs a new list of the dictionary's keys, which
we don't need here. Quietens pylint's consider-iterating-dictionary warning.

Signed-off-by: Brian Foley 
---
 autotools/check-imports | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/autotools/check-imports b/autotools/check-imports
index d82bd40..c90ea70 100755
--- a/autotools/check-imports
+++ b/autotools/check-imports
@@ -62,7 +62,7 @@ def main():
 
   for filename in args:
 # Reset global state
-for name in sys.modules.keys():
+for name in sys.modules:
   if name not in _STANDARD_MODULES:
 sys.modules.pop(name, None)
 
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 10/37] Cleanup: Remove some unneeded pylint disables

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Signed-off-by: Brian Foley 
---
 lib/cmdlib/cluster/verify.py | 1 -
 lib/cmdlib/test.py   | 1 -
 lib/server/noded.py  | 1 -
 3 files changed, 3 deletions(-)

diff --git a/lib/cmdlib/cluster/verify.py b/lib/cmdlib/cluster/verify.py
index dc5bac7..fe9e55e 100644
--- a/lib/cmdlib/cluster/verify.py
+++ b/lib/cmdlib/cluster/verify.py
@@ -141,7 +141,6 @@ class _VerifyErrors(object):
 # Report messages via the feedback_fn
 # pylint: disable=E1101
 self._feedback_fn(constants.ELOG_MESSAGE_LIST, prefixed_list)
-# pylint: enable=E1101
 
 # do not mark the operation as failed for WARN cases only
 if log_type == self.ETYPE_ERROR:
diff --git a/lib/cmdlib/test.py b/lib/cmdlib/test.py
index 5ec7c92..ce83cad 100644
--- a/lib/cmdlib/test.py
+++ b/lib/cmdlib/test.py
@@ -173,7 +173,6 @@ class LUTestDelay(NoHooksLU):
   # Instance of '_socketobject' has no ... member
   conn.settimeout(time_to_go)
   conn.recv(1)
-  # pylint: enable=E1101
 except socket.timeout, _:
   # A second timeout can occur if no data is sent
   return False
diff --git a/lib/server/noded.py b/lib/server/noded.py
index 880f2e1..31588b2 100644
--- a/lib/server/noded.py
+++ b/lib/server/noded.py
@@ -1370,7 +1370,6 @@ def SSLVerifyPeer(conn, cert, errnum, errdepth, ok):
   else:
 logging.error("Invalid errdepth value: %s.", errdepth)
 return False
-  # pylint: enable=W0613
 
 
 def PrepNoded(options, _):
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 04/37] Cleanup: del is a statement not a function

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Remove unnecessary parens to make pylint happy.

Signed-off-by: Brian Foley 
---
 lib/hypervisor/hv_kvm/monitor.py | 2 +-
 qa/qa_config.py  | 2 +-
 test/py/ganeti.hypervisor.hv_kvm_unittest.py | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/hypervisor/hv_kvm/monitor.py b/lib/hypervisor/hv_kvm/monitor.py
index f2b7d02..98c8a6f 100644
--- a/lib/hypervisor/hv_kvm/monitor.py
+++ b/lib/hypervisor/hv_kvm/monitor.py
@@ -104,7 +104,7 @@ class QmpMessage(object):
 """Delete the specified element from the QmpMessage.
 
 """
-del(self.data[key])
+del self.data[key]
 
   @staticmethod
   def BuildFromJsonString(json_string):
diff --git a/qa/qa_config.py b/qa/qa_config.py
index f84daf8..6035e4f 100644
--- a/qa/qa_config.py
+++ b/qa/qa_config.py
@@ -473,7 +473,7 @@ class _QaConfig(object):
 """Deletes a value from the configuration.
 
 """
-del(self._data[key])
+del self._data[key]
 
   def __len__(self):
 """Return the number of configuration items.
diff --git a/test/py/ganeti.hypervisor.hv_kvm_unittest.py 
b/test/py/ganeti.hypervisor.hv_kvm_unittest.py
index 4f0fbf6..09b74dc 100755
--- a/test/py/ganeti.hypervisor.hv_kvm_unittest.py
+++ b/test/py/ganeti.hypervisor.hv_kvm_unittest.py
@@ -174,7 +174,7 @@ class TestQmpMessage(testutils.GanetiTestCase):
 message = hv_kvm.QmpMessage(test_data)
 
 oldLen = len(message)
-del(message[toDelete])
+del message[toDelete]
 newLen = len(message)
 self.assertEqual(oldLen - 1, newLen)
 
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 11/37] Cleanup: Fix unidiomatic-typecheck

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Signed-off-by: Brian Foley 
---
 lib/cli.py  | 2 +-
 lib/ovf.py  | 2 +-
 lib/rpc_defs.py | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/cli.py b/lib/cli.py
index c8d8d1d..e29bc02 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -2796,7 +2796,7 @@ def _InitISpecsFromSplitOpts(ipolicy, ispecs_mem_size, 
ispecs_cpu_count,
   else:
 forced_type = TISPECS_CLUSTER_TYPES
   for specs in ispecs_transposed.values():
-assert type(specs) is dict
+assert isinstance(specs, dict)
 utils.ForceDictType(specs, forced_type)
 
   # then transpose
diff --git a/lib/ovf.py b/lib/ovf.py
index 4b65cb5..2662ca7 100644
--- a/lib/ovf.py
+++ b/lib/ovf.py
@@ -1278,7 +1278,7 @@ class OVFImporter(Converter):
   options
 
 """
-assert type(self.options.hypervisor) is tuple
+assert isinstance(self.options.hypervisor, tuple)
 assert len(self.options.hypervisor) == 2
 results = {}
 if self.options.hypervisor[0]:
diff --git a/lib/rpc_defs.py b/lib/rpc_defs.py
index 74b74d1..22637c6 100644
--- a/lib/rpc_defs.py
+++ b/lib/rpc_defs.py
@@ -147,7 +147,7 @@ def _NodeInfoPreProc(node, args):
   assert len(args) == 2
   # The storage_units argument is either a dictionary with one value for each
   # node, or a fixed value to be used for all the nodes
-  if type(args[0]) is dict:
+  if isinstance(args[0], dict):
 return [args[0][node], args[1]]
   else:
 return args
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 06/37] Cleanup: Fix for/else with no break in AddAuthorizedKeys

2016-12-05 Thread 'Brian Foley' via ganeti-devel
After a refactoring in c178396, the only break statement in the for/else
loop in AddAuthorizedKeys was removed. This means the else block is
always executed, so remove the else: to quell a pylint warning.

No functional change.

Signed-off-by: Brian Foley 
---
 lib/ssh.py | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/ssh.py b/lib/ssh.py
index 7d34f29..6e16674 100644
--- a/lib/ssh.py
+++ b/lib/ssh.py
@@ -170,13 +170,13 @@ def AddAuthorizedKeys(file_obj, keys):
in key_field_list
if split_key != line_key]
   nl = line.endswith("\n")
-else:
-  if not nl:
-f.write("\n")
-  for (key, _) in key_field_list:
-f.write(key.rstrip("\r\n"))
-f.write("\n")
-  f.flush()
+
+if not nl:
+  f.write("\n")
+for (key, _) in key_field_list:
+  f.write(key.rstrip("\r\n"))
+  f.write("\n")
+f.flush()
   finally:
 f.close()
 
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 07/37] Cleanup: StartInstance and RebootInstance return None

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Quell assignment-from-none warning

Signed-off-by: Brian Foley 
---
 lib/backend.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/backend.py b/lib/backend.py
index 82aa493..3cd07d1 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -2988,9 +2988,8 @@ def InstanceReboot(instance, reboot_type, 
shutdown_timeout, reason):
   elif reboot_type == constants.INSTANCE_REBOOT_HARD:
 try:
   InstanceShutdown(instance, shutdown_timeout, reason, store_reason=False)
-  result = StartInstance(instance, False, reason, store_reason=False)
+  StartInstance(instance, False, reason, store_reason=False)
   _StoreInstReasonTrail(instance.name, reason)
-  return result
 except errors.HypervisorError, err:
   _Fail("Failed to hard reboot instance '%s': %s", instance.name, err)
   else:
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 08/37] Cleanup: Remove unused format key

2016-12-05 Thread 'Brian Foley' via ganeti-devel
No functional change, but quietens pylint unused-format-string-argument
warning.

Signed-off-by: Brian Foley 
---
 lib/objects.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/objects.py b/lib/objects.py
index 96e7092..29dbbbc 100644
--- a/lib/objects.py
+++ b/lib/objects.py
@@ -2061,8 +2061,7 @@ class Cluster(TaggableObject):
   - at public visibility:  {public}
   - at private visibility: {private}
   - at secret visibility:  {secret}
-  """.format(dupes=formatter(duplicate_keys),
- public=formatter(params_public & duplicate_keys),
+  """.format(public=formatter(params_public & duplicate_keys),
  private=formatter(params_private & duplicate_keys),
  secret=formatter(params_secret & duplicate_keys))
   raise errors.OpPrereqError(msg)
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 02/37] Cleanup: Simplify boolean assignment

2016-12-05 Thread 'Brian Foley' via ganeti-devel
No functional change. Found by pylint's simplifiable-if-statement.

Signed-off-by: Brian Foley 
---
 lib/client/gnt_cluster.py | 7 +--
 tools/cluster-merge   | 5 +
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
index 40792e2..9ad82d8 100644
--- a/lib/client/gnt_cluster.py
+++ b/lib/client/gnt_cluster.py
@@ -294,11 +294,6 @@ def InitCluster(opts, args):
 
   default_ialloc_params = opts.default_iallocator_params
 
-  if opts.enabled_user_shutdown:
-enabled_user_shutdown = True
-  else:
-enabled_user_shutdown = False
-
   bootstrap.InitCluster(cluster_name=args[0],
 secondary_ip=opts.secondary_ip,
 vg_name=vg_name,
@@ -332,7 +327,7 @@ def InitCluster(opts, args):
 install_image=install_image,
 zeroing_image=zeroing_image,
 compression_tools=compression_tools,
-enabled_user_shutdown=enabled_user_shutdown,
+enabled_user_shutdown=opts.enabled_user_shutdown,
 )
   op = opcodes.OpClusterPostInit()
   SubmitOpCode(op, opts=opts)
diff --git a/tools/cluster-merge b/tools/cluster-merge
index 926b705..c83e350 100755
--- a/tools/cluster-merge
+++ b/tools/cluster-merge
@@ -448,10 +448,7 @@ class Merger(object):
   check_params_strict.append("shared_file_storage_dir")
 check_params.extend(check_params_strict)
 
-if self.params == _PARAMS_STRICT:
-  params_strict = True
-else:
-  params_strict = False
+params_strict = (self.params == _PARAMS_STRICT)
 
 for param_name in check_params:
   my_param = getattr(my_cluster, param_name)
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 00/37] Cleanup for pylint 1.6.4

2016-12-05 Thread 'Brian Foley' via ganeti-devel
Ganeti's python code passes the pylint checks in the version of pylint
included with Debian Jessie. Unfortunately this is a really old pylint
(0.26 from 2012) and the latest stable pylint (1.6.4 from 2016) has a
lot of additional checks. Ganeti produces many many warnings and errors
when run against 1.6.4.

This patchset is a first cut at fixing these, while still maintaining
 compatibility with pylint 0.26.

Brian Foley (37):
  Cleanup: Remove some unnecessary if (...) parens
  Cleanup: Simplify boolean assignment
  Cleanup: Use FOO not in BAR instead of not FOO in BAR
  Cleanup: del is a statement not a function
  Cleanup: Replace map/filters with list comprehensions
  Cleanup: Fix for/else with no break in AddAuthorizedKeys
  Cleanup: StartInstance and RebootInstance return None
  Cleanup: Remove unused format key
  Cleanup: Iterate dict rather than key list
  Cleanup: Remove some unneeded pylint disables
  Cleanup: Fix unidiomatic-typecheck
  Cleanup: Remove unused/duplicate module/fn import
  Fix pylint >1.4 pycurl no-member warnings
  Disable pylint warning about unrecognised --disable
  Remove pylint's star-args warning
  Disable pylint warning about use of deprecated pragmas
  Disable some pylint unused-import warnings
  Disable pylint import-self warning
  Disable pylint invalid-name warning
  Disable pylint eval-used warning
  Disable incorect pylint simplify-if-statement warning
  Disable pylint misplaced-comparison-constant warning
  Disable pylint unpacking-non-sequence warning
  Disable incorrect pylint not-callable warning
  Disable pylint unused-wildcard-import warning
  Disable unwanted pylint wrong-import-position warnings
  Disable pylint bare-except warning
  Disable pylint unsubscriptable-object warning
  Quell consider-using-enumerate warning
  Quell bad-whitespace warning
  Quell trailing newline
  Remove pylint tests removed in pylint 2.0
  Quell too-many-boolean-expressions
  Use default value lambda param to avoid cell-var-from-loop
  Quell cell-var-from-loop warning
  Quell the pylint wrong-import-order warnings
  Quell pylint socket.timeout warning

 Makefile.am   | 10 -
 autotools/build-bash-completion   | 14 +
 autotools/build-rpc   |  2 +-
 autotools/check-imports   |  5 +++--
 configure.ac  |  4 
 lib/__init__.py   |  2 +-
 lib/backend.py| 10 -
 lib/build/sphinx_ext.py   | 10 +
 lib/cli.py| 30 +--
 lib/cli_opts.py   |  7 ---
 lib/client/gnt_cluster.py | 15 ++
 lib/client/gnt_debug.py   |  8 +++
 lib/client/gnt_instance.py|  6 +++---
 lib/client/gnt_network.py |  1 -
 lib/cmdlib/backup.py  |  3 ++-
 lib/cmdlib/cluster/__init__.py|  2 +-
 lib/cmdlib/cluster/verify.py  |  9 
 lib/cmdlib/instance.py|  6 +++---
 lib/cmdlib/instance_create.py | 12 +--
 lib/cmdlib/instance_migration.py  |  3 ++-
 lib/cmdlib/instance_storage.py|  5 ++---
 lib/cmdlib/instance_utils.py  |  4 ++--
 lib/cmdlib/network.py |  4 ++--
 lib/cmdlib/query.py   |  2 --
 lib/cmdlib/test.py|  1 -
 lib/compat.py |  8 +++
 lib/config/__init__.py| 13 ++--
 lib/config/verify.py  |  4 ++--
 lib/errors.py |  1 -
 lib/http/__init__.py  |  5 +++--
 lib/http/auth.py  |  4 ++--
 lib/http/client.py|  6 --
 lib/hypervisor/hv_kvm/__init__.py |  4 ++--
 lib/hypervisor/hv_kvm/monitor.py  |  4 ++--
 lib/hypervisor/hv_xen.py  |  4 ++--
 lib/jqueue/__init__.py|  2 +-
 lib/luxi.py   |  9 
 lib/masterd/iallocator.py |  8 +++
 lib/metad.py  |  4 ++--
 lib/objects.py| 10 -
 lib/ovf.py|  5 +++--
 lib/rapi/baserlib.py  |  2 +-
 lib/rapi/client.py|  7 ---
 lib/rapi/testutils.py | 12 ++-
 lib/rpc/node.py   | 16 +++---
 lib/rpc_defs.py   |  2 +-
 lib/server/masterd.py |  2 +-
 lib/server/noded.py   |  6 +-
 lib/server/rapi.py

[PATCH stable-2.15 05/37] Cleanup: Replace map/filters with list comprehensions

2016-12-05 Thread 'Brian Foley' via ganeti-devel
No functional change, but more pythonic and quells pylint's
 deprecated-lambda warning.

Signed-off-by: Brian Foley 
---
 lib/cli.py | 14 ++
 lib/cmdlib/cluster/verify.py   |  6 +++---
 lib/cmdlib/instance_storage.py |  2 +-
 lib/config/__init__.py | 11 +--
 lib/http/client.py |  2 +-
 lib/hypervisor/hv_xen.py   |  4 ++--
 lib/jqueue/__init__.py |  2 +-
 lib/luxi.py|  9 +
 lib/rpc/node.py|  5 ++---
 lib/storage/bdev.py| 11 +--
 lib/storage/drbd.py|  4 ++--
 lib/storage/filestorage.py |  6 +++---
 lib/tools/node_cleanup.py  |  5 ++---
 lib/utils/livelock.py  |  4 ++--
 lib/utils/text.py  |  2 +-
 15 files changed, 41 insertions(+), 46 deletions(-)

diff --git a/lib/cli.py b/lib/cli.py
index 2971c60..c8d8d1d 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -2358,10 +2358,9 @@ def GetNodesSshPorts(nodes, cl):
   @rtype: a list of tuples
 
   """
-  return map(lambda t: t[0],
- cl.QueryNodes(names=nodes,
-   fields=["ndp/ssh_port"],
-   use_locking=False))
+  return [t[0] for t in cl.QueryNodes(names=nodes,
+  fields=["ndp/ssh_port"],
+  use_locking=False)]
 
 
 def GetNodeUUIDs(nodes, cl):
@@ -2375,10 +2374,9 @@ def GetNodeUUIDs(nodes, cl):
   @rtype: a list of tuples
 
   """
-  return map(lambda t: t[0],
- cl.QueryNodes(names=nodes,
-   fields=["uuid"],
-   use_locking=False))
+  return [t[0] for t in cl.QueryNodes(names=nodes,
+  fields=["uuid"],
+  use_locking=False)]
 
 
 def _ToStream(stream, txt, *args):
diff --git a/lib/cmdlib/cluster/verify.py b/lib/cmdlib/cluster/verify.py
index 77358dc..dc5bac7 100644
--- a/lib/cmdlib/cluster/verify.py
+++ b/lib/cmdlib/cluster/verify.py
@@ -685,7 +685,7 @@ class LUClusterVerifyGroup(LogicalUnit, _VerifyErrors):
 # exclusive_storage wants all PVs to have the same size (approximately),
 # if the smallest and the biggest ones are okay, everything is fine.
 # pv_min is None iff pv_max is None
-vals = filter((lambda ni: ni.pv_min is not None), node_image.values())
+vals = [ni for ni in node_image.values() if ni.pv_min is not None]
 if not vals:
   return
 (pvmin, minnode_uuid) = min((ni.pv_min, ni.uuid) for ni in vals)
@@ -1972,8 +1972,8 @@ class LUClusterVerifyGroup(LogicalUnit, _VerifyErrors):
 if self._exclusive_storage:
   node_verify_param[constants.NV_EXCLUSIVEPVS] = True
 
-node_group_uuids = dict(map(lambda n: (n.name, n.group),
-self.cfg.GetAllNodesInfo().values()))
+node_group_uuids = dict((n.name, n.group) for n in
+self.cfg.GetAllNodesInfo().values())
 groups_config = self.cfg.GetAllNodeGroupsInfoDict()
 
 # At this point, we have the in-memory data structures complete,
diff --git a/lib/cmdlib/instance_storage.py b/lib/cmdlib/instance_storage.py
index 92f5118..5508eb8 100644
--- a/lib/cmdlib/instance_storage.py
+++ b/lib/cmdlib/instance_storage.py
@@ -1347,7 +1347,7 @@ def ImageDisks(lu, instance, image, disks=None):
   if disks is None:
 disks = [(0, inst_disks[0])]
   else:
-disks = map(lambda idx: (idx, inst_disks[idx]), disks)
+disks = [(idx, inst_disks[idx]) for idx in disks]
 
   logging.info("Pausing synchronization of disks of instance '%s'",
instance.name)
diff --git a/lib/config/__init__.py b/lib/config/__init__.py
index 7011da2..954a09c 100644
--- a/lib/config/__init__.py
+++ b/lib/config/__init__.py
@@ -1253,8 +1253,7 @@ class ConfigWriter(object):
 if self._offline:
   raise errors.ProgrammerError("Can't call ComputeDRBDMap in offline mode")
 else:
-  return dict(map(lambda (k, v): (k, dict(v)),
-  self._wconfd.ComputeDRBDMap()))
+  return dict((k, dict(v)) for (k, v) in self._wconfd.ComputeDRBDMap())
 
   def AllocateDRBDMinor(self, node_uuids, disk_uuid):
 """Allocate a drbd minor.
@@ -1723,8 +1722,8 @@ class ConfigWriter(object):
 dictionaries.
 
 """
-return dict(map(lambda (uuid, ng): (uuid, ng.ToDict()),
-self._UnlockedGetAllNodeGroupsInfo().items()))
+return dict((uuid, ng.ToDict()) for (uuid, ng) in
+self._UnlockedGetAllNodeGroupsInfo().items())
 
   @ConfigSync(shared=1)
   def GetNodeGroupList(self):
@@ -2016,7 +2015,7 @@ class ConfigWriter(object):
 
 if expanded_name is not None:
   # there has to be exactly one instance with that name
-  inst = (filter(lambda n: n.name == expanded_name, all_insts)[0])
+  inst = [n for n in all_insts if n.name == expanded_name][0]
   return (inst.uuid, inst.name)
 else:
   return 

[PATCH stable-2.15 03/37] Cleanup: Use FOO not in BAR instead of not FOO in BAR

2016-12-05 Thread 'Brian Foley' via ganeti-devel
No functional change, but more pythonic, and makes pylint's
unneeded-not warning happy.

Signed-off-by: Brian Foley 
---
 lib/utils/io.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/utils/io.py b/lib/utils/io.py
index 9e74d68..49bab0d 100644
--- a/lib/utils/io.py
+++ b/lib/utils/io.py
@@ -190,7 +190,7 @@ def WriteFile(file_name, fn=None, data=None,
 raise errors.ProgrammerError("Both atime and mtime must be either"
  " set or None")
 
-  if not keep_perms in KEEP_PERMS_VALUES:
+  if keep_perms not in KEEP_PERMS_VALUES:
 raise errors.ProgrammerError("Invalid value for keep_perms: %s" %
  keep_perms)
   if keep_perms == KP_ALWAYS and (uid != -1 or gid != -1 or mode is not None):
-- 
2.8.0.rc3.226.g39d4020



[PATCH stable-2.15 01/37] Cleanup: Remove some unnecessary if (...) parens

2016-12-05 Thread 'Brian Foley' via ganeti-devel
No functional change, but makes pylint 1.1.0 a little happier, and
is more pythonic.

Signed-off-by: Brian Foley 
---
 lib/cli.py| 4 ++--
 lib/cmdlib/instance.py| 2 +-
 lib/hypervisor/hv_kvm/__init__.py | 4 ++--
 lib/uidpool.py| 2 +-
 lib/utils/process.py  | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/cli.py b/lib/cli.py
index 362f2ae..2971c60 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -2907,7 +2907,7 @@ def CreateIPolicyFromOpts(ispecs_mem_size=None,
 
   split_specs = (ispecs_mem_size or ispecs_cpu_count or ispecs_disk_count or
  ispecs_disk_size or ispecs_nic_count)
-  if (split_specs and (minmax_ispecs is not None or std_ispecs is not None)):
+  if split_specs and (minmax_ispecs is not None or std_ispecs is not None):
 raise errors.OpPrereqError("A --specs-xxx option cannot be specified"
" together with any --ipolicy-xxx-specs option",
errors.ECODE_INVAL)
@@ -2918,7 +2918,7 @@ def CreateIPolicyFromOpts(ispecs_mem_size=None,
 _InitISpecsFromSplitOpts(ipolicy_out, ispecs_mem_size, ispecs_cpu_count,
  ispecs_disk_count, ispecs_disk_size,
  ispecs_nic_count, group_ipolicy, fill_all)
-  elif (minmax_ispecs is not None or std_ispecs is not None):
+  elif minmax_ispecs is not None or std_ispecs is not None:
 _InitISpecsFromFullOpts(ipolicy_out, minmax_ispecs, std_ispecs,
 group_ipolicy, allowed_values)
 
diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
index 4f46910..a8335dc 100644
--- a/lib/cmdlib/instance.py
+++ b/lib/cmdlib/instance.py
@@ -179,7 +179,7 @@ class LUInstanceRename(LogicalUnit):
 renamed_storage = [d for d in disks
if (d.dev_type in constants.DTS_FILEBASED and
d.dev_type != constants.DT_GLUSTER)]
-if (renamed_storage and self.op.new_name != self.instance.name):
+if renamed_storage and self.op.new_name != self.instance.name:
   disks = self.cfg.GetInstanceDisks(self.instance.uuid)
   old_file_storage_dir = os.path.dirname(disks[0].logical_id[1])
   rename_file_storage = True
diff --git a/lib/hypervisor/hv_kvm/__init__.py 
b/lib/hypervisor/hv_kvm/__init__.py
index 174621f..cd29baa 100644
--- a/lib/hypervisor/hv_kvm/__init__.py
+++ b/lib/hypervisor/hv_kvm/__init__.py
@@ -153,7 +153,7 @@ def _GetDriveURI(disk, link, uri):
   access_mode = disk.params.get(constants.LDP_ACCESS,
 constants.DISK_KERNELSPACE)
   # If uri is available, use it during startup/hot-add
-  if (uri and access_mode == constants.DISK_USERSPACE):
+  if uri and access_mode == constants.DISK_USERSPACE:
 drive_uri = uri
   # Otherwise use the link previously created
   else:
@@ -1109,7 +1109,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
   # TODO (2.8): kernel_irqchip and kvm_shadow_mem machine properties, as
   # extra hypervisor parameters. We should also investigate whether and how
   # shadow_mem should be considered for the resource model.
-  if (hvp[constants.HV_KVM_FLAG] == constants.HT_KVM_ENABLED):
+  if hvp[constants.HV_KVM_FLAG] == constants.HT_KVM_ENABLED:
 specprop = ",accel=kvm"
   else:
 specprop = ""
diff --git a/lib/uidpool.py b/lib/uidpool.py
index 778c2f5..c1cd684 100644
--- a/lib/uidpool.py
+++ b/lib/uidpool.py
@@ -310,7 +310,7 @@ def RequestUnusedUid(all_uids):
   taken_uids = list(taken_uids)
   random.shuffle(taken_uids)
 
-  for uid in (unused_uids + taken_uids):
+  for uid in unused_uids + taken_uids:
 try:
   # Create the lock file
   # Note: we don't care if it exists. Only the fact that we can
diff --git a/lib/utils/process.py b/lib/utils/process.py
index 268ff54..29d8b0c 100644
--- a/lib/utils/process.py
+++ b/lib/utils/process.py
@@ -1097,7 +1097,7 @@ def CloseFDs(noclose_fds=None):
 MAXFD = 1024
 
   maxfd = resource.getrlimit(resource.RLIMIT_NOFILE)[1]
-  if (maxfd == resource.RLIM_INFINITY):
+  if maxfd == resource.RLIM_INFINITY:
 maxfd = MAXFD
 
   # Iterate through and close all file descriptors (except the standard ones)
-- 
2.8.0.rc3.226.g39d4020