Skip to content

Commit

Permalink
[docker_image_ctl.j2] Share UTS namespace with host OS (#4169)
Browse files Browse the repository at this point in the history 
Instead of updating hostname manualy on Config DB hostname change,
simply share containers UTS namespace with host OS.
Ideally, instead of setting `--uts=host` for every container in SONiC,
this setting can be set per container if feature requires.
One behaviour change is introduced in this commit, when `--privileged`
or `--cap-add=CAP_SYS_ADMIN` and `--uts=host` are combined, container
has privilege to change host OS and every other container hostname.
Such privilege should be fixed by limiting containers capabilities.

Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>
  • Loading branch information
@stepanblyschak
stepanblyschak committed Feb 26, 2020
1 parent 71592fc commit 1ef7403
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 94 deletions.
48 changes: 4 additions & 44 deletions files/build_templates/docker_image_ctl.j2
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/bin/bash

# single instance containers are still supported (even though it might not look like it)
# if no instance number is passed to this script, $DEV will simply be unset, resulting in docker
# if no instance number is passed to this script, $DEV will simply be unset, resulting in docker
# commands being sent to the base container name. E.g. `docker start database$DEV` simply starts
# the container `database` if no instance number is passed since `$DEV` is not defined

Expand Down Expand Up @@ -31,32 +31,6 @@ function getMountPoint()
echo $1 | python -c "import sys, json, os; mnts = [x for x in json.load(sys.stdin)[0]['Mounts'] if x['Destination'] == '/usr/share/sonic/hwsku']; print '' if len(mnts) == 0 else os.path.basename(mnts[0]['Source'])" 2>/dev/null
}

function updateHostName()
{
HOSTS=/etc/hosts
HOSTS_TMP=/etc/hosts.tmp

EXEC="docker exec -i {{docker_container_name}}$DEV bash -c"

NEW_HOSTNAME="$1"
HOSTNAME=`$EXEC "hostname"`
if ! [[ $HOSTNAME =~ ^[a-zA-Z0-9.\-]*$ ]]; then
HOSTNAME=`hostname`
fi

# copy HOSTS to HOSTS_TMP
$EXEC "cp $HOSTS $HOSTS_TMP"
# remove entry with hostname
$EXEC "sed -i \"/$HOSTNAME$/d\" $HOSTS_TMP"
# add entry with new hostname
$EXEC "echo -e \"127.0.0.1\t$NEW_HOSTNAME\" >> $HOSTS_TMP"

echo "Set hostname in {{docker_container_name}}$DEV container"
$EXEC "hostname '$NEW_HOSTNAME'"
$EXEC "cat $HOSTS_TMP > $HOSTS"
$EXEC "rm -f $HOSTS_TMP"
}

function getBootType()
{
# same code snippet in files/scripts/syncd.sh
Expand Down Expand Up @@ -161,11 +135,7 @@ start() {
{%- else %}
# Obtain our HWSKU as we will mount directories with these names in each docker
HWSKU=`sonic-cfggen -d -v 'DEVICE_METADATA["localhost"]["hwsku"]'`
HOSTNAME=`sonic-cfggen -d -v 'DEVICE_METADATA["localhost"]["hostname"]'`
{%- endif %}
if [ -z "$HOSTNAME" ] || ! [[ $HOSTNAME =~ ^[a-zA-Z0-9.\-]*$ ]]; then
HOSTNAME=`hostname`
fi

DOCKERCHECK=`docker inspect --type container {{docker_container_name}}$DEV 2>/dev/null`
if [ "$?" -eq "0" ]; then
Expand All @@ -183,11 +153,6 @@ start() {
preStartAction
docker start {{docker_container_name}}$DEV
postStartAction
CURRENT_HOSTNAME="$(docker exec {{docker_container_name}}$DEV hostname)"
if [ x"$HOSTNAME" != x"$CURRENT_HOSTNAME" ]; then
updateHostName "$HOSTNAME"
fi
updateHostName "$HOSTNAME"
exit $?
fi

Expand All @@ -210,7 +175,7 @@ start() {
if [ -z "$DEV" ]; then
NET="host"
else
{%- if docker_container_name == "database" %}
{%- if docker_container_name == "database" %}
NET="bridge"
{%- else %}
NET="container:database$DEV"
Expand All @@ -222,6 +187,7 @@ start() {
{%- endif %}
docker create {{docker_image_run_opt}} \
--net=$NET \
--uts=host \{# W/A: this should be set per-docker, for those dockers which really need host's UTS namespace #}
{%- if install_debug_image == "y" %}
-v /src:/src:ro -v /debug:/debug:rw \
{%- endif %}
Expand Down Expand Up @@ -258,7 +224,6 @@ start() {
preStartAction
docker start {{docker_container_name}}
postStartAction
updateHostName "$HOSTNAME"
}

wait() {
Expand All @@ -280,13 +245,8 @@ case "$1" in
start|wait|stop)
$1
;;
updateHostName)
cmd=$1
shift 2
$cmd $@
;;
*)
echo "Usage: $0 {start namespace(optional)|wait namespace(optional)|stop namespace(optional)|updateHostName namespace(optional) new_hostname}"
echo "Usage: $0 {start namespace(optional)|wait namespace(optional)|stop namespace(optional)}"
exit 1
;;
esac
51 changes: 1 addition & 50 deletions files/image_config/hostcfgd/hostcfgd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
# -*- coding: utf-8 -*-

import os
import re
import sys
import subprocess
import syslog
Expand All @@ -24,13 +23,6 @@ TACPLUS_SERVER_TIMEOUT_DEFAULT = "5"
TACPLUS_SERVER_AUTH_TYPE_DEFAULT = "pap"


def is_valid_hostname(hostname):
if hostname[-1] == "." or len(hostname) > 253:
return False
allowed = re.compile("(?!-)[A-Z\d-]{1,63}(?<!-)$", re.IGNORECASE)
return all(allowed.match(x) for x in hostname.split("."))


def is_true(val):
if val == 'True' or val == 'true':
return True
Expand Down Expand Up @@ -238,7 +230,6 @@ class HostConfigDaemon:
tacacs_server = self.config_db.get_table('TACPLUS_SERVER')
self.aaacfg = AaaCfg()
self.aaacfg.load(aaa, tacacs_global, tacacs_server)
self.hostname_cache=""
lpbk_table = self.config_db.get_table('LOOPBACK_INTERFACE')
self.iptables = Iptables()
self.iptables.load(lpbk_table)
Expand All @@ -260,45 +251,6 @@ class HostConfigDaemon:
log_data['passkey'] = obfuscate(log_data['passkey'])
syslog.syslog(syslog.LOG_INFO, 'value of {} changed to {}'.format(key, log_data))

def hostname_handler(self, key, data):
if key != "localhost":
return

hostname = data.get("hostname")

if not hostname:
syslog.syslog(syslog.LOG_WARNING, "hostname key is missing")
return
if not is_valid_hostname(hostname):
syslog.syslog(syslog.LOG_WARNING, "hostname {} is invalid".format(hostname))
return
if hostname == self.hostname_cache:
return

syslog.syslog(syslog.LOG_INFO, "Get all running containers")
cmd = 'docker ps --format "{{.Names}}"'
try:
containers = subprocess.check_output(cmd, shell=True).split("\n")[:-1]
except subprocess.CalledProcessError as err:
syslog.syslog(syslog.LOG_ERR, "{} - failed: return code - {}, output:\n{}"
.format(err.cmd, err.returncode, err.output))

for name in containers:
script = '/usr/bin/{}.sh'.format(name)
exists = os.path.isfile(script)
if not exists:
syslog.syslog(syslog.LOG_ERR, "Can't find control script for {}".format(name))
continue

cmd = "{} updateHostName {}".format(script, hostname)
try:
subprocess.check_call(cmd, shell=True)
except subprocess.CalledProcessError as err:
syslog.syslog(syslog.LOG_ERR, "{} - failed: return code - {}, output:\n{}"
.format(err.cmd, err.returncode, err.output))

self.hostname_cache = hostname

def lpbk_handler(self, key, data):
key = ConfigDBConnector.deserialize_key(key)
#Check if delete operation by fetch existing keys
Expand All @@ -309,7 +261,7 @@ class HostConfigDaemon:
add = False

self.iptables.iptables_handler(key, data, add)

def feature_status_handler(self, key, data):
status_data = self.config_db.get_table('FEATURE')
for key in status_data.keys():
Expand Down Expand Up @@ -353,7 +305,6 @@ class HostConfigDaemon:
self.config_db.subscribe('AAA', lambda table, key, data: self.aaa_handler(key, data))
self.config_db.subscribe('TACPLUS_SERVER', lambda table, key, data: self.tacacs_server_handler(key, data))
self.config_db.subscribe('TACPLUS', lambda table, key, data: self.tacacs_global_handler(key, data))
self.config_db.subscribe('DEVICE_METADATA', lambda table, key, data: self.hostname_handler(key, data))
self.config_db.subscribe('LOOPBACK_INTERFACE', lambda table, key, data: self.lpbk_handler(key, data))
self.config_db.subscribe('FEATURE', lambda table, key, data: self.feature_status_handler(key, data))
self.config_db.listen()
Expand Down

0 comments on commit 1ef7403

Please sign in to comment.