Merge pull request #5437 from c-po/containerlab

T9269: fix issues after recent GRUB serial console rewrites to support VyOS in container

7 files changed, 117 insertions, 28 deletions

diff --git a/python/vyos/airbag.py b/python/vyos/airbag.py
index 69b44dc9d..4e7bdfd06 100644
--- a/python/vyos/airbag.py
+++ b/python/vyos/airbag.py
@@ -78,8 +78,13 @@ def bug_report(dtype, value, trace):
         note = 'noteworthy:\n'
         note += '\n'.join(list(_noteworthy))
 
+    hardware = ''
+    if 'hardware_vendor' in information:
+        hardware = HARDWARE.format(**information)
+
     information.update({
         'date': datetime.now().strftime('%Y-%m-%d %H:%M:%S'),
+        'hardware': hardware,
         'trace': trace,
         'instructions': INSTRUCTIONS,
         'note': note,
@@ -141,14 +146,17 @@ Build commit ID:  {build_git}
 Architecture:     {system_arch}
 Boot via:         {boot_via}
 System type:      {system_type}
+{hardware}
+{trace}
+{note}
+"""
 
+# Optional section of FAULT - a container has no hardware of its own
+HARDWARE = """
 Hardware vendor:  {hardware_vendor}
 Hardware model:   {hardware_model}
 Hardware S/N:     {hardware_serial}
 Hardware UUID:    {hardware_uuid}
-
-{trace}
-{note}
 """
 
 INTRO = """\
diff --git a/python/vyos/system/grub_util.py b/python/vyos/system/grub_util.py
index edaea8ad3..98d3c1ddc 100644
--- a/python/vyos/system/grub_util.py
+++ b/python/vyos/system/grub_util.py
@@ -18,6 +18,8 @@ from vyos.system import grub
 from vyos.system import image
 from vyos.system import compat
 
+@image.if_not_container
+@image.if_persistence
 @compat.grub_cfg_update
 def set_serial_console(console_type: str, console_num: str,
                        console_speed: str, root_dir: str = '') -> None:
@@ -34,6 +36,8 @@ def set_serial_console(console_type: str, console_num: str,
     grub.set_serial_console(console_type, console_num, console_speed, root_dir)
 
 @image.if_not_live_boot
+@image.if_not_container
+@image.if_persistence
 def update_serial_console(console_type: str, console_num: str,
                           console_speed: str, root_dir: str = '') -> None:
     """Update console_speed if different from current value"""
@@ -53,6 +57,8 @@ def update_serial_console(console_type: str, console_num: str,
        console_speed != console_speed_current:
         set_serial_console(console_type, console_num, console_speed, root_dir)
 
+@image.if_not_container
+@image.if_persistence
 @compat.grub_cfg_update
 def set_kernel_cmdline_options(cmdline_options: str, version: str = '',
                                root_dir: str = '') -> None:
@@ -66,6 +72,8 @@ def set_kernel_cmdline_options(cmdline_options: str, version: str = '',
     grub.set_kernel_cmdline_options(cmdline_options, version, root_dir)
 
 @image.if_not_live_boot
+@image.if_not_container
+@image.if_persistence
 def update_kernel_cmdline_options(cmdline_options: str,
                                   root_dir: str = '',
                                   version = image.get_running_image()) -> None:
diff --git a/python/vyos/system/image.py b/python/vyos/system/image.py
index 02f2303b2..0b2eccf6d 100644
--- a/python/vyos/system/image.py
+++ b/python/vyos/system/image.py
@@ -283,6 +283,49 @@ def if_not_live_boot(func):
     return wrapper
 
 def is_running_as_container() -> bool:
-    if Path('/.dockerenv').exists():
+    """Detect if the system is running inside a container
+
+    Returns:
+        bool: True if running as container (Docker, Podman, LXC, ...)
+    """
+    # Docker and Podman drop a marker file into the container root
+    if Path('/.dockerenv').exists() or Path('/run/.containerenv').exists():
         return True
+    # systemd sets container= in the environment of PID 1 for all container
+    # types it knows about - this is only readable by root
+    try:
+        environ = Path('/proc/1/environ').read_bytes().decode(errors='ignore')
+        if 'container=' in environ:
+            return True
+    except (OSError, PermissionError):
+        pass
     return False
+
+def has_persistence() -> bool:
+    """Detect if a persistence storage partition is mounted
+
+    Returns:
+        bool: True if the persistence partition is available
+    """
+    return bool(disk.find_persistence())
+
+def if_persistence(func):
+    """Decorator to call function only if persistence storage is available.
+    Without it there is no writeable GRUB configuration to operate on"""
+    @wraps(func)
+    def wrapper(*args, **kwargs):
+        if has_persistence():
+            ret = func(*args, **kwargs)
+            return ret
+        return None
+    return wrapper
+
+def if_not_container(func):
+    """Decorator to call function only if not running inside a container"""
+    @wraps(func)
+    def wrapper(*args, **kwargs):
+        if not is_running_as_container():
+            ret = func(*args, **kwargs)
+            return ret
+        return None
+    return wrapper
diff --git a/python/vyos/utils/system.py b/python/vyos/utils/system.py
index fd32a486a..7cc6919a4 100644
--- a/python/vyos/utils/system.py
+++ b/python/vyos/utils/system.py
@@ -50,8 +50,12 @@ def sysctl_write(name: list[str], value: str | int) -> bool:
     # do not change anything if a value is already configured
     if sysctl_read(name) == value:
         return True
-    # return False if sysctl call failed
-    if run(['sysctl', '-wq', f'{key}={value}']).returncode != 0:
+    # return False if sysctl call failed - stderr is captured as the return
+    # code is the interface to the caller. A key may legitimately not exist,
+    # e.g. net.ipv4.neigh.default.gc_thresh* is not network namespace aware
+    # and thus not available when running inside a container
+    if run(['sysctl', '-wq', f'{key}={value}'],
+           capture_output=True).returncode != 0:
         return False
     # compare old and new values
     # sysctl may apply value, but its actual value will be
diff --git a/python/vyos/version.py b/python/vyos/version.py
index 4288a26ff..50359ddf6 100644
--- a/python/vyos/version.py
+++ b/python/vyos/version.py
@@ -71,38 +71,51 @@ def get_full_version_data(fname=version_file):
     # vyos.system.grub -> vyos.template -> jinja2, which every consumer of
     # vyos.version would otherwise pay for on import.
     from vyos.system.image import is_live_boot
+    from vyos.system.image import is_running_as_container
 
     version_data = get_version_data(fname)
 
     # Get system architecture (well, kernel architecture rather)
     version_data['system_arch'], _ = popen('uname -m', stderr=DEVNULL)
 
-    hypervisor,code = popen('hvinfo', stderr=DEVNULL)
-    if code == 1:
-         # hvinfo returns 1 if it cannot detect any hypervisor
-         version_data['system_type'] = 'bare metal'
+    # A container shares the Kernel with - and inherits the DMI/CPUID data of -
+    # its host, thus hvinfo would report the hypervisor of the host system
+    if is_running_as_container():
+        version_data['system_type'] = 'container'
     else:
-        version_data['system_type'] = f"{hypervisor} guest"
-
-    # Get boot type, it can be livecd or installed image
+        hypervisor,code = popen('hvinfo', stderr=DEVNULL)
+        if code == 1:
+             # hvinfo returns 1 if it cannot detect any hypervisor
+             version_data['system_type'] = 'bare metal'
+        else:
+            version_data['system_type'] = f"{hypervisor} guest"
+
+    # Get boot type, it can be a container, livecd or installed image
     # In installed images, the squashfs image file is named after its image version,
     # while on livecd it's just "filesystem.squashfs", that's how we tell a livecd boot
     # from an installed image
-    if is_live_boot():
+    if is_running_as_container():
+        # A container is never booted on its own - is_live_boot() would parse the
+        # Kernel cmdline of the host system
+        boot_via = "container image"
+    elif is_live_boot():
         boot_via = "livecd"
     else:
         boot_via = "installed image"
     version_data['boot_via'] = boot_via
 
-    # Get hardware details from DMI
-    dmi = '/sys/class/dmi/id'
-    version_data['hardware_vendor'] = read_file(dmi + '/sys_vendor', 'Unknown')
-    version_data['hardware_model'] = read_file(dmi +'/product_name','Unknown')
-
-    # These two assume script is run as root, normal users can't access those files
-    subsystem = '/sys/class/dmi/id/subsystem/id'
-    version_data['hardware_serial'] = read_file(subsystem + '/product_serial','Unknown')
-    version_data['hardware_uuid'] = read_file(subsystem + '/product_uuid', 'Unknown')
+    # Get hardware details from DMI - a container has no hardware of its own and
+    # would report the DMI data of the host system, thus the keys are left unset
+    # and consumers are expected to omit them
+    if not is_running_as_container():
+        dmi = '/sys/class/dmi/id'
+        version_data['hardware_vendor'] = read_file(dmi + '/sys_vendor', 'Unknown')
+        version_data['hardware_model'] = read_file(dmi +'/product_name','Unknown')
+
+        # These two assume script is run as root, normal users can't access those files
+        subsystem = '/sys/class/dmi/id/subsystem/id'
+        version_data['hardware_serial'] = read_file(subsystem + '/product_serial','Unknown')
+        version_data['hardware_uuid'] = read_file(subsystem + '/product_uuid', 'Unknown')
 
     return version_data
 
diff --git a/src/conf_mode/system_option.py b/src/conf_mode/system_option.py
index 190db232d..6e7ced37a 100755
--- a/src/conf_mode/system_option.py
+++ b/src/conf_mode/system_option.py
@@ -600,6 +600,11 @@ def generate_cmdline_for_kexec(options):
 
 def apply(options):
     kexec_required, cmdline_new = generate_cmdline_for_kexec(options)
+    # T9269: a container does not own the Kernel cmdline - it belongs to the
+    # host system, thus neither kexec nor a reboot would apply anything and the
+    # options always compare as changed
+    if image.is_running_as_container():
+        kexec_required = False
     if kexec_required:
         if not boot_configuration_complete() and os.getenv('VYOS_CONFIGD'):
             cmdl([
diff --git a/src/op_mode/version.py b/src/op_mode/version.py
index b93e3081b..bc74f83fc 100755
--- a/src/op_mode/version.py
+++ b/src/op_mode/version.py
@@ -26,6 +26,7 @@ import vyos.version
 import vyos.limericks
 
 from vyos.utils.boot import is_uefi_system
+from vyos.system.image import is_running_as_container
 from vyos.utils.system import get_secure_boot_state
 
 from jinja2 import Template
@@ -47,11 +48,13 @@ Architecture:     {{system_arch}}
 Boot via:         {{boot_via}}
 System type:      {{system_type}}
 Secure Boot:      {{secure_boot}}
+{%- if hardware_vendor is defined %}
 
 Hardware vendor:  {{hardware_vendor}}
 Hardware model:   {{hardware_model}}
 Hardware S/N:     {{hardware_serial}}
 Hardware UUID:    {{hardware_uuid}}
+{%- endif %}
 
 Copyright:        VyOS maintainers and contributors
 {%- if limerick %}
@@ -61,11 +64,16 @@ Copyright:        VyOS maintainers and contributors
 
 def _get_raw_data(funny=False):
     version_data = vyos.version.get_full_version_data()
-    version_data["secure_boot"] = "n/a (BIOS)"
-    if is_uefi_system():
-        version_data["secure_boot"] = "disabled"
-        if get_secure_boot_state():
-            version_data["secure_boot"] = "enabled"
+    # A container has no firmware of its own - it is not booted at all, thus
+    # neither the UEFI nor the BIOS wording applies
+    if is_running_as_container():
+        version_data["secure_boot"] = "n/a (container)"
+    else:
+        version_data["secure_boot"] = "n/a (BIOS)"
+        if is_uefi_system():
+            version_data["secure_boot"] = "disabled"
+            if get_secure_boot_state():
+                version_data["secure_boot"] = "enabled"
 
     if funny:
         version_data["limerick"] = vyos.limericks.get_random()