Skip to content

Commit

Permalink
[sonic-installer] use host docker startup arguments when running dock…
Browse files Browse the repository at this point in the history
…erd in chroot (#2179) (#2407)

Backport of #2179

- Why I did it
I attempted to fix an issue that happens on multi-asic devices after new SONiC image installation. The issue is caused by overriding docker0 bridge configuration as well as installing iptables rules by dockerd started in chroot environment.

Fixes sonic-net/sonic-buildimage#10135

- How I did it
I start dockerd in chroot using same parameters the host dockerd is started with.

- How to verify it
Run VS multi-asic device.
Install a new image on it.
Verify "show version", "show interface status"

Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
  • Loading branch information
stepanblyschak authored Oct 3, 2022
1 parent d112f7c commit be7da6b
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 0 deletions.
21 changes: 21 additions & 0 deletions sonic_installer/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,15 @@ def update_sonic_environment(bootloader, binary_image_version):
umount(new_image_mount)


def get_docker_opts():
""" Get options dockerd is started with """
with open("/var/run/docker.pid") as pid_file:
pid = int(pid_file.read())

with open("/proc/{}/cmdline".format(pid)) as cmdline_file:
return cmdline_file.read().strip().split("\x00")[1:]


def migrate_sonic_packages(bootloader, binary_image_version):
""" Migrate SONiC packages to new SONiC image. """

Expand All @@ -325,6 +334,8 @@ def migrate_sonic_packages(bootloader, binary_image_version):
new_image_docker_dir = os.path.join(new_image_dir, DOCKERDIR_NAME)
new_image_mount = os.path.join("/", tmp_dir, "image-{0}-fs".format(sonic_version))
new_image_docker_mount = os.path.join(new_image_mount, "var", "lib", "docker")
docker_default_config = os.path.join(new_image_mount, "etc", "default", "docker")
docker_default_config_backup = os.path.join(new_image_mount, tmp_dir, "docker_config_backup")

if not os.path.isdir(new_image_docker_dir):
# NOTE: This codepath can be reached if the installation process did not
Expand All @@ -349,6 +360,14 @@ def migrate_sonic_packages(bootloader, binary_image_version):
if not os.path.exists(os.path.join(new_image_mount, os.path.relpath(DOCKER_CTL_SCRIPT, os.path.abspath(os.sep)))):
echo_and_log("Warning: SONiC Application Extension is not supported in this image", LOG_WARN, fg="yellow")
return

# Start dockerd with same docker bridge, iptables configuration as on the host to not override docker configurations on the host.
# Dockerd has an option to start without creating a bridge, using --bridge=none option, however dockerd will remove the host docker0 in that case.
# Also, it is not possible to configure dockerd to start using a different bridge as it will also override the ip of the default docker0.
# Considering that, we start dockerd with same options the host dockerd is started.
run_command_or_raise(["cp", docker_default_config, docker_default_config_backup])
run_command_or_raise(["sh", "-c", "echo 'DOCKER_OPTS=\"$DOCKER_OPTS {}\"' >> {}".format(" ".join(get_docker_opts()), docker_default_config)])

run_command_or_raise(["chroot", new_image_mount, DOCKER_CTL_SCRIPT, "start"])
docker_started = True
run_command_or_raise(["cp", packages_path, os.path.join(new_image_mount, tmp_dir, packages_file)])
Expand All @@ -364,6 +383,8 @@ def migrate_sonic_packages(bootloader, binary_image_version):
finally:
if docker_started:
run_command_or_raise(["chroot", new_image_mount, DOCKER_CTL_SCRIPT, "stop"], raise_exception=False)
if os.path.exists(docker_default_config_backup):
run_command_or_raise(["mv", docker_default_config_backup, docker_default_config], raise_exception=False);
umount(new_image_mount, recursive=True, read_only=False, remove_dir=False, raise_exception=False)
umount(new_image_mount, raise_exception=False)

Expand Down
78 changes: 78 additions & 0 deletions tests/test_sonic_installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,84 @@
from click.testing import CliRunner
from unittest.mock import patch, Mock, call

@patch("sonic_installer.main.SWAPAllocator")
@patch("sonic_installer.main.get_bootloader")
@patch("sonic_installer.main.run_command_or_raise")
@patch("sonic_installer.main.run_command")
def test_install(run_command, run_command_or_raise, get_bootloader, swap, fs):
""" This test covers the execution of "sonic-installer install" command. """

sonic_image_filename = "sonic.bin"
current_image_version = "image_1"
new_image_version = "image_2"
new_image_folder = f"/images/{new_image_version}"
image_docker_folder = os.path.join(new_image_folder, "docker")
mounted_image_folder = f"/tmp/image-{new_image_version}-fs"
dockerd_opts = ["--iptables=false", "--bip=1.1.1.1/24"]

# Setup mock files needed for our test scenario
fs.create_file(sonic_image_filename)
fs.create_dir(image_docker_folder)
fs.create_dir(os.path.join(mounted_image_folder, "usr/lib/docker/docker.sh"))
fs.create_file("/var/run/docker.pid", contents="15")
fs.create_file("/proc/15/cmdline", contents="\x00".join(["dockerd"] + dockerd_opts))

# Setup bootloader mock
mock_bootloader = Mock()
mock_bootloader.get_binary_image_version = Mock(return_value=new_image_version)
mock_bootloader.get_installed_images = Mock(return_value=[current_image_version])
mock_bootloader.get_image_path = Mock(return_value=new_image_folder)

@contextmanager
def rootfs_path_mock(path):
yield mounted_image_folder

mock_bootloader.get_rootfs_path = rootfs_path_mock
get_bootloader.return_value=mock_bootloader

# Invoke CLI command
runner = CliRunner()
result = runner.invoke(sonic_installer.commands["install"], [sonic_image_filename, "-y"])
print(result.output)

assert result.exit_code == 0

# Assert bootloader install API was called
mock_bootloader.install_image.assert_called_with(f"./{sonic_image_filename}")
# Assert all below commands were called, so we ensure that
# 1. update SONiC environment works
# 2. package migration works
expected_call_list = [
call(["mkdir", "-p", mounted_image_folder]),
call(["mount", "-t", "squashfs", mounted_image_folder, mounted_image_folder]),
call(["sonic-cfggen", "-d", "-y", f"{mounted_image_folder}/etc/sonic/sonic_version.yml", "-t", f"{mounted_image_folder}/usr/share/sonic/templates/sonic-environment.j2"]),
call(["umount", "-r", "-f", mounted_image_folder], raise_exception=True),
call(["rm", "-rf", mounted_image_folder], raise_exception=True),
call(["mkdir", "-p", mounted_image_folder]),
call(["mount", "-t", "squashfs", mounted_image_folder, mounted_image_folder]),
call(["mkdir", "-p", f"{new_image_folder}/rw"]),
call(["mkdir", "-p", f"{new_image_folder}/work"]),
call(["mkdir", "-p", mounted_image_folder]),
call(["mount", "overlay", "-t", "overlay", "-o", f"rw,relatime,lowerdir={mounted_image_folder},upperdir={new_image_folder}/rw,workdir={new_image_folder}/work", mounted_image_folder]),
call(["mkdir", "-p", f"{mounted_image_folder}/var/lib/docker"]),
call(["mount", "--bind", f"{new_image_folder}/docker", f"{mounted_image_folder}/var/lib/docker"]),
call(["chroot", mounted_image_folder, "mount", "proc", "/proc", "-t", "proc"]),
call(["chroot", mounted_image_folder, "mount", "sysfs", "/sys", "-t", "sysfs"]),
call(["cp", f"{mounted_image_folder}/etc/default/docker", f"{mounted_image_folder}/tmp/docker_config_backup"]),
call(["sh", "-c", f"echo 'DOCKER_OPTS=\"$DOCKER_OPTS {' '.join(dockerd_opts)}\"' >> {mounted_image_folder}/etc/default/docker"]), # dockerd started with added options as host dockerd
call(["chroot", mounted_image_folder, "/usr/lib/docker/docker.sh", "start"]),
call(["cp", "/var/lib/sonic-package-manager/packages.json", f"{mounted_image_folder}/tmp/packages.json"]),
call(["touch", f"{mounted_image_folder}/tmp/docker.sock"]),
call(["mount", "--bind", "/var/run/docker.sock", f"{mounted_image_folder}/tmp/docker.sock"]),
call(["chroot", mounted_image_folder, "sh", "-c", "command -v sonic-package-manager"]),
call(["chroot", mounted_image_folder, "sonic-package-manager", "migrate", "/tmp/packages.json", "--dockerd-socket", "/tmp/docker.sock", "-y"]),
call(["chroot", mounted_image_folder, "/usr/lib/docker/docker.sh", "stop"], raise_exception=False),
call(["umount", "-f", "-R", mounted_image_folder], raise_exception=False),
call(["umount", "-r", "-f", mounted_image_folder], raise_exception=False),
call(["rm", "-rf", mounted_image_folder], raise_exception=False),
]
assert run_command_or_raise.call_args_list == expected_call_list

@patch("sonic_installer.main.get_bootloader")
def test_set_fips(get_bootloader):
""" This test covers the execution of "sonic-installer set-fips/get-fips" command. """
Expand Down

0 comments on commit be7da6b

Please sign in to comment.