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.

Sign up for GitHub

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

Jump to bottom

[tests] Fix issue with 127.0.1.1 IP being detected #3981

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[tests] Fix issue with 127.0.1.1 IP being detected #3981

wants to merge 1 commit into from

Conversation

arif-ali
Copy link
Member

@arif-ali arif-ali commented Apr 3, 2025

Running stageone tests in some environments sees the hostname of the machine to have an IP of 127.0.1.1 but the host is never set up in a way that this address is configured on the host. So if 127.0.1.1 is the one being detected, then ignore the test.

Closes: #3975

Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

  • Is the commit message split over multiple lines and hard-wrapped at 72 characters?
  • Is the subject and message clear and concise?
  • Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
  • Does the commit contain a Signed-off-by: First Lastname [email protected]?
  • Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?
  • Are all passwords or private data gathered by this PR obfuscated?

@packit-as-a-service Packit-as-a-Service
Copy link

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable packit/sosreport-sos-3981
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

@arif-ali arif-ali added the Status/Needs Review This issue still needs a review from project members label Apr 3, 2025
@pmoravec
Copy link
Contributor

pmoravec commented Apr 3, 2025

Isn't it better to change the ip_addr = socket.gethostbyname(hostname) assignment of self.sysinfo['pre']['networking']['ip_addr'], as it does not match ip addr output?

(.. which can have its caveats as well, so I am still OK-ish with the workaround kind of fix)

@arif-ali
Copy link
Member Author

arif-ali commented Apr 4, 2025

@pmoravec thanks for your input, I was looking for a quick way to solve it, but after further investigation, and looking through stuff, I think I may have found a better approach

Interestingly that I get 2 different results with the following 2 different hostnames

>>> print(socket.gethostbyname(f"{socket.gethostname()}.local"))
10.201.177.101

>>> print(socket.gethostbyname(f"{socket.gethostname()}"))
127.0.1.1

Maybe we can check to see if it's just hostname without the domain, we can append .local otherwise we keep it as is. The first result is the right result we should be getting

@arif-ali arif-ali force-pushed the sos-arif-avocado-autopkgtests-localhost-parser branch from 5e161e5 to 9ac24e1 Compare April 4, 2025 13:22
@arif-ali
[tests] Fix issue with 127.0.1.1 IP being detected
7064369 
Running stageone tests in some environments sees the hostname of the
machine to have an IP of 127.0.1.1 but the host is never set up in a
way that this address is configured on the host. We check for
hostname, if this returns a loopback address then we add .local to
the end of the hostname, which ensures we get the address of the host.

Closes: sosreport#3975
Signed-off-by: Arif Ali <[email protected]>
@arif-ali arif-ali force-pushed the sos-arif-avocado-autopkgtests-localhost-parser branch from 9ac24e1 to 7064369 Compare April 4, 2025 15:42
pmoravec
pmoravec approved these changes Apr 4, 2025
View reviewed changes
Copy link
Contributor

@pmoravec pmoravec left a comment

Choose a reason for hiding this comment

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

Really nice fix!

(maybe worth having a comment in the code explaining the "why we dont like 127.* addresses? :-o" but.. well.. git blame is enough)

@arif-ali
Copy link
Member Author

arif-ali commented Apr 4, 2025

Although I've tested this locally and all works where it wasn't before, need to still test on the Ubuntu servers.

So, please don't merge before that. Probably do further testing next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TurboTurtle TurboTurtle TurboTurtle approved these changes

@pmoravec pmoravec pmoravec approved these changes

Assignees
No one assigned
Labels
Status/Needs Review This issue still needs a review from project members
Projects
None yet
Milestone
No milestone
3 participants
@arif-ali @pmoravec @TurboTurtle