[lxc-devel] [PATCH 1/4] Honor network type and link from lxc-create -f

[Serge Hallyn]

Quoting Dwight Engen (dwight.engen at oracle.com):
> On Thu, 11 Oct 2012 13:04:27 -0500
> Serge Hallyn <serge.hallyn at canonical.com> wrote:
> 
> > Quoting Dwight Engen (dwight.engen at oracle.com):
> > > On Thu, 11 Oct 2012 11:48:41 -0500
> > > Serge Hallyn <serge.hallyn at canonical.com> wrote:
> > > 
> > > > Quoting Dwight Engen (dwight.engen at oracle.com):
> > > > > On Thu, 11 Oct 2012 10:10:03 -0500
> > > > > Serge Hallyn <serge.hallyn at canonical.com> wrote:
> > > > > 
> > > > > > Quoting Dwight Engen (dwight.engen at oracle.com):
> > > > > > > Make the oracle template honor the lxc.network.type and
> > > > > > > lxc.network.link configuration items if a "base"
> > > > > > > configuration file is passed to lxc-create. If no
> > > > > > > configuration file is passed, the template falls back to
> > > > > > > the default name created by libvirt.
> > > > > > > 
> > > > > > > Signed-off-by: Dwight Engen <dwight.engen at oracle.com>
> > > > > > > ---
> > > > > > >  templates/lxc-oracle.in |   16 ++++++++++++----
> > > > > > >  1 files changed, 12 insertions(+), 4 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/templates/lxc-oracle.in
> > > > > > > b/templates/lxc-oracle.in index ba62f8f..2d62396 100644
> > > > > > > --- a/templates/lxc-oracle.in
> > > > > > > +++ b/templates/lxc-oracle.in
> > > > > > > @@ -27,10 +27,6 @@
> > > > > > >  # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
> > > > > > > 02111-1307 USA #
> > > > > > >  
> > > > > > > -# use virbr0 that is setup by default by libvirtd
> > > > > > > -lxc_network_type=veth
> > > > > > > -lxc_network_link=virbr0
> > > > > > > -
> > > > > > >  die()
> > > > > > >  {
> > > > > > >      echo "failed: $1"
> > > > > > > @@ -250,6 +246,18 @@ container_config_create()
> > > > > > >  		      head -1 |awk '{print $2}' | cut
> > > > > > > -c1-10 |\ sed 's/\(..\)/\1:/g; s/.$//'`"
> > > > > > >      mkdir -p $cfg_dir || die "unable to create config dir
> > > > > > > $cfg_dir" +
> > > > > > > +    # see if the network settings are specified in the file
> > > > > > > thats handed to us
> > > > > > > +    lxc_network_type=`grep '^lxc.network.type'
> > > > > > > $cfg_dir/config | awk -F'[= \t]+' '{ print $2 }'`
> > > > > > > +    if [ -z "$lxc_network_type" ]; then
> > > > > > > +	lxc_network_type="veth"
> > > > > > > +    fi
> > > > > > > +
> > > > > > > +    lxc_network_link=`grep '^lxc.network.link'
> > > > > > > $cfg_dir/config | awk -F'[= \t]+' '{ print $2 }'`
> > > > > > > +    if [ -z "$lxc_network_link" ]; then
> > > > > > > +	lxc_network_link="virbr0"
> > > > > > > +    fi
> > > > > > > +
> > > > > > 
> > > > > > Hi,
> > > > > > 
> > > > > > the creator might want to put other things in the initial
> > > > > > config, such as lxc.cgroup.devices entries.
> > > > > 
> > > > > Yes that is what was bothering me, does the user/host config
> > > > > know better which devices should be imported to the container
> > > > > or the template? I guess we're okay since you must be root on
> > > > > the host to start them, so root just has to know that those
> > > > > devices make sense for the container. So instead of removing
> > > > > the config, I guess I'll just have a little function that adds
> > > > > config keys one at a time, checking to see that it's not
> > > > > already there, so that way anything can be specified in the
> > > > > copied in config and the template won't override it. Sound
> > > > > reasonable?
> > > > 
> > > > I wouldn't do each piece, just check if lxc.network is defined at
> > > > all, and if not then use your template defaults.
> > > 
> > > Okay, I'll do that.
> > > 
> > > > > > When you do 'lxc-create -t TEMPLATE -n p1 -f CONFIG",
> > > > > > lxc-create will copy CONFIG to /var/lib/lxc/p1/config.  I
> > > > > > think it would be better for your template to not remove the
> > > > > > config copied over by lxc-create.  So don't do the above
> > > > > > steps.  If you want the default to be to use virbr0, just
> > > > > > check whether 'lxc.network.type' is not in the config yet,
> > > > > > and if it is not then set 
> > > > > > 
> > > > > > lxc_network_type=veth
> > > > > > lxc_network_link=virbr0
> > > > > > 
> > > > > > as you were before.  (I'm sure you know this, but to be
> > > > > > clear, if there is no 'lxc.network.type' at all then the
> > > > > > container will share the host's network, and if it is
> > > > > > 'lxc.network.type = empty' then it will have a private netns
> > > > > > with only loopback.  So you can pick what you want for a
> > > > > > default, but this way the distro, by setting a
> > > > > > default /etc/lxc/lxc.conf, can easily choose a default bridge
> > > > > > for lxc.network.link while the template can choose what to do
> > > > > > if nothing is specified.
> > > > > 
> > > > > I do remember seeing that, but you're right that I wasn't
> > > > > thinking of that use case (shared network by not having
> > > > > lxc.network.type) since my goal was to keep the 'default'
> > > > > containers created fairly isolated, but still update-able
> > > > > through the network.
> > > > 
> > > > And admittedly the non-isolated network case may simply not be
> > > > valid for your template.  It's not safe for an ubuntu container
> > > > on an ubuntu host, for instance.
> > > > 
> > > > > This also gets back to the fact that lxc-create in git doesn't
> > > > > copy /etc/lxc/lxc.conf if no -f is specified, so I guess that
> > > > > only works on Ubuntu now? I'd like to add the 'distro' lxc.conf
> > > > > file and
> > > > 
> > > > Yeah.  It's a tiny patch, it's just not upstream because other
> > > > distros don't set up an lxc bridge right now.
> > > 
> > > Ahh, do you mean lxc specific bridge? At least on my Oracle Linux
> > > 6.3 and on Fedora 17 libvirt supplies virbr0 by default. I don't
> > > really know enough to say if lxc should be using that or not?
> > 
> > If libvirt is installed by default, than imo that's fine.  Before
> > we shipped lxcbr0, I always recommended using virbr0 for eash of use.
> > 
> > > Regardless of how the distro sets up the bridge, wouldn't it still
> > > be useful to have lxc-create do the copy from /etc if that file
> > > exists?
> > 
> > Yes.  Let's do that.
> > 
> > > That way the distro just has to put the bridge name in there.
> > 
> > Right.
> > 
> > > > > have the rpm .spec package it, but it won't do much good
> > > > > without the part in lxc-create :( Doing so would actually
> > > > > obviate the need for the template to have a "host default" for
> > > > > networking since it would just honor /etc/lxc/lxc.conf, making
> > > > > the template more 'host distro' agnostic. I'm happy to add the
> > > > > bits for this to lxc-create that Ubuntu already has, and add an
> > > > > lxc.conf to the source tree if you want.
> > > > 
> > > > What would you use for a default network?
> > > >
> > > > 'lxc.network.type = empty' might be a reasonable choice.  The
> > > > user can always pass a nic in by hand, and it keeps the container
> > > > from screwing up the host.  Any distro which cares to can then
> > > > override the lxc.conf with one that works for it.
> > > > 
> > > > -serge
> > > 
> > > Well I do like it that on Ubuntu for example the container
> > > networking 'just works', so I'd be inclined to use veth where it
> > > can just work out of the box. We could figure out what we're
> > > building for (ala HAVE_DEBIAN in the existing configure.ac) and
> > > then pick a distro specific .conf to package. If we don't detect
> > > the distro we could default to empty like you suggest.
> > > 
> > > If you think all this more properly belongs in vendor packaging, I
> > > can just do that instead :)
> > 
> > No, no, I prefer to do as much vendor-independently in upstream git as
> > possible.  Which is why I was wondering if we could do something neat
> > with augeas...   But I'm happy with what's layed out above, especially
> > to start with.
> > 
> > In the meantime did you want me to apply the 4 patches you already
> > sent, and send patches on top of that?  Or to send a whole new set?
> 
> Let me re-work it as described above and then I can send a new set if
> thats okay.

Great, thanks.

-serge