[Maas-devel] Signals for DHCP changes still broken

[Raphaël Badin]

On 02/09/2014 11:49 PM, Julian Edwards wrote:
> On Friday 07 Feb 2014 11:20:21 Raphaël Badin wrote:
>>> So far I've done my testing on canonistack; the test I'm planning to do
>>> today, with my microservers, will use a setup similar to yours.  I'll
>>> report back when I'm done.
>>
>> I managed to recreate the problem and I've identified the root cause: I
>> think there is a bug in src/maasserver/signals.py
>>
>> :connect_to_field_change.post_save_callback.  Trying to recreate the
>>
>> problem in a test case now.
>
> \o/  Excellent, thanks.

Let me explain what the core problem is:

connect_to_field_change (from src/maasserver/signals.py) is a small 
utility which uses Django's signal infrastructure to register a callback 
that should be fired when any field in a set of fields is modified on an 
particular object type.  We use this utility to limit the number of 
fields on nodegroupinterface objects whose change should cause the DHCP 
configuration to be rewritten — we basically want to consider all the 
fields but 'foreign_dhcp_ip'.

The bug stems from GenericIPAddressField's default cleanup method 
clashing with the default value we use for the GenericIPAddressField 
fields on nodegroupinterface:  the default value, as far as 
GenericIPAddressField is concerned, is the empty string and thus 
"cleaning" (in the Django sense) the value None gives the empty string. 
  That clashes with us using None as the default value for all the 
GenericIPAddressField fields on nodegroupinterface:  if a 
nodegroupinterface comes out of the database with its field 
'ip_range_low' empty, it's value is None because we have configured it 
this way on the nodegroupinterface model.  If this object goes through 
an edit cycle, right before being stored into the database, the value of 
the 'ip_range_low' field is the empty string because it's been converted 
by the field's "clean()" method.  This empty string will then be 
correctly converted back into None before being written to the DB but 
the problem is that, as the time the signal created by 
connect_to_field_change() was called, the value was still the empty 
string and was thus connect_to_field_change() interpreted that as change 
worth a DHCP/DNS rewrite.

The proper fix would be to change the default value of the 
GenericIPAddressField fields of the nodegroupinterface model to the 
empty string instead of None.

I haven't tested it but creating a subclass of GenericIPAddressField 
with a changed clean() method might work although it's a bit of a hack.

Now, we decided to actually fix two bugs with one change and create a 
dedicated API call to update the 'foreign_dhcp_ip' 
(https://code.launchpad.net/~jtv/maas/report-foreign-dhcp/+merge/205430). 
This will fix both the manifestation of this bug and bug 1276985.

We might still want to fix this bug properly but this is less urgent now 
that the branch above has landed.

Cheers,

R.