-
Notifications
You must be signed in to change notification settings - Fork 13
Add RSCT validity check in ServiceReport tool #11
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| # SPDX-License-Identifier: GPL-2.0-only | ||
| # | ||
| # (C) Copyright IBM Corp. 2020 | ||
| # Author: Seeteena Thoufeek <s1seetee@linux.vnet.ibm.com> | ||
|
|
||
| """Plugin to check RSCT configuration""" | ||
|
|
||
|
|
||
| import os | ||
| import sys | ||
|
|
||
| from servicereportpkg.check import Check, SysfsCheck | ||
| from servicereportpkg.validate.plugins import Plugin | ||
| from servicereportpkg.utils import execute_command | ||
| from servicereportpkg.check import PackageCheck,ServiceCheck | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Format the import statement, need space between PackageCheck and ServiceCheck.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK. Formatting will fix. |
||
| from servicereportpkg.utils import is_package_installed | ||
| from servicereportpkg.logger import get_default_logger | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No one using get_default_logger function anywhere in this file.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not needed. |
||
| from servicereportpkg.validate.schemes.schemes import PSeriesScheme | ||
|
|
||
| class RSCT(Plugin, PSeriesScheme): | ||
| """RSCT configuration check""" | ||
|
|
||
| def __init__(self): | ||
| Plugin.__init__(self) | ||
| self.name = RSCT.__name__ | ||
| self.description = RSCT.__doc__ | ||
| self.service_name = "ctrmc" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we don't need this variable anymore, right?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. not needed. will remove it. |
||
| self.installation_path = "/opt/rsct/bin" | ||
| self.packages = ["rsct.core","rsct.core.utils","rsct.basic","src", | ||
| "devices.chrp.base.ServiceRM","DynamicRM"] | ||
| self.subsystems = ["ctrmc","IBM.DRM","IBM.HostRM","IBM.ServiceRM", | ||
| "IBM.MgmtDomainRM"] | ||
| self.subsystem_active = "active" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really need to keep a class-level variable here? Can't we use the "active" string directly in get_subsystem_status function?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok. will do. |
||
|
|
||
| def check_rsct_installation_path(self): | ||
| """RSCT Installation path""" | ||
|
|
||
| installation_path_exists = True | ||
| self.log.info("RSCT Installation path check") | ||
| if not os.path.isdir(self.installation_path): | ||
| self.log.error("Missing RSCT installation directory %s", | ||
| self.installation_path) | ||
| installation_path_exists = False | ||
|
|
||
| return Check(self.check_rsct_installation_path.__doc__, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will you need installation path /opt/rsct/bin in the repair plugin?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes. In repair, with plugin_obj.check_rsct_installation_path() ...I am checking the installation path. |
||
| installation_path_exists) | ||
|
|
||
| def get_subsystem_status(self,subsystem): | ||
| """Checks the subsystem status by issuing the lssrc command""" | ||
|
|
||
| command = ["lssrc", "-s", subsystem] | ||
| (return_code,stdout,err) = execute_command(command) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Format issue. Please use pylint to find other similar issues.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok. will fix it. |
||
|
|
||
| if return_code is None or self.subsystem_active not in str(stdout): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No issue in hardcoding "active" string instead of self.subsystem_active.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok. will do |
||
| return False | ||
|
|
||
| return True | ||
|
|
||
| def check_rsct_subsystem_check(self): | ||
| """RSCT service status""" | ||
|
|
||
| status = True | ||
| self.log.info("RSCT Subsystem status check") | ||
| for subsystem in self.subsystems: | ||
| if self.get_subsystem_status(subsystem) ==0: | ||
| self.log.debug("%s Subsystem is not active", subsystem) | ||
| status = False | ||
|
|
||
| return ServiceCheck(self.check_rsct_subsystem_check.__doc__, | ||
| subsystem, status) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we discussed offline you might need to update this function to store all the subsystems along with their status?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. rsct repair plugin needs these subsystems along with their status so will update the revised code. |
||
|
|
||
| def check_rsct_package(self): | ||
| """RSCT package check""" | ||
|
|
||
| status = True | ||
| self.log.info("RSCT Package check") | ||
| for package in self.packages: | ||
| if not is_package_installed(package): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use four spaces to align you code. No tabs.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am using python formater to align the space. Is that okay?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. installed pylint and fixed the format error. |
||
| self.log.error("%s package is not installed", package) | ||
| status = False | ||
|
|
||
| return PackageCheck(self.check_rsct_package.__doc__, | ||
| package, status) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although you are checking all the packages defined in the self.packages list, but only the last package in the list is added to PackageCheck object. Not sure whether this is by intention?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ha. The time when validate code is posted, I did not think we need all those values needs to populate in repair. Now I am returning packagelist along with their status. |
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you don't need to import Check and SysfsCheck.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pylint will help you to find such errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to import Check. but SysfsCheck not needed.