Skip to content

[openshift_ovn] Allow specific container runtime for plugin command #4076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TurboTurtle merged 1 commit into from
Jul 31, 2025
Merged

[openshift_ovn] Allow specific container runtime for plugin command #4076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 34 additions & 12 deletions sos/report/plugins/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2111,7 +2111,8 @@ def add_cmd_output(self, cmds, suggest_filename=None,
sizelimit=None, pred=None, subdir=None,
changes=False, foreground=False, tags=[],
priority=10, cmd_as_tag=False, container=None,
to_file=False, runas=None, snap_cmd=False):
to_file=False, runas=None, snap_cmd=False,
runtime=None):
"""Run a program or a list of programs and collect the output

Output will be limited to `sizelimit`, collecting the last X amount
Expand Down Expand Up @@ -2190,6 +2191,9 @@ def add_cmd_output(self, cmds, suggest_filename=None,

:param snap_cmd: Are the commands being run from a snap?
:type snap_cmd: ``bool``

:param runtime: Specific runtime to use to run container cmd
:type runtime: ``str``
"""
if isinstance(cmds, str):
cmds = [cmds]
Expand All @@ -2204,7 +2208,8 @@ def add_cmd_output(self, cmds, suggest_filename=None,
if container:
ocmd = cmd
container_cmd = (ocmd, container)
cmd = self.fmt_container_cmd(container, cmd, runas=runas)
cmd = self.fmt_container_cmd(container, cmd, runtime=runtime,
runas=runas)
if not cmd:
self._log_debug(f"Skipping command '{ocmd}' as the "
f"requested container '{container}' does "
Expand Down Expand Up @@ -2636,7 +2641,7 @@ def collect_cmd_output(self, cmd, suggest_filename=None,
def exec_cmd(self, cmd, timeout=None, stderr=True, chroot=True,
runat=None, env=None, binary=False, pred=None,
foreground=False, container=False, quotecmd=False,
runas=None):
runas=None, runtime=None):
"""Execute a command right now and return the output and status, but
do not save the output within the archive.

Expand Down Expand Up @@ -2681,6 +2686,10 @@ def exec_cmd(self, cmd, timeout=None, stderr=True, chroot=True,
:param runas: Run the `cmd` as the `runas` user
:type runas: ``str``

:param runtime: Specific runtime to use to execute the
container command
:type runtime: ``str``

:returns: Command exit status and output
:rtype: ``dict``
"""
Expand All @@ -2703,8 +2712,9 @@ def exec_cmd(self, cmd, timeout=None, stderr=True, chroot=True,
self._log_info(f"Cannot run cmd '{cmd}' in container "
f"{container}: no runtime detected on host.")
return _default
if self.container_exists(container) or runas is not None:
cmd = self.fmt_container_cmd(container, cmd, quotecmd, runas)
if self.container_exists(container, runtime) or runas is not None:
cmd = self.fmt_container_cmd(container, cmd, quotecmd,
runtime, runas)
else:
self._log_info(f"Cannot run cmd '{cmd}' in container "
f"{container}: no such container is running.")
Expand Down Expand Up @@ -2784,17 +2794,20 @@ def _get_container_runtime(self, runtime=None):
return self.policy.runtimes[pol_runtime]
return None

def container_exists(self, name):
def container_exists(self, name, runtime=None):
"""If a container runtime is present, check to see if a container with
a given name is currently running

:param name: The name or ID of the container to check presence of
:type name: ``str``

:param runtime: The runtime to use
:type runtime: ``str``

:returns: ``True`` if `name` exists, else ``False``
:rtype: ``bool``
"""
_runtime = self._get_container_runtime()
_runtime = self._get_container_runtime(runtime)
if _runtime is not None:
return (_runtime.container_exists(name) or
_runtime.get_container_by_name(name) is not None)
Expand All @@ -2818,16 +2831,19 @@ def get_all_containers_by_regex(self, regex, get_all=False):
return [c for c in _containers if re.match(regex, c[1])]
return []

def get_container_by_name(self, name):
def get_container_by_name(self, name, runtime=None):
"""Get the container ID for a specific container

:param name: The name of the container
:type name: ``str``

:param runtime: The runtime to use
:type runtime: ``str``

:returns: The ID of the container if it exists
:rtype: ``str`` or ``None``
"""
_runtime = self._get_container_runtime()
_runtime = self._get_container_runtime(runtime)
if _runtime is not None:
return _runtime.get_container_by_name(name)
return None
Expand Down Expand Up @@ -2921,7 +2937,8 @@ def add_container_logs(self, containers, get_all=False, **kwargs):
cmd = _runtime.get_logs_command(_con[1])
self.add_cmd_output(cmd, **kwargs)

def fmt_container_cmd(self, container, cmd, quotecmd=False, runas=None):
def fmt_container_cmd(self, container, cmd, quotecmd=False, runtime=None,
runas=None):
"""Format a command to be executed by the loaded ``ContainerRuntime``
in a specified container

Expand All @@ -2934,6 +2951,10 @@ def fmt_container_cmd(self, container, cmd, quotecmd=False, runas=None):
:param quotecmd: Whether the cmd should be quoted.
:type quotecmd: ``bool``

:param runtime: The specific runtime to use to run the command
within the container
:type runtime: ``str``

:param runas: What user runs the container. If set, we trust
the container really runs (we dont keep them atm)
:type runas: ``str``
Expand All @@ -2942,8 +2963,9 @@ def fmt_container_cmd(self, container, cmd, quotecmd=False, runas=None):
within the `container` and not on the host
:rtype: ``str``
"""
if self.container_exists(container) or \
((_runtime := self._get_container_runtime()) and runas is not None):
if self.container_exists(container, runtime) or \
((_runtime := self._get_container_runtime(runtime)) and
runas is not None):
return _runtime.fmt_container_cmd(container, cmd, quotecmd)
return ''

Expand Down
37 changes: 23 additions & 14 deletions sos/report/plugins/openshift_ovn.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# Copyright (C) 2021 Nadia Pinaeva <npinaeva@redhat.com>

# This file is part of the sos project: https://github.com/sosreport/sos
#
# This copyrighted material is made available to anyone wishing to use,
Expand All @@ -8,6 +7,7 @@
#
# See the LICENSE file in the source distribution for further information.

import glob
from sos.report.plugins import Plugin, RedHatPlugin


Expand All @@ -21,6 +21,7 @@ class OpenshiftOVN(Plugin, RedHatPlugin):
profiles = ('openshift',)

def setup(self):

all_logs = self.get_option("all_logs")

self.add_copy_spec([
Expand Down Expand Up @@ -50,21 +51,29 @@ def setup(self):
'cluster/status OVN_Northbound',
'ovn-appctl -t /var/run/ovn/ovnsb_db.ctl ' +
'cluster/status OVN_Southbound'],
container='ovnkube-master')
self.add_cmd_output([
'ovs-appctl -t /var/run/ovn/ovn-controller.*.ctl ' +
'ct-zone-list'],
container='ovnkube-node')
self.add_cmd_output([
'ovs-appctl -t /var/run/ovn/ovn-controller.*.ctl ' +
'ct-zone-list'],
container='ovnkube-controller')
container='ovnkube-master',
runtime='crio')
# We need to determine the actual file name to send
# to the command
files = glob.glob("/var/run/ovn/ovn-controller.*.ctl")
for file in files:
Comment on lines -53 to +59

@TurboTurtle TurboTurtle Jul 9, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this collection not behave correctly when directly specifying the glob? Is that why we're also changing this to a per-file iteration?

@dwolstroRH dwolstroRH Jul 11, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When this command was getting executed, the filename '/var/run/ovn/ovn-controller..ctl' parameter within the command string was not getting expanded. The '' represents the current process id of the container, ie, '/var/run/ovn/ovn-controller.3127.ctl', which lead to a "File Not Found Error" condition when executing the command. I changed to find the filename before passing the command string into add_cmd_output.

self.add_cmd_output([
f"ovs-appctl -t {file} ct-zone-list"],
container='ovnkube-node',
runtime='crio')
self.add_cmd_output([
f"ovs-appctl -t {file} ct-zone-list"],
container='ovnkube-controller',
runtime='crio')
# Collect ovs ct-zone-list directly on host for interconnect setup.
self.add_cmd_output([
'ovs-appctl -t /var/run/ovn-ic/ovn-controller.*.ctl ' +
'ct-zone-list'])
files = glob.glob("/var/run/ovn-ic/ovn-controller.*.ctl")
for file in files:
self.add_cmd_output([
f"ovs-appctl -t {file} ct-zone-list'"],
runtime='crio')
self.add_cmd_output([
'ovs-appctl -t ovs-monitor-ipsec tunnels/show',
'ipsec status',
'certutil -L -d sql:/etc/ipsec.d'],
container='ovn-ipsec')
container='ovn-ipsec',
runtime='crio')
Loading