Skip to content
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.

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

[collect] Add cluster profile for RHOSP #2916

Merged
merged 1 commit into from May 9, 2022

Conversation

TurboTurtle
Copy link
Member

Adds a cluster profile for Red Hat OpenStack Platform to identify
controller and (optionally) compute nodes to collect sos reports from.

Signed-off-by: Jake Hunsaker jhunsake@redhat.com


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?

@TurboTurtle
Copy link
Member Author

@BryanQuigley and/or @slashdd - I made this with RHOSP in mind, but mainly because I'm not familiar with other flavors of OpenStack, personally. If this same profile can be used for Ubuntu/Deb deployments (meaning, the use of the ansible inventory yaml is not RH-specific) then I can drop the RH-specific verbiage.

@packit-as-a-service
Copy link

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable packit/sosreport-sos-2916
  • And now you can install the packages.

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

#
# See the LICENSE file in the source distribution for further information.

import yaml
Copy link
Contributor

@pmoravec pmoravec Apr 25, 2022

Choose a reason for hiding this comment

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

Some tests are failing with Could not initialize 'collect': No module named 'yaml'. The module is provided by python3-pyyaml which is would be a new sos requirement, then. Does this single instance of usage justify the new requirement? (I realy dont know..)

ACK to the PR otherwise, on torns if it enhance sos requirements just due to this use case or not (and reinvent the wheel by manual parsing the yaml file).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I had forgotten that yaml isn't in the standard library.

I'm checking back with the RHOSP folks to see if there is another way to get the node list, as I'm very hesitant to add yet another dependency, especially for such a specific use case.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've been looking into this a bit more.

There is a possibility of using something like openstack overcloud profiles list and then a follow up openstack command that maps the node names to addresses - however the architecture of OSP in regards to where this command is functional is not immediately clear to me.

I've also clarified with the support team that this ansible inventory file is updated as the environment changes, and is the defacto source of truth.

I'm a bit torn here. One the one hand I am really not fond of adding another dependency, but this approach to enumeration is a lot more straight forward and reliable than relying on a chain of commands that output a table and having to grok that.

@pmoravec
Copy link
Contributor

Maybe a stupid idea: python3-pyyaml does not need to be present on all systems, but maybe it is present on all OSP nodes?

Could we assume that / is that correct? If so, we can conditionally collect the data only when "try import yaml" succeeded.

@TurboTurtle
Copy link
Member Author

TurboTurtle commented Apr 28, 2022

No, because sos collect can be run from a non-OSP node - pyyaml needs to be available locally. In the case that collect is being run from a workstation, we still handle the yaml content locally by reading it from the remote node and parsing it as a string instead of an open file.

@TurboTurtle
Copy link
Member Author

Been thinking on this for a bit, and going back and forth with the RHOSP team contacts that I've had for this.

We really have only two choices here:

  1. Take on the new dependency, perhaps as a weakdep?
  2. Re-write to leverage the openstack commands and from there grok the output of one command, and fetch addresses from another. This in my opinion is a bit more fragile as the output format of the command could be more liable to change than the structure of the ansible inventory file.

I'm leaning towards option 1, and making it a weakdep. Thoughts, @pmoravec @jjansky1 @bmr-cymru ?

@BryanQuigley
Copy link
Member

I don't have much experience with ansible - and for the most part Ubuntu OpenStack deployments wouldn't use it.

Can you export the inventory as a json or another format Python can just use without a dependency?

  • I do see that this might require more changes to how it's detected - so maybe that's why it wouldn't work.

@TurboTurtle
Copy link
Member Author

Ansible is 100% yaml based for this stuff. Inventories, plays, playbooks, etc... yaml all the way down, so we wouldn't be able to get this as a json file unfortunately.

On that note, how are Ubuntu Openstack deployments done? I'm wondering if maybe there's some common ground we're overlooking that might be a suitable replacement to referencing the inventory file.

@pmoravec
Copy link
Contributor

pmoravec commented May 4, 2022

I feel the option 1 (new weak dependency) has less negs than the 2, indeed.

Adds a cluster profile for Red Hat OpenStack Platform to identify
controller and (optionally) compute nodes to collect sos reports from.

Note that this adds a dependency on pyyaml to sos. This should be
considered a weak dependency by downstreams. As such, it is added as a
'Recommends' in `sos.spec`. pip does not have the concept of 'weak
dependencies', and so is added as a regular requirement in `setup.py`.

Signed-off-by: Jake Hunsaker <jhunsake@redhat.com>
@TurboTurtle TurboTurtle merged commit 8aec430 into sosreport:main May 9, 2022
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants