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

Better handle errors in fetching the hostname on darwin (macOS) systems #884

Merged
merged 1 commit into from
Oct 18, 2016

Conversation

erikng
Copy link
Contributor

@erikng erikng commented Sep 30, 2016

This pull request mimics the code from :linux.

The current assumption on darwin machines is that there is an entry located in /etc/hosts. Given that most darwin machines are client devices, this more than likely is incorrect.

Issues that this can cause:

  • ohai returns a value of null in the JSON. In my testing only the fqdn attribute will return this as a value.
  • When uploading null data to a postgresql database, you will experience table issues.

This change drops the fqdn value if a hostname is not properly returned, but still returns a valid entry if there is an entry in /etc/hosts.

@mcquin
Copy link
Contributor

mcquin commented Oct 6, 2016

Hi @erikng . Thanks for contributing to Ohai!

Sometimes I get worried about behavior changing changes. However, since ruby returns nil if a key is not in a hash, checks on node['fqdn'] won't behave differently than if node['fqdn'] were nil. I suppose users with node.attribute?('fqdn') && node['fqdn'].nil? might have some problems, but it's unlikely that's a common pattern.

Overally, I'm 👍 .

@erikng Do you have any insight on why hostname --fqdn sometimes falsely returns a blank string? If so, could you add that information to your comment? Additionally, can you please remove the WTF. Thanks!

@erikng
Copy link
Contributor Author

erikng commented Oct 6, 2016

Sure, essentially it's the way ohai looks up the host name. The assumption is macOS is similar to Ubuntu/etc in that you must add an entry to etc/hosts to fully resolve. Apple has other native mechanisms that resolve the host name (and ohai uses them!) and while you can certainly add an entry to etc/hosts, most commonly on Apple devices this won't be done.

As for the WTF comment should I remove it from the other comments in this file? Almost the entirety of this PR is taken from other functions.

@mcquin
Copy link
Contributor

mcquin commented Oct 6, 2016

Ah. I see now that the comment was copied. Thank you for the delightful and descriptive comment @jaymzh. ❤️

sigh I guess just leave it.

@erikng
Copy link
Contributor Author

erikng commented Oct 6, 2016

Heh. Would you like me to still add the macOS comments about /etc/hosts?

@tas50 tas50 merged commit 7d440bc into chef:master Oct 18, 2016
@tas50 tas50 changed the title Add logic to darwin hostname Better handle errors in fetching the hostname on darwin (macOS) systems Oct 18, 2016
@thommay thommay added Type: Enhancement Adds new functionality. and removed Enhancement labels Jan 24, 2017
@chef chef locked and limited conversation to collaborators Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Enhancement Adds new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants