cloud-init-dev team mailing list archive
-
cloud-init-dev team
-
Mailing list archive
-
Message #04067
[Merge] ~ma-brothier/cloud-init:cs-multi-dhcp into cloud-init:master
Marc Brothier has proposed merging ~ma-brothier/cloud-init:cs-multi-dhcp into cloud-init:master.
Requested reviews:
cloud-init commiters (cloud-init-dev)
For more details, see:
https://code.launchpad.net/~ma-brothier/cloud-init/+git/cloud-init/+merge/336145
CloudStack changes on DHCP leases crawling mechanism for VR address
Currently the DHCP leases are search in reverse chronological order to look for the virtual router IP inside the lease information. In case more than one interface is added to a VM using a DHCP service to configure it, the latest DHCP lease will be linked to this new interface instead of the default one, thus waiting for the fault back method to fetch the default gateway address.
This change looks for all DHCP leases, grab all potential IP addresses (removing potential ducplicate due to multiple lease information in the same file) for the VR and try all of them in alphabetical order of the interface names until a good match is found (correct response), with a last default to the default gateway IP as originally.
--
Your team cloud-init commiters is requested to review the proposed merge of ~ma-brothier/cloud-init:cs-multi-dhcp into cloud-init:master.
diff --git a/cloudinit/sources/DataSourceCloudStack.py b/cloudinit/sources/DataSourceCloudStack.py
index 0df545f..bbe2f0e 100644
--- a/cloudinit/sources/DataSourceCloudStack.py
+++ b/cloudinit/sources/DataSourceCloudStack.py
@@ -13,16 +13,16 @@
# This file is part of cloud-init. See LICENSE file for license information.
import os
+import time
from socket import inet_ntoa
from struct import pack
-import time
from cloudinit import ec2_utils as ec2
from cloudinit import log as logging
-from cloudinit.net import dhcp
from cloudinit import sources
from cloudinit import url_helper as uhelp
from cloudinit import util
+from cloudinit.net import dhcp
LOG = logging.getLogger(__name__)
@@ -74,10 +74,9 @@ class DataSourceCloudStack(sources.DataSource):
# Cloudstack has its metadata/userdata URLs located at
# http://<virtual-router-ip>/latest/
self.api_ver = 'latest'
- self.vr_addr = get_vr_address()
- if not self.vr_addr:
+ self.vr_addresses = get_vr_addresses()
+ if not self.vr_addresses:
raise RuntimeError("No virtual router found!")
- self.metadata_address = "http://%s/"; % (self.vr_addr,)
self.cfg = {}
def _get_url_settings(self):
@@ -102,14 +101,17 @@ class DataSourceCloudStack(sources.DataSource):
def wait_for_metadata_service(self):
(max_wait, timeout) = self._get_url_settings()
- urls = [uhelp.combine_url(self.metadata_address,
- 'latest/meta-data/instance-id')]
+ urls = [uhelp.combine_url("http://%s/"; % vr_potential_ip,
+ 'latest/meta-data/instance-id')
+ for vr_potential_ip in self.vr_addresses]
start_time = time.time()
url = uhelp.wait_for_url(urls=urls, max_wait=max_wait,
timeout=timeout, status_cb=LOG.warn)
if url:
LOG.debug("Using metadata source: '%s'", url)
+ self.vr_addr = url.split('/')[2]
+ self.metadata_address = "http://%s/"; % (self.vr_addr,)
else:
LOG.critical(("Giving up on waiting for the metadata from %s"
" after %s seconds"),
@@ -168,7 +170,7 @@ class DataSourceCloudStack(sources.DataSource):
def get_default_gateway():
- # Returns the default gateway ip address in the dotted format.
+ """Returns the default gateway ip address in the dotted format."""
lines = util.load_file("/proc/net/route").splitlines()
for line in lines:
items = line.split("\t")
@@ -181,7 +183,7 @@ def get_default_gateway():
def get_dhclient_d():
- # find lease files directory
+ """Find lease files directory."""
supported_dirs = ["/var/lib/dhclient", "/var/lib/dhcp",
"/var/lib/NetworkManager"]
for d in supported_dirs:
@@ -191,15 +193,14 @@ def get_dhclient_d():
return None
-def get_latest_lease(lease_d=None):
- # find latest lease file
+def get_leases(lease_d=None):
+ """Find all lease files."""
if lease_d is None:
lease_d = get_dhclient_d()
if not lease_d:
return None
lease_files = os.listdir(lease_d)
- latest_mtime = -1
- latest_file = None
+ valid_lease_files = []
# lease files are named inconsistently across distros.
# We assume that 'dhclient6' indicates ipv6 and ignore it.
@@ -215,18 +216,17 @@ def get_latest_lease(lease_d=None):
continue
if not (fname.endswith(".lease") or fname.endswith(".leases")):
continue
-
- abs_path = os.path.join(lease_d, fname)
- mtime = os.path.getmtime(abs_path)
- if mtime > latest_mtime:
- latest_mtime = mtime
- latest_file = abs_path
- return latest_file
+ valid_lease_files.append(os.path.join(lease_d, fname))
+ return valid_lease_files
-def get_vr_address():
- # Get the address of the virtual router via dhcp leases
- # If no virtual router is detected, fallback on default gateway.
+def get_vr_addresses():
+ """Get a list of potential addresses for the virtual router via DHCP
+ leases. This overcomes the situation when multiple interfaces are
+ configured through DHCP, one of them will come from CloudStack VR and
+ all addresses should be tested until the meta-data server responds. The
+ latest entry will always be the default gateway.
+ """
# See http://docs.cloudstack.apache.org/projects/cloudstack-administration/en/4.8/virtual_machines/user-data.html # noqa
# Try networkd first...
@@ -234,27 +234,30 @@ def get_vr_address():
if latest_address:
LOG.debug("Found SERVER_ADDRESS '%s' via networkd_leases",
latest_address)
- return latest_address
+ return [latest_address]
# Try dhcp lease files next...
- lease_file = get_latest_lease()
- if not lease_file:
+ lease_files = get_leases()
+ if not lease_files:
LOG.debug("No lease file found, using default gateway")
- return get_default_gateway()
-
- with open(lease_file, "r") as fd:
- for line in fd:
- if "dhcp-server-identifier" in line:
- words = line.strip(" ;\r\n").split(" ")
- if len(words) > 2:
- dhcptok = words[2]
- LOG.debug("Found DHCP identifier %s", dhcptok)
- latest_address = dhcptok
- if not latest_address:
+ return [get_default_gateway()]
+
+ potential_addresses = []
+ for lease_file in lease_files:
+ with open(lease_file, "r") as fd:
+ for line in fd:
+ if "dhcp-server-identifier" in line:
+ words = line.strip(" ;\r\n").split(" ")
+ if len(words) > 2:
+ dhcptok = words[2]
+ if dhcptok not in potential_addresses:
+ LOG.debug("Found DHCP identifier %s", dhcptok)
+ potential_addresses.append(dhcptok)
+ if not potential_addresses:
# No virtual router found, fallback on default gateway
LOG.debug("No DHCP found, using default gateway")
- return get_default_gateway()
- return latest_address
+ return [get_default_gateway()]
+ return potential_addresses
# Used to match classes to dependencies
@@ -263,8 +266,8 @@ datasources = [
]
-# Return a list of data sources that match this set of dependencies
def get_datasource_list(depends):
+ """Return a list of data sources that match this set of dependencies."""
return sources.list_from_depends(depends, datasources)
# vi: ts=4 expandtab
diff --git a/tests/unittests/test_datasource/test_cloudstack.py b/tests/unittests/test_datasource/test_cloudstack.py
index d6d2d6b..8ee804d 100644
--- a/tests/unittests/test_datasource/test_cloudstack.py
+++ b/tests/unittests/test_datasource/test_cloudstack.py
@@ -1,15 +1,14 @@
# This file is part of cloud-init. See LICENSE file for license information.
+import os
+import time
+
from cloudinit import helpers
from cloudinit import util
from cloudinit.sources.DataSourceCloudStack import (
- DataSourceCloudStack, get_latest_lease)
-
+ DataSourceCloudStack, get_leases)
from cloudinit.tests.helpers import CiTestCase, ExitStack, mock
-import os
-import time
-
class TestCloudStackPasswordFetching(CiTestCase):
@@ -21,9 +20,9 @@ class TestCloudStackPasswordFetching(CiTestCase):
self.patches.enter_context(mock.patch('{0}.ec2'.format(mod_name)))
self.patches.enter_context(mock.patch('{0}.uhelp'.format(mod_name)))
default_gw = "192.201.20.0"
- get_latest_lease = mock.MagicMock(return_value=None)
+ get_leases = mock.MagicMock(return_value=None)
self.patches.enter_context(mock.patch(
- mod_name + '.get_latest_lease', get_latest_lease))
+ mod_name + '.get_leases', get_leases))
get_default_gw = mock.MagicMock(return_value=default_gw)
self.patches.enter_context(mock.patch(
@@ -105,7 +104,7 @@ class TestCloudStackPasswordFetching(CiTestCase):
self._check_password_not_saved_for('bad_request')
-class TestGetLatestLease(CiTestCase):
+class TestGetLeases(CiTestCase):
def _populate_dir_list(self, bdir, files):
"""populate_dir_list([(name, data), (name, data)])
@@ -122,8 +121,8 @@ class TestGetLatestLease(CiTestCase):
def _pop_and_test(self, files, expected):
lease_d = self.tmp_dir()
self._populate_dir_list(lease_d, files)
- self.assertEqual(self.tmp_path(expected, lease_d),
- get_latest_lease(lease_d))
+ self.assertEqual([self.tmp_path(expected, lease_d)],
+ get_leases(lease_d))
def test_skips_dhcpv6_files(self):
"""files started with dhclient6 should be skipped."""
@@ -154,8 +153,8 @@ class TestGetLatestLease(CiTestCase):
"dhclient.lease-old", "dhclient.leaselease"],
"dhclient.lease")
- def test_selects_newest_matching(self):
- """If multiple files match, the newest written should be used."""
+ def test_selects_all_matching(self):
+ """If multiple files match, all should be used."""
lease_d = self.tmp_dir()
valid_1 = "dhclient.leases"
valid_2 = "dhclient.lease"
@@ -163,13 +162,13 @@ class TestGetLatestLease(CiTestCase):
valid_2_path = self.tmp_path(valid_2, lease_d)
self._populate_dir_list(lease_d, [valid_1, valid_2])
- self.assertEqual(valid_2_path, get_latest_lease(lease_d))
+ self.assertEqual([valid_1_path, valid_2_path], get_leases(lease_d))
# now update mtime on valid_2 to be older than valid_1 and re-check.
mtime = int(os.path.getmtime(valid_1_path)) - 1
os.utime(valid_2_path, (mtime, mtime))
- self.assertEqual(valid_1_path, get_latest_lease(lease_d))
+ self.assertEqual([valid_1_path, valid_2_path], get_leases(lease_d))
# vi: ts=4 expandtab
