cloud-init

Merge ~johnsonshi/cloud-init:rhel-centos-dhclient-lease-path into cloud-init:master

  1. Git
  2. lp:~johnsonshi/cloud-init
  3. rhel-centos-dhclient-lease-path
  4. Merge into master
Proposed by Johnson Shi
Status: Work in progress
Proposed branch: ~johnsonshi/cloud-init:rhel-centos-dhclient-lease-path
Merge into: cloud-init:master
Diff against target: 617 lines (+293/-175) (has conflicts)
5 files modified
cloudinit/net/dhcp.py (+151/-0)
cloudinit/net/tests/test_dhcp.py (+128/-1)
cloudinit/sources/DataSourceAzure.py (+9/-6)
cloudinit/sources/DataSourceCloudStack.py (+1/-93)
tests/unittests/test_datasource/test_cloudstack.py (+4/-75)
Conflict in cloudinit/sources/DataSourceAzure.py
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
Chad Smith Needs Fixing
Scott Moser Pending
Review via email: mp+361108@code.launchpad.net

Commit message

dhcp: Refactor duplicate DHCP lease code in DataSources and dhcp.py

DataSourceCloudStack.py has good code for lease file handling,
getting the latest lease, and getting the virt address.
It handles lease file scraping for different Linux distros,
but it does not handle FreeBSD lease files.

The lease file hard-coded in DataSourceAzure.py is
'/var/lib/dhcp/dhclient.eth0.leases', which is non-existent
for RHEL and CentOS. This causes get_metadata_from_fabric to
fail, causing /run/cloud-init/instance-data.json to have
missing important metadata from IMDS.
This is because DataSourceAzure.py does not
take into account the different naming conventions of
lease files across different rhel/centos versions.
This DataSource also has hardcoded values for network
interface name which could be better obtained from
within the lease file. It also has hardcoded values
for lease file path and network interface name for FreeBSD.

This commit refactors common DHCP code out of the two
DataSources to cloudinit.net.dhcp.py instead.
The commmon DHCP lease file scrapers now handle FreeBSD
style lease file naming convention as well as centos/rhel
style naming. DataSourceAzure.py and DataSourceCloudStack.py
were also updated to use the common code to obtain lease
files and NIC names.

Tests that were relevant to getting lease files were moved
from test_cloudstack.py to test_dhcp.py. FreeBSD lease
file scraping tests were also added.

LP: #1571629

To post a comment you must log in.
Revision history for this message
Chad Smith (chad.smith) wrote :

Thanks for this branch Johnson Shi.

  Per your IRC conversation, I also agree that this looks a bit like duplication which already exists in DataSourceCloudStack.py. If we can add a common function in cloudinit.net.dhcp that'd find the right dhcp lease directory for us based on the presence of said directory, I think that would solve this problem for both Azure and CloudStack. Then we also could avoid adding additional utility functions in cloudinit.util for distro detection.

Also please add a unit test that exercises this function/feature once you've added it in a common location.
If you choose to add something in cloudinit.net.dhcp a unit test or two in cloudinit/net/test/test_dchp.py should suffice.

Reply
Revision history for this message
Scott Moser (smoser) wrote :

Hi,
This is kind of expected to work on rhel via the code added in 648dbbf6b090c81e989f1ab70bf99f4de16a6a70 .

That provides dhclient hooks in tools/hook-rhel.sh that should write
 /run/cloud-init/dhclient.hooks/<interface-name>.json
that should be read from 'find_endpoint' use of _load_dhclient_json.

I suspect that we just need to make sure that the dhclient hook is getting installed correctly and then this may "just work".

Reply
Revision history for this message
Johnson Shi (johnsonshi) wrote :

I have now migrated duplicate DHCP code out of DataSourceAzure and DataSourceCloudStack. I have also added FreeBSD lease file handling support to the common DHCP code. I have also migrated DHCP lease file getter unit tests out of test_cloudstack to test_dhcp. I have also added FreeBSD lease file scraping tests.

Reply
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:f218f506f82959a0985f9fdcb6669b69fb6376c7
https://jenkins.ubuntu.com/server/job/cloud-init-ci/516/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/516/rebuild

review: Needs Fixing (continuous-integration)
Reply
Revision history for this message
Jason Zions (jasonzio) wrote :

> Hi,
> This is kind of expected to work on rhel via the code added in
> 648dbbf6b090c81e989f1ab70bf99f4de16a6a70 .
>
> That provides dhclient hooks in tools/hook-rhel.sh that should write
> /run/cloud-init/dhclient.hooks/<interface-name>.json
> that should be read from 'find_endpoint' use of _load_dhclient_json.
>
> I suspect that we just need to make sure that the dhclient hook is getting
> installed correctly and then this may "just work".

The use of dhclient hooks is an optimization, but the underlying mechanism (scrape the lease files) still needs to work. This change is a refactoring of the underlying mechanism. Platforms that don't use dhclient or networkmanager (e.g. SLES12/15 which use Wicked, FreeBSD) need a clean, cross-cloud solution that doesn't assume dhclient is in use.

Reply
Revision history for this message
Johnson Shi (johnsonshi) wrote :

get_latest_lease_file() returns None according to Jenkins CI tests. However, when this is run through tox locally, all tests pass.

Reply
Revision history for this message
Johnson Shi (johnsonshi) wrote :

I think I figured out why. On my personal Ubuntu machine, /var/lib/dhcp/ and /var/lib/NetworkManager/ both exist. In cloudinit.net.dhcp.get_dhclient_d() (which was originally migrated from DataSourceCloudStack), it returns the *first* directory from a list of known directories where dhcp lease files reside in. In my personal Ubuntu, even though my leases are in /var/lib/NetworkManager/, the /var/lib/dhcp/ directory also exists.

Because /var/lib/dhcp/ is in front of /var/lib/NetworkManager/ in the list of supported directories in get_dhclient_d, /var/lib/dhcp/ is returned. This causes get_latest_lease() to look into this directory and fail to find a lease, causing it to return None.

My proposal is to have get_dhclient_d() return a list of supported and existing directories on the system. get_latest_lease() then checks into each and every one of these supported and existing directories, and returns the youngest lease. Why should we do this? There is no deterministic way of determining which one of the supported and existing directories actually contain the leases/latest lease unless we crawl them.

I'll make this change tomorrow.

Also, can I trigger Jenkins myself so that I wouldn't bother the cloud-init maintainers too much asking them to rebuild it for me? Thanks! =)

Reply
Revision history for this message
Johnson Shi (johnsonshi) :
Reply
Revision history for this message
Johnson Shi (johnsonshi) wrote :

Actually, len(os.listdir(d)) already handles the case where the first directory is empty. os.ilstdir(d) > 0 ensures that it tries to find a non-empty lease directory. get_dhclient_d() returns None only if all of these directories are either empty or non-existent. If get_dhclient_d() returns None, this causes get_latest_lease_file() to return None as well. This causes DataSourceAzure.LEASE_FILE to be set to None (since LEASE_FILE is set to the return value of get_latest_lease_file(). When DataSourceAzure.LEASE_FILE is passed as the argument to get_interface_name_from_lease(), it throws an exception since the file passed in is None.

Reply
Revision history for this message
Chad Smith (chad.smith) wrote :

per your IRC comment about unittests working on centos but failing on ubuntu, Generally we don't want our unit tests leaking out into the underlying environment, we should be mocking, patching or injection tmp_dir/file information into tested functions so we don't have different results on different systems.

Here's a very quick pass against your branch with a couple of inline comments.

Reply
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:054c12f20c6aa2b4db5a97ebe5654b87f6d99315
https://jenkins.ubuntu.com/server/job/cloud-init-ci/523/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/523/rebuild

review: Approve (continuous-integration)
Reply
Revision history for this message
Johnson Shi (johnsonshi) wrote :

I have addressed Chad Smith's comments. I have refactored test_dhcp.py as per Chad's comments. DataSourceAzure.py has also been refactored to not rely on top-level LEASE_FILE.

Reply
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:78d0507770bec2c3104fdb76c278704925ddd2e9
https://jenkins.ubuntu.com/server/job/cloud-init-ci/524/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/524/rebuild

review: Approve (continuous-integration)
Reply
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:917901d1e5d65c79a24ae7f0c6bd5d550d89f537
https://jenkins.ubuntu.com/server/job/cloud-init-ci/525/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/525/rebuild

review: Approve (continuous-integration)
Reply
Revision history for this message
Chad Smith (chad.smith) wrote :

Thanks for patience on this Johnson Shi.

Here are some inline concerns that maybe we can shore up with Azure to ensure we don't break existing users of dhclient_lease_file config option.

review: Needs Fixing
Reply
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:e3cc0cad54679ad42b79d73d8f0de69742cfe6f9
https://jenkins.ubuntu.com/server/job/cloud-init-ci/590/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/590/rebuild

review: Needs Fixing (continuous-integration)
Reply
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:917901d1e5d65c79a24ae7f0c6bd5d550d89f537
https://jenkins.ubuntu.com/server/job/cloud-init-ci/592/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/592/rebuild

review: Approve (continuous-integration)
Reply
Revision history for this message
Ryan Harper (raharper) wrote :

Hi Johnson Shi,

I've moved this branch to Work in Progress state. Chad Smith left you some in-line comments. Once those have been addressed, please push your changes and then change the merge proposal state back to "Needs Review" and we'll pick things up from there.

Reply
Revision history for this message
Johnson Shi (johnsonshi) wrote :

Hi Ryan,

Thank you! I will continue work on it once my semester finishes.

Regards,
Johnson Shi

On Tue, Apr 9, 2019 at 4:32 AM Ryan Harper <email address hidden>
wrote:

> Hi Johnson Shi,
>
> I've moved this branch to Work in Progress state. Chad Smith left you
> some in-line comments. Once those have been addressed, please push your
> changes and then change the merge proposal state back to "Needs Review" and
> we'll pick things up from there.
> --
>
> https://code.launchpad.net/~johnsonshi/cloud-init/+git/cloud-init/+merge/361108
> You are the owner of
> ~johnsonshi/cloud-init:rhel-centos-dhclient-lease-path.
>

Reply

Unmerged commits

917901d... by Johnson Shi

Address PR comments: refactor tests and DataSourceAzure

b4fee53... by Johnson Shi

Refactored DataSourceAzure's lease file usages

36e12da... by Johnson Shi

Removed dynamic function calls in top level variables

2fc0184... by Johnson Shi

dhcp: add test for parsing nic name from lease file

fefef05... by Johnson Shi

dhcp: nic name getter now gets name from latest lease in file

7a29b7e... by Johnson Shi

azure: remove hardcoded lease file path and nic names

90560ef... by Johnson Shi

Add network interface scraping from lease file

29c5e26... by Johnson Shi

dhcp: add tests for FreeBSD lease scraping

2ff27aa... by Johnson Shi

cloudstack: Fix missing imports.

f9d0463... by Johnson Shi

dhcp: Add support lease file scraping for FreeBSD

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py
2index c98a97c..182d7d0 100644
3--- a/cloudinit/net/dhcp.py
4+++ b/cloudinit/net/dhcp.py
5@@ -10,6 +10,8 @@ import os
6 import re
7 import signal
8 import time
9+from socket import inet_ntoa
10+from struct import pack
11
12 from cloudinit.net import (
13 EphemeralIPv4Network, find_fallback_nic, get_devicelist,
14@@ -272,4 +274,153 @@ def networkd_get_option_from_leases(keyname, leases_d=None):
15 return data[keyname]
16 return None
17
18+
19+def get_default_gateway():
20+ """Scrapes /proc/net/route for the default gateway ip addr.
21+
22+ @return: default gateway ipaddr in dotted format,
23+ or None if nothing is found.
24+ """
25+
26+ lines = util.load_file("/proc/net/route").splitlines()
27+ for line in lines:
28+ items = line.split("\t")
29+ if items[1] == "00000000":
30+ # Found the default route, get the gateway
31+ gw = inet_ntoa(pack("<L", int(items[2], 16)))
32+ LOG.debug("Found default route, gateway is %s", gw)
33+ return gw
34+ return None
35+
36+
37+def get_dhclient_d():
38+ """Finds the first existing lease file directory from
39+ a list of supported (known) lease file directories.
40+
41+ @return: first existing lease file directory, or None
42+ """
43+
44+ supported_dirs = ["/var/lib/dhclient", "/var/lib/dhcp",
45+ "/var/lib/NetworkManager", "/var/db"]
46+ for d in supported_dirs:
47+ if os.path.exists(d) and len(os.listdir(d)) > 0:
48+ LOG.debug("Using %s lease directory", d)
49+ return d
50+ return None
51+
52+
53+def get_interface_name_from_lease(lease_file):
54+ """Obtains the network interface name associated
55+ with the lease file. This function is needed because
56+ Linux distros and FreeBSD have different naming
57+ naming conventions for lease files (different naming
58+ convention for the NIC name in the lease filename),
59+ so parsing the interface name from the lease file
60+ is the safest solution.
61+
62+ @param lease_file: the lease file to grab the interface
63+ name from.
64+
65+ @return: the interface name
66+ """
67+ lease_dict = None
68+ try:
69+ lease_dict = parse_dhcp_lease_file(lease_file)
70+ except InvalidDHCPLeaseFileError:
71+ return None
72+ return lease_dict[-1]['interface']
73+
74+
75+def get_latest_lease_file(lease_d=None):
76+ """Obtains the abs path to the latest dhcp lease file
77+ in the lease_d directory, or from a list of supported
78+ (known) lease directories if lease_d param is None.
79+
80+ @param lease_d: the lease file directory to search in
81+
82+ @return: absolute path to the latest lease file in lease_d
83+ """
84+
85+ if lease_d is None:
86+ lease_d = get_dhclient_d()
87+ if not lease_d:
88+ return None
89+ lease_files = os.listdir(lease_d)
90+ latest_mtime = -1
91+ latest_file = None
92+ freeBSD_lease_name_re = re.compile(r"dhclient\.leases\.\w+")
93+
94+ # lease files are named inconsistently across distros.
95+ # We assume that 'dhclient6' indicates ipv6 and ignore it.
96+ # ubuntu:
97+ # dhclient.<iface>.leases, dhclient.leases, dhclient6.leases
98+ # centos6:
99+ # dhclient-<iface>.leases, dhclient6.leases
100+ # centos7: ('--' is not a typo)
101+ # dhclient--<iface>.lease, dhclient6.leases
102+ # Systems running NetworkManager:
103+ # dhclient-XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX-<iface>.lease
104+ # FreeBSD:
105+ # dhclient.leases.<iface>
106+ for fname in lease_files:
107+ if fname.startswith("dhclient6"):
108+ # avoid files that start with dhclient6 assuming dhcpv6.
109+ continue
110+ if "/var/db" not in lease_d:
111+ if not (fname.endswith(".lease") or
112+ fname.endswith(".leases")):
113+ continue
114+ # FreeBSD lease file scraping
115+ # if (lease_d.startswith("/var/db") and
116+ elif not freeBSD_lease_name_re.match(fname):
117+ # If scraping within the FreeBSD lease dir,
118+ # only the FreeBSD lease filename format is allowed.
119+ # See DHCLIENT.LEASES(5) on FreeBSD.
120+ continue
121+ abs_path = os.path.join(lease_d, fname)
122+ mtime = os.path.getmtime(abs_path)
123+ if mtime > latest_mtime:
124+ latest_mtime = mtime
125+ latest_file = abs_path
126+ return latest_file
127+
128+
129+def get_vr_address():
130+ """Get the address of the virtual router via dhcp leases
131+ If no virtual router is detected, fallback on default gateway.
132+
133+ @return: the virtual router obtained in the following order:
134+ networkd lease, then dhcp lease file, then if DHCP is not found,
135+ default gateway. If no default gateway is found as well, then
136+ None is returned.
137+ """
138+
139+ # Try networkd first...
140+ latest_address = networkd_get_option_from_leases('SERVER_ADDRESS')
141+ if latest_address:
142+ LOG.debug("Found SERVER_ADDRESS '%s' via networkd_leases",
143+ latest_address)
144+ return latest_address
145+
146+ # Try dhcp lease files next...
147+ lease_file = get_latest_lease_file()
148+ if not lease_file:
149+ LOG.debug("No lease file found, using default gateway")
150+ return get_default_gateway()
151+
152+ with open(lease_file, "r") as fd:
153+ for line in fd:
154+ if "dhcp-server-identifier" in line:
155+ words = line.strip(" ;\r\n").split(" ")
156+ if len(words) > 2:
157+ dhcptok = words[2]
158+ LOG.debug("Found DHCP identifier %s", dhcptok)
159+ latest_address = dhcptok
160+ if not latest_address:
161+ # No virtual router found, fallback on default gateway
162+ LOG.debug("No DHCP found, using default gateway")
163+ return get_default_gateway()
164+ return latest_address
165+
166+
167 # vi: ts=4 expandtab
168diff --git a/cloudinit/net/tests/test_dhcp.py b/cloudinit/net/tests/test_dhcp.py
169index 79e8842..4121c82 100644
170--- a/cloudinit/net/tests/test_dhcp.py
171+++ b/cloudinit/net/tests/test_dhcp.py
172@@ -1,6 +1,7 @@
173 # This file is part of cloud-init. See LICENSE file for license information.
174
175 import httpretty
176+import time
177 import os
178 import signal
179 from textwrap import dedent
180@@ -8,7 +9,8 @@ from textwrap import dedent
181 import cloudinit.net as net
182 from cloudinit.net.dhcp import (
183 InvalidDHCPLeaseFileError, maybe_perform_dhcp_discovery,
184- parse_dhcp_lease_file, dhcp_discovery, networkd_load_leases)
185+ parse_dhcp_lease_file, dhcp_discovery, networkd_load_leases,
186+ get_latest_lease_file, get_interface_name_from_lease)
187 from cloudinit.util import ensure_file, write_file
188 from cloudinit.tests.helpers import (
189 CiTestCase, HttprettyTestCase, mock, populate_dir, wrap_and_call)
190@@ -63,6 +65,131 @@ class TestParseDHCPLeasesFile(CiTestCase):
191 write_file(lease_file, content)
192 self.assertItemsEqual(expected, parse_dhcp_lease_file(lease_file))
193
194+ def test_parse_network_interface_name(self):
195+ """get_interface_name_from_lease returns the interface
196+ name from the last lease in the specified lease file.
197+ As per convention, newer leases are appended to the end
198+ of the file, so the latest lease is the last lease."""
199+ lease_file = self.tmp_path('leases')
200+ content = dedent("""
201+ lease {
202+ interface "eth0";
203+ fixed-address 192.168.2.74;
204+ option subnet-mask 255.255.255.0;
205+ option routers 192.168.2.1;
206+ renew 4 2017/07/27 18:02:30;
207+ expire 5 2017/07/28 07:08:15;
208+ }
209+ lease {
210+ interface "wlp3s0";
211+ fixed-address 192.168.2.74;
212+ option subnet-mask 255.255.255.0;
213+ option routers 192.168.2.1;
214+ }
215+ """)
216+ expected = "wlp3s0"
217+ write_file(lease_file, content)
218+ self.assertItemsEqual(expected,
219+ get_interface_name_from_lease(lease_file))
220+
221+
222+class TestGetLatestLease(CiTestCase):
223+
224+ def _update_increasing_timestamp(self, bdir, files):
225+ """Updates the timestamps of the files in bdir
226+ to ensure their mtime increases with each file"""
227+
228+ start = int(time.time())
229+ for num, fname in enumerate(reversed(files)):
230+ fpath = os.path.sep.join((bdir, fname))
231+ os.utime(fpath, (start - num, start - num))
232+
233+ def _pop_and_test(self, filenames, expected, path=None):
234+ """Each file in filenames is written to a directory
235+ in successive order. Their mtimes are also updated
236+ so that they increase with each file in filenames.
237+ The directory that the files are written to is
238+ either a tmp directory, or a tmp directory that
239+ ends with path."""
240+
241+ lease_d = None
242+ if path is not None:
243+ lease_d = self.tmp_path(path)
244+ else:
245+ lease_d = self.tmp_dir()
246+ files = {fname: '' for fname in filenames}
247+ populate_dir(lease_d, files)
248+ self._update_increasing_timestamp(lease_d, filenames)
249+ self.assertEqual(self.tmp_path(expected, lease_d),
250+ get_latest_lease_file(lease_d))
251+
252+ def test_skips_dhcpv6_files(self):
253+ """files started with dhclient6 should be skipped."""
254+
255+ expected = "dhclient.lease"
256+ self._pop_and_test([expected, "dhclient6.lease"], expected)
257+
258+ def test_selects_dhclient_dot_files(self):
259+ """files named dhclient.lease or dhclient.leases should be used.
260+
261+ Ubuntu names files dhclient.eth0.leases dhclient6.leases and
262+ sometimes dhclient.leases."""
263+
264+ self._pop_and_test(["dhclient.lease"], "dhclient.lease")
265+ self._pop_and_test(["dhclient.leases"], "dhclient.leases")
266+
267+ def test_selects_dhclient_dash_files(self):
268+ """files named dhclient-lease or dhclient-leases should be used.
269+
270+ Redhat/Centos names files with dhclient--eth0.lease (centos 7) or
271+ dhclient-eth0.leases (centos 6).
272+ """
273+
274+ self._pop_and_test(["dhclient-eth0.lease"], "dhclient-eth0.lease")
275+ self._pop_and_test(["dhclient--eth0.lease"], "dhclient--eth0.lease")
276+
277+ def test_selects_network_manager_files(self):
278+ """files named
279+ dhclient-XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX-<iface>.lease
280+ should be used.
281+
282+ On systems running NetworkManager, the lease file naming convention
283+ is similar to Ubuntu and RHEL/CentOS names, except that it has a
284+ random string identifier before the interface name.
285+ """
286+
287+ lease = "dhclient-e4599575-5a21-46c1-bfe3-62cb23268b69-eth0.lease"
288+ self._pop_and_test([lease], lease)
289+
290+ def test_ignores_by_extension(self):
291+ """only .lease or .leases file should be considered."""
292+
293+ self._pop_and_test(["dhclient.lease", "dhclient.lease.bk",
294+ "dhclient.lease-old", "dhclient.leaselease"],
295+ "dhclient.lease")
296+
297+ def test_select_freeBSD_lease_file(self):
298+ """when selecting lease file within /var/db (FreeBSD location),
299+ only lease files following "dhclient.leases.<iface>"
300+ format are allowed.
301+ """
302+
303+ # simulate the file scraping within /tmp/XXXXX/var/db
304+ self._pop_and_test(["dhclient.leases.eth0", "dhclient.lease.bk",
305+ "dhclient.lease-old", "dhclient.leaselease",
306+ "dhclient-eth0.lease", "dhclient--eth0.lease",
307+ "dhclient.lease", "dhclient.leases"],
308+ "dhclient.leases.eth0", path="var/db")
309+
310+ def test_selects_newest_matching(self):
311+ """If multiple files match, the newest written should be used."""
312+
313+ valid_1 = "dhclient.leases"
314+ valid_2 = "dhclient.lease"
315+
316+ self._pop_and_test([valid_1, valid_2], valid_2)
317+ self._pop_and_test([valid_2, valid_1], valid_1)
318+
319
320 class TestDHCPDiscoveryClean(CiTestCase):
321 with_logs = True
322diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
323index eccbee5..15d1279 100644
324--- a/cloudinit/sources/DataSourceAzure.py
325+++ b/cloudinit/sources/DataSourceAzure.py
326@@ -19,7 +19,8 @@ import xml.etree.ElementTree as ET
327 from cloudinit import log as logging
328 from cloudinit import net
329 from cloudinit.event import EventType
330-from cloudinit.net.dhcp import EphemeralDHCPv4
331+from cloudinit.net.dhcp import (
332+ EphemeralDHCPv4, get_latest_lease_file)
333 from cloudinit import sources
334 from cloudinit.sources.helpers.azure import get_metadata_from_fabric
335 from cloudinit.sources.helpers import netlink
336@@ -46,8 +47,8 @@ BOUNCE_COMMAND_FREEBSD = [
337 # ensures that it gets linked to this path.
338 RESOURCE_DISK_PATH = '/dev/disk/cloud/azure_resource'
339 DEFAULT_PRIMARY_NIC = 'eth0'
340-LEASE_FILE = '/var/lib/dhcp/dhclient.eth0.leases'
341 DEFAULT_FS = 'ext4'
342+
343 # DMI chassis-asset-tag is set static for all azure instances
344 AZURE_CHASSIS_ASSET_TAG = '7783-7084-3265-9085-8269-3286-77'
345 REPROVISION_MARKER_FILE = "/var/lib/cloud/data/poll_imds"
346@@ -186,8 +187,6 @@ def get_resource_disk_on_freebsd(port_id):
347
348 # update the FreeBSD specific information
349 if util.is_FreeBSD():
350- DEFAULT_PRIMARY_NIC = 'hn0'
351- LEASE_FILE = '/var/db/dhclient.leases.hn0'
352 DEFAULT_FS = 'freebsd-ufs'
353 res_disk = get_resource_disk_on_freebsd(1)
354 if res_disk is not None:
355@@ -207,7 +206,6 @@ BUILTIN_DS_CONFIG = {
356 'hostname_command': 'hostname',
357 },
358 'disk_aliases': {'ephemeral0': RESOURCE_DISK_PATH},
359- 'dhclient_lease_file': LEASE_FILE,
360 'apply_network_config': True, # Use IMDS published network configuration
361 }
362 # RELEASE_BLOCKER: Xenial and earlier apply_network_config default is False
363@@ -277,7 +275,6 @@ class DataSourceAzure(sources.DataSource):
364 self.ds_cfg = util.mergemanydict([
365 util.get_cfg_by_path(sys_cfg, DS_CFG_PATH, {}),
366 BUILTIN_DS_CONFIG])
367- self.dhclient_lease_file = self.ds_cfg.get('dhclient_lease_file')
368 self._network_config = None
369 # Regenerate network config new_instance boot and every boot
370 self.update_events['network'].add(EventType.BOOT)
371@@ -627,11 +624,17 @@ class DataSourceAzure(sources.DataSource):
372 if self.ds_cfg['agent_command'] == AGENT_START_BUILTIN:
373 self.bounce_network_with_azure_hostname()
374
375+<<<<<<< cloudinit/sources/DataSourceAzure.py
376 pubkey_info = self.cfg.get('_pubkeys', None)
377 metadata_func = partial(get_metadata_from_fabric,
378 fallback_lease_file=self.
379 dhclient_lease_file,
380 pubkey_info=pubkey_info)
381+=======
382+ metadata_func = partial(
383+ get_metadata_from_fabric,
384+ fallback_lease_file=get_latest_lease_file())
385+>>>>>>> cloudinit/sources/DataSourceAzure.py
386 else:
387 metadata_func = self.get_metadata_from_agent
388
389diff --git a/cloudinit/sources/DataSourceCloudStack.py b/cloudinit/sources/DataSourceCloudStack.py
390index d4b758f..944717e 100644
391--- a/cloudinit/sources/DataSourceCloudStack.py
392+++ b/cloudinit/sources/DataSourceCloudStack.py
393@@ -13,8 +13,6 @@
394 # This file is part of cloud-init. See LICENSE file for license information.
395
396 import os
397-from socket import inet_ntoa
398-from struct import pack
399 import time
400
401 from cloudinit import ec2_utils as ec2
402@@ -78,7 +76,7 @@ class DataSourceCloudStack(sources.DataSource):
403 # Cloudstack has its metadata/userdata URLs located at
404 # http://<virtual-router-ip>/latest/
405 self.api_ver = 'latest'
406- self.vr_addr = get_vr_address()
407+ self.vr_addr = dhcp.get_vr_address()
408 if not self.vr_addr:
409 raise RuntimeError("No virtual router found!")
410 self.metadata_address = "http://%s/" % (self.vr_addr,)
411@@ -156,96 +154,6 @@ class DataSourceCloudStack(sources.DataSource):
412 return self.metadata['availability-zone']
413
414
415-def get_default_gateway():
416- # Returns the default gateway ip address in the dotted format.
417- lines = util.load_file("/proc/net/route").splitlines()
418- for line in lines:
419- items = line.split("\t")
420- if items[1] == "00000000":
421- # Found the default route, get the gateway
422- gw = inet_ntoa(pack("<L", int(items[2], 16)))
423- LOG.debug("Found default route, gateway is %s", gw)
424- return gw
425- return None
426-
427-
428-def get_dhclient_d():
429- # find lease files directory
430- supported_dirs = ["/var/lib/dhclient", "/var/lib/dhcp",
431- "/var/lib/NetworkManager"]
432- for d in supported_dirs:
433- if os.path.exists(d) and len(os.listdir(d)) > 0:
434- LOG.debug("Using %s lease directory", d)
435- return d
436- return None
437-
438-
439-def get_latest_lease(lease_d=None):
440- # find latest lease file
441- if lease_d is None:
442- lease_d = get_dhclient_d()
443- if not lease_d:
444- return None
445- lease_files = os.listdir(lease_d)
446- latest_mtime = -1
447- latest_file = None
448-
449- # lease files are named inconsistently across distros.
450- # We assume that 'dhclient6' indicates ipv6 and ignore it.
451- # ubuntu:
452- # dhclient.<iface>.leases, dhclient.leases, dhclient6.leases
453- # centos6:
454- # dhclient-<iface>.leases, dhclient6.leases
455- # centos7: ('--' is not a typo)
456- # dhclient--<iface>.lease, dhclient6.leases
457- for fname in lease_files:
458- if fname.startswith("dhclient6"):
459- # avoid files that start with dhclient6 assuming dhcpv6.
460- continue
461- if not (fname.endswith(".lease") or fname.endswith(".leases")):
462- continue
463-
464- abs_path = os.path.join(lease_d, fname)
465- mtime = os.path.getmtime(abs_path)
466- if mtime > latest_mtime:
467- latest_mtime = mtime
468- latest_file = abs_path
469- return latest_file
470-
471-
472-def get_vr_address():
473- # Get the address of the virtual router via dhcp leases
474- # If no virtual router is detected, fallback on default gateway.
475- # See http://docs.cloudstack.apache.org/projects/cloudstack-administration/en/4.8/virtual_machines/user-data.html # noqa
476-
477- # Try networkd first...
478- latest_address = dhcp.networkd_get_option_from_leases('SERVER_ADDRESS')
479- if latest_address:
480- LOG.debug("Found SERVER_ADDRESS '%s' via networkd_leases",
481- latest_address)
482- return latest_address
483-
484- # Try dhcp lease files next...
485- lease_file = get_latest_lease()
486- if not lease_file:
487- LOG.debug("No lease file found, using default gateway")
488- return get_default_gateway()
489-
490- with open(lease_file, "r") as fd:
491- for line in fd:
492- if "dhcp-server-identifier" in line:
493- words = line.strip(" ;\r\n").split(" ")
494- if len(words) > 2:
495- dhcptok = words[2]
496- LOG.debug("Found DHCP identifier %s", dhcptok)
497- latest_address = dhcptok
498- if not latest_address:
499- # No virtual router found, fallback on default gateway
500- LOG.debug("No DHCP found, using default gateway")
501- return get_default_gateway()
502- return latest_address
503-
504-
505 # Used to match classes to dependencies
506 datasources = [
507 (DataSourceCloudStack, (sources.DEP_FILESYSTEM, sources.DEP_NETWORK)),
508diff --git a/tests/unittests/test_datasource/test_cloudstack.py b/tests/unittests/test_datasource/test_cloudstack.py
509index d6d2d6b..49219ce 100644
510--- a/tests/unittests/test_datasource/test_cloudstack.py
511+++ b/tests/unittests/test_datasource/test_cloudstack.py
512@@ -1,15 +1,11 @@
513 # This file is part of cloud-init. See LICENSE file for license information.
514
515 from cloudinit import helpers
516-from cloudinit import util
517 from cloudinit.sources.DataSourceCloudStack import (
518- DataSourceCloudStack, get_latest_lease)
519+ DataSourceCloudStack)
520
521 from cloudinit.tests.helpers import CiTestCase, ExitStack, mock
522
523-import os
524-import time
525-
526
527 class TestCloudStackPasswordFetching(CiTestCase):
528
529@@ -21,13 +17,13 @@ class TestCloudStackPasswordFetching(CiTestCase):
530 self.patches.enter_context(mock.patch('{0}.ec2'.format(mod_name)))
531 self.patches.enter_context(mock.patch('{0}.uhelp'.format(mod_name)))
532 default_gw = "192.201.20.0"
533- get_latest_lease = mock.MagicMock(return_value=None)
534+ get_latest_lease_file = mock.MagicMock(return_value=None)
535 self.patches.enter_context(mock.patch(
536- mod_name + '.get_latest_lease', get_latest_lease))
537+ mod_name + '.dhcp.get_latest_lease_file', get_latest_lease_file))
538
539 get_default_gw = mock.MagicMock(return_value=default_gw)
540 self.patches.enter_context(mock.patch(
541- mod_name + '.get_default_gateway', get_default_gw))
542+ mod_name + '.dhcp.get_default_gateway', get_default_gw))
543
544 get_networkd_server_address = mock.MagicMock(return_value=None)
545 self.patches.enter_context(mock.patch(
546@@ -105,71 +101,4 @@ class TestCloudStackPasswordFetching(CiTestCase):
547 self._check_password_not_saved_for('bad_request')
548
549
550-class TestGetLatestLease(CiTestCase):
551-
552- def _populate_dir_list(self, bdir, files):
553- """populate_dir_list([(name, data), (name, data)])
554-
555- writes files to bdir, and updates timestamps to ensure
556- that their mtime increases with each file."""
557-
558- start = int(time.time())
559- for num, fname in enumerate(reversed(files)):
560- fpath = os.path.sep.join((bdir, fname))
561- util.write_file(fpath, fname.encode())
562- os.utime(fpath, (start - num, start - num))
563-
564- def _pop_and_test(self, files, expected):
565- lease_d = self.tmp_dir()
566- self._populate_dir_list(lease_d, files)
567- self.assertEqual(self.tmp_path(expected, lease_d),
568- get_latest_lease(lease_d))
569-
570- def test_skips_dhcpv6_files(self):
571- """files started with dhclient6 should be skipped."""
572- expected = "dhclient.lease"
573- self._pop_and_test([expected, "dhclient6.lease"], expected)
574-
575- def test_selects_dhclient_dot_files(self):
576- """files named dhclient.lease or dhclient.leases should be used.
577-
578- Ubuntu names files dhclient.eth0.leases dhclient6.leases and
579- sometimes dhclient.leases."""
580- self._pop_and_test(["dhclient.lease"], "dhclient.lease")
581- self._pop_and_test(["dhclient.leases"], "dhclient.leases")
582-
583- def test_selects_dhclient_dash_files(self):
584- """files named dhclient-lease or dhclient-leases should be used.
585-
586- Redhat/Centos names files with dhclient--eth0.lease (centos 7) or
587- dhclient-eth0.leases (centos 6).
588- """
589- self._pop_and_test(["dhclient-eth0.lease"], "dhclient-eth0.lease")
590- self._pop_and_test(["dhclient--eth0.lease"], "dhclient--eth0.lease")
591-
592- def test_ignores_by_extension(self):
593- """only .lease or .leases file should be considered."""
594-
595- self._pop_and_test(["dhclient.lease", "dhclient.lease.bk",
596- "dhclient.lease-old", "dhclient.leaselease"],
597- "dhclient.lease")
598-
599- def test_selects_newest_matching(self):
600- """If multiple files match, the newest written should be used."""
601- lease_d = self.tmp_dir()
602- valid_1 = "dhclient.leases"
603- valid_2 = "dhclient.lease"
604- valid_1_path = self.tmp_path(valid_1, lease_d)
605- valid_2_path = self.tmp_path(valid_2, lease_d)
606-
607- self._populate_dir_list(lease_d, [valid_1, valid_2])
608- self.assertEqual(valid_2_path, get_latest_lease(lease_d))
609-
610- # now update mtime on valid_2 to be older than valid_1 and re-check.
611- mtime = int(os.path.getmtime(valid_1_path)) - 1
612- os.utime(valid_2_path, (mtime, mtime))
613-
614- self.assertEqual(valid_1_path, get_latest_lease(lease_d))
615-
616-
617 # vi: ts=4 expandtab

Subscribers

People subscribed via source and target branches