cloud-init-dev team mailing list archive
-
cloud-init-dev team
-
Mailing list archive
-
Message #03136
Re: [Merge] ~rjschwei/cloud-init:suseIntegrate into cloud-init:master
Some comments inline.
Diff comments:
> diff --git a/cloudinit/distros/opensuse.py b/cloudinit/distros/opensuse.py
> new file mode 100644
> index 0000000..0386e10
> --- /dev/null
> +++ b/cloudinit/distros/opensuse.py
> @@ -0,0 +1,233 @@
> +# vi: ts=4 expandtab
for header information.
if you want to put copyright, you can.
see cloudinit/distros/parsers/hosts.py (example with author and copyrihgt)
or cloudinit/net/eni.py (just the single line ehader).
# This file is part of cloud-init. See LICENSE file for license information.
then, put the 'vi' line as last line in file.
> +#
> +# Copyright (C) 2017 SUSE LLC
> +# Copyright (C) 2013 Hewlett-Packard Development Company, L.P.
> +#
> +# Author: Robert Schweikert <rjschwei@xxxxxxxx>
> +# Author: Juerg Haefliger <juerg.haefliger@xxxxxx>
> +#
> +# Leaning very heavily on the RHEL and Debian implementation
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License version 3, as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +from cloudinit import distros
> +
> +from cloudinit.distros.parsers.hostname import HostnameConf
> +
> +from cloudinit import helpers
> +from cloudinit import log as logging
> +from cloudinit import util
> +
> +from cloudinit.distros import net_util
> +from cloudinit.distros import rhel_util as rhutil
> +#from cloudinit.net import sysconfig
remove these comment lines.
> +#from cloudinit.net.network_state import parse_net_config_data
> +from cloudinit.settings import PER_INSTANCE
> +
> +LOG = logging.getLogger(__name__)
> +
> +class Distro(distros.Distro):
> + clock_conf_fn = '/etc/sysconfig/clock'
> + hostname_conf_fn = '/etc/HOSTNAME'
> + init_cmd = ['service']
> + locale_conf_fn = '/etc/sysconfig/language'
> + network_conf_fn = '/etc/sysconfig/network'
> + network_script_tpl = '/etc/sysconfig/network/ifcfg-%s'
> + resolve_conf_fn = '/etc/resolv.conf'
> + route_conf_tpl = '/etc/sysconfig/network/ifroute-%s'
> + systemd_hostname_conf_fn = '/etc/hostname'
> + systemd_locale_conf_fn = '/etc/locale.conf'
> + tz_local_fn = '/etc/localtime'
> +
> + def __init__(self, name, cfg, paths):
> + distros.Distro.__init__(self, name, cfg, paths)
> +# self._net_renderer = sysconfig.Renderer()
> + # This will be used to restrict certain
these comments must have been copied. i'm not sure what they mean here.
> + # calls from repeatly happening (when they
> + # should only happen say once per instance...)
> + self._runner = helpers.Runners(paths)
> + self.osfamily = 'suse'
> + cfg['ssh_svcname'] = 'sshd'
> + self.systemdDist = util.which('systemctl')
> + if self.systemdDist:
can we use the 'uses_systemd()' facility for consistency?
> + self.init_cmd = ['systemctl']
> + cfg['ssh_svcname'] = 'sshd.service'
> +
> + def apply_locale(self, locale, out_fn=None):
> + if self.systemdDist:
> + if not out_fn:
> + out_fn = self.systemd_locale_conf_fn
> + locale_cfg = {'LANG': locale}
> + else:
> + if not out_fn:
> + out_fn = self.locale_conf_fn
> + locale_cfg = {'RC_LANG': locale}
> + rhutil.update_sysconfig_file(out_fn, locale_cfg)
> +
> + def install_packages(self, pkglist):
> + self.package_command('install', args='-l', pkgs=pkglist)
can you change -l to
--auto-agree-with-licenses
unless some zypper version didn't have the long format, this is more self documenting.
> +
> + def package_command(self, command, args=None, pkgs=None):
> + if pkgs is None:
> + pkgs = []
> +
> + cmd = ['zypper']
> + # No user interaction possible, enable non-interactive mode
> + cmd.append('--non-interactive')
> +
> + # Comand is the operation, such as install
> + if command == 'upgrade':
> + command = 'update'
> + cmd.append(command)
> +
> + # args are the arguments to the command, not global options
> + if args and isinstance(args, str):
> + cmd.append(args)
> + elif args and isinstance(args, list):
> + cmd.extend(args)
> +
> + pkglist = util.expand_package_list('%s-%s', pkgs)
> + cmd.extend(pkglist)
> +
> + # Allow the output of this to flow outwards (ie not be captured)
> + util.subp(cmd, capture=False)
> +
> + def set_timezone(self, tz):
> + tz_file = self._find_tz_file(tz)
> + if self.systemdDist:
> + # Currently, timedatectl complains if invoked during startup
> + # so for compatibility, create the link manually.
> + util.del_file(self.tz_local_fn)
> + util.sym_link(tz_file, self.tz_local_fn)
> + else:
> + # Adjust the sysconfig clock zone setting
> + clock_cfg = {
> + 'TIMEZONE': str(tz),
> + }
> + rhutil.update_sysconfig_file(self.clock_conf_fn, clock_cfg)
> + # This ensures that the correct tz will be used for the system
> + util.copy(tz_file, self.tz_local_fn)
> +
> + 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.systemdDist and filename.endswith('/previous-hostname'):
> + return util.load_file(filename).strip()
> + elif self.systemdDist:
> + (out, _err) = util.subp(['hostname'])
> + if len(out):
> + return out
> + else:
> + return default
> + else:
> + try:
> + conf = self._read_hostname_conf(filename)
> + hostname = conf.hostname
> + except IOError:
> + pass
> + if not hostname:
> + return default
> + return hostname
> +
> + def _read_hostname_conf(self, filename):
> + conf = HostnameConf(util.load_file(filename))
> + conf.parse()
> + return conf
> +
> + def _read_system_hostname(self):
> + if self.systemdDist:
> + host_fn = self.systemd_hostname_conf_fn
> + else:
> + host_fn = self.hostname_conf_fn
> + return (host_fn, self._read_hostname(host_fn))
> +
> + def _write_hostname(self, hostname, out_fn):
> + if self.systemdDist and out_fn.endswith('/previous-hostname'):
> + util.write_file(out_fn, hostname)
> + elif self.systemdDist:
> + util.subp(['hostnamectl', 'set-hostname', str(hostname)])
> + else:
> + conf = None
> + try:
> + # Try to update the previous one
> + # so lets see if we can read it first.
> + conf = self._read_hostname_conf(out_fn)
> + except IOError:
> + pass
> + if not conf:
> + conf = HostnameConf('')
> + conf.set_hostname(hostname)
> + util.write_file(out_fn, str(conf), 0o644)
> +
> + def _write_network(self, settings):
> + # Convert debian settings to ifcfg format
> + entries = net_util.translate_network(settings)
> + LOG.debug("Translated ubuntu style network settings %s into %s",
> + settings, entries)
> + # Make the intermediate format as the suse format...
> + nameservers = []
> + searchservers = []
> + dev_names = entries.keys()
> + for (dev, info) in entries.items():
> + net_fn = self.network_script_tpl % (dev)
> + route_fn = self.route_conf_tpl % (dev)
> + mode = None
> + if info.get('auto', None):
> + mode = 'auto'
> + else:
> + mode = 'manual'
> + bootproto = info.get('bootproto', None)
> + gateway = info.get('gateway', None)
> + net_cfg = {
> + 'BOOTPROTO': bootproto,
> + 'BROADCAST': info.get('broadcast'),
> + 'GATEWAY': gateway,
> + 'IPADDR': info.get('address'),
> + 'LLADDR': info.get('hwaddress'),
> + 'NETMASK': info.get('netmask'),
> + 'STARTMODE': mode,
> + 'USERCONTROL': 'no'
> + }
> + if dev != 'lo':
> + net_cfg['ETHTOOL_OPTIONS'] = ''
> + else:
> + net_cfg['FIREWALL'] = 'no'
> + rhutil.update_sysconfig_file(net_fn, net_cfg, True)
> + if gateway and bootproto == 'static':
> + default_route = 'default %s' %gateway
> + util.write_file(route_fn, default_route, 0o644)
> + if 'dns-nameservers' in info:
> + nameservers.extend(info['dns-nameservers'])
> + if 'dns-search' in info:
> + searchservers.extend(info['dns-search'])
> + if nameservers or searchservers:
> + rhutil.update_resolve_conf_file(self.resolve_conf_fn,
> + nameservers, searchservers)
> + return dev_names
> +
> +# New interface cannot yet be implemented/used as we have to figure out
for now just drop this comments.
> +# how to have a distro specific renderer
> +# def _write_network_config(self, netconfig):
> +# ns = parse_net_config_data(netconfig)
> +# self._net_renderer.render_network_state("/", ns)
> +# return []
> diff --git a/cloudinit/distros/sles.py b/cloudinit/distros/sles.py
> index dbec2ed..f58db99 100644
> --- a/cloudinit/distros/sles.py
> +++ b/cloudinit/distros/sles.py
> @@ -1,167 +1,28 @@
> -# Copyright (C) 2013 Hewlett-Packard Development Company, L.P.
> +# vi: ts=4 expandtab
> #
> -# Author: Juerg Haefliger <juerg.haefliger@xxxxxx>
> +# Copyright (C) 2017 SUSE LLC
> #
> -# This file is part of cloud-init. See LICENSE file for license information.
> -
> -from cloudinit import distros
> +# Leaning very heavily on the RHEL and Debian implementation
headers and such cleanup.
> +# Author: Robert Schweikert <rjschwei@xxxxxxxx>
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License version 3, as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
>
> -from cloudinit.distros.parsers.hostname import HostnameConf
> +from cloudinit.distros import opensuse
>
> -from cloudinit import helpers
> from cloudinit import log as logging
> -from cloudinit import util
> -
> -from cloudinit.distros import net_util
> -from cloudinit.distros import rhel_util
> -from cloudinit.settings import PER_INSTANCE
>
> LOG = logging.getLogger(__name__)
>
> +class Distro(opensuse.Distro):
> + pass
>
> -class Distro(distros.Distro):
> - clock_conf_fn = '/etc/sysconfig/clock'
> - locale_conf_fn = '/etc/sysconfig/language'
> - network_conf_fn = '/etc/sysconfig/network'
> - hostname_conf_fn = '/etc/HOSTNAME'
> - network_script_tpl = '/etc/sysconfig/network/ifcfg-%s'
> - resolve_conf_fn = '/etc/resolv.conf'
> - tz_local_fn = '/etc/localtime'
> -
> - def __init__(self, name, cfg, paths):
> - distros.Distro.__init__(self, name, cfg, paths)
> - # This will be used to restrict certain
> - # calls from repeatly happening (when they
> - # should only happen say once per instance...)
> - self._runner = helpers.Runners(paths)
> - self.osfamily = 'suse'
> -
> - def install_packages(self, pkglist):
> - self.package_command('install', args='-l', pkgs=pkglist)
> -
> - def _write_network(self, settings):
> - # Convert debian settings to ifcfg format
> - entries = net_util.translate_network(settings)
> - LOG.debug("Translated ubuntu style network settings %s into %s",
> - settings, entries)
> - # Make the intermediate format as the suse format...
> - nameservers = []
> - searchservers = []
> - dev_names = entries.keys()
> - for (dev, info) in entries.items():
> - net_fn = self.network_script_tpl % (dev)
> - mode = info.get('auto')
> - if mode and mode.lower() == 'true':
> - mode = 'auto'
> - else:
> - mode = 'manual'
> - net_cfg = {
> - 'BOOTPROTO': info.get('bootproto'),
> - 'BROADCAST': info.get('broadcast'),
> - 'GATEWAY': info.get('gateway'),
> - 'IPADDR': info.get('address'),
> - 'LLADDR': info.get('hwaddress'),
> - 'NETMASK': info.get('netmask'),
> - 'STARTMODE': mode,
> - 'USERCONTROL': 'no'
> - }
> - if dev != 'lo':
> - net_cfg['ETHERDEVICE'] = dev
> - net_cfg['ETHTOOL_OPTIONS'] = ''
> - else:
> - net_cfg['FIREWALL'] = 'no'
> - rhel_util.update_sysconfig_file(net_fn, net_cfg, True)
> - if 'dns-nameservers' in info:
> - nameservers.extend(info['dns-nameservers'])
> - if 'dns-search' in info:
> - searchservers.extend(info['dns-search'])
> - if nameservers or searchservers:
> - rhel_util.update_resolve_conf_file(self.resolve_conf_fn,
> - nameservers, searchservers)
> - return dev_names
> -
> - def apply_locale(self, locale, out_fn=None):
> - if not out_fn:
> - out_fn = self.locale_conf_fn
> - locale_cfg = {
> - 'RC_LANG': locale,
> - }
> - rhel_util.update_sysconfig_file(out_fn, locale_cfg)
> -
> - def _write_hostname(self, hostname, out_fn):
> - conf = None
> - try:
> - # Try to update the previous one
> - # so lets see if we can read it first.
> - conf = self._read_hostname_conf(out_fn)
> - except IOError:
> - pass
> - if not conf:
> - conf = HostnameConf('')
> - conf.set_hostname(hostname)
> - util.write_file(out_fn, str(conf), 0o644)
> -
> - def _read_system_hostname(self):
> - host_fn = self.hostname_conf_fn
> - return (host_fn, self._read_hostname(host_fn))
> -
> - def _read_hostname_conf(self, filename):
> - conf = HostnameConf(util.load_file(filename))
> - conf.parse()
> - return conf
> -
> - def _read_hostname(self, filename, default=None):
> - hostname = None
> - try:
> - conf = self._read_hostname_conf(filename)
> - hostname = conf.hostname
> - except IOError:
> - pass
> - if not hostname:
> - return default
> - return hostname
> -
> - 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)
> - # Adjust the sysconfig clock zone setting
> - clock_cfg = {
> - 'TIMEZONE': str(tz),
> - }
> - rhel_util.update_sysconfig_file(self.clock_conf_fn, clock_cfg)
> - # This ensures that the correct tz will be used for the system
> - util.copy(tz_file, self.tz_local_fn)
> -
> - def package_command(self, command, args=None, pkgs=None):
> - if pkgs is None:
> - pkgs = []
> -
> - cmd = ['zypper']
> - # No user interaction possible, enable non-interactive mode
> - cmd.append('--non-interactive')
> -
> - # Comand is the operation, such as install
> - cmd.append(command)
> -
> - # args are the arguments to the command, not global options
> - if args and isinstance(args, str):
> - cmd.append(args)
> - elif args and isinstance(args, list):
> - cmd.extend(args)
> -
> - pkglist = util.expand_package_list('%s-%s', pkgs)
> - cmd.extend(pkglist)
> -
> - # Allow the output of this to flow outwards (ie not be captured)
> - util.subp(cmd, capture=False)
> -
> - def update_package_sources(self):
> - self._runner.run("update-sources", self.package_command,
> - ['refresh'], freq=PER_INSTANCE)
> -
> -# vi: ts=4 expandtab
> diff --git a/tests/unittests/test_handler/test_handler_set_hostname.py b/tests/unittests/test_handler/test_handler_set_hostname.py
> index 4b18de7..8678859 100644
> --- a/tests/unittests/test_handler/test_handler_set_hostname.py
> +++ b/tests/unittests/test_handler/test_handler_set_hostname.py
> @@ -70,7 +70,8 @@ class TestHostname(t_help.FilesystemMockingTestCase):
> cc = cloud.Cloud(ds, paths, {}, distro, None)
> self.patchUtils(self.tmp)
> cc_set_hostname.handle('cc_set_hostname', cfg, cc, LOG, [])
> - contents = util.load_file("/etc/HOSTNAME")
> - self.assertEqual('blah', contents.strip())
these tests are bad.
the set_hostname tests really should just ensure that the distro's set_hostname gets called, and we should have tests for those classes that they do the right thing rather than testing the distro classes' behavior here. I think i'm' ok with this change here, but its really a pain.
the check for 'distro.systemdDist' ends up actually checking the test environment, as it doesn't get mocked at all.
> + if not distro.systemdDist:
> + contents = util.load_file(distro.hostname_conf_fn)
> + self.assertEqual('blah', contents.strip())
>
> # vi: ts=4 expandtab
> diff --git a/tox.ini b/tox.ini
> index 1e7ca2d..a17156c 100644
> --- a/tox.ini
> +++ b/tox.ini
> @@ -92,6 +92,22 @@ deps =
> six==1.9.0
> -r{toxinidir}/test-requirements.txt
>
> +[testenv:opensusel42]
> +basepython = python2.7
> +commands = nosetests {posargs:tests/unittests}
> +deps =
> + # requirements
> + argparse==1.3.0
> + jinja2==2.8
> + PyYAML==3.11
> + PrettyTable==0.7.2
> + oauthlib==0.7.2
> + configobj==5.0.6
> + requests==2.11.1
> + jsonpatch==1.11
> + six==1.9.0
> + -r{toxinidir}/test-requirements.txt
> +
this is good thank you.
> [testenv:tip-pycodestyle]
> commands = {envpython} -m pycodestyle {posargs:cloudinit/ tests/ tools/}
> deps = pycodestyle
--
https://code.launchpad.net/~rjschwei/cloud-init/+git/cloud-init/+merge/329616
Your team cloud-init commiters is requested to review the proposed merge of ~rjschwei/cloud-init:suseIntegrate into cloud-init:master.
References
