Code review comment for lp:~allenap/maas/remove-ip-from-amt

> Mostly fine but a question and a nit!  Setting to approved though as I don't
> want to block you.
>
> [1]
> 49      +    if 'mac_address' in kwargs:
> 50      +        kwargs['ip_address'] = find_ip_via_arp(kwargs['mac_address'])
>
> If ip_address is not set, then the template will blow up.  Why would
> mac_address not be set in kwargs here?

It's only in the power parameters for Etherwake and AMT.

>
> [2]
> You're using a bit of a testing antipattern by initialising
> sentinel.ip_address in the setup and then depending on it in the test
> assertions.  I think it would be better if each test patched out this
> separately, despite the lack of factoring.

Okay, I'll fix that up.

>
> BTW I think we should use more sentinel, it's much easier on the eye that the
> factory.makeRandomXXX stuff!

Indeed! It doesn't always work - sometimes we really do need quacking
ducks - but when it does it's concise and clear.