MAAS

Merge lp://staging/~julian-edwards/maas/periodic-lease-upload-pserv into lp://staging/~maas-committers/maas/trunk

periodic-lease-upload-pserv
Merge into trunk

Proposed by Julian Edwards on 2014-08-18

Status:	Merged
Approved by:	Julian Edwards on 2014-08-28
Approved revision:	no longer in the source branch.
Merged at revision:	2830
Proposed branch:	lp://staging/~julian-edwards/maas/periodic-lease-upload-pserv
Merge into:	lp://staging/~maas-committers/maas/trunk
Diff against target:	906 lines (+558/-64) 15 files modified docs/development/lease-scanning-and-dns.rst (+4/-10) etc/celeryconfig_cluster.py (+0/-8) src/maasserver/rpc/leases.py (+45/-0) src/maasserver/rpc/regionservice.py (+10/-0) src/maasserver/rpc/tests/test_regionservice.py (+40/-0) src/provisioningserver/dhcp/leases.py (+18/-8) src/provisioningserver/dhcp/tests/test_leases.py (+11/-11) src/provisioningserver/plugin.py (+13/-0) src/provisioningserver/pserv_services/lease_upload_service.py (+145/-0) src/provisioningserver/pserv_services/tests/test_lease_upload_service.py (+235/-0) src/provisioningserver/rpc/region.py (+16/-0) src/provisioningserver/tasks.py (+0/-13) src/provisioningserver/testing/testcase.py (+18/-0) src/provisioningserver/tests/test_plugin.py (+2/-1) src/provisioningserver/tests/test_tasks.py (+1/-13)
To merge this branch:	bzr merge lp://staging/~julian-edwards/maas/periodic-lease-upload-pserv
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Gavin Panella (community)		2014-08-18	Approve on 2014-08-27
Review via email: mp+231164@code.staging.launchpad.net

Commit message

Add a pserv service that replaces the Celery task that periodically uploads DHCP leases.

Description of the change

This branch contains everything needed to remove the old Celery task to upload leases, and replace it with a pserv-based one.

Roughly, the following changes were made:
* Removing all trace of the celery config, code and tests
* Update dev documentation
* RPC command definition for UpdateLeases call
* Code to translate between the RPC format and the format needed by the existing code that does the actual updating.
* New pserv service.
* Tests. Lots of tests!

Sorry it's on the large side, but it is a complete cut-over. I've tested that it works on my local rig.

Revision history for this message

Gavin Panella (allenap) wrote on 2014-08-19:

Looking good so far :) Lots of comments, but none of them are blockers.

Revision history for this message

Julian Edwards (julian-edwards) wrote on 2014-08-20:

Thanks for the comments!

I tried your fixture suggestion in the last test but I can't get it to work. Can you take a look please?

Take this branch and apply this diff:
http://paste.ubuntu.com/8095590/

I get this output:
======================================================================
FAIL: provisioningserver.tests.test_lease_upload_service.TestPeriodicImageDownloadService.test_upload_is_initiated
----------------------------------------------------------------------
_StringException: Failed expectation: {{{
File "/home/ed/canonical/maas/sandbox/src/maastesting/crochet.py", line 101, in __checkResults
    fail_count, Equals(0), "Unfired and/or unhandled "
  File "/usr/lib/python2.7/dist-packages/testtools/testcase.py", line 447, in expectThat
    postfix_content="MismatchError: " + str(mismatch_error)
MismatchError: 0 != 1: Unfired and/or unhandled EventualResult(s); see test details.
}}}

Unfired/unhandled EventualResult #1: {{{
*** EventualResult has not fired:
<crochet._eventloop.EventualResult object at 0x7f9e58fcfbd0>
*** It was connected to a Deferred:
<Deferred at 0x7f9e58fd35a8>
}}}

traceback-1: {{{
Traceback (most recent call last):
AssertionError: Forced Test Failure
}}}

Traceback (most recent call last):
  File "/home/ed/canonical/maas/sandbox/src/provisioningserver/tests/test_lease_upload_service.py", line 214, in test_upload_is_initiated
    MockCalledOnceWith(uuid=uuid, mappings=mappings))
  File "/usr/lib/python2.7/dist-packages/testtools/testcase.py", line 406, in assertThat
    raise mismatch_error
MismatchError: Expected to be called once. Called 0 times.

-------------------- >> begin captured logging << --------------------
twisted: INFO: AMPTestProtocol#1 connection established (HOST:<twisted.test.iosim.FakeAddress object at 0x7f9e58fc64d0> PEER:<twisted.test.iosim.FakeAddress object at 0x7f9e58fc6510>)
twisted: INFO: ClusterClient connection established (HOST:<twisted.test.iosim.FakeAddress object at 0x7f9e58fc6550> PEER:<twisted.test.iosim.FakeAddress object at 0x7f9e58fc6590>)
--------------------- >> end captured logging << ---------------------

Thanks for the comments!

I tried your fixture suggestion in the last test but I can't get it to work.  Can you take a look please?

Take this branch and apply this diff:
http://paste.ubuntu.com/8095590/

traceback-1: {{{
Traceback (most recent call last):
AssertionError: Forced Test Failure
}}}

Revision history for this message

Gavin Panella (allenap) wrote on 2014-08-20:

Have a look at https://code.launchpad.net/~allenap/maas/periodic-lease-upload-pserv/+merge/231538.

Revision history for this message

Julian Edwards (julian-edwards) wrote on 2014-08-25:

This is now all working on my test rig. I still can't get your modified tests to pass though, so I'll get a review and land without the changes as your off today and then we can look at it again when you're back.

Revision history for this message

Julian Edwards (julian-edwards) wrote on 2014-08-27:

I made a couple more changes:
- Catch service errors and log them — the code may look somewhat familiar to you :)
- moved the code to the new pserv_services dir

Revision history for this message

Gavin Panella (allenap) wrote on 2014-08-27:

Looks grand.

review: Approve

Revision history for this message

Julian Edwards (julian-edwards) wrote on 2014-08-28:

Thanks for reviewing! I'm going to leave the lock for now, see my comment. It's easily changed later if needed.

Revision history for this message

Julian Edwards (julian-edwards) wrote on 2014-08-28:

Tested again on my local rig and it seems fine, so landing now.