Just to be sure, I am talking about kvm-ifup-os, which I doubt will work without root permission given that it is setting up ifconfig, ip route, etc. Without root permission it will probably just fail.
On Thu, Feb 6, 2014 at 11:10 AM, Michele Tartara <[email protected]>wrote: > On Thu, Feb 6, 2014 at 11:06 AM, Jose A. Lopes <[email protected]> > wrote: > > Should I test for the root user when the script starts and refuse to > > run the script otherwise ? > > I don't think so. The script runs with the permissions of the node > daemon, which usually is root, and if it's not root it's because the > user explicitly changed this and supposedly knows what he/she is > doing. > > Cheers, > Michele > > > > > On Thu, Feb 06, 2014 at 10:50:29AM +0100, Michele Tartara wrote: > >> On Wed, Feb 5, 2014 at 10:10 AM, Jose A. Lopes <[email protected]> > wrote: > >> > The script 'tools/kvm-ifup-os' configures TAP network interfaces for > >> > for instances, routing, DHCP server, etc. Note that this script only > >> > configures TAP network interfaces that are used by the instance > >> > communication, that is, network interfaces named according to the > >> > pattern 'gnt.com.%d', where '%d' is a number unique within a given > >> > node. > >> > > >> > Signed-off-by: Jose A. Lopes <[email protected]> > >> > --- > >> > Makefile.am | 11 ++ > >> > tools/kvm-ifup-os.in | 296 > +++++++++++++++++++++++++++++++++++++++++++++++++++ > >> > tools/kvm-ifup.in | 21 +++- > >> > tools/net-common.in | 17 ++- > >> > 4 files changed, 335 insertions(+), 10 deletions(-) > >> > create mode 100644 tools/kvm-ifup-os.in > >> > > >> > diff --git a/Makefile.am b/Makefile.am > >> > index e5d0594..aa631da 100644 > >> > --- a/Makefile.am > >> > +++ b/Makefile.am > >> > @@ -95,6 +95,7 @@ toolsdir = $(pkglibdir)/tools > >> > iallocatorsdir = $(pkglibdir)/iallocators > >> > pytoolsdir = $(pkgpythondir)/tools > >> > docdir = $(versiondir)$(datadir)/doc/$(PACKAGE) > >> > +ifupdir = $(sysconfdir)/ganeti > >> > > >> > SYMLINK_TARGET_DIRS = \ > >> > $(sysconfdir)/ganeti \ > >> > @@ -258,6 +259,7 @@ CLEANFILES = \ > >> > $(man_MANS) \ > >> > $(manhtml) \ > >> > tools/kvm-ifup \ > >> > + tools/kvm-ifup-os \ > >> > tools/vif-ganeti \ > >> > tools/net-common \ > >> > tools/users-setup \ > >> > @@ -320,6 +322,9 @@ BUILT_EXAMPLES = \ > >> > doc/examples/gnt-config-backup \ > >> > doc/examples/hooks/ipsec > >> > > >> > +dist_ifup_SCRIPTS = \ > >> > + tools/kvm-ifup-os > >> > + > >> > nodist_pkgpython_PYTHON = \ > >> > $(BUILT_PYTHON_SOURCES) > >> > > >> > @@ -1131,6 +1136,7 @@ pkglib_python_basenames = \ > >> > myexeclib_SCRIPTS = \ > >> > daemons/daemon-util \ > >> > tools/kvm-ifup \ > >> > + tools/kvm-ifup-os \ > >> > tools/vif-ganeti \ > >> > tools/net-common \ > >> > $(HS_MYEXECLIB_PROGS) > >> > @@ -1169,6 +1175,7 @@ EXTRA_DIST = \ > >> > devel/upload \ > >> > devel/webserver \ > >> > tools/kvm-ifup.in \ > >> > + tools/kvm-ifup-os.in \ > >> > tools/vif-ganeti.in \ > >> > tools/net-common.in \ > >> > tools/vcluster-setup.in \ > >> > @@ -1653,6 +1660,10 @@ tools/kvm-ifup: > >> > tools/kvm-ifup.in$(REPLACE_VARS_SED) > >> > sed -f $(REPLACE_VARS_SED) < $< > $@ > >> > chmod +x $@ > >> > > >> > +tools/kvm-ifup-os: tools/kvm-ifup-os.in $(REPLACE_VARS_SED) > >> > + sed -f $(REPLACE_VARS_SED) < $< > $@ > >> > + chmod +x $@ > >> > + > >> > tools/vif-ganeti: tools/vif-ganeti.in $(REPLACE_VARS_SED) > >> > sed -f $(REPLACE_VARS_SED) < $< > $@ > >> > chmod +x $@ > >> > diff --git a/tools/kvm-ifup-os.in b/tools/kvm-ifup-os.in > >> > new file mode 100644 > >> > index 0000000..b696da6 > >> > --- /dev/null > >> > +++ b/tools/kvm-ifup-os.in > >> > @@ -0,0 +1,296 @@ > >> > +#!/bin/bash > >> > +# > >> > + > >> > +# Copyright (C) 2014 Google Inc. > >> > +# > >> > +# This program is free software; you can redistribute it and/or > modify > >> > +# it under the terms of the GNU General Public License as published > by > >> > +# the Free Software Foundation; either version 2 of the License, or > >> > +# (at your option) any later version. > >> > +# > >> > +# 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, write to the Free Software > >> > +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > >> > +# 02110-1301, USA. > >> > + > >> > +# This script is a hook called to configure new TAP network > interfaces > >> > +# used for instance communication, and it should be called whenever a > >> > +# new instance is started. > >> > +# > >> > +# This script configures the new interface but it also performs > >> > +# maintenance on the network interfaces that have been configured > >> > +# before, by checking whether those TAP interfaces still exist, etc. > >> > +# > >> > +# This script also controls the DHCP server that leases IP address > for > >> > +# instances, i.e., the NICs inside the instances, not the TAP > >> > +# interfaces. The DHCP server is started and restarted (or reHUPed) > >> > +# as necessary and always with up-to-date configuration files. > >> > +# > >> > +# This script expects the following environment variables > >> > +# > >> > +# INTERFACE: network interface name to be configured > >> > +# MODE: networking mode for 'INTERFACE' (must be 'routed') > >> > +# MAC: MAC address for 'INTERFACE' > >> > +# IP: IP address for 'INTERFACE' > >> > + > >> > +source @PKGLIBDIR@/net-common > >> > + > >> > +readonly NETMASK=255.255.255.255 > >> > +readonly DNSMASQ_CONF=/var/run/ganeti/dnsmasq.conf > >> > +readonly DNSMASQ_HOSTS=/var/run/ganeti/dnsmasq.hosts > >> > +readonly DNSMASQ_PID=/var/run/ganeti/dnsmasq.pid > >> > + > >> > +# join intercalates a sequence of arguments using the given separator > >> > +function join { > >> > + local IFS="$1" > >> > + shift > >> > + echo "$*" > >> > +} > >> > + > >> > +# restart_dnsmasq restarts the DHCP server dnsmasq with the (possibly > >> > +# up-to-date) configuration file. > >> > +# > >> > +# If all instances have been terminated, which means there are no > more > >> > +# TAP network interfaces to monitor or IP addresses to lease, the > DHCP > >> > +# server is terminated through 'SIGTERM'. > >> > +# > >> > +# If there are still instances running, then: > >> > +# - if the DHCP server is running, a 'SIGHUP' will be sent to the > >> > +# dnsmasq process which will cause the configuration file to be > >> > +# re-read, while keeping the process running > >> > +# - if the DHCP server is not running, it will be started, and the > >> > +# configuration file will be passed it > >> > +function restart_dnsmasq { > >> > + SIGNAL= > >> > + > >> > + if [ -z "$ALIVE_INTERFACES" -o -z "$ALIVE_LEASES" ] > >> > + then > >> > + SIGNAL=TERM > >> > + else > >> > + SIGNAL=HUP > >> > + fi > >> > + > >> > + RUNNING= > >> > + > >> > + if [ -f "$DNSMASQ_PID" ] > >> > + then > >> > + PID=$(cat $DNSMASQ_PID) > >> > + if [ -n "$PID" ] && ps -p "$PID" > >> > + then > >> > + RUNNING=yes > >> > + fi > >> > + fi > >> > + > >> > + KILLED= > >> > + > >> > + if [ "$RUNNING" = yes ] > >> > + then > >> > + kill -$SIGNAL $PID > >> > + > >> > + if [ "$SIGNAL" = TERM ] > >> > + then > >> > + KILLED=yes > >> > + fi > >> > + fi > >> > + > >> > + if [ "$KILLED" = yes ] > >> > + then > >> > + rm -f $DNSMASQ_PID > >> > + fi > >> > + > >> > + if [ "$RUNNING" != yes -o "$KILLED" == yes ] > >> > + then > >> > + if [ -n "$ALIVE_INTERFACES" -a -n "$ALIVE_LEASES" ] > >> > + then > >> > + dnsmasq -C $DNSMASQ_CONF > >> > + fi > >> > + fi > >> > + > >> > + return 0 > >> > +} > >> > + > >> > +# Check that environment variable 'INTERFACE' exists. > >> > +# > >> > +# This environment variable holds the TAP network interface that > >> > +# should be configured by this script. Ganeti always passes it, > >> > +# but... :) > >> > +if [ -z "$INTERFACE" ] > >> > +then > >> > + echo kvm-vif-bridge: Failed to configure communication mechanism \ > >> > + interface because the \'INTERFACE\' environment variable was \ > >> > + not specified to the \'kvm-vif-bridge\' script > >> > + exit 1 > >> > +fi > >> > + > >> > +# Check that environment variable 'MODE' exists. > >> > +# > >> > +# See comment about environment variable 'INTERFACE'. > >> > +if [ -z "$MODE" ] > >> > +then > >> > + echo kvm-vif-bridge: Failed to configure communication mechanism \ > >> > + interface because the \'MODE\' environment variable was \ > >> > + not specified to the \'kvm-vif-bridge\' script > >> > + exit 1 > >> > +fi > >> > + > >> > +# Check whether the interface being configured has instance > >> > +# communication enabled, otherwise exit this script. > >> > +if ! is_instance_communication_tap; then exit 0; fi > >> > + > >> > +# Check that environment variable 'MAC' exists. > >> > +# > >> > +# See comment about environment variable 'INTERFACE'. > >> > +if [ -z "$MAC" ] > >> > +then > >> > + echo kvm-vif-bridge: Failed to configure communication mechanism \ > >> > + interface because the \'MAC\' environment variable was \ > >> > + not specified to the \'kvm-vif-bridge\' script > >> > + exit 1 > >> > +fi > >> > + > >> > +# Check that environment variable 'IP' exists. > >> > +# > >> > +# See comment about environment variable 'INTERFACE'. > >> > +if [ -z "$IP" ] > >> > +then > >> > + echo kvm-vif-bridge: Failed to configure communication mechanism \ > >> > + interface because the \'IP\' environment variable was \ > >> > + not specified to the \'kvm-vif-bridge\' script > >> > + exit 1 > >> > +fi > >> > + > >> > +# Configure the TAP interface > >> > +# > >> > +# Ganeti defers the configuration of instance network interfaces to > >> > +# hooks, therefore, we must configure the interface's network > address, > >> > +# netmask, and IP address. > >> > +# > >> > +# The TAP network interface, which is used by the instance > >> > +# communication, is part of the network 169.254.0.0/16 and has the > IP > >> > +# 169.254.169.254. Because all network interfaces used in the > >> > +# instance communication have the same IP, the routing table must > also > >> > +# be configured, and that is done at a later step. > >> > +# > >> > +# Note the interface must be marked as up before configuring the > >> > +# routing table and before starting/restarting the DHCP server. > >> > +# > >> > +# Note also that we don't have to check whether the interface is > >> > +# already configured because reconfiguring the interface with the > same > >> > +# parameters does not produce an error. > >> > +ifconfig $INTERFACE 169.254.169.254 netmask $NETMASK up > >> > + > >> > +# Configure the routing table > >> > +# > >> > +# Given that all TAP network interfaces in the instance communication > >> > +# have the same IP address, the routing table must be configured in > >> > +# order to properly route traffic from the host to the guests. > >> > +# > >> > +# Note that we must first check if a duplicate routing rule has > >> > +# already been added to the routing table, as this operation will > fail > >> > +# if we try to add a routing rule that already exists. > >> > +ACTIVE_IP=$(ip route | grep "dev $INTERFACE" | awk '{ print $1 }') > >> > + > >> > +if [ -z "$ACTIVE_IP" -o "$ACTIVE_IP" != "$IP" ] > >> > +then > >> > + route add -host $IP dev $INTERFACE > >> > +fi > >> > + > >> > +# Ensure the DHCP server configuration files exist > >> > +touch $DNSMASQ_CONF > >> > +chmod 0644 $DNSMASQ_CONF > >> > + > >> > +touch $DNSMASQ_HOSTS > >> > +chmod 0644 $DNSMASQ_HOSTS > >> > + > >> > +# Determine dnsmasq operational mode. > >> > +# > >> > +# The DHCP server dnsmasq can run in different modes. In this > version > >> > +# of the script, only the mode 'bind-dynamic' is supported. Please > >> > +# refer to the dnsmasq FAQ for a detailed of each mode. > >> > +# > >> > +# Note that dnsmasq might already be running, therefore, we don't > need > >> > +# to determine which modes are supported by this DHCP server. > >> > +# Instead, we just read the current mode from the configuration file. > >> > +DNSMASQ_MODE=$(head -n 1 $DNSMASQ_CONF) > >> > + > >> > +if [ -z "$DNSMASQ_MODE" ] > >> > +then > >> > + BIND_DYNAMIC=$(dnsmasq --help | grep -e --bind-dynamic) > >> > + > >> > + if [ -z "$BIND_DYNAMIC" ] > >> > + then > >> > + echo kvm-vif-bridge: dnsmasq mode \"bind-dynamic\" is not > supported > >> > + exit 1 > >> > + fi > >> > + > >> > + DNSMASQ_MODE=bind-dynamic > >> > +fi > >> > + > >> > +# Determine the interfaces that should go in the configuration file. > >> > +# > >> > +# The TAP network interfaces used by the instance communication are > >> > +# named after the following pattern > >> > +# > >> > +# gnt.com.%d > >> > +# > >> > +# where '%d' is a unique number within the host. Fortunately, > dnsmasq > >> > +# supports binding to specific network interfaces via a pattern. > >> > +ALIVE_INTERFACES=${GANETI_TAP}.* > >> > + > >> > +# Determine which of the leases are not duplicated and should go in > >> > +# the new configuration file for the DHCP server. > >> > +# > >> > +# Given that instances come and go, it is possible that we offer more > >> > +# leases that necessary and, worse, that we have duplicate leases, > >> > +# that is, the same IP address for the same/different MAC addresses. > >> > +# Duplicate leases must be eliminated before being written to the > >> > +# configuration file. > >> > +CONF_LEASES=$(cat $DNSMASQ_HOSTS) > >> > +CONF_LEASES=$(join $'\n' $CONF_LEASES | sort -u) > >> > + > >> > +ALIVE_LEASES=( $MAC,$IP ) > >> > + > >> > +for i in $CONF_LEASES > >> > +do > >> > + LEASE_MAC=$(echo $i | cut -d "," -f 1) > >> > + LEASE_IP=$(echo $i | cut -d "," -f 2) > >> > + if [ "$LEASE_MAC" != "$MAC" -a "$LEASE_IP" != "$IP" ] > >> > + then > >> > + ALIVE_LEASES=( ${ALIVE_LEASES[@]} $i ) > >> > + fi > >> > +done > >> > + > >> > +ALIVE_LEASES=$(echo ${ALIVE_LEASES[@]} | sort -u) > >> > + > >> > +# Update dnsmasq configuration. > >> > +# > >> > +# Write the parameters we have collected before into the new dnsmasq > >> > +# configuration file. Also, write the new leases into the new > dnsmasq > >> > +# hosts file. Finally, restart dnsmasq with the new configuration > >> > +# files. > >> > +cat > $DNSMASQ_CONF <<EOF > >> > +$DNSMASQ_MODE > >> > +dhcp-authoritative > >> > +dhcp-hostsfile=$DNSMASQ_HOSTS > >> > +dhcp-range=169.254.0.0,static,255.255.0.0 > >> > +except-interface=eth* > >> > +except-interface=lo > >> > +leasefile-ro > >> > +no-hosts > >> > +no-ping > >> > +no-resolv > >> > +pid-file=$DNSMASQ_PID > >> > +port=0 > >> > +strict-order > >> > +EOF > >> > +for i in $ALIVE_INTERFACES; do echo interface=$i >> $DNSMASQ_CONF; > done > >> > + > >> > +echo -n > $DNSMASQ_HOSTS > >> > +for i in $ALIVE_LEASES; do echo $i >> $DNSMASQ_HOSTS; done > >> > + > >> > +restart_dnsmasq > >> > diff --git a/tools/kvm-ifup.in b/tools/kvm-ifup.in > >> > index 5a53cee..f7ce37d 100644 > >> > --- a/tools/kvm-ifup.in > >> > +++ b/tools/kvm-ifup.in > >> > @@ -1,7 +1,7 @@ > >> > #!/bin/bash > >> > # > >> > > >> > -# Copyright (C) 2011, 2012 Google Inc. > >> > +# Copyright (C) 2011, 2012, 2014 Google Inc. > >> > # > >> > # This program is free software; you can redistribute it and/or > modify > >> > # it under the terms of the GNU General Public License as published > by > >> > @@ -20,12 +20,23 @@ > >> > > >> > source @PKGLIBDIR@/net-common > >> > > >> > +check > >> > + > >> > +# Execute the user-supplied network script, if applicable > >> I'd change this into something like "Execute the script for setting up > >> the communication with the instance os, if available" > >> No reference to "user-supplied" as it is user-modifiable but supplied > by us. > >> > >> > +# > >> > +# This additional hook is placed here for backwards-compatibility > with > >> > +# the other hook below. > >> > >> Without the whole discussion we had offline, this comment about > >> backwards-compatibility does not make much sense, so I'd just remove > >> it. > >> > >> > +if is_instance_communication_tap && [ -x "$CONF_DIR/kvm-ifup-os" ]; > then > >> > + . $CONF_DIR/kvm-ifup-os > >> > +fi > >> > + > >> > # Execute the user-supplied network script, if applicable > >> > if [ -x "$CONF_DIR/kvm-vif-bridge" ]; then > >> > exec $CONF_DIR/kvm-vif-bridge > >> > fi > >> > > >> > -check > >> > -setup_bridge > >> > -setup_ovs > >> > -setup_route > >> > +if ! is_instance_communication_tap; then > >> > + setup_bridge > >> > + setup_ovs > >> > + setup_route > >> > +fi > >> > diff --git a/tools/net-common.in b/tools/net-common.in > >> > index 7305875..c0d5bdf 100644 > >> > --- a/tools/net-common.in > >> > +++ b/tools/net-common.in > >> > @@ -20,8 +20,9 @@ > >> > > >> > @SHELL_ENV_INIT@ > >> > > >> > -function check { > >> > +readonly GANETI_TAP="gnt.com" > >> > > >> > +function check { > >> > if [ -z "$INTERFACE" ]; then > >> > echo "No network interface specified" > >> > exit 1 > >> > @@ -31,22 +32,29 @@ function check { > >> > echo "MODE not specified" > >> > exit 1 > >> > fi > >> > +} > >> > > >> > +function is_instance_communication_tap { > >> > + COMMUNICATION=$(echo "$INTERFACE" | cut -d "." -f 1-2) > >> > + > >> > + if [ "$MODE" = "routed" -a "$COMMUNICATION" = "$GANETI_TAP" ] > >> > + then > >> > + return 0 > >> > + else > >> > + return 1 > >> > + fi > >> > } > >> > > >> > function fix_mac { > >> > - > >> > # Fix the autogenerated MAC to have the first octet set to "fe" > >> > # to discourage the bridge from using the TAP dev's MAC > >> > FIXED_MAC=$(ip link show $INTERFACE | \ > >> > awk '{if ($1 == "link/ether") printf("fe%s",substr($2,3,15))}') > >> > # in case of a vif (xen_netback device) this action is not allowed > >> > ip link set $INTERFACE address $FIXED_MAC || true > >> > - > >> > } > >> > > >> > function setup_bridge { > >> > - > >> > if [ "$MODE" = "bridged" ]; then > >> > fix_mac > >> > ip link set $INTERFACE up > >> > @@ -55,7 +63,6 @@ function setup_bridge { > >> > # Connect the interface to the bridge > >> > brctl addif $LINK $INTERFACE > >> > fi > >> > - > >> > } > >> > > >> > function setup_ovs { > >> > -- > >> > 1.9.0.rc1.175.g0b1dcb5 > >> > > >> > >> Rest LGTM, no need to resend. > >> > >> Thanks, > >> Michele > >> > >> -- > >> Google Germany GmbH > >> Dienerstr. 12 > >> 80331 München > >> > >> Registergericht und -nummer: Hamburg, HRB 86891 > >> Sitz der Gesellschaft: Hamburg > >> Geschäftsführer: Graham Law, Christine Elizabeth Flores > > > > -- > > Jose Antonio Lopes > > Ganeti Engineering > > Google Germany GmbH > > Dienerstr. 12, 80331, München > > > > Registergericht und -nummer: Hamburg, HRB 86891 > > Sitz der Gesellschaft: Hamburg > > Geschäftsführer: Graham Law, Christine Elizabeth Flores > > Steuernummer: 48/725/00206 > > Umsatzsteueridentifikationsnummer: DE813741370 > > > > -- > Google Germany GmbH > Dienerstr. 12 > 80331 München > > Registergericht und -nummer: Hamburg, HRB 86891 > Sitz der Gesellschaft: Hamburg > Geschäftsführer: Graham Law, Christine Elizabeth Flores >
