Skip to content

2121350: Implement "force" register option in rhsm dbus python binding#3160

Merged
jirihnidek merged 1 commit intomainfrom
jajerome/ENT-5350
Nov 11, 2022
Merged

2121350: Implement "force" register option in rhsm dbus python binding#3160
jirihnidek merged 1 commit intomainfrom
jajerome/ENT-5350

Conversation

@DuckBoss
Copy link
Contributor

@DuckBoss DuckBoss commented Nov 1, 2022

Testing

This implementation can be tested in either Python or CLI by sending a register command with the "force" option enabled to the rhsm dbus python binding as seen below:

Testing with a shell script

Testing with a python script

  • Modified script from original BZ reproduction steps:
import dbus

REGISTER_OPTS_DICT = dbus.Dictionary({"force": True}, signature="ss")
system_bus = dbus.SystemBus()
register_server = system_bus.get_object("com.redhat.RHSM1", "/com/redhat/RHSM1/RegisterServer")
address = register_server.Start("C", dbus_interface="com.redhat.RHSM1.RegisterServer")
private_bus = dbus.connection.Connection(address)
register_object = private_bus.get_object("com.redhat.RHSM1", "/com/redhat/RHSM1/Register")
connection_opts = {
    "host": "<stage_server_address>"
}
connection_opts = dbus.Dictionary(connection_opts, signature="ss")

# Register w/ username and password
register_object.Register(
    "<org_id>",
    "<username>",
    "<password>",
    REGISTER_OPTS_DICT,
    connection_opts,
    "C",
    dbus_interface="com.redhat.RHSM1.Register",
)

# Register w/ activation keys
register_object.RegisterWithActivationKeys(
    "<org_id>",
    [<list_of_activation_keys>],
    REGISTER_OPTS_DICT,
    connection_opts,
    "C",
    dbus_interface="com.redhat.RHSM1.Register",
)

@cnsnyder cnsnyder requested review from a team and cnsnyder and removed request for a team November 1, 2022 17:21
@DuckBoss DuckBoss marked this pull request as ready for review November 1, 2022 17:49
@cnsnyder cnsnyder requested a review from a team November 1, 2022 17:49
@DuckBoss DuckBoss force-pushed the jajerome/ENT-5350 branch 2 times, most recently from 0ace972 to 37bd012 Compare November 1, 2022 20:30
@DuckBoss
Copy link
Contributor Author

DuckBoss commented Nov 1, 2022

Side note for anyone reviewing: I added unit tests for the related dbus binding changes, however I'm not too sure if this is the proper implementation. Please let me know!

@jirihnidek jirihnidek self-assigned this Nov 2, 2022
Copy link
Contributor

@jirihnidek jirihnidek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall it LGTM 👍 . I have only small requests.

BTW: it is good to add some instruction how to test this case. Something like: Use this script for testing: https://github.com/jirihnidek/rhsm-dbus-examples/blob/main/register-org-username-password-force.sh

@ptoscano
Copy link
Contributor

ptoscano commented Nov 2, 2022

This seems good now at a quick glance; also RegisterWithActivationKeys needs the same changes, as that API has the same issue.

@DuckBoss DuckBoss force-pushed the jajerome/ENT-5350 branch 2 times, most recently from c223ab1 to 7802812 Compare November 2, 2022 17:46
@jirihnidek
Copy link
Contributor

jirihnidek commented Nov 3, 2022

BTW: When I try to run unit tests: py.test ./test/rhsmlib/dbus/test_register.py, then there is many failed unit test even for main branch. There is PR fixing this issue: #3156. We should wait until this is merged to main.

@DuckBoss
Copy link
Contributor Author

DuckBoss commented Nov 3, 2022

BTW: When I try to run unit tests: py.test ./test/rhsmlib/dbus/test_register.py, then there is many failed unit test even for main branch. There is PR fixing this issue: #3156. We should wait until this is merged to main.

Thanks for letting me know, but I'm not able to reproduce the unit test errors for the main branch PR. I'm getting the dbus-related unit test errors only in the 1.28 backport PR.


Edit: I've added the additional unit test for checking registration with activation keys if the system is already registered.

Card ID: ENT-5350
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2121350

- Implemented missing behavior of the "force" option
in Dbus API by unregistering the system if already registered
and re-registering the system.
- Added unit tests to check registration with force option
and registration when already registered.
Copy link
Contributor

@jirihnidek jirihnidek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks for updates.

@jirihnidek jirihnidek merged commit 81d0b4c into main Nov 11, 2022
@jirihnidek jirihnidek deleted the jajerome/ENT-5350 branch November 11, 2022 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants