CRAYSAT-1866, CRAYSAT-1871: Backport sat status fixes to release/3.25 for CSM 1.5 and SAT 2.6 #232
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix the `sat status` command to only make the necessary API requests to CSM services. Specifically, the `BOSStatusModule`, `SLSStatusModule`, and the `CFSStatusModule`, only apply to components of type `Node`, and so they should not be used when getting status of components of other types, e.g. `NodeBMC`. Using those modules in that case results in many unneccessary API requests to BOS, SLS, and CFS. Simplify code a little by making `component_types` a required argument for the `get_relevant_modules` and `get_poopulated_rows` methods. It was confusing to have a lot of code that handled `None` when it really would never be `None` because the corresponding argparse argument had a default value of `["Node"]`. Test Description: Unit tests pass. Tested by running `sat status` against drax with debug logging enabled and different values for `--types`. E.g. tested with `--types NodeBMC RouterBMC` and verified that no requests were made to BOS, CFS, or SLS. Then tested with `--types Node` (the default) and `--types Node NodeBMC` to verify that requests were made to BOS, CFS, and SLS. (cherry picked from commit 0d692fe)
The `SLSStatusModule` used by `sat status` has only `Node` as its `component_types`, meaning information from SLS is only added for nodes. The query, however, obtains SLS data for all components known to SLS. Modify this query to use the `/search/hardware` endpoint of SLS with `type=comptype_node` as a query parameter to limit the results to only the node components. Test Description: Ran `sat status` from my macbook against drax with debug logging enabled to verify API requests made. Executed with multiple option combinations, including: * `--types Node` * `--types NodeBMC` * `--types Node NodeBMC` * `--types all` * Same as above but with addition of `--sls-fields` option (cherry picked from commit 67cd146)
haasken-hpe
changed the title
haasken-hpe
changed the title
|
The resulting build was sanity tested on drax as follows: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary and Scope
Backport the following changes to release/3.25 for inclusion in CSM 1.5 and SAT 2.6.
sat statusto only make necessary API requests #221sat statusto only Nodes #228Issues and Related PRs
Testing
See linked PRs above.
Risks and Mitigations
See linked PRs above.
Pull Request Checklist