Bug 1494970 - [RFE]: Replace vdsm sos report code with sos plugin
Summary: [RFE]: Replace vdsm sos report code with sos plugin
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: vdsm
Classification: oVirt
Component: General
Version: 4.20.4
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: ovirt-4.3.5
: ---
Assignee: Marcin Sobczyk
QA Contact: Lucie Leistnerova
URL:
Whiteboard:
: 1381185 (view as bug list)
Depends On:
Blocks: oVirt_on_Fedora 1700780
TreeView+ depends on / blocked
 
Reported: 2017-09-24 12:40 UTC by Yaniv Bronhaim
Modified: 2019-07-30 14:08 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1700780 (view as bug list)
Environment:
Last Closed: 2019-07-30 14:08:14 UTC
oVirt Team: Infra
Embargoed:
mperina: ovirt-4.3?
mperina: ovirt-4.4?
rule-engine: planning_ack?
mperina: devel_ack+
lsvaty: testing_ack+


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Github sosreport sos pull 1205 0 None None None 2018-02-05 11:44:24 UTC

Description Yaniv Bronhaim 2017-09-24 12:40:47 UTC
Description of problem:
Following sos guidelines [1], we will be able to remove current vdsm/sos code and be aligned with sos project standards.


[1] https://github.com/sosreport/sos/wiki/How-to-Write-a-Plugin

Comment 1 Yaniv Bronhaim 2017-09-28 09:49:58 UTC
Hi Bryn, I sent you some questions regarding the implementation. How will we do the review process? May I submit it by mail to your maillist?..
Thanks

Comment 2 Yaniv Bronhaim 2017-10-16 12:30:14 UTC
*** Bug 1381185 has been marked as a duplicate of this bug. ***

Comment 3 Yaniv Bronhaim 2017-10-16 12:36:31 UTC
Bug #1381185 was about supporting the sos code with py3 and new fedora releases. Using sos plugins will allow us to benefit from sos team support in those regards (builds and compatibility).

To continue this work, Bryn already replied to me offline - copying answers here:


> 1. what replaces Plugin.add_forbidden_path - If I want to copy stuff from
> /etc/pki we used to use this function. will add_copy_spec work here?

Nothing replaced it - it's still there in the current tree:

    def add_forbidden_path(self, forbidden):
        """Specify a path to not copy, even if it's part of a copy_specs[]
        entry.
        """

You should be able to use it exactly as previously (modulo any case
conversion from the old method names).

> 2. how can I run out internal client from within the plugin? is it the same
> as we do today with the add_cmd_output?

Is this to do with the module import problem, or actually calling into
the client? Can you paste an example from the existing plugin?


>> we still haven't discussed about it, but this might be handled same as we did before.


> another question - once I have it written, how do I check that ? how do I
> package it as plugin?

If it's going upstream you just need to commit it to a git tree that's
based on a recent pull from sos master, and then submit a PR.

Otherwise if you are packaging it independently it's really down to
you - it can be as simple as distributing the file with instructions
or built into an RPM or other package.

Comment 4 Sandro Bonazzola 2017-10-17 06:52:19 UTC
adding Pavel

Comment 5 Pavel Moravec 2017-10-17 20:20:35 UTC
I understand the requirement is to incorporate VDSM independent plugin [1] into sos. But I dont understand:

- is the change required in RHEL[|7[|.?]] or in general in sos?

- what particular info is needed from me and Bryn?

I suggest opening a PR to include the plugin in upstream in either case. If it should be available in general, upstream is the best. If in Fedora, well we need somebody to build Fedora sos package :) but after it's in upstream. If in RHEL, again upstream first.

Very quick review of the plugin:
- line 110 (collectVdsmCommand method):
  - why dont use add_cmd_output / get_cmd_output_now / call_ext_prog methods from Plugin class? Due to collecting commands inside vdsm shell/CLI?
- line 132 (_addVdsmRunDir method):
  - cant it be replaced by classic add_copy_spec(whole_path) and then add_forbidden_path ?
- line 137 ("p = subprocess.Popen") - why dont call get_cmd_output_now or call_ext_prog ? See e.g. [2] as an example of usage
- the CLI calls are hard to follow in code. Isnt there a bash command calling them one after another? Or in other words, does the invocation of the CLI allow an equivalent to "-c" parameter from postgres? (connect to the database _and_ directly execute a command and collect its output)?


[1] https://gerrit.ovirt.org/gitweb?p=vdsm.git;a=blob;f=lib/sos/vdsm.py.in;hb=HEAD
[2] https://github.com/sosreport/sos/blob/master/sos/plugins/networking.py#L82

Comment 6 Irit Goihman 2017-11-07 09:40:59 UTC
(In reply to Pavel Moravec from comment #5)
> I understand the requirement is to incorporate VDSM independent plugin [1]
> into sos. But I dont understand:
> 
> - is the change required in RHEL[|7[|.?]] or in general in sos?

The required change is to add VDSM independent plugin to sos and get rid of VDSM plugin in VDSM project.

> 
> - what particular info is needed from me and Bryn?

When will the next sosreport version will be released and in what RHEL version is it expected to be installed? 

> 
> I suggest opening a PR to include the plugin in upstream in either case. If
> it should be available in general, upstream is the best. If in Fedora, well
> we need somebody to build Fedora sos package :) but after it's in upstream.
> If in RHEL, again upstream first.
> 
> Very quick review of the plugin:
> - line 110 (collectVdsmCommand method):
>   - why dont use add_cmd_output / get_cmd_output_now / call_ext_prog methods
> from Plugin class? Due to collecting commands inside vdsm shell/CLI?
> - line 132 (_addVdsmRunDir method):
>   - cant it be replaced by classic add_copy_spec(whole_path) and then
> add_forbidden_path ?
> - line 137 ("p = subprocess.Popen") - why dont call get_cmd_output_now or
> call_ext_prog ? See e.g. [2] as an example of usage
> - the CLI calls are hard to follow in code. Isnt there a bash command
> calling them one after another? Or in other words, does the invocation of
> the CLI allow an equivalent to "-c" parameter from postgres? (connect to the
> database _and_ directly execute a command and collect its output)?
> 
> 
> [1]
> https://gerrit.ovirt.org/gitweb?p=vdsm.git;a=blob;f=lib/sos/vdsm.py.in;
> hb=HEAD
> [2]
> https://github.com/sosreport/sos/blob/master/sos/plugins/networking.py#L82

Comment 7 Bryn M. Reeves 2017-11-07 10:31:03 UTC
> When will the next sosreport version will be released and in what RHEL version 
> is it expected to be installed?

See the milestones page on GitHub:

  https://github.com/sosreport/sos/milestones

The sos-3.6 release is tentatively scheduled for release next April, coinciding with the beginning of the rhel-7.6 development phase.

Comment 8 Lucie Leistnerova 2019-07-16 13:29:08 UTC
# rpm -qf /usr/lib/python2.7/site-packages/sos/plugins/vdsm.py
sos-3.7-5.el7.noarch

verified with vdsm-4.30.24-2.el7ev.x86_64

Comment 9 Sandro Bonazzola 2019-07-30 14:08:14 UTC
This bugzilla is included in oVirt 4.3.5 release, published on July 30th 2019.

Since the problem described in this bug report should be
resolved in oVirt 4.3.5 release, it has been closed with a resolution of CURRENT RELEASE.

If the solution does not work for you, please open a new bug report.


Note You need to log in before you can comment on or make changes to this bug.