[1.29] feat: new status messages for SCA & unregistered systems #3507
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
This way it can be done also in a different place. This is only refactoring with no behaviour changes.
Stop setting the 'consumerIdentity' attribute in the StatusCommand instance of each test: - that attribute does not exist - TestStatusCommand inherits from SubManFixture, which already sets a fake identity (i.e. all the tests assume a registered status by default) Since there are no tests here currently attempting any status check for not registered systems, there are no behaviour changes, only dead code is dropped.
As it may be useful also in other tests outside the test_identitycertlib module, then - move it together with the other stubs - adapt test_identitycertlib to it This is a simple refactoring with no behaviour changes.
So far _print_status() did both: 1) try hard to get the status of the system, possibily refreshing caches 2) print the actual status of the system Since the result of (1) will be needed also outside of this function, then move its logic in its own method with self-explaining name. As result, adapt _print_status() to get the SCA mode determination result, and simplify its code to do only the printing. There should be no behaviour change to the output.
Testing for the various states of syspurpose compliance makes sense only in entitlement mode. The test test_disabled_status_sca_mode already tests the only syspurpose compliance state when using SCA (i.e. "invalid"), so that situation is already covered.
In case the system is not registered do not attempt to get the compliance/entitlements state, as it will not reflect the actual status of the system (i.e. not registered). In that case there is also no need to print the syspurpose status, as it is not relevant on unregistered systems. Add a simple unit test for this situation.
The message printed for the "overall status" comes directly from the compliance status of the system. Compliance is a concept strictly specific to entitlement mode, and in case of SCA there is nothing to be said, and thus the "unknown" status gets used. While this is technically correct, in practice it tends to confuse users a lot, as it does not clearly give the important detail about the system properly registered. Hence, in case the content access mode is really SCA (after checking the certificates, potentially refreshing caches and status), use the simple status string "registered" for a system in SCA mode. This, together with the message already printed out about the fact that the host should get access to content, should improve the feedback that users get when reading the output of "status" on a system registered to an SCA organization. Adapt the existing test to the new messaging. Related to: CCT-848
… mode Both are features strictly specific to entitlement mode, and their status is less than optimal (and sometimes actually confusing) in an SCA system. Hence, limit their output only in case the content access mode of the system is not SCA.
The test used to rely on patching get_terminal_width() as imported from a CLI module. Change it to patch it directly as imported from its own module. There is no behaviour change, as the patched default (500) still applies, and few tests properly override that as they needed.
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.
The message printed for the "overall status" comes directly from the compliance status of the system. Compliance is a concept strictly specific to entitlement mode, and in case of SCA there is nothing to be said, and thus the "unknown" status gets used. While this is technically correct, in practice it tends to confuse users a lot, as it does not clearly give the important detail about the system properly registered.
Hence, in case the content access mode is really SCA (after checking the certificates, potentially refreshing caches and status), use the simple status string "registered" for a system in SCA mode. This, together with the message already printed out about the fact that the host should get access to content, should improve the feedback that users get when reading the output of "status" on a system registered to an SCA organization.
The output of
subscription-manager statuson a SCA system thus changes from this:to this:
Change/simplify the message also for a not registered systems: because of the reasons mentioned above, printing "unknown" is confusing, and also not correct (subscription-manager knows the system is not registered). In that case there is also no need to print the syspurpose status, as it is not relevant on unregistered systems.
The output of
subscription-manager statuson an unregistered system thus changes from this:to this:
The changes requires some additional cleanups/refactors in tests.
Card ID: CCT-848
Backport of most of PR #3504 excluding the removal of non-SCA bits (still supported in this branch).