Merge lp:~tycho-s/maas/add-hardcode-amt-option into lp:~maas-committers/maas/trunk

[Tycho Andersen]

Add optional power_address field to AMT template to allow manual overriding of the default IP address.

Add optional power_address field to AMT template to allow manual overriding of the default IP address.

[Tycho Andersen (tycho-s)]

One thing to note is that this needs to go into Trusty as I understand it, so it probably needs to go in 1.5 as well. I'm not exactly sure how to lp-fu that, so if someone has some instrucitons, that would be great :-)

[Raphaël Badin (rvb)]

Looks good, but please confirm that you've QAed this. This close to the release, we're being cautious for things that touch that part of the code (see [0]).

[0]

I'm assuming this has been tested in the field… right?

(Note that the existing unit tests will only check that the template can be rendered correctly)

[1]

31 + 'power_address', "Optional IP to use instead of MAC"),

This is a bit vague (what does it mean to use an IP instead of a MAC). *I* understand what you mean because I know what this is about but you have to put yourself in the shoes of a user. Maybe have a look at the comments for IPMI parameters for inspiration.

[2]

Please confirm this with Andres but I think this needs to be linked to a bug in order to make it into 1.5.

[3]

> One thing to note is that this needs to go into Trusty as I understand it, so it probably needs to go in 1.5 as well.
> I'm not exactly sure how to lp-fu that, so if someone has some instrucitons, that would be great :-)

Once this is landed, create a branch off lp:maas/1.5, cherry pick the revision where this branch has landed: bzr -c <rev_number> lp:maas then create a MP for this branch with lp:maas/1.5 as the target.

review: Needs Information

[Tycho Andersen (tycho-s)]

On Wed, Apr 16, 2014 at 12:48:40PM -0000, Raphaël Badin wrote:
> Review: Needs Information
>
> Looks good, but please confirm that you've QAed this. This close to the release, we're being cautious for things that touch that part of the code (see [0]).
>
> [0]
>
> I'm assuming this has been tested in the field… right?

Yep. However, it is worth noting that the power_address field doesn't
have any validation (that it is an actual IP address), so if you type
something wrong in there it won't work. This is a bug with every other
IP address field as well, though.

> (Note that the existing unit tests will only check that the template can be rendered correctly)
>
>
> [1]
>
> 31 + 'power_address', "Optional IP to use instead of MAC"),
>
> This is a bit vague (what does it mean to use an IP instead of a MAC). *I* understand what you mean because I know what this is about but you have to put yourself in the shoes of a user. Maybe have a look at the comments for IPMI parameters for inspiration.

Done.

> [2]
>
> Please confirm this with Andres but I think this needs to be linked to a bug in order to make it into 1.5.

Checked with him, we do, and I've created one and linked it everywhere
I think (?). Let me know if there are more places that I missed.

> [3]
>
> > One thing to note is that this needs to go into Trusty as I understand it, so it probably needs to go in 1.5 as well.
> > I'm not exactly sure how to lp-fu that, so if someone has some instrucitons, that would be great :-)
>
> Once this is landed, create a branch off lp:maas/1.5, cherry pick the revision where this branch has landed: bzr -c <rev_number> lp:maas then create a MP for this branch with lp:maas/1.5 as the target.

Excellent, thank you.

\t

[Raphaël Badin (rvb)]

All right, thanks for the replies.

review: Approve

=== modified file 'etc/maas/templates/power/amt.template'
--- etc/maas/templates/power/amt.template	2014-02-17 21:00:17 +0000
+++ etc/maas/templates/power/amt.template	2014-04-16 21:49:03 +0000
@@ -2,10 +2,17 @@
 # -*- mode: shell-script -*-
 #
 # Control a system via amttool
+power_address='{{power_address}}'
 power_change='{{power_change}}'
 power_pass='{{power_pass}}'
 ip_address='{{ip_address}}'
 
+# The user specified power_address overrides any automatically determined
+# ip_address.
+if [ -n "$power_address" ]; then
+  ip_address=$power_address
+fi
+
 echo amt.template starting $*
 
 echo ip_address $ip_address

=== modified file 'src/provisioningserver/power_schema.py'
--- src/provisioningserver/power_schema.py	2014-03-27 04:15:45 +0000
+++ src/provisioningserver/power_schema.py	2014-04-16 21:49:03 +0000
@@ -226,6 +226,10 @@
             make_json_field(
                 'mac_address', "MAC Address", field_type='mac_address'),
             make_json_field('power_pass', "Power password"),
+            make_json_field(
+                'power_address',
+                "An IP address to use instead of the node's primary NIC's IP "
+                "(i.e. the IP of the MAC above, looked up with ARP)."),
         ],
     },
     {