Improve Linux EC2 detection, fix false detection, and add Windows detection #793
Conversation
This false detects any Xen system and is less than ideal
| @@ -19,8 +20,9 @@ | |||
|
|
|||
| # How we detect EC2 from easiest to hardest & least reliable | |||
| # 1. Ohai ec2 hint exists. This always works | |||
| # 2. Xen hypervisor UUID starts with 'ec2'. This catches Linux HVM & paravirt instances | |||
| # 2. DMI data mentions amazon. This catches HVM instances in a VPC | |||
There was a problem hiding this comment.
Should this line be removed?
tas50
force-pushed
the
better_ec2_round_200
branch
from
fbaade3 to
509c105
Compare
| Ohai::Log.debug("ec2 plugin: has_amazon_org? == true") | ||
| true | ||
| end | ||
| rescue NoMethodError |
There was a problem hiding this comment.
I'm slightly uncomfortable catching the nonexistance of kernel[:os_info][:organization] with an exception. If it's set to not Amazon, then you'll also miss the logging.
There was a problem hiding this comment.
I was pretty aggressive there since the keys aren't going to exist on a lot of platforms. kernel[:os_info][:organization] only exists on Windows and I haven't checked, but I assume we don't have kernel on every platform. Do we have a better way to check for the existence of the complete path to that key and it's content?
There was a problem hiding this comment.
There's a DSL method attribute? that may be helpful
Lines 124 to 128 in 8f48805
There was a problem hiding this comment.
so switch that to this?:
if kernel.attribute?('os_info') && kernel[:os_info].attribute?('organization') && kernel[:os_info][:organization] =~ /Amazon/
There was a problem hiding this comment.
After some offline discussion we're going to stick with this for now and add a helper to allow better checking nested Mashes with logging #794
Previously we only got false logging when we rescued
| return true | ||
| end | ||
| else | ||
| Ohai::Log.debug("ec2 plugin: has_ec2_xen_uuid? == false") |
There was a problem hiding this comment.
This won't show up if the file exists and does not contain ec2. If you just remove the else line and put this at the end of the method unconditionally, it will probably do what you want since the if statement terminates with a return.
|
I'm normally not a fan of empty rescues, but I think it makes sense here to avoid duplicating the debug logging in each method. |
| end | ||
| rescue NoMethodError | ||
| Ohai::Log.debug("ec2 plugin: has_amazon_org? == false") | ||
| false |
There was a problem hiding this comment.
You could do the same if else reduction here and save yourself a repeat of this debug line, but this one isn't as important since it works.
|
👍 |
|
@mcquin Any chance you can look over the updates to this? |
|
👍 |
|
Looks great @tas50 👍 |
Remove the Xen MAC address detection method, which we previously deprecated
Detect Windows hosts based on Kernel owner information
Detect Linux hosts based on the Xen UUID starting with ec2