>
>
>
> @@ -3535,6 +3542,22 @@ def GetOnlineNodes(nodes, cl=None, nowarn=False,
> secondary_ips=False,
> return map(fn, online)
>
>
> +def GetNodesSshPorts(nodes, cl):
> + """Retrieves SSH ports of given nodes.
> +
> + @param nodes: the names of nodes
> + @type nodes: a list of strings
> + @param cl: a client to use for the query
> + @type cl: L{Client}
> + @return: the list SSH ports corresponding to the nodes
>
s/the list SSH ports/the list of SSH ports/
> diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
> index 64594ce..6733f49 100644
> --- a/lib/client/gnt_cluster.py
> +++ b/lib/client/gnt_cluster.py
> @@ -570,17 +570,20 @@ def ClusterCopyFile(opts, args):
> errors.ECODE_INVAL)
>
> cl = GetClient()
> + try:
> + cluster_name = cl.QueryConfigValues(["cluster_name"])[0]
>
> - cluster_name = cl.QueryConfigValues(["cluster_name"])[0]
> -
> - results = GetOnlineNodes(nodes=opts.nodes, cl=cl, filter_master=True,
> - secondary_ips=opts.use_replication_network,
> - nodegroup=opts.nodegroup)
> + results = GetOnlineNodes(nodes=opts.nodes, cl=cl, filter_master=True,
> + secondary_ips=opts.use_replication_network,
> + nodegroup=opts.nodegroup)
> + ports = GetNodesSshPorts(opts.nodes, cl)
> + finally:
> + cl.Close()
>
>
Actually, this is the first use I have seen of the client's Close method in
our codebase. Could we perhaps remove it?
Either the resources of the client are collected without the use of close,
or the whole codebase is doing everything horribly wrong.
If the first is true, and it is more likely to be true, then adding an
explicit close with no reason is just confusing.
As a result of looking into this, you will probably find out when and why
Close should be invoked.
Could you adjust the function documentation as well, perhaps in a separate
patch?
> # Retrieve relevant parameters of the node group.
> ssh_port = None
> - if opts.nodegroup:
> - try:
> - output = cl.QueryGroups(names=[opts.nodegroup],
> fields=["ndp/ssh_port"],
> - use_locking=False)
> - (ssh_port, ) = output[0]
> - except (errors.OpPrereqError, errors.OpExecError):
> - pass
> + try:
> + node_groups = [opts.nodegroup] if opts.nodegroup is not None else []
> + output = cl.QueryGroups(names=node_groups, fields=["ndp/ssh_port"],
> + use_locking=False)
> + (ssh_port, ) = output[0]
> + except (errors.OpPrereqError, errors.OpExecError):
> + pass
>
> try:
> output = cl.QueryNodes(names=[node],
>
If the node group is not specified, you are querying for [], probably
returning no results in the query.
The output[0] would give you an IndexError here, which the except does not
catch.
Does this work? I assume it does, but I am curious to know why.
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
