>
>
>
> @@ -992,7 +997,12 @@ def VerifyNode(what, cluster_name, all_hvparams):
> # Try to contact all nodes
> val = {}
> for node in nodes:
> - success, message =
> _GetSshRunner(cluster_name).VerifyNodeHostname(node)
> + params = groups_cfg.get(node_groups.get(node))
> + ssh_port = params["ndparams"].get(constants.ND_SSH_PORT)
> + logging.debug("Ssh port %s (None = default), derived from %s for
> %s",
> + str(ssh_port), str(params), node)
>
While debugging messages are just fine, printing all the group params could
clutter up logs, and provides little benefit here.
Perhaps remove this part of the output, or print just the group name?
> diff --git a/lib/cmdlib/node.py b/lib/cmdlib/node.py
> index 89155ac..62e3795 100644
> --- a/lib/cmdlib/node.py
> +++ b/lib/cmdlib/node.py
> @@ -313,7 +313,10 @@ class LUNodeAdd(LogicalUnit):
> cname = self.cfg.GetClusterName()
> result = rpcrunner.call_node_verify_light(
> [node_name], vparams, cname,
> - self.cfg.GetClusterInfo().hvparams)[node_name]
> + self.cfg.GetClusterInfo().hvparams,
> + {},
> + self.cfg.GetAllNodeGroupsInfoDict()
> + )[node_name]
> (errmsgs, _) = CheckNodePVs(result.payload, excl_stor)
> if errmsgs:
> raise errors.OpPrereqError("Checks on node PVs failed: %s" %
>
Why is an empty dictionary being passed to the node_verify_light call here?
It just invokes node_verify in the current code, and all node_verify calls
provide a dictionary even for one node.
If there is a reason, it would be nice to add a comment in the code to
explain why.
> diff --git a/lib/rpc_defs.py b/lib/rpc_defs.py
> index e56341d..712fb74 100644
> --- a/lib/rpc_defs.py
> +++ b/lib/rpc_defs.py
> @@ -489,6 +489,9 @@ _NODE_CALLS = [
> ("checkdict", None, "What to verify"),
> ("cluster_name", None, "Cluster name"),
> ("all_hvparams", None, "Dictionary mapping hypervisor names to
> hvparams"),
> + ("node_groups", None, "node names mapped to their group uuids"),
> + ("groups_cfg", None,
> + "a dictionary mapping group uuids to their configuration"),
> ], None, None, "Request verification of given parameters"),
> ("node_volumes", MULTI, None, constants.RPC_TMO_FAST, [], None, None,
> "Gets all volumes on node(s)"),
> @@ -611,6 +614,9 @@ CALLS = {
> ("checkdict", None, "What to verify"),
> ("cluster_name", None, "Cluster name"),
> ("hvparams", None, "Dictionary mapping hypervisor names to
> hvparams"),
> + ("node_groups", None, "node names mapped to their group uuids"),
> + ("groups_cfg", None,
> + "a dictionary mapping group uuids to their configuration"),
> ], None, None, "Request verification of given parameters"),
> ]),
> "RpcClientConfig": _Prepare([
Nitpicking time: all other calls align the continued entries with the first
entry - you have an extra space.
> @classmethod
> def perspective_node_verify_light(cls, params):
> """Run a light verify sequence on this node.
>
> + This means verifying a node just before it's being added to a cluster.
> +
> """
> # So far it's the same as the normal node_verify
> return cls.perspective_node_verify(params)
>
Nitpicking: It is currently used only to verify a node just before it is
added to a cluster.
And it could be a good idea to move the comment about this call being
equivalent to node_verify into the docstring.
> diff --git a/lib/ssh.py b/lib/ssh.py
> index c694341..32a35e5 100644
> --- a/lib/ssh.py
> +++ b/lib/ssh.py
> @@ -123,7 +123,8 @@ class SshRunner:
> self.ipv6 = ipv6
>
> def _BuildSshOptions(self, batch, ask_key, use_cluster_key,
> - strict_host_check, private_key=None, quiet=True):
> + strict_host_check, private_key=None, quiet=True,
> + port=None):
> """Builds a list with needed SSH options.
>
> @param batch: same as ssh's batch option
> @@ -134,6 +135,7 @@ class SshRunner:
> @param strict_host_check: this makes the host key checking strict
> @param private_key: use this private key instead of the default
> @param quiet: whether to enable -q to ssh
> + @param port: the SSH port to use, or None to use teh default
>
s/teh/the/
Thanks,
Hrvoje Ribicic
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370
