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

Check if instance has Ipv4 or IPv6 enabled #128

Merged
merged 16 commits into from May 20, 2024
Merged

Conversation

jesusbv
Copy link
Contributor

@jesusbv jesusbv commented Aug 29, 2023

When using '--force-new --smt-ip ...' options,
validate that the instance can use the --smt-ip value in order to save debugging or unnecessary errors

When using '--force-new --smt-ip ...' options,
validate that the instance can use the --smt-ip value
in order to save debugging or unnecessary errors
@jesusbv jesusbv self-assigned this Aug 29, 2023
@jesusbv jesusbv marked this pull request as draft August 29, 2023 13:42
- Leverage has_ipv6_access
- Refactor has_ipv6_access
  to accomodate IPv4 check
@jesusbv jesusbv marked this pull request as ready for review October 25, 2023 14:50
@jesusbv
Copy link
Contributor Author

jesusbv commented Oct 25, 2023

once approve, I will squash the commits into one

Copy link
Contributor

@rjschwei rjschwei left a comment

Choose a reason for hiding this comment

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

This litters various pieces of knowledge in many places making it difficult to follow the code and the intention. Combined with generic names such as __connection_check that appears to be implemented for IPv4 use only.

lib/cloudregister/registerutils.py Outdated Show resolved Hide resolved
lib/cloudregister/registerutils.py Outdated Show resolved Hide resolved
lib/cloudregister/registerutils.py Outdated Show resolved Hide resolved
- Less knowledge littering
- Check request exception for
  - established connection
  - timeout
- Handle general request exception
- The message is checked if connection failed,
  use that and remove the if to set the message
@jesusbv jesusbv requested review from rjschwei and removed request for rjschwei November 14, 2023 10:25
Copy link
Contributor

@rjschwei rjschwei left a comment

Choose a reason for hiding this comment

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

The state purpose, which I agree with does not match the implementation. If we want an early exit when the user gives us an IP address, then construct an smt object in the cli code and call is_responsive then get out, it will amount to about a 3 line change.

lib/cloudregister/registerutils.py Outdated Show resolved Hide resolved
lib/cloudregister/registerutils.py Outdated Show resolved Hide resolved
@jesusbv
Copy link
Contributor Author

jesusbv commented Nov 21, 2023

The state purpose, which I agree with does not match the implementation. If we want an early exit when the user gives us an IP address, then construct an smt object in the cli code and call is_responsive then get out, it will amount to about a 3 line change.

We want to check if the IP provided (IPv4 or IPv6) is enabled on the client, so they can actually use that IP

- Rename method to reflect the correct intend
- Check if a request could be made, handle behaviour
  accordingly
- Check socket connection
- Restore has_ipv6_access as it was
- Remove __connection_check
lib/cloudregister/registerutils.py Outdated Show resolved Hide resolved
lib/cloudregister/registerutils.py Outdated Show resolved Hide resolved
lib/cloudregister/registerutils.py Outdated Show resolved Hide resolved
usr/sbin/registercloudguest Outdated Show resolved Hide resolved
usr/sbin/registercloudguest Outdated Show resolved Hide resolved
- Close socket connection
- Return False for any socket error
- When checking if instance can access IPv6 infra server,
  make sure the instance can handle IPv6 addresses
@jesusbv jesusbv requested a review from rjschwei May 10, 2024 07:52
lib/cloudregister/registerutils.py Outdated Show resolved Hide resolved
@rjschwei rjschwei merged commit 7492190 into master May 20, 2024
1 check passed
@jesusbv jesusbv deleted the ipv6-ipv4-enabled-instance branch May 21, 2024 08:43
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.

None yet

3 participants