Bug 1895316
| Summary: | collector _get_nodes not called when --cluster-type specified | ||
|---|---|---|---|
| Product: | Red Hat Enterprise Linux 8 | Reporter: | Miroslav Hradílek <mhradile> |
| Component: | sos | Assignee: | Pavel Moravec <pmoravec> |
| Status: | CLOSED ERRATA | QA Contact: | Miroslav Hradílek <mhradile> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | 8.4 | CC: | agk, bmr, jhunsaker, plambri, sbradley |
| Target Milestone: | rc | Keywords: | Triaged |
| Target Release: | 8.4 | Flags: | pm-rhel:
mirror+
|
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | sos-4.0-3.el8 | Doc Type: | If docs needed, set a value |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2021-05-18 14:48:58 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
So, this is currently intentional. The thought being if we can't reliably determine the cluster type, then we won't be able to reliably determine the node list. oVirt/RHV is probably the only potential exception to this, since node determination is done via DB query that is separated from the "normal" processes that make a system a RHV manager. For pacemaker for example, if we can't determine that a system is part of a pacemaker cluster then we have no way to assume that the `pcs` command is present or functional on the system. The assumption here is that when specifying --cluster-type, users will also specify --nodes. That very well may be a wrong assumption to make, so let's tackle that first before deciding on a code change for this. (In reply to Jake Hunsaker from comment #2) > The assumption here is that when specifying --cluster-type, users will also > specify --nodes. That very well may be a wrong assumption to make, so let's > tackle that first before deciding on a code change for this. Shouldn't we enforce that assumption in code, then? I.e. raise an error when --cluster-type is provided but --nodes not? (I dont have much preference to either option of the quoted assumption) Definitely a solid point, that. Also, thinking on this more I'm convincing myself that the original thinking is wrong. We should be more in line with the idea of "since the user is explicitly saying this is the cluster type, then they know the mechanism to determine nodes is present". Whether or not that mechanism is _functional_ is not really a determination for us to make. +1 to running get_nodes() when --cluster-type is manually specified. I will drop you an email as the point I will try to make is not UX related but rather testing related. From the UX point of view it makes sense to me that you specify also nodes and that it should fail if you do not. Not sure how users are expecting it to work. (In reply to Jake Hunsaker from comment #4) > Definitely a solid point, that. Also, thinking on this more I'm convincing > myself that the original thinking is wrong. We should be more in line with > the idea of "since the user is explicitly saying this is the cluster type, > then they know the mechanism to determine nodes is present". Whether or not > that mechanism is _functional_ is not really a determination for us to make. > > +1 to running get_nodes() when --cluster-type is manually specified. Jake, will you file upstream PR for this or shall I? (directly the patch that Mirek proposed, right?) Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory (sos bug fix and enhancement update), and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHEA-2021:1604 |
Description of problem: It looks like master node is not queried for nodes when '--cluster-type' option is specified. Version-Release number of selected component (if applicable): # Unreleased package # rpm -q sos sos-collector sos-4.0-2.el8.noarch package sos-collector is not installed How reproducible: 100% Steps to Reproduce: # Setup # ssh-keygen -t rsa -f $HOME/.ssh/id_rsa -N '' # cat $HOME/.ssh/id_rsa.pub >> $HOME/.ssh/authorized_keys # chmod og-wx ~/.ssh/authorized_keys # cat >/usr/share/ovirt-engine/dbscripts/engine-psql.sh <<'EOF' #!/bin/bash # /usr/share/ovirt-engine/dbscripts/engine-psql.sh -c "select host_name from vds_static..." echo "$@" >> /root/engine-psql.debug EOF # chmod +x /usr/share/ovirt-engine/dbscripts/engine-psql.sh # cat /root/engine-psql.debug cat: /root/engine-psql.debug: No such file or directory # Repoduce # echo -e "\n"| sos-collector -vvv --case-id=666 --no-local --cluster-type ovirt --master "127.0.0.1" -c "ovirt.cluster=whatever" . . . [ovirt] Query command for ovirt DB set to: SELECT host_name from vds where cluster_id in (select cluster_id FROM cluster WHERE name like 'whatever' and storage_pool_id in (SELECT id FROM storage_pool WHERE name like '%')) [sos_collector:reduce_node_list] Node list reduced to [] . . . # cat /root/engine-psql.debug cat: /root/engine-psql.debug: No such file or directory Actual results: engine-psql.sh is not called to query database for nodes. (called through _get_nodes()) Expected results: engine-psql.sh is not called to query database for nodes. (called through _get_nodes()) This is from sos-collector-1.8-2.el8.noarch for comparison # Repoduce # echo -e "\n"| sos-collector -vvv --case-id=666 --no-local --cluster-type ovirt --master "127.0.0.1" -c "ovirt.cluster=whatever" . . . [['ovirt']] Query command for ovirt DB set to: SELECT host_name from vds where cluster_id in (select cluster_id FROM cluster WHERE name like 'whatever' and storage_pool_id in (SELECT id FROM storage_pool WHERE name like '%')) [sweetpig-16.4a2m.lab.eng.bos.redhat.com:run_command] Running command /usr/share/ovirt-engine/dbscripts/engine-psql.sh -c 'SELECT host_name from vds where cluster_id in (select cluster_id FROM cluster WHERE name like '"'"'whatever'"'"' and storage_pool_id in (SELECT id FROM storage_pool WHERE name like '"'"'%'"'"'))' . . . # cat engine-psql.debug -c SELECT host_name from vds where cluster_id in (select cluster_id FROM cluster WHERE name like 'whatever' and storage_pool_id in (SELECT id FROM storage_pool WHERE name like '%')) Additional info: I managed to trace it to the following method: def get_nodes_from_cluster(self): """Collects the list of nodes from the determined cluster cluster""" if self.cluster_type: nodes = self.cluster._get_nodes() self.log_debug('Node list: %s' % nodes) return nodes return [] When I change following code: if self.opts.cluster_type: if self.opts.cluster_type == 'none': self.cluster = self.clusters['jbon'] else: self.cluster = self.clusters[self.opts.cluster_type] self.cluster.master = self.master to if self.opts.cluster_type: if self.opts.cluster_type == 'none': self.cluster = self.clusters['jbon'] else: self.cluster = self.clusters[self.opts.cluster_type] self.cluster_type = self.opts.cluster_type self.cluster.master = self.master it works as expected. I'm not suggesting that as a patch. The issue might be that the self.cluster_type is no longer set by parsing options implicitly?