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

Feat register no machine-id #3554

Merged
merged 2 commits into from
Dec 5, 2022
Merged

Feat register no machine-id #3554

merged 2 commits into from
Dec 5, 2022

Conversation

ahitacat
Copy link
Contributor

@ahitacat ahitacat commented Oct 14, 2022

All Pull Requests:

Check all that apply:

  • Have you followed the guidelines in our Contributing document, including the instructions about commit messages?
  • Is this PR to correct an issue?
  • Is this PR an enhancement?

Complete Description of Additions/Changes:

If insights-client is not register and there is a machine-id file in the system the registration process should fail.
It prints a new message asking to unregister the system again.

This resolves: rhbz#2071093

@ahitacat ahitacat added the client These issues represent work to be done by the "client" team. label Oct 17, 2022
Copy link
Contributor

@strider strider left a comment

Choose a reason for hiding this comment

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

LGTM. Only some interrogations.


# check registration with API
check = get_registration_status(config, pconn)

if machine_id_present and check['status'] is False:
Copy link
Contributor

Choose a reason for hiding this comment

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

Would prefer:

if machine_id_present and not check['status']:

insights/client/client.py Outdated Show resolved Hide resolved
# create a machine id first thing. we'll need it for all uploads
logger.debug('Machine ID: %s', client.get_machine_id())
logger.debug("CONFIG: %s", config)
print_egg_versions()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why logging all the available eggs' versions here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to maintain the structure of the logs, but I think it can be done first in this phase

# create a machine id first thing. we'll need it for all uploads
logger.debug('Machine ID: %s', client.get_machine_id())
logger.debug("CONFIG: %s", config)
print_egg_versions()
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

# create a machine id first thing. we'll need it for all uploads
logger.debug('Machine ID: %s', client.get_machine_id())
logger.debug("CONFIG: %s", config)
print_egg_versions()
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@strider strider self-requested a review October 20, 2022 12:17
@Ichimonji10
Copy link

ESSNTL-3782

@patriksuba
Copy link

Changes works as stated in description. While machine-id file exists client will fail with error message. Unregistering client deletes machine-id file. In BZ is also stated if the "--force" flag is also provided, print a message explaining what will happen, ask the user to confirm whether to proceed, and continue only if the user types "y" - this feature doesn't work.

@ahitacat
Copy link
Contributor Author

ahitacat commented Dec 5, 2022

I did not introduce the --force flag in this PR. It is planned to be deprecated at the same time this PR is merged BZ#2071082.
cc. @subpop

…ered insights-client fails executing

Signed-off-by: ahitacat <[email protected]>
@ahitacat ahitacat marked this pull request as ready for review December 5, 2022 17:18
@subpop subpop merged commit 1857a42 into RedHatInsights:master Dec 5, 2022
subpop pushed a commit that referenced this pull request Dec 5, 2022
* Registration mechanism looks for machine-id file and if is not registered insights-client fails executing
* Test to check this new logic

Signed-off-by: ahitacat <[email protected]>
xiangce pushed a commit that referenced this pull request Sep 6, 2024
* Registration mechanism looks for machine-id file and if is not registered insights-client fails executing
* Test to check this new logic

Signed-off-by: ahitacat <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client These issues represent work to be done by the "client" team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants