-
Notifications
You must be signed in to change notification settings - Fork 59
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
IPv6: dhcp/provisioner: make ipv6 aware #1723
base: master
Are you sure you want to change the base?
Changes from all commits
c956414
b223609
c03c408
4e70134
ee488f9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -92,6 +92,7 @@ class Network | |
attr_reader :vlan, :use_vlan | ||
attr_reader :add_bridge, :add_ovs_bridge, :bridge_name | ||
attr_reader :conduit | ||
attr_reader :ip_version | ||
|
||
def initialize(node, net, data) | ||
@node = node | ||
|
@@ -112,6 +113,13 @@ def initialize(node, net, data) | |
# let's resolve this only if needed | ||
@interface = nil | ||
@interface_list = nil | ||
|
||
require "ipaddr" | ||
@ip_version = if IPAddr.new(@subnet).ipv6? | ||
"6" | ||
else | ||
"4" | ||
end | ||
end | ||
|
||
def interface | ||
|
@@ -124,6 +132,30 @@ def interface_list | |
@interface_list | ||
end | ||
|
||
def cidr_to_netmask | ||
if @ip_version == "6" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style/ConditionalAssignment: Use the return of the conditional for variable assignment and comparison. |
||
range = 128 | ||
else | ||
range = 32 | ||
end | ||
|
||
cidr = Integer(@netmask) rescue nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style/RescueModifier: Avoid using rescue in its modifier form. (https://github.com/bbatsov/ruby-style-guide#no-rescue-modifiers) |
||
if cidr && !(1..range).cover?(cidr) | ||
Chef::Log.error("Invalid CIDR netmask '#{cidr}' > '#{range}'") | ||
return @netmask | ||
end | ||
|
||
if cidr | ||
if @ip_version == "6" | ||
IPAddr.new('ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff').mask(cidr).to_s | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://github.com/SUSE/style-guides/blob/master/Ruby.md#stylestringliterals) |
||
else | ||
IPAddr.new('255.255.255.255').mask(cidr).to_s | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://github.com/SUSE/style-guides/blob/master/Ruby.md#stylestringliterals) |
||
end | ||
else | ||
@netmask | ||
end | ||
end | ||
|
||
protected | ||
|
||
def resolve_interface_info | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -152,6 +152,10 @@ def make_zone(zone) | |
|
||
zonefile_entries | ||
end | ||
def address_version(address) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Layout/EmptyLineBetweenDefs: Use empty lines between method definitions. (https://github.com/bbatsov/ruby-style-guide#empty-lines-between-methods) |
||
'ip6addr' if IPAddr.new(address).ipv6? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://github.com/SUSE/style-guides/blob/master/Ruby.md#stylestringliterals) |
||
'ip4addr' if IPAddr.new(address).ipv4? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://github.com/SUSE/style-guides/blob/master/Ruby.md#stylestringliterals) |
||
end | ||
|
||
# Create our basic zone infrastructure. | ||
zones = Mash.new | ||
|
@@ -207,7 +211,7 @@ def make_zone(zone) | |
alias_name = "#{net_name}-#{alias_name_no_net}" if alias_name_no_net | ||
end | ||
cluster_zone[:hosts][base_name] = Mash.new | ||
cluster_zone[:hosts][base_name][:ip4addr] = network.address | ||
cluster_zone[:hosts][base_name][address_version(network.address)] = network.address | ||
cluster_zone[:hosts][base_name][:alias] = alias_name if alias_name | ||
end | ||
|
||
|
@@ -238,7 +242,7 @@ def make_zone(zone) | |
temporary_dhcp.each_pair do |address, value| | ||
_, base_name, alias_name = value | ||
cluster_zone[:hosts][base_name] = Mash.new | ||
cluster_zone[:hosts][base_name][:ip4addr] = address | ||
cluster_zone[:hosts][base_name][address_version(address)] = address | ||
cluster_zone[:hosts][base_name][:alias] = alias_name if alias_name | ||
end | ||
|
||
|
@@ -257,7 +261,8 @@ def make_zone(zone) | |
base_name="#{net_name}-#{base_name}" | ||
end | ||
cluster_zone[:hosts][base_name] = Mash.new | ||
cluster_zone[:hosts][base_name][:ip4addr] = network[:allocated_by_name][host][:address] | ||
address = network[:allocated_by_name][host][:address] | ||
cluster_zone[:hosts][base_name][address_version(address)] = address | ||
end | ||
end | ||
|
||
|
@@ -388,8 +393,7 @@ def make_zone(zone) | |
end | ||
end | ||
|
||
### FIXME Change to "any" once IPv6 support has been implemented | ||
admin_addr6 = "none" | ||
admin_addr6 = "any" | ||
if node[:dns][:enable_designate] && !node[:dns][:master] | ||
node[:dns][:forwarders].push master_ip | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
|
||
default[:dhcp][:interfaces] = ["eth0"] | ||
default[:dhcp][:options] = [ | ||
default[:dhcp][:options][:v4] = [ | ||
"ddns-update-style none", | ||
"allow booting", | ||
"option option-128 code 128 = string", | ||
|
@@ -10,4 +10,17 @@ | |
"option dhcp-client-debug code 226 = unsigned integer 16", | ||
"option dhcp-client-debug 0" | ||
] | ||
default[:dhcp][:options][:v6] = [ | ||
"ddns-update-style none", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Layout/IndentArray: Use 2 spaces for indentation in an array, relative to the start of the line where the left square bracket is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a sensible comment from hound 😉 It seems we are using 4 spaces everywhere else. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, it's times like this I wonder should I just fix the hound "error" everywhere in the file to get it to pass or just ignore it. I mean we use 4 spaces everywhere here, so am keeping with the style. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You have two choices: Ignore Hound, or push a fix-up commit and then base your changes on top of that. |
||
"allow booting", | ||
"option option-128 code 128 = string", | ||
"option option-129 code 129 = text", | ||
"option dhcp-client-state code 225 = unsigned integer 16", | ||
"option dhcp-client-state 0", | ||
"option dhcp-client-debug code 226 = unsigned integer 16", | ||
"option dhcp-client-debug 0", | ||
"option dhcp6.bootfile-url code 59 = string", | ||
"option dhcp6.client-arch-type code 61 = array of unsigned integer 16", | ||
"option dhcp6.vendor-class code 16 = {integer 32, integer 16, string}" | ||
] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
module DhcpHelper | ||
def self.config_filename(base, ip_version, extension = ".conf") | ||
if ip_version == "4" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style/ConditionalAssignment: Use the return of the conditional for variable assignment and comparison. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style/ConditionalAssignment: Use the return of the conditional for variable assignment and comparison. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style/ConditionalAssignment: Use the return of the conditional for variable assignment and comparison. |
||
extra = "" | ||
else | ||
extra = ip_version | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be fair I had envisioned this as always returning "#{base}#{extra}#{extension}" where extra gets set to 6 if ip_version is 6, but this is very much a nit. |
||
"#{base}#{extra}#{extension}" | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,17 +33,25 @@ | |
directory "/etc/dhcp3/subnets.d" | ||
directory "/etc/dhcp3/hosts.d" | ||
|
||
file "/etc/dhcp3/groups.d/group_list.conf" do | ||
# This needs to be evaled. | ||
admin_network = Chef::Recipe::Barclamp::Inventory.get_network_by_type(node, "admin") | ||
address = admin_network.address | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lint/UselessAssignment: Useless assignment to variable - address. (https://github.com/bbatsov/ruby-style-guide#underscore-unused-vars) |
||
intfs = [admin_network.interface] | ||
|
||
group_list = DhcpHelper.config_filename("group_list", admin_network.ip_version) | ||
file "/etc/dhcp3/groups.d/#{group_list}" do | ||
owner "root" | ||
group "root" | ||
mode 0644 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style/NumericLiteralPrefix: Use 0o for octal literals. (https://github.com/bbatsov/ruby-style-guide#numeric-literal-prefixes) |
||
end | ||
file "/etc/dhcp3/subnets.d/subnet_list.conf" do | ||
subnet_list = DhcpHelper.config_filename("subnet_list", admin_network.ip_version) | ||
file "/etc/dhcp3/subnets.d/#{subnet_list}" do | ||
owner "root" | ||
group "root" | ||
mode 0644 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style/NumericLiteralPrefix: Use 0o for octal literals. (https://github.com/bbatsov/ruby-style-guide#numeric-literal-prefixes) |
||
end | ||
file "/etc/dhcp3/hosts.d/host_list.conf" do | ||
host_list = DhcpHelper.config_filename("host_list", admin_network.ip_version) | ||
file "/etc/dhcp3/hosts.d/#{host_list}" do | ||
owner "root" | ||
group "root" | ||
mode 0644 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style/NumericLiteralPrefix: Use 0o for octal literals. (https://github.com/bbatsov/ruby-style-guide#numeric-literal-prefixes) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to only write these out if ip_version is 6? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True, when originally writing this I was thinking about an environment that was dual stack. But your right, I'm already making assumptions based on the admin network and what version it is in order to create the correct dhcpd[6] config. I'll make the change to only write out for the ip_version of the admin net. |
||
|
@@ -59,22 +67,20 @@ | |
not_if "test -f /etc/dhcp3/omapi.key" | ||
end | ||
|
||
# This needs to be evaled. | ||
intfs = [Chef::Recipe::Barclamp::Inventory.get_network_by_type(node, "admin").interface] | ||
address = Chef::Recipe::Barclamp::Inventory.get_network_by_type(node, "admin").address | ||
|
||
d_opts = node[:dhcp][:options] | ||
d_opts = node[:dhcp][:options]["v#{admin_network.ip_version}"] | ||
dhcpd_conf = DhcpHelper.config_filename("dhcpd", admin_network.ip_version) | ||
|
||
case node[:platform_family] | ||
when "debian" | ||
case node[:lsb][:codename] | ||
when "natty","oneiric","precise" | ||
template "/etc/dhcp/dhcpd.conf" do | ||
template "/etc/dhcp/#{dhcpd_conf}" do | ||
owner "root" | ||
group "root" | ||
mode 0644 | ||
source "dhcpd.conf.erb" | ||
variables(options: d_opts) | ||
variables(options: d_opts, ip_version: admin_network.ip_version) | ||
if node[:provisioner][:enable_pxe] | ||
notifies :restart, "service[dhcp3-server]" | ||
end | ||
|
@@ -90,12 +96,12 @@ | |
end | ||
end | ||
else | ||
template "/etc/dhcp3/dhcpd.conf" do | ||
template "/etc/dhcp3/#{dhcpd_conf}" do | ||
owner "root" | ||
group "root" | ||
mode 0644 | ||
source "dhcpd.conf.erb" | ||
variables(options: d_opts) | ||
variables(options: d_opts, ip_version: admin_network.ip_version) | ||
if node[:provisioner][:enable_pxe] | ||
notifies :restart, "service[dhcp3-server]" | ||
end | ||
|
@@ -115,17 +121,17 @@ | |
|
||
dhcp_config_file = case | ||
when node[:platform_version].to_f >= 6 | ||
"/etc/dhcp/dhcpd.conf" | ||
"/etc/dhcp/#{dhcpd_conf}" | ||
else | ||
"/etc/dhcpd.conf" | ||
"/etc/#{dhcpd_conf}" | ||
end | ||
|
||
template dhcp_config_file do | ||
owner "root" | ||
group "root" | ||
mode 0644 | ||
source "dhcpd.conf.erb" | ||
variables(options: d_opts) | ||
variables(options: d_opts, ip_version: admin_network.ip_version) | ||
if node[:provisioner][:enable_pxe] | ||
notifies :restart, "service[dhcp3-server]" | ||
end | ||
|
@@ -143,12 +149,12 @@ | |
end | ||
|
||
when "suse" | ||
template "/etc/dhcpd.conf" do | ||
template "/etc/#{dhcpd_conf}" do | ||
owner "root" | ||
group "root" | ||
mode 0644 | ||
source "dhcpd.conf.erb" | ||
variables(options: d_opts) | ||
variables(options: d_opts, ip_version: admin_network.ip_version) | ||
if node[:provisioner][:enable_pxe] | ||
notifies :restart, "service[dhcp3-server]" | ||
end | ||
|
@@ -168,7 +174,7 @@ | |
|
||
service "dhcp3-server" do | ||
if %w(suse rhel).include?(node[:platform_family]) | ||
service_name "dhcpd" | ||
service_name DhcpHelper.config_filename("dhcpd", admin_network.ip_version, "") | ||
elsif node[:platform] == "ubuntu" | ||
case node[:lsb][:codename] | ||
when "maverick" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,12 @@ log-facility local7; | |
# Fix for https://bugzilla.opensuse.org/show_bug.cgi?id=961536 | ||
always-reply-rfc1048 true; | ||
|
||
<% if @ip_version == "6" -%> | ||
include "/etc/dhcp3/groups.d/group_list6.conf"; | ||
include "/etc/dhcp3/subnets.d/subnet_list6.conf"; | ||
include "/etc/dhcp3/hosts.d/host_list6.conf"; | ||
<% else -%> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are most of the included options commented out? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They are the default values from the "sample" config, there commented out there too. Really there there because maybe there something we may or may not what to tweak and left them here seeing as I haven't really testing the actual running dhcpd6 yet.. other then it starts. Also it was WIP so figured doesn't hurt. I'll remove them for now. |
||
include "/etc/dhcp3/groups.d/group_list.conf"; | ||
include "/etc/dhcp3/subnets.d/subnet_list.conf"; | ||
include "/etc/dhcp3/hosts.d/host_list.conf"; | ||
<% end -%> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/CyclomaticComplexity: Cyclomatic complexity for cidr_to_netmask is too high. [7/6]
Metrics/PerceivedComplexity: Perceived complexity for cidr_to_netmask is too high. [10/7]