Merge branch 'devel-2.5'

* devel-2.5:
  Delete master IPs from mergee master nodes
  Use pep8 utility in “make lint”
  Two more PEP8 fixes
  check-python-code: Give location(s) of lines longer than 80 chars

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

diff --git a/Makefile.am b/Makefile.am
index a9ef001..444b53e 100644 (file)
--- a/Makefile.am
+++ b/Makefile.am
@@ -9,6 +9,11 @@
 abs_top_builddir = @abs_top_builddir@
 abs_top_srcdir = @abs_top_srcdir@
 
+# Helper values for calling builtin functions
+empty :=
+space := $(empty) $(empty)
+comma := ,
+
 # Use bash in order to be able to use pipefail
 SHELL=/bin/bash
 
@@ -150,15 +155,17 @@ CLEANFILES = \
 BUILT_SOURCES = \
        ganeti \
        stamp-srclinks \
-       lib/_autoconf.py \
-       lib/_vcsversion.py \
        $(all_dirfiles) \
-       $(PYTHON_BOOTSTRAP)
+       $(PYTHON_BOOTSTRAP) \
+       $(BUILT_PYTHON_SOURCES)
 
-nodist_pkgpython_PYTHON = \
+BUILT_PYTHON_SOURCES = \
        lib/_autoconf.py \
        lib/_vcsversion.py
 
+nodist_pkgpython_PYTHON = \
+       $(BUILT_PYTHON_SOURCES)
+
 noinst_PYTHON = \
        lib/build/__init__.py \
        lib/build/sphinx_ext.py
@@ -365,7 +372,7 @@ $(RUN_IN_TEMPDIR): | $(all_dirfiles)
 # it changes
 doc/html/index.html: $(docrst) $(docpng) doc/conf.py configure.ac \
        $(RUN_IN_TEMPDIR) lib/build/sphinx_ext.py lib/opcodes.py lib/ht.py \
-       | lib/_autoconf.py lib/_vcsversion.py
+       | $(BUILT_PYTHON_SOURCES)
        @test -n "$(SPHINX)" || \
            { echo 'sphinx-build' not found during configure; exit 1; }
        @mkdir_p@ $(dir $@)
@@ -1063,9 +1070,23 @@ hs-check: htools/test
        @rm -f test.tix
        ./htools/test
 
+# E111: indentation is not a multiple of four
+# E261: at least two spaces before inline comment
+# E501: line too long (80 characters)
+PEP8_IGNORE = E111,E261,E501
+
+# For excluding pep8 expects filenames only, not whole paths
+PEP8_EXCLUDE = $(subst $(space),$(comma),$(strip $(notdir $(BUILT_PYTHON_SOURCES))))
+
 .PHONY: lint
 lint: $(BUILT_SOURCES)
        @test -n "$(PYLINT)" || { echo 'pylint' not found during configure; exit 1; }
+       if test -z "$(PEP8)"; then \
+               echo '"pep8" not found during configure' >&2; \
+       else \
+               $(PEP8) --repeat --ignore='$(PEP8_IGNORE)' --exclude='$(PEP8_EXCLUDE)' \
+                       $(lint_python_code); \
+       fi
        $(PYLINT) $(LINT_OPTS) $(lint_python_code)
        cd $(top_srcdir)/qa && \
          PYTHONPATH=$(abs_top_srcdir) $(PYLINT) $(LINT_OPTS) \

diff --git a/autotools/check-python-code b/autotools/check-python-code
index ec1d96b..166e12d 100755 (executable)
--- a/autotools/check-python-code
+++ b/autotools/check-python-code
@@ -20,6 +20,13 @@
 
 set -e
 
+readonly maxlinelen=$(for ((i=0; i<81; ++i)); do echo -n .; done)
+
+if [[ "${#maxlinelen}" != 81 ]]; then
+  echo "Internal error: Check for line length is incorrect" >&2
+  exit 1
+fi
+
 # "[...] If the last ARG evaluates to 0, let returns 1; 0 is returned
 # otherwise.", hence ignoring the return value.
 let problems=0 || :
@@ -47,7 +54,7 @@ for script; do
     echo "Found editor-specific settings in $script" >&2
   fi
 
-  if [[ "$(wc --max-line-length < "$script")" -gt 80 ]]; then
+  if grep -n -H "^$maxlinelen" "$script"; then
     let ++problems
     echo "Longest line in $script is longer than 80 characters" >&2
   fi

diff --git a/configure.ac b/configure.ac
index 25491a1..764a590 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -317,6 +317,14 @@ then
   AC_MSG_WARN([pylint not found, checking code will not be possible])
 fi
 
+# Check for pep8
+AC_ARG_VAR(PEP8, [pep8 path])
+AC_PATH_PROG(PEP8, [pep8], [])
+if test -z "$PEP8"
+then
+  AC_MSG_WARN([pep8 not found, checking code will not be complete])
+fi
+
 # Check for socat
 AC_ARG_VAR(SOCAT, [socat path])
 AC_PATH_PROG(SOCAT, [socat], [])

diff --git a/doc/devnotes.rst b/doc/devnotes.rst
index 35d223c..f9646fb 100644 (file)
--- a/doc/devnotes.rst
+++ b/doc/devnotes.rst
@@ -16,6 +16,7 @@ Most dependencies from :doc:`install-quick`, plus (for Python):
 - the `en_US.UTF-8` locale must be enabled on the system
 - `pylint <http://www.logilab.org/857>`_ and its associated
   dependencies
+- `pep8 <https://github.com/jcrocholl/pep8/>`_
 
 Note that for pylint, at the current moment the following versions
 need to be used::

diff --git a/lib/client/gnt_group.py b/lib/client/gnt_group.py
index 538c1cb..2fb73e1 100644 (file)
--- a/lib/client/gnt_group.py
+++ b/lib/client/gnt_group.py
@@ -130,17 +130,13 @@ def SetGroupParams(opts, args):
   @return: the desired exit code
 
   """
-  all_changes = {
-    "ndparams": opts.ndparams,
-    "alloc_policy": opts.alloc_policy,
-  }
-
-  if all_changes.values().count(None) == len(all_changes):
+  if opts.ndparams is None and opts.alloc_policy is None:
     ToStderr("Please give at least one of the parameters.")
     return 1
 
-  op = opcodes.OpGroupSetParams(group_name=args[0], # pylint: disable-msg=W0142
-                                **all_changes)
+  op = opcodes.OpGroupSetParams(group_name=args[0],
+                                ndparams=opts.ndparams,
+                                alloc_policy=opts.alloc_policy)
   result = SubmitOrSend(op, opts)
 
   if result:

diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index a57a0d9..d64e6c7 100644 (file)
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -9828,6 +9828,8 @@ class TLReplaceDisks(Tasklet):
     """
     steps_total = 6
 
+    pnode = self.instance.primary_node
+
     # Step: check device activation
     self.lu.LogStep(1, steps_total, "Check device existence")
     self._CheckDisksExistence([self.instance.primary_node])
@@ -9902,10 +9904,8 @@ class TLReplaceDisks(Tasklet):
                                  " soon as possible"))
 
     self.lu.LogInfo("Detaching primary drbds from the network (=> standalone)")
-    result = self.rpc.call_drbd_disconnect_net([self.instance.primary_node],
-                                               self.node_secondary_ip,
-                                               self.instance.disks)\
-                                              [self.instance.primary_node]
+    result = self.rpc.call_drbd_disconnect_net([pnode], self.node_secondary_ip,
+                                               self.instance.disks)[pnode]
 
     msg = result.fail_msg
     if msg:

diff --git a/tools/cluster-merge b/tools/cluster-merge
index 7d7091f..e2fc848 100755 (executable)
--- a/tools/cluster-merge
+++ b/tools/cluster-merge
@@ -40,6 +40,7 @@ from ganeti import constants
 from ganeti import errors
 from ganeti import ssh
 from ganeti import utils
+from ganeti import netutils
 
 
 _GROUPS_MERGE = "merge"
@@ -109,13 +110,16 @@ class MergerData(object):
   """Container class to hold data used for merger.
 
   """
-  def __init__(self, cluster, key_path, nodes, instances, config_path=None):
+  def __init__(self, cluster, key_path, nodes, instances, master_node,
+               master_ip, config_path=None):
     """Initialize the container.
 
     @param cluster: The name of the cluster
     @param key_path: Path to the ssh private key used for authentication
     @param nodes: List of online nodes in the merging cluster
     @param instances: List of instances running on merging cluster
+    @param master_node: Name of the master node
+    @param master_ip: Cluster IP
     @param config_path: Path to the merging cluster config
 
     """
@@ -123,6 +127,8 @@ class MergerData(object):
     self.key_path = key_path
     self.nodes = nodes
     self.instances = instances
+    self.master_node = master_node
+    self.master_ip = master_ip
     self.config_path = config_path
 
 
@@ -206,7 +212,26 @@ class Merger(object):
                                  (cluster, result.fail_reason, result.output))
       instances = result.stdout.splitlines()
 
-      self.merger_data.append(MergerData(cluster, key_path, nodes, instances))
+      path = utils.PathJoin(constants.DATA_DIR, "ssconf_%s" %
+                            constants.SS_MASTER_NODE)
+      result = self._RunCmd(cluster, "cat %s" % path, private_key=key_path)
+      if result.failed:
+        raise errors.RemoteError("Unable to retrieve the master node name from"
+                                 " %s. Fail reason: %s; output: %s" %
+                                 (cluster, result.fail_reason, result.output))
+      master_node = result.stdout.strip()
+
+      path = utils.PathJoin(constants.DATA_DIR, "ssconf_%s" %
+                            constants.SS_MASTER_IP)
+      result = self._RunCmd(cluster, "cat %s" % path, private_key=key_path)
+      if result.failed:
+        raise errors.RemoteError("Unable to retrieve the master IP from"
+                                 " %s. Fail reason: %s; output: %s" %
+                                 (cluster, result.fail_reason, result.output))
+      master_ip = result.stdout.strip()
+
+      self.merger_data.append(MergerData(cluster, key_path, nodes, instances,
+                                         master_node, master_ip))
 
   def _PrepareAuthorizedKeys(self):
     """Prepare the authorized_keys on every merging node.
@@ -289,6 +314,31 @@ class Merger(object):
                                  " Fail reason: %s; output: %s" %
                                  (cluster, result.fail_reason, result.output))
 
+  def _RemoveMasterIps(self):
+    """Removes the master IPs from the master nodes of each cluster.
+
+    """
+    for data in self.merger_data:
+      master_ip_family = netutils.IPAddress.GetAddressFamily(data.master_ip)
+      master_ip_len = netutils.IP4Address.iplen
+      if master_ip_family == netutils.IP6Address.family:
+        master_ip_len = netutils.IP6Address.iplen
+      # Not using constants.IP_COMMAND_PATH because the command might run on a
+      # machine in which the ip path is different, so it's better to rely on
+      # $PATH.
+      cmd = "ip address del %s/%s dev $(cat %s)" % (
+             data.master_ip,
+             master_ip_len,
+             utils.PathJoin(constants.DATA_DIR, "ssconf_%s" %
+                            constants.SS_MASTER_NETDEV))
+      result = self._RunCmd(data.master_node, cmd, max_attempts=3)
+      if result.failed:
+        raise errors.RemoteError("Unable to remove master IP on %s."
+                                 " Fail reason: %s; output: %s" %
+                                 (data.master_node,
+                                  result.fail_reason,
+                                  result.output))
+
   def _StopDaemons(self):
     """Stop all daemons on merging nodes.
 
@@ -693,6 +743,8 @@ class Merger(object):
       self._StopDaemons()
       logging.info("Merging config")
       self._FetchRemoteConfig()
+      logging.info("Removing master IPs on mergee master nodes")
+      self._RemoveMasterIps()
 
       logging.info("Stopping master daemon")
       self._KillMasterDaemon()