Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix case when switching license from any not empty non SLES_BYOS to SLES_BYOS without cache file #138

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
2 changes: 1 addition & 1 deletion cloud-regionsrv-client.spec
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ Requires: python3-dnspython
Guest registration plugin for images intended for Microsoft Azure

%package addon-azure
Version: 1.0.5
Version: 1.0.6
Release: 0
Summary: Enable/Disable Guest Registration for Microsoft Azure
Group: Productivity/Networking/Web/Servers
Expand Down
41 changes: 33 additions & 8 deletions usr/sbin/regionsrv-enabler-azure
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,21 @@
# Lesser General Public License for more details.
# You should have received a copy of the GNU Lesser General Public
# License along with this library.

"""
jesusbv marked this conversation as resolved.
Show resolved Hide resolved
Check the different states of the license type:
START LICENSE CHANGED TO CACHE ACTION
jesusbv marked this conversation as resolved.
Show resolved Hide resolved
BYOS SLES N/A Register
BYOS SLES BYOS Register
BYOS BYOS N/A Do nothing
BYOS BYOS BYOS Do nothing

SLES BYOS N/A De-register
SLES BYOS SLES De-register
SLES SLES N/A Do nothing
SLES SLES SLES Do nothing
"""

import logging
import os
import requests
Expand All @@ -20,6 +35,7 @@ import sys

import cloudregister.registerutils as utils


CACHE_LICENSE_PATH = os.path.join(utils.get_state_dir(), 'cached_license')


Expand All @@ -37,7 +53,7 @@ def has_license_changed(license_type):
return license_type != old_license
except FileNotFoundError:
update_license_cache(license_type)

return False
jesusbv marked this conversation as resolved.
Show resolved Hide resolved

def get_license_type():
proxies = {
Expand Down Expand Up @@ -96,21 +112,30 @@ def run_command(command):
utils.start_logging()
service_name = 'guestregister'
license_type = get_license_type()

if ('BYOS' in license_type and has_license_changed(license_type) and
not utils.uses_rmt_as_scc_proxy()):
update_server = utils.get_smt()

if ('BYOS' in license_type and not utils.uses_rmt_as_scc_proxy()):
jesusbv marked this conversation as resolved.
Show resolved Hide resolved
license_has_changed = has_license_changed(license_type)
rjschwei marked this conversation as resolved.
Show resolved Hide resolved
if (
license_has_changed or
(update_server and utils.is_registered(update_server.get_FQDN()))
):
# either license has changed or
# system was previously registered
if license_has_changed:
Copy link
Contributor

Choose a reason for hiding this comment

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

This condition is tested twice in consecutive conditionals, on line 138 and then again here. Can be changed to (starting at line 137)

if has_license_changed(license_type):
    update_license_cache(license_type)
    if update_server and utils.is_registered(update_server.get_FQDN()):
        run_command(['registercloudguest', '--clean'])
        run_command(['systemctl', 'disable', service_name])

to avoid checking the same condition in consecutive instructions and make the flow a little easier to follow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should not do that, the de-registration must happen even if license hasnt changed
I will think a better way to check the conditions

Copy link
Contributor Author

@jesusbv jesusbv Jan 5, 2024

Choose a reason for hiding this comment

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

I think you meant outside the if of the license change

if has_license_changed(license_type):
    update_license_cache(license_type)
if update_server and utils.is_registered(update_server.get_FQDN()):
    run_command(['registercloudguest', '--clean'])
    run_command(['systemctl', 'disable', service_name])

update_license_cache(license_type)
rjschwei marked this conversation as resolved.
Show resolved Hide resolved
run_command(['registercloudguest', '--clean'])
run_command(['systemctl', 'disable', service_name])
update_license_cache(license_type)
elif license_type and 'BYOS' not in license_type:
# if no license type, system could be a BYOS without AHB
# or PAYG without AHB
update_server = utils.get_smt()
if update_server:
if (not has_license_changed(license_type) and
license_has_changed = has_license_changed(license_type)
if (not license_has_changed and
utils.is_registered(update_server.get_FQDN())):
sys.exit(0)
update_license_cache(license_type)
if license_has_changed:
update_license_cache(license_type)
update_server = None
if not update_server:
run_command(['registercloudguest', '--force-new'])
Expand Down