cloud-init-dev team mailing list archive

[Scott Moser <smoser@xxxxxxxxxx>]

update the commit message to address the different behavior (we now write json for 1).

is it possible to put cloud-init-dhclient-hook outside of /usr/bin ?
we could put it in /usr/lib/cloud-init and hten just name it 'dhclient-hook' . i say this because its not really intende to be used as an executable.

other suggestions in line. 

this is looking good though, thanks Brent and Lars.

Diff comments:

> === added file 'cloudinit/cmd/dhclient_hook.py'
> --- cloudinit/cmd/dhclient_hook.py	1970-01-01 00:00:00 +0000
> +++ cloudinit/cmd/dhclient_hook.py	2016-07-13 22:57:56 +0000
> @@ -0,0 +1,51 @@
> +import os
> +import json
> +from cloudinit import stages
> +from cloudinit import log as logging
> +
> +
> +class LogDhclient():
> +
> +    LOG = logging.getLogger(__name__)
> +
> +    def __init__(self):
> +        self.hooks_dir = self._get_hooks_dir()
> +
> +    @staticmethod
> +    def _get_hooks_dir():
> +        i = stages.Init()
> +        return os.path.join(i.paths.get_runpath(), 'dhclient.hooks')
> +
> +    def check_hooks_dir(self):
> +        if not os.path.exists(self.hooks_dir):
> +            os.makedirs(self.hooks_dir)
> +        else:
> +            hook_files = [os.path.join(self.hooks_dir, x)
> +                          for x in os.listdir(self.hooks_dir)]
> +            for hook_file in hook_files:
> +                os.remove(hook_file)
> +
> +    @staticmethod
> +    def get_vals(info):
> +        new_info = {}
> +        for k, v in info.iteritems():
> +            if k.startswith("DHCP4_") or k.startswith("new_"):
> +                key = (k.replace('DHCP4_', '').replace('new_', '')).lower()
> +                new_info[key] = v
> +        return new_info
> +
> +    def record(self):
> +        envs = os.environ
> +        ifc_name = envs.get("interface", envs.get("DEVICE_IFACE", None))
> +        if ifc_name is None:
> +            return
> +        ifc_file_name = os.path.join(self.hooks_dir, ifc_name + '.json')
> +        with open(ifc_file_name, 'w') as outfile:
> +            json.dump(self.get_vals(envs), outfile, indent=4)

return json.dumps(data, indent=1, sort_keys=True,
                      separators=(',', ': ')).encode('utf-8')

that is nice and pretty output. ie, please use sort_keys and separators also.

> +            self.LOG.debug("Wrote dhclient options in %s", ifc_file_name)
> +
> +
> +def main():
> +    record = LogDhclient()
> +    record.check_hooks_dir()
> +    record.record()
> 
> === modified file 'cloudinit/sources/helpers/azure.py'
> --- cloudinit/sources/helpers/azure.py	2016-05-12 20:52:56 +0000
> +++ cloudinit/sources/helpers/azure.py	2016-07-13 22:57:56 +0000
> @@ -213,15 +228,74 @@
>          return socket.inet_ntoa(packed_bytes)
>  
>      @staticmethod
> -    def find_endpoint():
> +    def _get_value_from_leases_file(lease_file):
> +        leases = []
> +        content = util.load_file(lease_file)
> +        LOG.debug("content is {}".format(content))
> +        for line in content.splitlines():
> +            if 'unknown-245' in line:

can you give an example here of what that line looks like ? line.strip(''..... that is hard to parse in your head.

> +                leases.append(line.strip(' ').split(' ', 2)[-1].strip(';\n"'))
> +        # Return the "most recent" one in the list
> +        if len(leases) < 1:
> +            return None
> +        else:
> +            return leases[-1]
> +
> +    @staticmethod
> +    def _load_dhclient_json():
> +        dhcp_options = {}
> +        hooks_dir = WALinuxAgentShim._get_hooks_dir()
> +        if not os.path.exists(hooks_dir):
> +            LOG.debug("%s not found.", hooks_dir)
> +            return None
> +        hook_files = [os.path.join(hooks_dir, x)
> +                      for x in os.listdir(hooks_dir)]
> +        for hook_file in hook_files:
> +            try:
> +                _file_dict = json.load(open(hook_file))
> +                dhcp_options[os.path.basename(
> +                    hook_file).replace('.json', '')] = _file_dict
> +            except ValueError:
> +                LOG.info("%s is not valid JSON data", hook_file)

Probably best to raise some exception here so caller can tell the difference between "there just wasnt data in the file" and "the file did not exist"

> +        return dhcp_options
> +
> +    @staticmethod
> +    def _get_value_from_dhcpoptions(dhcp_options):
> +        if dhcp_options is None:
> +            return None
> +        # the MS endpoint server is given to us as DHPC option 245
> +        _value = None
> +        for interface in dhcp_options:
> +            _value = dhcp_options[interface].get('unknown_245', None)
> +            if _value is not None:
> +                break
> +        return _value
> +
> +    @staticmethod
> +    def find_endpoint(lease_file):
>          LOG.debug('Finding Azure endpoint...')
> -        content = util.load_file('/var/lib/dhcp/dhclient.eth0.leases')
>          value = None
> -        for line in content.splitlines():
> -            if 'unknown-245' in line:
> -                value = line.strip(' ').split(' ', 2)[-1].strip(';\n"')
> -        if value is None:
> -            raise ValueError('No endpoint found in DHCP config.')
> +        # Option-245 stored in /run/cloud-init/dhclient.hooks/<ifc>.json
> +        # a dhclient exit hook that calls cloud-init-dhclient-hook
> +        LOG.debug("Lease file %s found", lease_file)
> +        dhcp_options = WALinuxAgentShim._load_dhclient_json()
> +        value = WALinuxAgentShim._get_value_from_dhcpoptions(dhcp_options)
> +        LOG.debug("value is {}".format(value))
> +        if value is None:
> +            # Fallback and check the leases file if unsuccessful
> +            LOG.debug("Unable to find endpoint in dhclient logs. "
> +                      " Falling back to check lease files")
> +            if lease_file is None:
> +                LOG.warn("No lease file was specified.")
> +                value = None
> +            else:
> +                value = WALinuxAgentShim._get_value_from_leases_file(
> +                    lease_file)
> +                LOG.debug("second value is {}".format(value))
> +
> +        if value is None:
> +            raise ValueError('No endpoint found.')
> +
>          endpoint_ip_address = WALinuxAgentShim.get_ip_from_lease_value(value)
>          LOG.debug('Azure endpoint found at %s', endpoint_ip_address)
>          return endpoint_ip_address
> @@ -231,10 +305,11 @@
>          http_client = AzureEndpointHttpClient(self.openssl_manager.certificate)
>          LOG.info('Registering with Azure...')
>          attempts = 0
> +        end_point = self.endpoint
>          while True:
>              try:
>                  response = http_client.get(
> -                    'http://{0}/machine/?comp=goalstate'.format(self.endpoint))
> +                    'http://{0}/machine/?comp=goalstate'.format(end_point))

revert this, as i dont think you need the saved chars for line lenght, do you? and then recvert line end_point= above also. just to get rid of unrelated small changes.

>              except Exception:
>                  if attempts < 10:
>                      time.sleep(attempts + 1)
> 
> === added file 'tools/NM-cloudinit'
> --- tools/NM-cloudinit	1970-01-01 00:00:00 +0000
> +++ tools/NM-cloudinit	2016-07-13 22:57:56 +0000
> @@ -0,0 +1,4 @@
> +#!/bin/bash
> +# Writes DHCP lease information into the cloud-init data directory.
> +/usr/bin/cloud-init-dhclient-hook

Reading NetworkManager(8) i see that Network Manger says

| NetworkManager will execute scripts in the
| /etc/NetworkManager/dispatcher.d directory or subdirectories in
| alphabetical order in response to network events. Each script should be a
| regular executable file owned by root. Furthermore, it must not be
| writable by group or other, and not setuid.

| Each script receives two arguments, the first being the interface name
| of the device an operation just happened on, and second the action. For
| device actions, the interface is the name of the kernel interface suitable
| for IP configuration. Thus it is either VPN_IP_IFACE, DEVICE_IP_IFACE, or
| DEVICE_IFACE, as applicable. For the hostname action it is always "none".

So lets do the following:
a.) change to /bin/sh
b.) use 'exec /usr/bin/cloud-init-dhclient-hook'

we probably only want to do this on *some* actions probably 'up' or
'dhcp4-change' ?

#!/bin/sh
case "$1:$2" in
   up:*) exec /usr/bin/cloud-init-dhclient-hook;;
   down:*) ;; # do we need to clean up?;;
esac

> +
> 
> === added file 'tools/cloud-init-log-dhcp'
> --- tools/cloud-init-log-dhcp	1970-01-01 00:00:00 +0000
> +++ tools/cloud-init-log-dhcp	2016-07-13 22:57:56 +0000
> @@ -0,0 +1,5 @@
> +#!/bin/bash

i believe this file is *sourced* by dhclient when executing hooks, and dhclient (i believe) uses bash to do that.  so #!/bin/bash is reasonable here, but we should mention this is *sourced*, so we should not 'exec cloud-init-dhclient-hook'

> +# Writes DHCP lease information into the cloud-init data directory.
> +
> +cloud-init-dhclient-hook
> +


-- 
https://code.launchpad.net/~bbaude/cloud-init/azure_dhcp/+merge/298677
Your team cloud init development team is requested to review the proposed merge of lp:~bbaude/cloud-init/azure_dhcp into lp:cloud-init.