Skip to content

[collect/RHCOS] Fix sos collect on RHCOS nodes with SSH transport - #4393

Open
dwolstroRH wants to merge 2 commits into
sosreport:mainfrom
dwolstroRH:supdev-226-rhcos-ssh-fix
Open

[collect/RHCOS] Fix sos collect on RHCOS nodes with SSH transport#4393
dwolstroRH wants to merge 2 commits into
sosreport:mainfrom
dwolstroRH:supdev-226-rhcos-ssh-fix

Conversation

@dwolstroRH

Copy link
Copy Markdown
Contributor
            nodes, two issues occurred:
  1. sos was not detected because RHELPolicy matched RHCOS nodes before RedHatCoreOSPolicy could be checked (both have ID=rhel in os-release)

  2. Spurious namespace deletion error occurred with SSH transport because cleanup() tried to delete a namespace that was never created (namespaces are only created for 'oc' transport)

Fixes:

  • Add check() override to RHELPolicy to explicitly reject RHCOS nodes
  • Add RHEL version detection (8/9/10) from os-release for dynamic support-tools image selection
  • Update create_sos_container() to use support-tools image with proper mount points and 'sleep infinity' to keep container running
  • Fix ocp cluster cleanup() to only delete namespace when using 'oc' transport, not SSH transport

Related: SUPDEV-226


Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

  • Is the commit message split over multiple lines and hard-wrapped at 72 characters?
  • Is the subject and message clear and concise?
  • Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
  • Does the commit contain a Signed-off-by: First Lastname email@example.com?
  • Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?
  • Are all passwords or private data gathered by this PR obfuscated?

@packit-as-a-service

Copy link
Copy Markdown

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo dnf install -y 'dnf*-command(copr)'
  • dnf copr enable packit/sosreport-sos-4393
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

Comment thread sos/collector/clusters/ocp.py
@pmoravec

Copy link
Copy Markdown
Contributor

What is the typical /etc/os-release content on CoreOS, please? I would prefer some other method to distinguish among the classes, other than chaning the parent class check method.

Also, I feel the one commit aims to beat multiple flies, it is worth splitting it into more, for the sake of clarity (this makes sense to do once we agree on the first point).

@dwolstroRH
dwolstroRH force-pushed the supdev-226-rhcos-ssh-fix branch from 25816de to 1f12138 Compare August 3, 2026 21:01
Comment thread sos/policies/distros/coreos.py Fixed
@dwolstroRH
dwolstroRH force-pushed the supdev-226-rhcos-ssh-fix branch from 1f12138 to 15c3b1b Compare August 3, 2026 21:18
@pmoravec

pmoravec commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

sos was not detected because RHELPolicy matched RHCOS nodes before RedHatCoreOSPolicy could be checked (both have ID=rhel in os-release)

This should be fixed by #4397 so no need to extract RedHatCoreOSPolicy to a new file.

@dwolstroRH
dwolstroRH force-pushed the supdev-226-rhcos-ssh-fix branch from 15c3b1b to de2feeb Compare August 4, 2026 19:34
Comment thread sos/policies/distros/redhat.py
Comment thread sos/policies/distros/redhat.py
@dwolstroRH
dwolstroRH force-pushed the supdev-226-rhcos-ssh-fix branch from de2feeb to 7aad9fe Compare August 10, 2026 17:34

@pmoravec pmoravec left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tentatively approving it - I would like to see the self.project = None "merged" from both if-then-else branches, and got confirmation on the "sleep forever" understanding. Plus the DCO is still wrong. Otherwise LGTM.

  The cleanup() method unconditionally attempted to delete the temporary
  namespace, but setup() only creates it when using the 'oc' transport.
  With SSH (control_persist) transport, the delete command would fail with
  a spurious error. Guard the deletion with a set_transport_type() check
  so cleanup matches setup's behavior.

Related: SUPDEV-226
Signed-off-by: David Wolstromer <dwolstro@redhat.com>
@dwolstroRH
dwolstroRH force-pushed the supdev-226-rhcos-ssh-fix branch from 7aad9fe to bc6bc09 Compare August 13, 2026 20:14
@dwolstroRH

Copy link
Copy Markdown
Contributor Author

Tentatively approving it - I would like to see the self.project = None "merged" from both if-then-else branches, and got confirmation on the "sleep forever" understanding. Plus the DCO is still wrong. Otherwise LGTM.

Sleep infinity keeps the container process alive so we can exec into it for sos report collection. This makes the -i
(interactive) flag unnecessary since the container no longer needs an interactive shell to stay running. The force_pull option is now honored as well.

The DCO should hopefully be fixed as well

@pmoravec pmoravec added Reviewed/Needs 2nd Ack Require a 2nd ack from a maintainer Status/Needs Review This issue still needs a review from project members Kind/RedHat RedHat related item Kind/collect collect component of sos labels Aug 14, 2026
Comment thread sos/policies/distros/redhat.py Outdated
Comment on lines +518 to +525
_image = (
'$(if [ -f /root/.toolboxrc ]; then '
'. /root/.toolboxrc && echo "${REGISTRY}/${IMAGE}"; '
'else '
f'echo "registry.redhat.io/rhel{self._rhel_version}/'
f'support-tools:latest"; '
'fi)'
)

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.

Absolutely not, no embedded bash scripting in sos.

@dwolstroRH
dwolstroRH force-pushed the supdev-226-rhcos-ssh-fix branch from bc6bc09 to 7b1ba22 Compare August 18, 2026 19:41
Comment thread sos/policies/distros/redhat.py Fixed
Add _get_rhel_version() to detect the underlying RHEL major version
from RHEL_VERSION in os-release (with PLATFORM_ID fallback), and use
it to select the correct support-tools image (rhel8 vs rhel9). Also
add .toolboxrc support to honor custom container registries, honor
the force_pull option, and use 'sleep infinity' to keep the container
running for exec.

Related: SUPDEV-226
Signed-off-by: David Wolstromer <dwolstro@redhat.com>
@dwolstroRH
dwolstroRH requested a review from TurboTurtle August 19, 2026 17:16
@dwolstroRH
dwolstroRH force-pushed the supdev-226-rhcos-ssh-fix branch from 7b1ba22 to 9f0a517 Compare August 19, 2026 17:39
@pmoravec
pmoravec self-requested a review August 20, 2026 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Kind/collect collect component of sos Kind/RedHat RedHat related item Reviewed/Needs 2nd Ack Require a 2nd ack from a maintainer Status/Needs Review This issue still needs a review from project members

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants