Merge lp:~allenap/maas/move-dhcp-helper-to-lib into lp:~maas-committers/maas/trunk

[Gavin Panella]

Move maas-dhcp-helper off PATH to /usr/lib/maas.

Goes with lp:~allenap/maas/packaging.move-dhcp-helper-to-lib.

[Blake Rouse (blake-rouse)]

Just trying to make my life harder, by moving stuff around. ;-)

Please land ASAP so I can place into my snapable-rack branch.

review: Approve

[Blake Rouse (blake-rouse)]

Actually now I wonder if this will break the leases file? As I believe the path to this file is present in already given leases, so it knows where to send the message when it expires or is released.

Might need to keep a symlink around at the old place not to break already given out leases.

review: Needs Information

[Gavin Panella (allenap)]

I will check this carefully. If that's the case then I'm going to sit on this change: risking that kind of breakage is not worth it for this clean-up branch. Thanks for checking this out.

[Gavin Panella (allenap)]

I can't find any traces of scripts in the lease file so I think we're safe on this. Andres has asked that I wait before landing this, and that was my intention anyway, but I think we'll be good to go next week.

[Mike Pontillo (mpontillo)]

Blake is correct; the path is hard-coded in some of the leases, so that MAAS knows what binary to call when the lease expires, such as:

lease 172.16.100.150 {
  starts 1 2017/03/27 21:42:51;
  ends 1 2017/03/27 21:44:51;
  tstp 1 2017/03/27 21:44:51;
  cltt 1 2017/03/27 21:42:51;
  binding state free;
  hardware ethernet 0c:c4:7a:c0:31:fa;
  set vendor-class-identifier = "Linux ipconfig";
  client-hostname "skylake";
  on expiry {
    set clhw =
       binary-to-ascii (16, 8, ":",
                        substring (hardware, 1, 6)) ;
    set clip =
       binary-to-ascii (10, 8, ".", leased-address) ;
    execute ("/usr/sbin/maas-dhcp-helper", "notify", "--action", "expiry",
        "--mac", clhw, "--ip-family", "ipv4", "--ip", clip);
  }
  on release {
    set clhw =
       binary-to-ascii (16, 8, ":",
                        substring (hardware, 1, 6)) ;
    set clip =
       binary-to-ascii (10, 8, ".", leased-address) ;
    execute ("/usr/sbin/maas-dhcp-helper", "notify", "--action", "release",
        "--mac", clhw, "--ip-family", "ipv4", "--ip", clip);
  }
}

So I don't think we can move this around.

review: Disapprove

[Mike Pontillo (mpontillo)]

s/MAAS/dhcpd/

[Gavin Panella (allenap)]

Thanks for finding that Mike. That's a curious design decision by ISC, but anyway, that's definitely a blocker. We could still move this script but it would be more involved — stop dhcpd, rewrite leases, start dhcpd — so I'll park this work. Once we get to Snapland it'll be irrelevant anyway, right?

Unmerged revisions

5897. By Gavin Panella on 2017-03-31

Move maas-dhcp-helper to /usr/lib/maas; it shouldn't be on the PATH.

=== modified file 'setup.py'
--- setup.py	2017-03-22 15:27:26 +0000
+++ setup.py	2017-03-31 08:10:06 +0000
@@ -114,10 +114,9 @@
         ('/usr/bin',
             ['scripts/maas-generate-winrm-cert',
              'scripts/uec2roottar']),
-        ('/usr/sbin',
-            ['scripts/maas-dhcp-helper']),
         ('/usr/lib/maas',
-            ['scripts/maas-dhcp-monitor',
+            ['scripts/maas-dhcp-helper',
+             'scripts/maas-dhcp-monitor',
              'scripts/maas-network-monitor',
              'scripts/maas-delete-file',
              'scripts/maas-write-file']),

=== modified file 'src/provisioningserver/dhcp/tests/test_helper_clean.py'
--- src/provisioningserver/dhcp/tests/test_helper_clean.py	2016-03-22 17:04:10 +0000
+++ src/provisioningserver/dhcp/tests/test_helper_clean.py	2017-03-31 08:10:06 +0000
@@ -5,9 +5,11 @@
 
 __all__ = []
 
-from maastesting import root
 from maastesting.testcase import MAASTestCase
-from provisioningserver.utils.fs import read_text_file
+from provisioningserver.utils.fs import (
+    get_library_script_path,
+    read_text_file,
+)
 from provisioningserver.utils.shell import call_and_check
 
 
@@ -96,7 +98,7 @@
     def test_removes_hosts_from_leases_file(self):
         path = self.make_file(contents=LEASES_FILE_WITH_HOSTS)
         call_and_check([
-            "%s/scripts/maas-dhcp-helper" % root,
+            get_library_script_path("maas-dhcp-helper"),
             "clean",
             path,
         ])

=== modified file 'src/provisioningserver/dhcp/tests/test_helper_notify.py'
--- src/provisioningserver/dhcp/tests/test_helper_notify.py	2016-10-18 11:55:44 +0000
+++ src/provisioningserver/dhcp/tests/test_helper_notify.py	2017-03-31 08:10:06 +0000
@@ -9,7 +9,6 @@
 import random
 from unittest.mock import sentinel
 
-from maastesting import root
 from maastesting.factory import factory
 from maastesting.testcase import (
     MAASTestCase,
@@ -19,6 +18,7 @@
 from provisioningserver.rackdservices.lease_socket_service import (
     LeaseSocketService,
 )
+from provisioningserver.utils.fs import get_library_script_path
 from provisioningserver.utils.shell import call_and_check
 from provisioningserver.utils.twisted import DeferredValue
 from testtools.matchers import (
@@ -69,7 +69,7 @@
         self.addCleanup(service.stopService)
 
         call_and_check([
-            "%s/scripts/maas-dhcp-helper" % root,
+            get_library_script_path("maas-dhcp-helper"),
             "notify",
             '--action', action,
             '--mac', mac,

=== modified file 'src/provisioningserver/templates/dhcp/dhcpd.conf.template'
--- src/provisioningserver/templates/dhcp/dhcpd.conf.template	2016-10-12 22:07:21 +0000
+++ src/provisioningserver/templates/dhcp/dhcpd.conf.template	2017-03-31 08:10:06 +0000
@@ -143,7 +143,7 @@
    set cllt = binary-to-ascii(10, 32, "", encode-int(lease-time, 32));
    set clht = pick-first-value(option host-name, "(none)");
    execute(
-       "/usr/sbin/maas-dhcp-helper", "notify",
+       "/usr/lib/maas/maas-dhcp-helper", "notify",
        "--action", "commit", "--mac", clhw,
        "--ip-family", "ipv4", "--ip", clip,
        "--lease-time", cllt, "--hostname", clht);
@@ -152,7 +152,7 @@
    set clhw = binary-to-ascii(16, 8, ":", substring(hardware, 1, 6));
    set clip = binary-to-ascii(10, 8, ".", leased-address);
    execute(
-       "/usr/sbin/maas-dhcp-helper", "notify",
+       "/usr/lib/maas/maas-dhcp-helper", "notify",
        "--action", "expiry", "--mac", clhw,
        "--ip-family", "ipv4", "--ip", clip);
 }
@@ -160,7 +160,7 @@
    set clhw = binary-to-ascii(16, 8, ":", substring(hardware, 1, 6));
    set clip = binary-to-ascii(10, 8, ".", leased-address);
    execute(
-       "/usr/sbin/maas-dhcp-helper", "notify",
+       "/usr/lib/maas/maas-dhcp-helper", "notify",
        "--action", "release", "--mac", clhw,
        "--ip-family", "ipv4", "--ip", clip);
 }