Hi,

I'm not reviewing as I don't really know the code, but I have a suggestion
for this part. Right now I'm implementing a test framework for the cmdlib
module which makes it rather easy to unit test such logic as you have
implemented here. So it would be great if you could take a look at the
tests (cluster_unittest.py for example) and implement some for your code as
well.

Cheers,
Thomas


On Tue, Aug 20, 2013 at 12:44 PM, Sebastian Gebhard <[email protected]>wrote:

> This patch adds functionality to LUNodeAdd to
>   - check the arguments given. It will warn if no physical link is given
>     and fail if OpenvSwitch is not enabled, but parameters are given
>   - call the RPC to configure OpenvSwitch on the node
> ---
>  lib/cmdlib/node.py | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/lib/cmdlib/node.py b/lib/cmdlib/node.py
> index ca4a5a6..c62e691 100644
> --- a/lib/cmdlib/node.py
> +++ b/lib/cmdlib/node.py
> @@ -112,6 +112,20 @@ class LUNodeAdd(LogicalUnit):
>        raise errors.OpPrereqError("Cannot pass a node group when a node is"
>                                   " being readded", errors.ECODE_INVAL)
>
> +    # OpenvSwitch: Warn user if link is missing
> +    if (self.op.ndparams[constants.ND_OVS] and not
> +        self.op.ndparams[constants.ND_OVS_LINK]):
> +      self.LogInfo("No physical interface for OpenvSwitch was given."
> +                   " OpenvSwitch will not have an outside connection.
> This"
> +                   " might not be what you want.")
> +    # OpenvSwitch: Fail if parameters are given, but OVS is not enabled.
> +    if (not self.op.ndparams[constants.ND_OVS] and
> +        (self.op.ndparams[constants.ND_OVS_NAME] or
> +         self.op.ndparams[constants.ND_OVS_LINK])):
> +      raise errors.OpPrereqError("OpenvSwitch name or link were given,
> but"
> +                                 " OpenvSwitch is not enabled. Please
> enable"
> +                                 " OpenvSwitch with --ovs",
> errors.ECODE_INVAL)
> +
>    def BuildHooksEnv(self):
>      """Build hooks env.
>
> @@ -375,6 +389,13 @@ class LUNodeAdd(LogicalUnit):
>                        (verifier, nl_payload[failed]))
>          raise errors.OpExecError("ssh/hostname verification failed")
>
> +    # OpenvSwitch initialization on the node
> +    if self.new_node.ndparams[constants.ND_OVS]:
> +      result = self.rpc.call_node_configure_ovs(
> +                 self.new_node.name,
> +                 self.new_node.ndparams[constants.ND_OVS_NAME],
> +                 self.new_node.ndparams[constants.ND_OVS_LINK])
> +
>      if self.op.readd:
>        self.context.ReaddNode(self.new_node)
>        RedistributeAncillaryFiles(self)
> --
> 1.8.1.2
>
>


-- 
Thomas Thrainer | Software Engineer | [email protected] |

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

Reply via email to