Skip to content

Commit 049d62b

Browse files
sshedisshedi
authored
photon: refactor hostname handling and add networkd activator (canonical#958)
1 parent 00dbaf1 commit 049d62b

File tree

6 files changed

+117
-44
lines changed

6 files changed

+117
-44
lines changed

‎cloudinit/distros/photon.py‎

Lines changed: 24 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,12 @@
1313
from cloudinit import log as logging
1414
from cloudinit.settings import PER_INSTANCE
1515
from cloudinit.distros import rhel_util as rhutil
16-
from cloudinit.distros.parsers.hostname import HostnameConf
1716

1817
LOG = logging.getLogger(__name__)
1918

2019

2120
class Distro(distros.Distro):
22-
hostname_conf_fn = '/etc/hostname'
21+
systemd_hostname_conf_fn = '/etc/hostname'
2322
network_conf_dir = '/etc/systemd/network/'
2423
systemd_locale_conf_fn = '/etc/locale.conf'
2524
resolve_conf_fn = '/etc/systemd/resolved.conf'
@@ -43,17 +42,18 @@ def __init__(self, name, cfg, paths):
4342
self.osfamily = 'photon'
4443
self.init_cmd = ['systemctl']
4544

46-
def exec_cmd(self, cmd, capture=False):
45+
def exec_cmd(self, cmd, capture=True):
4746
LOG.debug('Attempting to run: %s', cmd)
4847
try:
4948
(out, err) = subp.subp(cmd, capture=capture)
5049
if err:
5150
LOG.warning('Running %s resulted in stderr output: %s',
5251
cmd, err)
53-
return True, out, err
52+
return True, out, err
53+
return False, out, err
5454
except subp.ProcessExecutionError:
5555
util.logexc(LOG, 'Command %s failed', cmd)
56-
return False, None, None
56+
return True, None, None
5757

5858
def generate_fallback_config(self):
5959
key = 'disable_fallback_netcfg'
@@ -85,41 +85,32 @@ def apply_locale(self, locale, out_fn=None):
8585
# For locale change to take effect, reboot is needed or we can restart
8686
# systemd-localed. This is equivalent of localectl
8787
cmd = ['systemctl', 'restart', 'systemd-localed']
88-
_ret, _out, _err = self.exec_cmd(cmd)
88+
self.exec_cmd(cmd)
8989

9090
def install_packages(self, pkglist):
9191
# self.update_package_sources()
9292
self.package_command('install', pkgs=pkglist)
9393

94-
def _bring_up_interfaces(self, device_names):
95-
cmd = ['systemctl', 'restart', 'systemd-networkd', 'systemd-resolved']
96-
LOG.debug('Attempting to run bring up interfaces using command %s',
97-
cmd)
98-
ret, _out, _err = self.exec_cmd(cmd)
99-
return ret
100-
10194
def _write_hostname(self, hostname, filename):
102-
conf = None
103-
try:
104-
# Try to update the previous one
105-
# Let's see if we can read it first.
106-
conf = HostnameConf(util.load_file(filename))
107-
conf.parse()
108-
except IOError:
109-
pass
110-
if not conf:
111-
conf = HostnameConf('')
112-
conf.set_hostname(hostname)
113-
util.write_file(filename, str(conf), mode=0o644)
95+
if filename and filename.endswith('/previous-hostname'):
96+
util.write_file(filename, hostname)
97+
else:
98+
ret, _out, err = self.exec_cmd(['hostnamectl', 'set-hostname',
99+
str(hostname)])
100+
if ret:
101+
LOG.warning(('Error while setting hostname: %s\n'
102+
'Given hostname: %s', err, hostname))
114103

115104
def _read_system_hostname(self):
116-
sys_hostname = self._read_hostname(self.hostname_conf_fn)
117-
return (self.hostname_conf_fn, sys_hostname)
105+
sys_hostname = self._read_hostname(self.systemd_hostname_conf_fn)
106+
return (self.systemd_hostname_conf_fn, sys_hostname)
118107

119108
def _read_hostname(self, filename, default=None):
120-
_ret, out, _err = self.exec_cmd(['hostname'])
109+
if filename and filename.endswith('/previous-hostname'):
110+
return util.load_file(filename).strip()
121111

122-
return out if out else default
112+
_ret, out, _err = self.exec_cmd(['hostname', '-f'])
113+
return out.strip() if out else default
123114

124115
def _get_localhost_ip(self):
125116
return '127.0.1.1'
@@ -128,7 +119,7 @@ def set_timezone(self, tz):
128119
distros.set_etc_timezone(tz=tz, tz_file=self._find_tz_file(tz))
129120

130121
def package_command(self, command, args=None, pkgs=None):
131-
if pkgs is None:
122+
if not pkgs:
132123
pkgs = []
133124

134125
cmd = ['tdnf', '-y']
@@ -142,8 +133,9 @@ def package_command(self, command, args=None, pkgs=None):
142133
pkglist = util.expand_package_list('%s-%s', pkgs)
143134
cmd.extend(pkglist)
144135

145-
# Allow the output of this to flow outwards (ie not be captured)
146-
_ret, _out, _err = self.exec_cmd(cmd, capture=False)
136+
ret, _out, err = self.exec_cmd(cmd)
137+
if ret:
138+
LOG.error('Error while installing packages: %s', err)
147139

148140
def update_package_sources(self):
149141
self._runner.run('update-sources', self.package_command,

‎cloudinit/net/activators.py‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from cloudinit import util
99
from cloudinit.net.eni import available as eni_available
1010
from cloudinit.net.netplan import available as netplan_available
11+
from cloudinit.net.networkd import available as networkd_available
1112
from cloudinit.net.network_state import NetworkState
1213
from cloudinit.net.sysconfig import NM_CFG_FILE
1314

@@ -213,12 +214,38 @@ def bring_down_all_interfaces(network_state: NetworkState) -> bool:
213214
return _alter_interface(NetplanActivator.NETPLAN_CMD, 'all')
214215

215216

217+
class NetworkdActivator(NetworkActivator):
218+
@staticmethod
219+
def available(target=None) -> bool:
220+
"""Return true if ifupdown can be used on this system."""
221+
return networkd_available(target=target)
222+
223+
@staticmethod
224+
def bring_up_interface(device_name: str) -> bool:
225+
""" Return True is successful, otherwise return False """
226+
cmd = ['ip', 'link', 'set', 'up', device_name]
227+
return _alter_interface(cmd, device_name)
228+
229+
@staticmethod
230+
def bring_up_all_interfaces(network_state: NetworkState) -> bool:
231+
""" Return True is successful, otherwise return False """
232+
cmd = ['systemctl', 'restart', 'systemd-networkd', 'systemd-resolved']
233+
return _alter_interface(cmd, 'all')
234+
235+
@staticmethod
236+
def bring_down_interface(device_name: str) -> bool:
237+
""" Return True is successful, otherwise return False """
238+
cmd = ['ip', 'link', 'set', 'down', device_name]
239+
return _alter_interface(cmd, device_name)
240+
241+
216242
# This section is mostly copied and pasted from renderers.py. An abstract
217243
# version to encompass both seems overkill at this point
218244
DEFAULT_PRIORITY = [
219245
IfUpDownActivator,
220246
NetworkManagerActivator,
221247
NetplanActivator,
248+
NetworkdActivator,
222249
]
223250

224251

‎cloudinit/net/networkd.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ def _render_content(self, ns):
246246

247247

248248
def available(target=None):
249-
expected = ['systemctl']
249+
expected = ['ip', 'systemctl']
250250
search = ['/usr/bin', '/bin']
251251
for p in expected:
252252
if not subp.which(p, search=search, target=target):

‎tests/unittests/test_distros/test_photon.py‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,28 @@ def test_network_renderer(self):
2525
def test_get_distro(self):
2626
self.assertEqual(self.distro.osfamily, 'photon')
2727

28-
def test_write_hostname(self):
28+
@mock.patch("cloudinit.distros.photon.subp.subp")
29+
def test_write_hostname(self, m_subp):
2930
hostname = 'myhostname'
30-
hostfile = self.tmp_path('hostfile')
31+
hostfile = self.tmp_path('previous-hostname')
3132
self.distro._write_hostname(hostname, hostfile)
32-
self.assertEqual(hostname + '\n', util.load_file(hostfile))
33+
self.assertEqual(hostname, util.load_file(hostfile))
34+
35+
ret = self.distro._read_hostname(hostfile)
36+
self.assertEqual(ret, hostname)
37+
38+
m_subp.return_value = (None, None)
39+
hostfile += 'hostfile'
40+
self.distro._write_hostname(hostname, hostfile)
41+
42+
m_subp.return_value = (hostname, None)
43+
ret = self.distro._read_hostname(hostfile)
44+
self.assertEqual(ret, hostname)
45+
46+
self.logs.truncate(0)
47+
m_subp.return_value = (None, 'bla')
48+
self.distro._write_hostname(hostname, None)
49+
self.assertIn('Error while setting hostname', self.logs.getvalue())
3350

3451
@mock.patch('cloudinit.net.generate_fallback_config')
3552
def test_fallback_netcfg(self, m_fallback_cfg):

‎tests/unittests/test_handler/test_handler_set_hostname.py‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,8 @@ def test_write_hostname_sles(self, m_uses_systemd):
120120
contents = util.load_file(distro.hostname_conf_fn)
121121
self.assertEqual('blah', contents.strip())
122122

123-
@mock.patch('cloudinit.distros.Distro.uses_systemd', return_value=False)
124-
def test_photon_hostname(self, m_uses_systemd):
123+
@mock.patch('cloudinit.distros.photon.subp.subp')
124+
def test_photon_hostname(self, m_subp):
125125
cfg1 = {
126126
'hostname': 'photon',
127127
'prefer_fqdn_over_hostname': True,
@@ -134,17 +134,31 @@ def test_photon_hostname(self, m_uses_systemd):
134134
}
135135

136136
ds = None
137+
m_subp.return_value = (None, None)
137138
distro = self._fetch_distro('photon', cfg1)
138139
paths = helpers.Paths({'cloud_dir': self.tmp})
139140
cc = cloud.Cloud(ds, paths, {}, distro, None)
140-
self.patchUtils(self.tmp)
141141
for c in [cfg1, cfg2]:
142142
cc_set_hostname.handle('cc_set_hostname', c, cc, LOG, [])
143-
contents = util.load_file(distro.hostname_conf_fn, decode=True)
143+
print("\n", m_subp.call_args_list)
144144
if c['prefer_fqdn_over_hostname']:
145-
self.assertEqual(contents.strip(), c['fqdn'])
145+
assert [
146+
mock.call(['hostnamectl', 'set-hostname', c['fqdn']],
147+
capture=True)
148+
] in m_subp.call_args_list
149+
assert [
150+
mock.call(['hostnamectl', 'set-hostname', c['hostname']],
151+
capture=True)
152+
] not in m_subp.call_args_list
146153
else:
147-
self.assertEqual(contents.strip(), c['hostname'])
154+
assert [
155+
mock.call(['hostnamectl', 'set-hostname', c['hostname']],
156+
capture=True)
157+
] in m_subp.call_args_list
158+
assert [
159+
mock.call(['hostnamectl', 'set-hostname', c['fqdn']],
160+
capture=True)
161+
] not in m_subp.call_args_list
148162

149163
def test_multiple_calls_skips_unchanged_hostname(self):
150164
"""Only new hostname or fqdn values will generate a hostname call."""

‎tests/unittests/test_net_activators.py‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111
from cloudinit.net.activators import (
1212
IfUpDownActivator,
1313
NetplanActivator,
14-
NetworkManagerActivator
14+
NetworkManagerActivator,
15+
NetworkdActivator
1516
)
1617
from cloudinit.net.network_state import parse_net_config_data
1718
from cloudinit.safeyaml import load
@@ -116,11 +117,17 @@ def test_none_available(self, unavailable_mocks):
116117
(('nmcli',), {'target': None}),
117118
]
118119

120+
NETWORKD_AVAILABLE_CALLS = [
121+
(('ip',), {'search': ['/usr/bin', '/bin'], 'target': None}),
122+
(('systemctl',), {'search': ['/usr/bin', '/bin'], 'target': None}),
123+
]
124+
119125

120126
@pytest.mark.parametrize('activator, available_calls', [
121127
(IfUpDownActivator, IF_UP_DOWN_AVAILABLE_CALLS),
122128
(NetplanActivator, NETPLAN_AVAILABLE_CALLS),
123129
(NetworkManagerActivator, NETWORK_MANAGER_AVAILABLE_CALLS),
130+
(NetworkdActivator, NETWORKD_AVAILABLE_CALLS),
124131
])
125132
class TestActivatorsAvailable:
126133
def test_available(
@@ -140,11 +147,18 @@ def test_available(
140147
((['nmcli', 'connection', 'up', 'ifname', 'eth1'], ), {}),
141148
]
142149

150+
NETWORKD_BRING_UP_CALL_LIST = [
151+
((['ip', 'link', 'set', 'up', 'eth0'], ), {}),
152+
((['ip', 'link', 'set', 'up', 'eth1'], ), {}),
153+
((['systemctl', 'restart', 'systemd-networkd', 'systemd-resolved'], ), {}),
154+
]
155+
143156

144157
@pytest.mark.parametrize('activator, expected_call_list', [
145158
(IfUpDownActivator, IF_UP_DOWN_BRING_UP_CALL_LIST),
146159
(NetplanActivator, NETPLAN_CALL_LIST),
147160
(NetworkManagerActivator, NETWORK_MANAGER_BRING_UP_CALL_LIST),
161+
(NetworkdActivator, NETWORKD_BRING_UP_CALL_LIST),
148162
])
149163
class TestActivatorsBringUp:
150164
@patch('cloudinit.subp.subp', return_value=('', ''))
@@ -159,8 +173,11 @@ def test_bring_up_interface(
159173
def test_bring_up_interfaces(
160174
self, m_subp, activator, expected_call_list, available_mocks
161175
):
176+
index = 0
162177
activator.bring_up_interfaces(['eth0', 'eth1'])
163-
assert expected_call_list == m_subp.call_args_list
178+
for call in m_subp.call_args_list:
179+
assert call == expected_call_list[index]
180+
index += 1
164181

165182
@patch('cloudinit.subp.subp', return_value=('', ''))
166183
def test_bring_up_all_interfaces_v1(
@@ -191,11 +208,17 @@ def test_bring_up_all_interfaces_v2(
191208
((['nmcli', 'connection', 'down', 'eth1'], ), {}),
192209
]
193210

211+
NETWORKD_BRING_DOWN_CALL_LIST = [
212+
((['ip', 'link', 'set', 'down', 'eth0'], ), {}),
213+
((['ip', 'link', 'set', 'down', 'eth1'], ), {}),
214+
]
215+
194216

195217
@pytest.mark.parametrize('activator, expected_call_list', [
196218
(IfUpDownActivator, IF_UP_DOWN_BRING_DOWN_CALL_LIST),
197219
(NetplanActivator, NETPLAN_CALL_LIST),
198220
(NetworkManagerActivator, NETWORK_MANAGER_BRING_DOWN_CALL_LIST),
221+
(NetworkdActivator, NETWORKD_BRING_DOWN_CALL_LIST),
199222
])
200223
class TestActivatorsBringDown:
201224
@patch('cloudinit.subp.subp', return_value=('', ''))

0 commit comments

Comments
 (0)