[lxc-devel] [PATCH] Rework init scripts
Stéphane Graber
stgraber at ubuntu.com
Fri Sep 26 15:06:00 UTC 2014
On Wed, Sep 24, 2014 at 07:16:32PM -0400, Dwight Engen wrote:
> On Wed, 24 Sep 2014 16:13:48 -0400
> Stéphane Graber <stgraber at ubuntu.com> wrote:
>
> > On Wed, Sep 24, 2014 at 04:05:33PM -0400, Dwight Engen wrote:
> > > On Wed, 24 Sep 2014 13:17:28 -0400
> > > Stéphane Graber <stgraber at ubuntu.com> wrote:
> > >
> > > > I've confirmed that the testsuite on all arches passes fine with
> > > > that change and close inspection of the resulting package looked
> > > > good too.
> > > >
> > > > It'd be nice to have someone confirm that make rpm works with that
> > > > change as I don't have an easy way to try that.
> > >
> > > Hi Stéphane, make rpm is borken for the sysvinit case without (the
> > > scripts get installed in the wrong place as can be seen with "make
> > > install" since initdir isn't actually set):
> > >
> > > diff --git a/config/init/sysvinit/Makefile.am
> > > b/config/init/sysvinit/Makefile.am index d166400..e8b9f4f 100644
> > > --- a/config/init/sysvinit/Makefile.am
> > > +++ b/config/init/sysvinit/Makefile.am
> > > @@ -5,9 +5,9 @@ if INIT_SCRIPT_SYSV
> > > # directly to the rc directory under the appropriate name.
> > >
> > > if HAVE_DEBIAN
> > > - initdir = "init.d"
> > > +initdir = "init.d"
> > > else
> > > - initdir = "rc.d/init.d"
> > > +initdir = "rc.d/init.d"
> > > endif
> > >
> > > install-sysvinit: lxc-containers lxc-net
> >
> > Thanks, I'll integrate that in my patch when pushing it upstream.
> >
> > > > There's the issue of sysconfig/lxc-net which I dropped and will
> > > > need some other RPM-specific workaround if the file really needs
> > > > to exist (as shipping a file upstream for the sole purpose of
> > > > making the rpm DB happy isn't something I'm happy with).
> > >
> > > I have not yet tested that the built rpms actually work, will get to
> > > that in a bit. I don't really like that sysconfig/lxc-net is
> > > created in %post, this really should just be a file that gets
> > > installed by make install. Also, I think there needs to be a
> > > %postun that does a userdel lxc-dnsmasq. I'll send a patch for that
> > > too if you agree.
> >
> > +1 on the postun
>
> Way down below is a patch with that and a few more things I had to do
> to get this running right on my OL6.5 box.
>
> > For lxc-net, at least in Ubuntu we wanted to minimize the potential
> > for configuration files conflicts on upgrade, which block the upgrade
> > and require user intervention.
>
> Yeah, this is handled in rpm by marking it as %config. So in the .spec
> we have:
>
> %config(noreplace) %{_sysconfdir}/sysconfig/*
>
> which makes /etc/sysconfig/lxc owned by the pkg but
> not /etc/sysconfig/lxc-net since that is generated from %post. I think
> that's what we want.
>
> > The way to do that was simply to have lxc-net not be part of the
> > package at all but instead be entirely constructed from the
> > maintainer script and only have it built that way if it doesn't exist.
> >
> > Additions to lxc-net won't appear on a system where LXC is already
> > present, but that's not an issue since the default values are all set
> > in the lxc-net script, the config file only serves as an override.
> >
> > Another reason not to have the file be registered as a conffile is
> > that since the subnet is determined at installation time and so will
> > likely vary between machines, this will cause a configuration file
> > prompt with every single package update as even if the user never
> > edited the file manually, it'll differ from whatever default one we
> > may include in the package.
>
> Yeah, its nice that it picks a subnet for you, but it does worry me a
> bit. I don't think its exactly foolproof, for example what if something
> else was using the same algorithm to pick a subnet and it did so in
> between the time our pkg is installed and the time the service is
> started? I guess its the best we can do, I had a look at the .spec from
> libvirt and it has this block:
>
> # We want to install the default network for initial RPM installs
> # or on the first upgrade from a non-network aware libvirt only.
> # We check this by looking to see if the daemon is already installed
> /sbin/chkconfig --list libvirtd 1>/dev/null 2>&1
> if test $? != 0 && test ! -f %{_sysconfdir}/libvirt/qemu/networks/default.xml
> then
> UUID=`/usr/bin/uuidgen`
> sed -e "s,</name>,</name>\n <uuid>$UUID</uuid>," \
> < %{_datadir}/libvirt/networks/default.xml \
> > %{_sysconfdir}/libvirt/qemu/networks/default.xml
> ln -s ../default.xml %{_sysconfdir}/libvirt/qemu/networks/autostart/default.xml
> fi
>
> ... so it looks like they install the default bridge with a static IP.
> Anyway, this is now working for me, thanks!
>
> > >
> > > > Once this lands, I think we'll be good for alpha-2.
> > > >
> > > >
> > > > On Wed, Sep 24, 2014 at 01:05:26PM -0400, Stéphane Graber wrote:
> > > > > From: "Michael H. Warfield" <mhw at WittsEnd.com>
> > > > >
> > > > > This commit is based on the work of:
> > > > > Signed-off-by: Michael H. Warfield <mhw at WittsEnd.com>
> > > > >
> > > > > A generic changelog would be:
> > > > > - Bring support for lxcbr0 to all distributions
> > > > > - Share the container startup and network configuration logic
> > > > > across distributions and init systems.
> > > > > - Have all the init scripts call the helper script.
> > > > > - Support for the various different distro-specific
> > > > > configuration locations to configure lxc-net and container
> > > > > startup.
> > > > >
> > > > > Changes on top of Mike's original version:
> > > > > - Remove sysconfig/lxc-net as it's apparently only there as a
> > > > > workaround for an RPM limitation and is breaking Debian
> > > > > systems by including a useless file which will get registered
> > > > > as a package provided conffile in the dpkg database and will
> > > > > therefore cause conffile prompts on upgrades...
> > > > > - Go with a consistant coding style in the various init
> > > > > scripts.
> > > > > - Split out the common logic from the sysvinit scripts and ship
> > > > > both in their respective location rather than have them be
> > > > > copies.
> > > > > - Fix the upstart jobs so they actually work (there's no such
> > > > > thing as libexec on Debian systems).
> > > > >
> > > > > Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
> > > > > ---
> > > [snip so msg is small enough to not await moderator approval]
> > > _______________________________________________
> > > lxc-devel mailing list
> > > lxc-devel at lists.linuxcontainers.org
> > > http://lists.linuxcontainers.org/listinfo/lxc-devel
> >
>
>
>
>
>
> From: Dwight Engen <dwight.engen at oracle.com>
> Date: Wed, 24 Sep 2014 18:24:34 -0400
> Subject: [PATCH] minor fixups to init script rework
>
> - fix bug in action() fallback, need to shift away msg before executing action
>
> - make lxc-net 98 so it starts before lxc-container (99), otherwise the lxcbr0
> won't be available when containers are autostarted
>
> - make the default RUNTIME_PATH be /var/run instead of /run. On older
> distros (like ol6.5) /run doesn't exist. lxc-net will create this directory
> and attempt to create the dnsmasq.pid file in it, but this will fail when
> SELinux is enabled because the directory will have the default_t type.
> Newer systems have /var/run symlinked to /run so you get to the same place
> in that case.
>
> - add %postun to remove lxc-dnsmasq user when pkgs are removed
>
> - fix bug in lxc-oracle template that was creating /var/lock/subsys/lxc as
> a dir and interfering with the init scripts
>
> Signed-off-by: Dwight Engen <dwight.engen at oracle.com>
Thanks, some comments below.
> ---
> .gitignore | 7 ++++++-
> config/init/common/lxc-containers.in | 3 ++-
> config/init/common/lxc-net.in | 19 +++++++++++++++++++
> config/init/sysvinit/Makefile.am | 4 ++--
> config/init/sysvinit/lxc-net.in | 2 +-
> configure.ac | 4 ++--
> lxc.spec.in | 7 ++++++-
> templates/lxc-oracle.in | 2 +-
> 8 files changed, 39 insertions(+), 9 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index 0b6ec69..bd96fa4 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -113,10 +113,15 @@ config/missing
> config/libtool.m4
> config/lt*.m4
> config/bash/lxc
> +config/init/common/lxc-containers
> +config/init/common/lxc-net
> config/init/systemd/lxc-autostart-helper
> -config/init/systemd/lxc.service
> config/init/systemd/lxc-net.service
> +config/init/systemd/lxc.service
> config/init/sysvinit/lxc
> +config/init/sysvinit/lxc-containers
> +config/init/sysvinit/lxc-net
> +config/sysconfig/lxc
>
> doc/*.1
> doc/*.5
> diff --git a/config/init/common/lxc-containers.in b/config/init/common/lxc-containers.in
> index 9d1d604..77ee0f9 100644
> --- a/config/init/common/lxc-containers.in
> +++ b/config/init/common/lxc-containers.in
> @@ -41,7 +41,8 @@ if ! type action >/dev/null 2>&1; then
> # Real basic fallback for sysvinit "action" verbage.
> action() {
> echo -n "$1 "
> - "$@" && echo "OK" || echo "Failed"
> + shift
> + "$@" && echo "OK" || echo "Failed"
> }
> fi
>
> diff --git a/config/init/common/lxc-net.in b/config/init/common/lxc-net.in
> index c921ab7..4bd69c3 100644
> --- a/config/init/common/lxc-net.in
> +++ b/config/init/common/lxc-net.in
> @@ -1,5 +1,6 @@
> #!/bin/sh -
>
> +sysconfdir="@SYSCONFDIR@"
> distrosysconfdir="@LXC_DISTRO_SYSCONF@"
> localstatedir="@LOCALSTATEDIR@"
> varrun="@RUNTIME_PATH@/lxc"
> @@ -17,6 +18,20 @@ LXC_DHCP_MAX="253"
> LXC_DHCP_CONFILE=""
> LXC_DOMAIN=""
>
> +# Source function library.
> +test ! -r "$sysconfdir"/rc.d/init.d/functions ||
> + . "$sysconfdir"/rc.d/init.d/functions
> +
> +# provide action() fallback
> +if ! type action >/dev/null 2>&1; then
> + # Real basic fallback for sysvinit "action" verbage.
> + action() {
> + echo -n "$1 "
> + shift
> + "$@" && echo "OK" || echo "Failed"
> + }
> +fi
> +
> [ ! -f $distrosysconfdir/lxc ] || . $distrosysconfdir/lxc
>
> if [ -d "$localstatedir"/lock/subsys ]; then
> @@ -30,6 +45,8 @@ start() {
>
> [ "x$USE_LXC_BRIDGE" = "xtrue" ] || { exit 0; }
>
> + action $"Starting LXC network bridge: " /bin/true
> +
> use_iptables_lock="-w"
> iptables -w -L -n > /dev/null 2>&1 || use_iptables_lock=""
> cleanup() {
> @@ -80,6 +97,8 @@ stop() {
> # if $LXC_BRIDGE has attached interfaces, don't shut it down
> ls /sys/class/net/${LXC_BRIDGE}/brif/* > /dev/null 2>&1 && exit 0;
>
> + action $"Stopping LXC network bridge: " /bin/true
> +
> if [ -d /sys/class/net/${LXC_BRIDGE} ]; then
> use_iptables_lock="-w"
> iptables -w -L -n > /dev/null 2>&1 || use_iptables_lock=""
I'm not fond of adding init script logic into those scripts as this
means that for example upstart will be logging those messages every time
the script is called.
I'd prefer we make sure the common scripts exit with the proper exit
codes and print errors on stderr and then have the usual status messages
be written by config/init/sysvinit/* rather than config/init/common/*
> diff --git a/config/init/sysvinit/Makefile.am b/config/init/sysvinit/Makefile.am
> index d166400..e8b9f4f 100644
> --- a/config/init/sysvinit/Makefile.am
> +++ b/config/init/sysvinit/Makefile.am
> @@ -5,9 +5,9 @@ if INIT_SCRIPT_SYSV
> # directly to the rc directory under the appropriate name.
>
> if HAVE_DEBIAN
> - initdir = "init.d"
> +initdir = "init.d"
> else
> - initdir = "rc.d/init.d"
> +initdir = "rc.d/init.d"
> endif
>
> install-sysvinit: lxc-containers lxc-net
> diff --git a/config/init/sysvinit/lxc-net.in b/config/init/sysvinit/lxc-net.in
> index 6e22505..abf941d 100644
> --- a/config/init/sysvinit/lxc-net.in
> +++ b/config/init/sysvinit/lxc-net.in
> @@ -2,7 +2,7 @@
> #
> # lxc-net Start/Stop LXC Networking
> #
> -# chkconfig: 345 99 01
> +# chkconfig: 345 98 01
> # description: Starts/Stops LXC Network Bridge
> #
> ### BEGIN INIT INFO
> diff --git a/configure.ac b/configure.ac
> index 7b54587..8c5487a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -435,12 +435,12 @@ AC_ARG_ENABLE([tests],
> [], [enable_tests=no])
> AM_CONDITIONAL([ENABLE_TESTS], [test "x$enable_tests" = "xyes"])
>
> -# Allow overriding the default runtime dir (/run)
> +# Allow overriding the default runtime dir (/var/run)
> AC_ARG_WITH([runtime-path],
> [AC_HELP_STRING(
> [--with-runtime-path=dir],
> [runtime directory (default: /run)]
> - )], [], [with_runtime_path=['/run']])
> + )], [], [with_runtime_path=['/var/run']])
/run has been the standard cross-distro location for a while, I don't
see any reason for changing that.
If that's actually a problem for you and I suspect that may only be on
pretty old versions of Oracle/RedHat, you can just pass
--with-runtime-path when making the package (after doing the appropriate
distro+release check).
>
> # LXC container path, where the containers are actually stored
> # This is overridden by an entry in the file called LXCCONF
> diff --git a/lxc.spec.in b/lxc.spec.in
> index 52b6326..1de8bc4 100644
> --- a/lxc.spec.in
> +++ b/lxc.spec.in
> @@ -63,6 +63,8 @@ Group: Applications/System
> License: LGPLv2+
> BuildRoot: %{_tmppath}/%{name}-%{version}-build
> Requires: openssl rsync dnsmasq
> +Requires(pre): /usr/sbin/useradd
> +Requires(postun): /usr/sbin/userdel
> # Note for Suse. The "docbook2X" BuildRequires does properly
> # match docbook2x on Suse in a case insensitive manner
> BuildRequires: libcap libcap-devel docbook2X graphviz libxslt pkgconfig
> @@ -147,7 +149,7 @@ find %{buildroot} -type f -name '*.la' -exec rm -f {} ';'
> rm -rf %{buildroot}
>
> %pre
> -# Ensure that lxcdnsmasq uid & gid gets correctly allocated
> +# Ensure that lxc-dnsmasq uid & gid gets correctly allocated
> if getent passwd lxc-dnsmasq >/dev/null 2>&1 ; then : ; else \
> /usr/sbin/useradd -M -r -s /sbin/nologin \
> -c "LXC Networking Service" -d %_localstatedir/%name lxc-dnsmasq 2> /dev/null \
> @@ -200,6 +202,9 @@ LXC_DHCP_MAX="253"
> EOF
> fi
>
> +%postun
> +/usr/sbin/userdel lxc-dnsmasq > /dev/null 2>&1 || :
> +
> %post libs -p /sbin/ldconfig
> %postun libs -p /sbin/ldconfig
>
> diff --git a/templates/lxc-oracle.in b/templates/lxc-oracle.in
> index 830ce5c..1f65d4c 100644
> --- a/templates/lxc-oracle.in
> +++ b/templates/lxc-oracle.in
> @@ -598,7 +598,7 @@ container_rootfs_create()
> fi
> done
>
> - mkdir -p @LOCALSTATEDIR@/lock/subsys/lxc
> + mkdir -p @LOCALSTATEDIR@/lock/subsys
> (
> flock -x 9
> if [ $? -ne 0 ]; then
> --
> 1.7.1
>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140926/28bcd195/attachment.sig>
More information about the lxc-devel
mailing list