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

Properly poll Openstack metadata + other Openstack improvements #818

Merged
merged 8 commits into from
May 11, 2016
Merged

Properly poll Openstack metadata + other Openstack improvements #818

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5912cc3
Properly poll Openstack detection + other Openstack improvements
tas50 May 9, 2016
85bc7ec
Update the specs for the updated plugin
tas50 May 9, 2016
be8c7a9
Set metadata to {} not nil if it doesn't respond
tas50 May 9, 2016
b18d70f
Detect Dreamhost DreamCompute
tas50 May 9, 2016
4c9a5b0
Fix typo in comment
tas50 May 10, 2016
cbca3e0
Move metadata attributes back to openstack namespace
tas50 May 10, 2016
7809966
Allow setting dreamhost as the provider in the cloud plugins
tas50 May 10, 2016
c11de20
Use the mixin variable for the metadata server
tas50 May 11, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/ohai/plugins/cloud.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ def get_openstack_values
cloud[:public_hostname] = openstack["public_hostname"]
cloud[:local_ipv4] = openstack["local_ipv4"]
cloud[:local_hostname] = openstack["local_hostname"]
cloud[:provider] = openstack["provider"]
cloud[:provider] = "openstack"
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this can sometimes be "dreamhost"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this one gets a bit weird and here's why I went with openstack. When you're in the context of the cloud plugin says provider you'd think openstack (right? maybe...). When you're in the context of the openstack plugin you know you're openstack so when someone says provider you're thinking about what openstack provider and that could be dreamhost. I guess it could really go either way though.

Copy link
Contributor

Choose a reason for hiding this comment

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

It could be breaking for some users. Though I understand your argument, I think it's safer to keep it as it was.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The previous behavior was HP or Openstack, but I'll switch this one back. People on Dreamhost can figure it out probably.

end

# ----------------------------------------
Expand Down
78 changes: 44 additions & 34 deletions lib/ohai/plugins/openstack.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#
# Author:: Matt Ray (<matt@chef.io>)
# Author:: Tim Smith (<tsmith@chef.io>)
# Copyright:: Copyright (c) 2012-2016 Chef Software, Inc.
# License:: Apache License, Version 2.0
#
Expand All @@ -18,53 +19,62 @@
require "ohai/mixin/ec2_metadata"

Ohai.plugin(:Openstack) do
include Ohai::Mixin::Ec2Metadata

provides "openstack"
depends "dmi"
depends "etc"

include Ohai::Mixin::Ec2Metadata
# do we have the openstack dmi data
def openstack_dmi?
# detect a manufacturer of OpenStack Foundation
if dmi[:system][:all_records][0][:Manufacturer] =~ /OpenStack/
Ohai::Log.debug("Plugin Openstack: has_openstack_dmi? == true")
true
end
rescue NoMethodError
Ohai::Log.debug("Plugin Openstack: has_openstack_dmi? == false")
false
end

def collect_openstack_metadata(addr = Ohai::Mixin::Ec2Metadata::EC2_METADATA_ADDR, api_version = "2013-04-04")
path = "/openstack/#{api_version}/meta_data.json"
uri = "http://#{addr}#{path}"
# check for the ohai hint and log debug messaging
def openstack_hint?
if hint?("openstack")
Ohai::Log.debug("Plugin Openstack: openstack hint present")
return true
else
Ohai::Log.debug("Plugin Openstack: openstack hint not present")
return false
end
end

# dreamhost systems have the dhc-user on them
def openstack_provider
begin
response = http_client.get_response(URI.parse(uri), nil, nil)
case response.code
when "200"
FFI_Yajl::Parser.parse(response.body)
when "404"
Ohai::Log.debug("Encountered 404 response retreiving OpenStack specific metadata path: #{path} ; continuing.")
nil
else
raise "Encountered error retrieving OpenStack specific metadata (#{path} returned #{response.code} response)"
end
rescue => e
Ohai::Log.debug("Encountered error retrieving OpenStack specific metadata (#{uri}), due to #{e.class}")
nil
return "dreamhost" if etc["passwd"]["dhc-user"]
rescue NoMethodError
# handle etc not existing on non-linux systems
end
return "openstack"
end
Copy link
Contributor

@thommay thommay May 10, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

def openstack_provider
  if  etc["passwd"]["dhc-user"]
    "dreamhost"
  else 
    "openstack"
  end
rescue NoMethodError
end

might be cleaner?

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 had it that way and it ends up returning nil when it can't find the key, which isn't what you want.

Scenarios:
etc plugin doesn't exist at all and the rescue is necessary -> openstack
etc plugin exists, but dhc-user doesn't exist -> openstack
etc plugin exists and dhc-user does exist -> dreamhost


collect_data do
# Adds openstack Mash
if hint?("openstack") || hint?("hp")
Ohai::Log.debug("ohai openstack")

if can_metadata_connect?(Ohai::Mixin::Ec2Metadata::EC2_METADATA_ADDR, 80)
openstack Mash.new
Ohai::Log.debug("connecting to the OpenStack metadata service")
fetch_metadata.each { |k, v| openstack[k] = v }
# fetch data if we look like openstack
if openstack_hint? || openstack_dmi?
openstack Mash.new
openstack[:provider] = openstack_provider

if hint?("hp")
openstack["provider"] = "hp"
else
openstack["provider"] = "openstack"
Ohai::Log.debug("connecting to the OpenStack specific metadata service")
openstack["metadata"] = collect_openstack_metadata
# fetch the metadata if we can do a simple socket connect first
if can_metadata_connect?("169.254.169.254", 80)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to use Ohai::Mixin::Ec2Metadata::EC2_METADATA_ADDR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While it is the same thing it seems a bit odd to me to use that variable

Copy link
Contributor

@mcquin mcquin May 10, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

You're using fetch_metadata which relies on that variable. Best to use it here too in the unlikely case something changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

The variable is referenced in the tests, as well.

fetch_metadata.each do |k, v|
openstack[k] = v
end

Ohai::Log.debug("Plugin Openstack: Successfully fetched Openstack metadata from the metadata endpoint")
else
Ohai::Log.debug("unable to connect to the OpenStack metadata service")
Ohai::Log.debug("Plugin Openstack: Timed out connecting to Openstack metadata endpoint. Skipping metadata.")
end
else
Ohai::Log.debug("NOT ohai openstack")
Ohai::Log.debug("Plugin Openstack: Node does not appear to be an Openstack node")
end
end
end
162 changes: 80 additions & 82 deletions spec/unit/plugins/openstack_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,45 +21,68 @@

describe "OpenStack Plugin" do

let(:openstack_hint) { false }
let(:hp_hint) { false }
let(:plugin) { get_plugin("openstack") }

let(:ohai_system) { Ohai::System.new }
let(:ohai_data) { ohai_system.data }

let(:openstack_plugin) do
plugin = get_plugin("openstack", ohai_system)
allow(plugin).to receive(:hint?).with("openstack").and_return(openstack_hint)
allow(plugin).to receive(:hint?).with("hp").and_return(hp_hint)
plugin
before(:each) do
allow(plugin).to receive(:hint?).with("openstack").and_return(false)
plugin[:dmi] = nil
end

before do
context "when there is no relevant hint or dmi data" do
it "does not set any openstack data" do
plugin.run
expect(plugin[:openstack]).to be_nil
end
end

context "when there is no relevant hint" do
context "when DMI data is Openstack" do
context "and the metadata service is not available" do
before do
allow(plugin).to receive(:can_metadata_connect?).
with(Ohai::Mixin::Ec2Metadata::EC2_METADATA_ADDR, 80).
and_return(false)
plugin[:dmi] = { :system => { :all_records => [ { :Manufacturer => "OpenStack Foundation" } ] } }
plugin.run
end

it "does not set any openstack data" do
openstack_plugin.run
expect(ohai_data).to_not have_key("openstack")
end
it "sets openstack attribute" do
expect(plugin[:openstack][:provider]).to eq("openstack")
end

it "doesn't set metadata attributes" do
expect(plugin[:openstack][:instance_id]).to be_nil
end
end
end

context "when there is an `openstack` hint" do
let(:openstack_hint) { true }
context "when running on dreamhost" do
it "sets openstack provider attribute to dreamhost" do
plugin["etc"] = { "passwd" => { "dhc-user" => {} } }
allow(plugin).to receive(:can_metadata_connect?).
with(Ohai::Mixin::Ec2Metadata::EC2_METADATA_ADDR, 80).
and_return(false)
plugin[:dmi] = { :system => { :all_records => [ { :Manufacturer => "OpenStack Foundation" } ] } }
plugin.run
expect(plugin[:openstack][:provider]).to eq("dreamhost")
end
end

context "when the hint is present" do
context "and the metadata service is not available" do

before do
expect(openstack_plugin).to receive(:can_metadata_connect?).
allow(plugin).to receive(:can_metadata_connect?).
with(Ohai::Mixin::Ec2Metadata::EC2_METADATA_ADDR, 80).
and_return(false)
allow(plugin).to receive(:hint?).with("openstack").and_return(true)
plugin.run
end

it "sets openstack provider attribute if the hint is provided" do
expect(plugin[:openstack][:provider]).to eq("openstack")
end

it "does not set any openstack data" do
openstack_plugin.run
expect(ohai_data).to_not have_key("openstack")
it "doesn't set metadata attributes" do
expect(plugin[:openstack][:instance_id]).to be_nil
end
end

Expand Down Expand Up @@ -118,8 +141,8 @@
}
end

let(:openstack_metadata_version) { "2013-04-04" }
let(:openstack_metadata_endpoint) { "http://169.254.169.254/openstack/" }
let(:openstack_metadata_version) { "2009-04-04" }
let(:openstack_metadata_endpoint) { "http://169.254.169.254/" }

let(:openstack_metadata_values) do
'{
Expand All @@ -140,128 +163,103 @@

let(:http_client) { double("Net::HTTP", :read_timeout= => nil) }

def expect_get(url, response_body)
expect(http_client).to receive(:get).
def allow_get(url, response_body)
allow(http_client).to receive(:get).
with(url).
and_return(double("HTTP Response", :code => "200", :body => response_body))
end

def expect_get_response(url, response_body)
expect(http_client).to receive(:get_response).
def allow_get_response(url, response_body)
allow(http_client).to receive(:get_response).
with(url, nil, nil).
and_return(double("HTTP Response", :code => "200", :body => response_body))
end

before do
expect(openstack_plugin).to receive(:can_metadata_connect?).
allow(plugin).to receive(:hint?).with("openstack").and_return(true)
allow(plugin).to receive(:can_metadata_connect?).
with(Ohai::Mixin::Ec2Metadata::EC2_METADATA_ADDR, 80).
and_return(true)

allow(Net::HTTP).to receive(:start).
with(Ohai::Mixin::Ec2Metadata::EC2_METADATA_ADDR).
and_return(http_client)

allow(openstack_plugin).to receive(:best_api_version).and_return(metadata_version)
allow(plugin).to receive(:best_api_version).and_return(metadata_version)

expect_get("/#{metadata_version}/meta-data/", metadata_root)
allow_get("/#{metadata_version}/meta-data/", metadata_root)

metadata_values.each do |md_id, md_value|
expect_get("/#{metadata_version}/meta-data/#{md_id}", md_value)
allow_get("/#{metadata_version}/meta-data/#{md_id}", md_value)
end

expect_get_response(
allow_get_response(
URI.parse("#{openstack_metadata_endpoint}#{openstack_metadata_version}/meta_data.json"),
openstack_metadata_values
)

openstack_plugin.run
plugin.run
end

it "reads the reservation_id from the metadata service" do
expect(ohai_data["openstack"]["reservation_id"]).to eq("r-4tjvl99h")
expect(plugin["openstack"]["reservation_id"]).to eq("r-4tjvl99h")
end
it "reads the public_keys_0_openssh_key from the metadata service" do
expect(ohai_data["openstack"]["public_keys_0_openssh_key"]).to eq("SSH KEY DATA")
expect(plugin["openstack"]["public_keys_0_openssh_key"]).to eq("SSH KEY DATA")
end
it "reads the security_groups from the metadata service" do
expect(ohai_data["openstack"]["security_groups"]).to eq(["default"])
expect(plugin["openstack"]["security_groups"]).to eq(["default"])
end
it "reads the public_ipv4 from the metadata service" do
expect(ohai_data["openstack"]["public_ipv4"]).to eq("")
expect(plugin["openstack"]["public_ipv4"]).to eq("")
end
it "reads the ami_manifest_path from the metadata service" do
expect(ohai_data["openstack"]["ami_manifest_path"]).to eq("FIXME")
expect(plugin["openstack"]["ami_manifest_path"]).to eq("FIXME")
end
it "reads the instance_type from the metadata service" do
expect(ohai_data["openstack"]["instance_type"]).to eq("opc-tester")
expect(plugin["openstack"]["instance_type"]).to eq("opc-tester")
end
it "reads the instance_id from the metadata service" do
expect(ohai_data["openstack"]["instance_id"]).to eq("i-0000162a")
expect(plugin["openstack"]["instance_id"]).to eq("i-0000162a")
end
it "reads the local_ipv4 from the metadata service" do
expect(ohai_data["openstack"]["local_ipv4"]).to eq("172.31.7.23")
expect(plugin["openstack"]["local_ipv4"]).to eq("172.31.7.23")
end
it "reads the ari_id from the metadata service" do
expect(ohai_data["openstack"]["ari_id"]).to eq("ari-00000037")
expect(plugin["openstack"]["ari_id"]).to eq("ari-00000037")
end
it "reads the local_hostname from the metadata service" do
expect(ohai_data["openstack"]["local_hostname"]).to eq("ohai-7-system-test.opscode.us")
expect(plugin["openstack"]["local_hostname"]).to eq("ohai-7-system-test.opscode.us")
end
it "reads the placement_availability_zone from the metadata service" do
expect(ohai_data["openstack"]["placement_availability_zone"]).to eq("nova")
expect(plugin["openstack"]["placement_availability_zone"]).to eq("nova")
end
it "reads the ami_launch_index from the metadata service" do
expect(ohai_data["openstack"]["ami_launch_index"]).to eq("0")
expect(plugin["openstack"]["ami_launch_index"]).to eq("0")
end
it "reads the public_hostname from the metadata service" do
expect(ohai_data["openstack"]["public_hostname"]).to eq("ohai-7-system-test.opscode.us")
expect(plugin["openstack"]["public_hostname"]).to eq("ohai-7-system-test.opscode.us")
end
it "reads the hostname from the metadata service" do
expect(ohai_data["openstack"]["hostname"]).to eq("ohai-7-system-test.opscode.us")
expect(plugin["openstack"]["hostname"]).to eq("ohai-7-system-test.opscode.us")
end
it "reads the ami_id from the metadata service" do
expect(ohai_data["openstack"]["ami_id"]).to eq("ami-00000035")
expect(plugin["openstack"]["ami_id"]).to eq("ami-00000035")
end
it "reads the instance_action from the metadata service" do
expect(ohai_data["openstack"]["instance_action"]).to eq("none")
expect(plugin["openstack"]["instance_action"]).to eq("none")
end
it "reads the aki_id from the metadata service" do
expect(ohai_data["openstack"]["aki_id"]).to eq("aki-00000036")
expect(plugin["openstack"]["aki_id"]).to eq("aki-00000036")
end
it "reads the block_device_mapping_ami from the metadata service" do
expect(ohai_data["openstack"]["block_device_mapping_ami"]).to eq("vda")
expect(plugin["openstack"]["block_device_mapping_ami"]).to eq("vda")
end
it "reads the block_device_mapping_root from the metadata service" do
expect(ohai_data["openstack"]["block_device_mapping_root"]).to eq("/dev/vda")
expect(plugin["openstack"]["block_device_mapping_root"]).to eq("/dev/vda")
end
it "reads the provider from the metadata service" do
expect(ohai_data["openstack"]["provider"]).to eq("openstack")
end

context "Retreive openStack specific metadata" do
it "reads the availability_zone from the openstack metadata service" do
expect(ohai_data["openstack"]["metadata"]["availability_zone"]).to eq("nova")
end
it "reads the hostname from the openstack metadata service" do
expect(ohai_data["openstack"]["metadata"]["hostname"]).to eq("ohai.novalocal")
end
it "reads the launch_index from the openstack metadata service" do
expect(ohai_data["openstack"]["metadata"]["launch_index"]).to eq(0)
end
it "reads the meta from the openstack metadata service" do
expect(ohai_data["openstack"]["metadata"]["meta"]).to eq({ "priority" => "low", "role" => "ohaiserver" })
end
it "reads the name from the openstack metadata service" do
expect(ohai_data["openstack"]["metadata"]["name"]).to eq("ohai_spec")
end
it "reads the public_keys from the openstack metadata service" do
expect(ohai_data["openstack"]["metadata"]["public_keys"]).to eq({ "mykey" => "SSH KEY DATA" })
end
it "reads the uuid from the openstack metadata service" do
expect(ohai_data["openstack"]["metadata"]["uuid"]).to eq("00000000-0000-0000-0000-100000000000")
end
it "sets the provider to openstack" do
expect(plugin["openstack"]["provider"]).to eq("openstack")
end
end

end
end