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

fail with useful error on unmatched interface (bsc#990745) #575

Closed
wants to merge 7 commits into from
Closed

fail with useful error on unmatched interface (bsc#990745) #575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
893bfbb
add comments to help explain cryptic code
Jul 28, 2016
498bfb1
don't use cryptic 1- or 2-byte variable names
Jul 28, 2016
b434a5d
make #lookup_interface_info always return a 3-element Array
Jul 28, 2016
df4d076
output useful debugging on unmatched interface (bsc#990745)
Jul 27, 2016
d257291
fail with useful error on unmatched admin interface (bsc#990745)
Jul 28, 2016
8d84239
remove unnecessary rescue nil
Jul 28, 2016
15bc4b2
fix indentation
Aug 12, 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
155 changes: 125 additions & 30 deletions chef/cookbooks/barclamp/libraries/barclamp_library.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ def self.bus_index(bus_order, path)
999
end

# Returns an array of interfaces like ["em1", "em2", "em3", "em4"],
# sorted by bus_order
def self.sort_ifs(map, bus_order)
answer = map.sort{|a,b|
aindex = Barclamp::Inventory.bus_index(bus_order, a[1]["path"])
Expand All @@ -95,6 +97,14 @@ def self.sort_ifs(map, bus_order)

# IMPORTANT: This needs to be kept in sync with the get_bus_order method in
# node_object.rb in the Crowbar framework.
#
# Returns something like
# [
# "0000:00/0000:00:03.0/0000:01:00.0",
# "0000:00/0000:00:03.0/0000:01:00.1",
# "0000:00/0000:00:1c.4/0000:06:00.0",
# "0000:00/0000:00:1c.4/0000:06:00.1"
# ]
def self.get_bus_order(node)
bus_order = nil
node["network"]["interface_map"].each do |data|
Expand All @@ -111,6 +121,22 @@ def self.get_bus_order(node)
bus_order
end

# Each Hash in the network.conduit_map list looks something like:
#
# {
# "pattern": "dual/.*/crowbar$",
# "conduit_list": {
# "intf0" => { "if_list" => ["?1g1"] },
# "intf1" => { "if_list" => ["?1g2"] },
# "intf2" => { "if_list" => ["?1g1"] },
# "bastion" => { "if_list" => [ "1g2"] }
# }
# }
#
# This method searches that list for the Hash whose value of the
# "pattern" key matches the selected network mode / number of
# interfaces / role, and from the matching Hash, returns the
# sub-Hash corresponding to the value of the "conduit_list" key.
def self.get_conduits(node)
conduits = nil
node["network"]["conduit_map"].each do |data|
Expand All @@ -137,10 +163,40 @@ def self.get_conduits(node)
conduits
end

# Returns something like
# {
# "em4" => {
# "path" => "0000:00/0000:00:1c.4/0000:06:00.1",
# "speeds" => ["10m", "100m", "1g"]
# },
# "em1" => {
# "path" => "0000:00/0000:00:03.0/0000:01:00.0",
# "speeds" => ["100m", "1g", "10g"]
# },
# "em2" => {
# "path" => "0000:00/0000:00:03.0/0000:01:00.1",
# "speeds" => ["100m", "1g", "10g"]
# },
# "em3" => {
# "path" => "0000:00/0000:00:1c.4/0000:06:00.0",
# "speeds" => ["10m", "100m", "1g"]
# }
# }
def self.get_detected_intfs(node)
node.automatic_attrs["crowbar_ohai"]["detected"]["network"]
end

# Creates a copy of the nested Hash returned by get_conduits,
# with every interface designator pattern (e.g. "?1g2") replaced
# with a matching interface of the correct speed which the node has.
#
# Returns something like
# {
# "intf0" => { "if_list" => [ "eth0" ] },
# "intf1" => { "if_list" => [ "eth0" ] },
# "intf2" => { "if_list" => [ "eth0" ] },
# "bastion" => { "if_list" => [ "eth1" ] }
# }
def self.build_node_map(node)
bus_order = Barclamp::Inventory.get_bus_order(node)
conduits = Barclamp::Inventory.get_conduits(node)
Expand All @@ -149,11 +205,14 @@ def self.build_node_map(node)

if_list = get_detected_intfs(node)

# build a set of maps <intf-designator> -> <OS intf>
# designators are <speed><#> (speed = 100m, 1g etc). # is a count of interfaces of the same speed.
# OS intf is the name given to the interface by the operating system
# The intf-designator is 'stable' in terms of renumbering because of addition/removal of add on cards (across machines)

# build an if_remap Hash mapping <intf-designator> to <OS-intf>
# intf-designators are of the format <speed><i>, where:
# - speed is a value like 100m, 1g, 10g etc.
# - i is an integer ordering interfaces of the same speed
# OS-intf is the name given to the interface by the operating system
#
# The intf-designator is 'stable' in terms of renumbering because of
# addition/removal of add on cards (across machines)
sorted_ifs = Barclamp::Inventory.sort_ifs(if_list, bus_order)
if_remap = {}
count_map = {}
Expand All @@ -167,20 +226,39 @@ def self.build_node_map(node)
count_map[speed] = count + 1
end
end

# if_remap now contains something like
# {
# "100m1" => "em1",
# "1g1" => "em1",
# "10g1" => "em1",
# "100m2" => "em2",
# "1g2" => "em2",
# "10g2" => "em2",
# "10m1" => "em3",
# "100m3" => "em3",
# "1g3" => "em3",
# "10m2" => "em4",
# "100m4" => "em4",
# "1g4" => "em4"
# }
ans = {}
conduits.each do |k,v|
conduits.each do |conduit, conduit_data|
hash = {}
v.each do |mk, mv|
if mk == "if_list"
hash["if_list"] = v["if_list"].map do |if_ref|
map_if_ref(if_remap, if_ref)
conduit_data.each do |conduit_key, conduit_val|
if conduit_key == "if_list"
hash["if_list"] = conduit_data["if_list"].map do |if_ref|
r = map_if_ref(if_remap, if_ref)
if r.nil?
Chef::Log.debug "Unable to find interface for #{node.fqdn} " \
"matching #{if_ref} out of #{if_remap}"
end
r
end
else
hash[mk] = mv
hash[conduit_key] = conduit_val
end
end
ans[k] = hash
ans[conduit] = hash
end

ans
Expand All @@ -198,31 +276,48 @@ def self.map_if_ref(if_map, ref)
if_cnt = m[3]
desired = speeds.index(m[2])
found = nil
filter = lambda { |x|
filter = lambda do |x|
found = if_map["#{speeds[x]}#{if_cnt}"] unless found
}
end
case m[1]
when "+"
(desired..speeds.length).each(&filter)
when "-"
desired.downto(0,&filter)
when "?"
(desired..speeds.length).each(&filter)
desired.downto(0,&filter) unless found
else
found = if_map[ref]
end
found
when "+"
(desired..speeds.length).each(&filter)
when "-"
desired.downto(0, &filter)
when "?"
(desired..speeds.length).each(&filter)
desired.downto(0, &filter) unless found
else
found = if_map[ref]
end
found
end

# Find the interface(s) and teaming mode to use for the given
# conduit on the given node. Optionally provide a
# intf_to_if_map precalculated by #build_node_map if you are
# calling this in a loop and want to avoid it being calculated
# on each call.
#
# Returns an [interface, if_list, teaming_mode] Array:
# interface:
# The interface providing access to the given conduit.
# If teaming is enabled, this will be something like bond0.
# if_list:
# The list of physical interfaces used. If teaming is enabled,
# this will be the backing interfaces behind bondN, otherwise
# it will be a singleton list.
# team_mode:
# The integer representing the teaming mode, as specified
# by the relevant conduit_list entry in the network barclamp proposal
def self.lookup_interface_info(node, conduit, intf_to_if_map = nil)
intf_to_if_map = Barclamp::Inventory.build_node_map(node) if intf_to_if_map.nil?

return [nil, nil] if intf_to_if_map[conduit].nil?
return [nil, nil, nil] if intf_to_if_map[conduit].nil?

c_info = intf_to_if_map[conduit]
interface_list = c_info["if_list"]
team_mode = c_info["team_mode"] rescue nil
conduit_info = intf_to_if_map[conduit]
interface_list = conduit_info["if_list"]
team_mode = conduit_info["team_mode"]

return [interface_list[0], interface_list, nil] if interface_list.size == 1

Expand Down
4 changes: 4 additions & 0 deletions chef/cookbooks/provisioner/recipes/update_nodes.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ def find_node_boot_mac_addresses(node, admin_data_net)
result = []
admin_interfaces = admin_data_net.interface_list
admin_interfaces.each do |interface|
if interface.nil?
raise "Failed to find admin node interface for node #{node.fqdn}! " \
"Check your network barclamp proposal."
end
Copy link
Member

Choose a reason for hiding this comment

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

Thinking more about this: no objection about this part, as it's no regression, but I think it's not enough. We do want to avoid a crash in chef-client here, as having a rogue node could result, thanks to this, in a DoS on part of the deployment infrastructure. I'll submit a patch.

Copy link
Member

Choose a reason for hiding this comment

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

See #803

node["network"]["interfaces"][interface]["addresses"].each do |addr, addr_data|
next if addr_data["family"] != "lladdr"
result << addr unless result.include? addr
Expand Down