Skip to content

Commit

Permalink
[docker_image_ctl.j2] Share UTS namespace with host OS (sonic-net#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>

Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>

Conflicts:
	files/build_templates/docker_image_ctl.j2
  • Loading branch information
Stepan Blyschak committed Feb 26, 2020
1 parent c72d3af commit 121de33
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 92 deletions.
44 changes: 2 additions & 42 deletions files/build_templates/docker_image_ctl.j2
Original file line number Diff line number Diff line change
Expand Up @@ -43,32 +43,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 @@ -211,11 +185,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 @@ -233,11 +203,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 Down Expand Up @@ -272,6 +237,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 @@ -310,7 +276,6 @@ start() {
preStartAction
docker start {{docker_container_name}}
postStartAction
updateHostName "$HOSTNAME"
}

wait() {
Expand Down Expand Up @@ -351,13 +316,8 @@ case "$1" in
start|wait|stop|stopPost)
$1
;;
updateHostName)
cmd=$1
shift 2
$cmd $@
;;
*)
echo "Usage: $0 {start namespace(optional)|wait namespace(optional)|stop namespace(optional)|stopPost namespace(optional)|updateHostName namespace(optional) new_hostname}"
echo "Usage: $0 {start namespace(optional)|wait namespace(optional)|stop namespace(optional)|stopPost namespace(optional)}"
exit 1
;;
esac
51 changes: 1 addition & 50 deletions files/image_config/hostcfgd/hostcfgd
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 @@ -263,45 +254,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 @@ -312,7 +264,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 @@ -356,7 +308,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 121de33

Please sign in to comment.