← Back to team overview

maas-devel team mailing list archive

Re: Signals for DHCP changes still broken

 

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.



Follow ups

References