Merge lp://qastaging/~blake-rouse/maas/dhcp-service into lp://qastaging/~maas-committers/maas/trunk

Proposed by Blake Rouse
Status: Merged
Approved by: Andres Rodriguez
Approved revision: no longer in the source branch.
Merged at revision: 3926
Proposed branch: lp://qastaging/~blake-rouse/maas/dhcp-service
Merge into: lp://qastaging/~maas-committers/maas/trunk
Prerequisite: lp://qastaging/~blake-rouse/maas/service-monitor-service
Diff against target: 1360 lines (+642/-451)
12 files modified
src/provisioningserver/dhcp/__init__.py (+93/-0)
src/provisioningserver/dhcp/control.py (+0/-106)
src/provisioningserver/dhcp/omshell.py (+23/-0)
src/provisioningserver/dhcp/tests/test_control.py (+0/-147)
src/provisioningserver/dhcp/tests/test_omshell.py (+45/-0)
src/provisioningserver/drivers/service/__init__.py (+6/-0)
src/provisioningserver/drivers/service/dhcp.py (+89/-0)
src/provisioningserver/drivers/service/tests/test_dhcp.py (+116/-0)
src/provisioningserver/rpc/dhcp.py (+88/-104)
src/provisioningserver/rpc/tests/test_dhcp.py (+159/-94)
src/provisioningserver/service_monitor.py (+8/-0)
src/provisioningserver/tests/test_service_monitor.py (+15/-0)
To merge this branch: bzr merge lp://qastaging/~blake-rouse/maas/dhcp-service
Reviewer Review Type Date Requested Status
Mike Pontillo (community) Approve
Review via email: mp+259464@code.qastaging.launchpad.net

Commit message

Add DHCPv4Service and DHCPv6Service to the ServiceRegistry. Ensure that the services are running before using the omshell.

This allows for the DHCP services used on the cluster controller to be monitored and to be in the expected state before performing operations over the omshell.

To post a comment you must log in.
Revision history for this message
Blake Rouse (blake-rouse) wrote :

This has some restructure to fix circular imports and to remove old code that is no longer needed now that service control is done in the service monitor instead of helpers in the dhcp code.

So sorry for the size! :-(

Revision history for this message
Mike Pontillo (mpontillo) wrote :

Please take a look at my inline comments; I see a couple of minor concerns. Do you think either of these issues could cause regressions?

review: Needs Information
Revision history for this message
Blake Rouse (blake-rouse) :
Revision history for this message
Blake Rouse (blake-rouse) wrote :

I updated the code to check for working omshell. If you could give it another look.

Revision history for this message
Mike Pontillo (mpontillo) wrote :

Nice improvement. I tested with omshell on my system, and your code is consistent with what I see.

One thing I would consider doing is increasing the timeout from 1.5 seconds. That was a lot of effort to just gain an additional .5 second window. ;-) (Maybe 5-10 seconds, in case the system is under heavy load?)

review: Approve
Revision history for this message
Andres Rodriguez (andreserl) wrote :

I'm gonna land it. If we need to increase the timeout, that's easy to do!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.