Dev: corosync: Default 'crm corosync status' to all components (#2038)#2122
Open
nicholasyang2022 wants to merge 1 commit into
Open
Dev: corosync: Default 'crm corosync status' to all components (#2038)#2122nicholasyang2022 wants to merge 1 commit into
nicholasyang2022 wants to merge 1 commit into
Conversation
Collaborator
Author
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the crm corosync status command to display health information for all corosync-related components by default (instead of only ring), aligning behavior with the expectation in #2038.
Changes:
- Default
crm corosync statusto query and print status for all supported components. - Move
COROSYNC_STATUS_TYPESintocrmsh/corosync.pyand introduce aSTATUS_FUNC_DICTdispatch table. - Replace
crm_node -lexecution viacluster_shell()with a directsubprocess.run()call, and add fail-fast behavior when node status query fails.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/unittests/test_corosync.py | Updates/extends unit tests for the new default “all components” behavior and new error handling paths. |
| crmsh/ui_corosync.py | Changes corosync status default argument to query all components and updates completer source. |
| crmsh/corosync.py | Refactors status querying to support “all by default”, adds dispatch dict/constants, and switches crm_node -l to subprocess.run. |
| crmsh/constants.py | Removes COROSYNC_STATUS_TYPES constant after moving it to corosync.py. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+200
to
+209
| STATUS_FUNC_DICT = { | ||
| "ring": query_ring_status, | ||
| "quorum": query_quorum_status, | ||
| "qdevice": query_qdevice_status, | ||
| "cpg": query_cpg_status, | ||
| "qnetd": query_qnetd_status, | ||
| } | ||
|
|
||
| COROSYNC_STATUS_TYPES = tuple(STATUS_FUNC_DICT.keys()) | ||
|
|
…erLabs#2038) - Update 'crm corosync status' to show all components by default - Refactor COROSYNC_STATUS_TYPES from constants.py to corosync.py - Optimize crm_node -l to use subprocess.run directly - Ensure ValueError is raised on status query failures for 'fail fast' behavior Fixes: ClusterLabs#2038
nicholasyang2022
force-pushed
the
issue-2038-corosync-status
branch
from
June 1, 2026 05:24
dbbdae6 to
f6571cf
Compare
Collaborator
Author
|
This PR targets to v5.2. |
nicholasyang2022
marked this pull request as ready for review
June 1, 2026 05:59
Member
|
I'd prefer the output to be a little more self-explained to its usage. Something as below, |
Collaborator
Author
I tried this and it is not quite readable. Currently, subcommands like |
This file contains hidden or 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
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.
Fixes: #2038