Skip to content

Commit

Permalink
Auto-attach subscription when there is only one (#385)
Browse files Browse the repository at this point in the history
Also, previous to this change, if a pool had been specified on the
convert2rhel command line and attach_subscription() was called more than
once, the subscription-manager command line would be constructed
incorrectly.  It would have "--pool" added to the front of the pool id
each time attach-subscription was called leading to something like this:
"subscription-manager attach --pool --pool POOLID".

To fix this, we now treat tool_opts.pool as read-only and do not modify
it on each call to attach_subscription().  This goes along with our
overall move to treat global singletons as read-only whenever possible.
  • Loading branch information
Andrew-ang9 authored Apr 13, 2022
1 parent e5606a9 commit 154ac63
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 12 deletions.
25 changes: 16 additions & 9 deletions convert2rhel/subscription.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,29 +304,36 @@ def attach_subscription():
pool = ["subscription-manager", "attach"]
if tool_opts.auto_attach:
pool.append("--auto")
tool_opts.pool = "-a"
loggerinst.info("Auto-attaching compatible subscriptions to the system ...")
elif tool_opts.pool:
# The subscription pool ID has been passed through a command line
# option
pool.extend(["--pool", tool_opts.pool])
tool_opts.pool = pool
loggerinst.info("Attaching provided subscription pool ID to the system ...")
else:
# Let the user choose the subscription appropriate for the conversion
loggerinst.info("Manually select subscription appropriate for the conversion")
subs_list = get_avail_subs()

if len(subs_list) == 0:
loggerinst.warning("No subscription available for the conversion.")
return False

print_avail_subs(subs_list)
sub_num = utils.let_user_choose_item(len(subs_list), "subscription")
pool.extend(["--pool", subs_list[sub_num].pool_id])
tool_opts.pool = pool
loggerinst.info("Attaching subscription with pool ID %s to the system ..." % subs_list[sub_num].pool_id)
elif len(subs_list) == 1:
sub_num = 0
loggerinst.info(
" %s is the only subscription available, it will automatically be selected for the conversion."
% subs_list[0].pool_id
)

else:
# Let the user choose the subscription appropriate for the conversion
loggerinst.info("Manually select subscription appropriate for the conversion")
print_avail_subs(subs_list)
sub_num = utils.let_user_choose_item(len(subs_list), "subscription")
loggerinst.info("Attaching subscription with pool ID %s to the system ..." % subs_list[sub_num].pool_id)

pool.extend(["--pool", subs_list[sub_num].pool_id])
_, ret_code = utils.run_subprocess(pool)

if ret_code != 0:
# Unsuccessful attachment, e.g. the pool ID is incorrect or the
# number of purchased attachments has been depleted.
Expand Down
34 changes: 32 additions & 2 deletions convert2rhel/unit_tests/subscription_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,20 @@ def __call__(self, *args, **kwargs):


class TestSubscription(unittest.TestCase):
class GetOneSubMocked(unit_tests.MockFunction):
def __call__(self, *args, **kwargs):
Sub = namedtuple("Sub", ["pool_id", "sub_raw"])

subscription1 = Sub("samplepool", "Subscription description")
return [subscription1]

class GetAvailSubsMocked(unit_tests.MockFunction):
def __call__(self, *args, **kwargs):
return [namedtuple("Sub", ["pool_id", "sub_raw"])("samplepool", "Subscription description")]
Sub = namedtuple("Sub", ["pool_id", "sub_raw"])

subscription1 = Sub("samplepool", "Subscription description")
subscription2 = Sub("pool0", "sub desc")
return [subscription1, subscription2]

class GetNoAvailSubsMocked(unit_tests.MockFunction):
def __call__(self, *args, **kwargs):
Expand All @@ -67,7 +78,11 @@ def __call__(self, *args, **kwargs):
return [namedtuple("Sub", ["pool_id", "sub_raw"])("samplepool", "Subscription description")]

class LetUserChooseItemMocked(unit_tests.MockFunction):
def __init__(self):
self.called = 0

def __call__(self, *args, **kwargs):
self.called += 1
return 0

class GetRegistrationCmdMocked(unit_tests.MockFunction):
Expand Down Expand Up @@ -166,6 +181,13 @@ def test_get_registration_cmd(self):
expected = ["subscription-manager", "register", "--force", "--username=user", "--password=pass with space"]
self.assertEqual(subscription.get_registration_cmd(), expected)

@unit_tests.mock(subscription, "get_avail_subs", GetOneSubMocked())
@unit_tests.mock(utils, "let_user_choose_item", LetUserChooseItemMocked())
@unit_tests.mock(utils, "run_subprocess", RunSubprocessMocked())
def test_attach_subscription_one_sub_available(self):
self.assertEqual(subscription.attach_subscription(), True)
self.assertEqual(utils.let_user_choose_item.called, 0)

def test_get_registration_cmd_activation_key(self):
tool_opts.activation_key = "activation_key"
tool_opts.org = "org"
Expand All @@ -185,11 +207,19 @@ def test_get_registration_cmd_empty_string(self):
self.assertEqual(subscription.get_registration_cmd(), expected)
self.assertEqual(utils.prompt_user.called, {"Username: ": 2, "Password: ": 2})

@unit_tests.mock(subscription, "get_avail_subs", GetOneSubMocked())
@unit_tests.mock(utils, "let_user_choose_item", LetUserChooseItemMocked())
@unit_tests.mock(utils, "run_subprocess", RunSubprocessMocked())
def test_attach_subscription_one_sub_available(self):
self.assertEqual(subscription.attach_subscription(), True)
self.assertEqual(utils.let_user_choose_item.called, 0)

@unit_tests.mock(subscription, "get_avail_subs", GetAvailSubsMocked())
@unit_tests.mock(utils, "let_user_choose_item", LetUserChooseItemMocked())
@unit_tests.mock(utils, "run_subprocess", RunSubprocessMocked())
def test_attach_subscription_available(self):
def test_attach_subscription_multiple_subs_available(self):
self.assertEqual(subscription.attach_subscription(), True)
self.assertEqual(utils.let_user_choose_item.called, 1)

@unit_tests.mock(subscription, "get_avail_subs", GetAvailSubsMocked())
@unit_tests.mock(utils, "let_user_choose_item", LetUserChooseItemMocked())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@


def test_check_user_response_user_and_password(convert2rhel):

# Run c2r registration with no username and password provided
# check for user prompt enforcing input, then continue with registration
with convert2rhel(
Expand Down Expand Up @@ -52,3 +51,20 @@ def test_check_user_response_organization(convert2rhel):
# the Ctrl+d is used to terminate the process instead
c2r.sendcontrol("d")
assert c2r.exitstatus != 0


def test_auto_attach_pool_submgr(convert2rhel):
single_pool_id = env.str("RHSM_SINGLE_SUB_POOL")
with convert2rhel(
"-y --no-rpm-va --serverurl {} --username {} --password {} --debug".format(
env.str("RHSM_SERVER_URL"),
env.str("RHSM_SINGLE_SUB_USERNAME"),
env.str("RHSM_SINGLE_SUB_PASSWORD"),
)
) as c2r:
c2r.expect(
f"{single_pool_id} is the only subscription available, it will automatically be selected for the conversion."
)
c2r.sendcontrol("d")

assert c2r.exitstatus != 0

0 comments on commit 154ac63

Please sign in to comment.