Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new network activators to bring up interfaces (SC-85) #919

Merged
merged 8 commits into from
Jul 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions cloudinit/cmd/devel/net_convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,6 @@ def handle_args(name, args):
pre_ns = ovf.get_network_config_from_conf(config, False)

ns = network_state.parse_net_config_data(pre_ns)
if not ns:
raise RuntimeError("No valid network_state object created from"
" input data")

if args.debug:
sys.stderr.write('\n'.join(
Expand Down
39 changes: 17 additions & 22 deletions cloudinit/distros/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@
import string
import urllib.parse
from io import StringIO
from typing import Any, Mapping

from cloudinit import importer
from cloudinit import log as logging
from cloudinit import net
from cloudinit.net import activators
from cloudinit.net import eni
from cloudinit.net import network_state
from cloudinit.net import renderers
from cloudinit.net.network_state import parse_net_config_data
from cloudinit import persistence
from cloudinit import ssh_util
from cloudinit import type_utils
Expand Down Expand Up @@ -72,7 +75,7 @@ class Distro(persistence.CloudInitPickleMixin, metaclass=abc.ABCMeta):
hostname_conf_fn = "/etc/hostname"
tz_zone_dir = "/usr/share/zoneinfo"
init_cmd = ['service'] # systemctl, service etc
renderer_configs = {}
renderer_configs = {} # type: Mapping[str, Mapping[str, Any]]
_preferred_ntp_clients = None
networking_cls = LinuxNetworking
# This is used by self.shutdown_command(), and can be overridden in
Expand Down Expand Up @@ -106,23 +109,20 @@ def install_packages(self, pkglist):
raise NotImplementedError()

def _write_network(self, settings):
raise RuntimeError(
"""Deprecated. Remove if/when arch and gentoo support renderers."""
raise NotImplementedError(
"Legacy function '_write_network' was called in distro '%s'.\n"
"_write_network_config needs implementation.\n" % self.name)

def _write_network_config(self, settings):
raise NotImplementedError()

def _supported_write_network_config(self, network_config):
def _write_network_state(self, network_state):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good naming, makes it more clear

priority = util.get_cfg_by_path(
self._cfg, ('network', 'renderers'), None)

name, render_cls = renderers.select(priority=priority)
LOG.debug("Selected renderer '%s' from priority list: %s",
name, priority)
renderer = render_cls(config=self.renderer_configs.get(name))
renderer.render_network_config(network_config)
return []
renderer.render_network_state(network_state)

def _find_tz_file(self, tz):
tz_file = os.path.join(self.tz_zone_dir, str(tz))
Expand Down Expand Up @@ -174,6 +174,7 @@ def get_package_mirror_info(self, arch=None, data_source=None):
mirror_info=arch_info)

def apply_network(self, settings, bring_up=True):
"""Deprecated. Remove if/when arch and gentoo support renderers."""
# this applies network where 'settings' is interfaces(5) style
# it is obsolete compared to apply_network_config
# Write it out
Expand All @@ -188,6 +189,7 @@ def apply_network(self, settings, bring_up=True):
return False

def _apply_network_from_network_config(self, netconfig, bring_up=True):
"""Deprecated. Remove if/when arch and gentoo support renderers."""
distro = self.__class__
LOG.warning("apply_network_config is not currently implemented "
"for distribution '%s'. Attempting to use apply_network",
Expand All @@ -208,16 +210,18 @@ def apply_network_config(self, netconfig, bring_up=False):
# apply network config netconfig
# This method is preferred to apply_network which only takes
# a much less complete network config format (interfaces(5)).
network_state = parse_net_config_data(netconfig)
try:
dev_names = self._write_network_config(netconfig)
self._write_network_state(network_state)
except NotImplementedError:
# backwards compat until all distros have apply_network_config
return self._apply_network_from_network_config(
netconfig, bring_up=bring_up)

# Now try to bring them up
if bring_up:
return self._bring_up_interfaces(dev_names)
network_activator = activators.select_activator()
network_activator.bring_up_all_interfaces(network_state)
return False

def apply_network_config_names(self, netconfig):
Expand Down Expand Up @@ -393,20 +397,11 @@ def preferred_ntp_clients(self):
return self._preferred_ntp_clients

def _bring_up_interface(self, device_name):
cmd = ['ifup', device_name]
LOG.debug("Attempting to run bring up interface %s using command %s",
device_name, cmd)
try:
(_out, err) = subp.subp(cmd)
if len(err):
LOG.warning("Running %s resulted in stderr output: %s",
cmd, err)
return True
except subp.ProcessExecutionError:
util.logexc(LOG, "Running interface command %s failed", cmd)
return False
"""Deprecated. Remove if/when arch and gentoo support renderers."""
raise NotImplementedError
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break both gentoo which directly calls Distros._bring_up_interface.

Don't we need to either:

  1. leave the _bring_up_interface stub in place still in the base class?
    OR
  2. Migrate this function declaration into gentoo.py._bring_up_interface? Or do we want to correct the gentoo reference to just call self._bring_up_interface

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't actually. The call hierarchy would work like this:
base:apply_network -> gentoo:_bring_up_interfaces->base:_bring_up_interfaces->gentoo:_bring_up_interface

The distros.Distro._bring_up_interfaces is confusing (it should just call super), but since it's passing self to the base class, the "normal" method calling rules still apply.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

D'oh right.


def _bring_up_interfaces(self, device_names):
"""Deprecated. Remove if/when arch and gentoo support renderers."""
am_failed = 0
for d in device_names:
if not self._bring_up_interface(d):
Expand Down
13 changes: 0 additions & 13 deletions cloudinit/distros/alpine.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,6 @@ def install_packages(self, pkglist):
self.update_package_sources()
self.package_command('add', pkgs=pkglist)

def _write_network_config(self, netconfig):
return self._supported_write_network_config(netconfig)

def _bring_up_interfaces(self, device_names):
use_all = False
for d in device_names:
if d == 'all':
use_all = True
if use_all:
return distros.Distro._bring_up_interface(self, '-a')
else:
return distros.Distro._bring_up_interfaces(self, device_names)

def _write_hostname(self, your_hostname, out_fn):
conf = None
try:
Expand Down
10 changes: 2 additions & 8 deletions cloudinit/distros/arch.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ def install_packages(self, pkglist):
self.update_package_sources()
self.package_command('', pkgs=pkglist)

def _write_network_config(self, netconfig):
def _write_network_state(self, network_state):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also drop "_bring_up_interfaces" from arch as it as a near equivalent (yet less functional) of the parent class Distro._bring_up_interfaces

try:
return self._supported_write_network_config(netconfig)
super()._write_network_state(network_state)
except RendererNotFoundError as e:
# Fall back to old _write_network
raise NotImplementedError from e
Expand Down Expand Up @@ -101,12 +101,6 @@ def _bring_up_interface(self, device_name):
util.logexc(LOG, "Running interface command %s failed", cmd)
return False

def _bring_up_interfaces(self, device_names):
for d in device_names:
if not self._bring_up_interface(d):
return False
return True

def _write_hostname(self, your_hostname, out_fn):
conf = None
try:
Expand Down
3 changes: 0 additions & 3 deletions cloudinit/distros/bsd.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,6 @@ def package_command(self, command, args=None, pkgs=None):
# Allow the output of this to flow outwards (ie not be captured)
subp.subp(cmd, env=self._get_pkg_cmd_environ(), capture=False)

def _write_network_config(self, netconfig):
return self._supported_write_network_config(netconfig)

def set_timezone(self, tz):
distros.set_etc_timezone(tz=tz, tz_file=self._find_tz_file(tz))

Expand Down
14 changes: 2 additions & 12 deletions cloudinit/distros/debian.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,19 +111,9 @@ def install_packages(self, pkglist):
self.update_package_sources()
self.package_command('install', pkgs=pkglist)

def _write_network_config(self, netconfig):
def _write_network_state(self, network_state):
_maybe_remove_legacy_eth0()
return self._supported_write_network_config(netconfig)

def _bring_up_interfaces(self, device_names):
use_all = False
for d in device_names:
if d == 'all':
use_all = True
if use_all:
return distros.Distro._bring_up_interface(self, '--all')
else:
return distros.Distro._bring_up_interfaces(self, device_names)
return super()._write_network_state(network_state)

def _write_hostname(self, your_hostname, out_fn):
conf = None
Expand Down
9 changes: 0 additions & 9 deletions cloudinit/distros/opensuse.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,6 @@ def update_package_sources(self):
self._runner.run("update-sources", self.package_command,
['refresh'], freq=PER_INSTANCE)

def _bring_up_interfaces(self, device_names):
if device_names and 'all' in device_names:
raise RuntimeError(('Distro %s can not translate '
'the device name "all"') % (self.name))
return distros.Distro._bring_up_interfaces(self, device_names)

def _read_hostname(self, filename, default=None):
if self.uses_systemd() and filename.endswith('/previous-hostname'):
return util.load_file(filename).strip()
Expand Down Expand Up @@ -174,9 +168,6 @@ def _write_hostname(self, hostname, out_fn):
conf.set_hostname(hostname)
util.write_file(out_fn, str(conf), 0o644)

def _write_network_config(self, netconfig):
return self._supported_write_network_config(netconfig)

@property
def preferred_ntp_clients(self):
"""The preferred ntp client is dependent on the version."""
Expand Down
3 changes: 0 additions & 3 deletions cloudinit/distros/photon.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,6 @@ def install_packages(self, pkglist):
# self.update_package_sources()
self.package_command('install', pkgs=pkglist)

def _write_network_config(self, netconfig):
return self._supported_write_network_config(netconfig)

def _bring_up_interfaces(self, device_names):
cmd = ['systemctl', 'restart', 'systemd-networkd', 'systemd-resolved']
LOG.debug('Attempting to run bring up interfaces using command %s',
Expand Down
9 changes: 0 additions & 9 deletions cloudinit/distros/rhel.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,6 @@ def __init__(self, name, cfg, paths):
def install_packages(self, pkglist):
self.package_command('install', pkgs=pkglist)

def _write_network_config(self, netconfig):
return self._supported_write_network_config(netconfig)

def apply_locale(self, locale, out_fn=None):
if self.uses_systemd():
if not out_fn:
Expand Down Expand Up @@ -117,12 +114,6 @@ def _read_hostname(self, filename, default=None):
else:
return default

def _bring_up_interfaces(self, device_names):
if device_names and 'all' in device_names:
raise RuntimeError(('Distro %s can not translate '
'the device name "all"') % (self.name))
return distros.Distro._bring_up_interfaces(self, device_names)

def set_timezone(self, tz):
tz_file = self._find_tz_file(tz)
if self.uses_systemd():
Expand Down
Loading