Fix logic for enabling 'swiftcheck' service
Currently used logic for enabling swiftcheck service is
not correct. Additional checks for Keystone availability
from Swift node should be added to HAProxy if management VIP
and Swift proxy IP addresses are from different L3 networks.
Change-Id: I9513fc9da02abdc24cc61a60c33181bb0fc9235b
Related-bug: #1548275
(cherry picked from commit 766335df60
)
This commit is contained in:
parent
545061d6c7
commit
5fe0ca3d3c
|
@ -42,8 +42,8 @@
|
|||
# (required) Array. This is an array of server names for the haproxy service
|
||||
#
|
||||
# [*bind_to_one*]
|
||||
# (optional) Boolean. If true, uses custom script checker w/ additional tests
|
||||
# Defaults to false.
|
||||
# (optional) Boolean. If false, uses custom script checker w/ additional tests
|
||||
# Defaults to true.
|
||||
#
|
||||
class openstack::ha::swift (
|
||||
$internal_virtual_ip,
|
||||
|
@ -55,17 +55,17 @@ class openstack::ha::swift (
|
|||
$internal_ssl = false,
|
||||
$internal_ssl_path = undef,
|
||||
$baremetal_virtual_ip = undef,
|
||||
$bind_to_one = false,
|
||||
$bind_to_one = true,
|
||||
) {
|
||||
|
||||
$bm_opt_tail = 'inter 15s fastinter 2s downinter 8s rise 3 fall 3'
|
||||
|
||||
if $bind_to_one {
|
||||
$http_check = 'httpchk'
|
||||
$balancermember_options = "check port 49001 ${bm_opt_tail}"
|
||||
} else {
|
||||
$http_check = 'httpchk HEAD /healthcheck HTTP/1.0'
|
||||
$balancermember_options = "check ${bm_opt_tail}"
|
||||
} else {
|
||||
$http_check = 'httpchk'
|
||||
$balancermember_options = "check port 49001 ${bm_opt_tail}"
|
||||
}
|
||||
|
||||
# defaults for any haproxy_service within this class
|
||||
|
|
|
@ -20,11 +20,11 @@ require 'spec_helper'
|
|||
|
||||
before :each do
|
||||
if params[:bind_to_one]
|
||||
@http_check = 'httpchk'
|
||||
@balancermember_options = "check port 49001 #{bm_opt_tail}"
|
||||
else
|
||||
@http_check = 'httpchk HEAD /healthcheck HTTP/1.0'
|
||||
@balancermember_options = "check #{bm_opt_tail}"
|
||||
else
|
||||
@http_check = 'httpchk'
|
||||
@balancermember_options = "check port 49001 #{bm_opt_tail}"
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -38,7 +38,7 @@ require 'spec_helper'
|
|||
:server_names => ['node-1', 'node-2'],
|
||||
:public_ssl => true,
|
||||
:public_ssl_path => '/var/lib/fuel/haproxy/public_swift.pem',
|
||||
:bind_to_one => true,
|
||||
:bind_to_one => false,
|
||||
}
|
||||
end
|
||||
|
||||
|
@ -72,6 +72,7 @@ require 'spec_helper'
|
|||
:ipaddresses => ['127.0.0.2', '127.0.0.3'],
|
||||
:public_virtual_ip => '192.168.0.1',
|
||||
:server_names => ['node-1', 'node-2'],
|
||||
:bind_to_one => true,
|
||||
}
|
||||
end
|
||||
|
||||
|
|
|
@ -0,0 +1,15 @@
|
|||
require 'ipaddr'
|
||||
|
||||
module Puppet::Parser::Functions
|
||||
newfunction(:has_ip_in_network, :type => :rvalue, :arity => 2, :doc => <<-EOS
|
||||
This function checks if IP addresses (1st arg) is a part of given IP network (2nd arg).
|
||||
EOS
|
||||
) do |args|
|
||||
begin
|
||||
return IPAddr.new(args[1]).include?(args[0])
|
||||
rescue ArgumentError => e
|
||||
raise Puppet::ParseError, "Can't check if #{args[1]} includes #{args[0]}: #{e}"
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
@ -37,10 +37,13 @@ class osnailyfacter::openstack_haproxy::openstack_haproxy_swift {
|
|||
|
||||
prepare_network_config(hiera_hash('network_scheme', {}))
|
||||
|
||||
# Check proxy and storage daemons binds on the same ip address
|
||||
$swift_api_ipaddr = get_network_role_property('swift/api', 'ipaddr')
|
||||
$swift_storage_ipaddr = get_network_role_property('swift/replication', 'ipaddr')
|
||||
$bind_to_one = ($swift_api_ipaddr == $swift_storage_ipaddr)
|
||||
# Check swift proxy and internal VIP are from the same IP network. If no then it's possible
|
||||
# to get network failure, so proxy couldn't access Keystone via VIP. In such cases swift
|
||||
# health check returns OK, but all requests forwarded from HAproxy fail, see LP#1459772
|
||||
# In order to detect such bad swift backends we enable a service which checks Keystone
|
||||
# availability from swift node. HAProxy monitors that service to get proper backend status.
|
||||
$swift_api_network = get_network_role_property('swift/api', 'network')
|
||||
$bind_to_one = has_ip_in_network($internal_virtual_ip, $swift_api_network)
|
||||
|
||||
# configure swift ha proxy
|
||||
class { '::openstack::ha::swift':
|
||||
|
|
|
@ -0,0 +1,34 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe 'has_ip_in_network' do
|
||||
let(:scope) { PuppetlabsSpec::PuppetInternals.scope }
|
||||
|
||||
it 'should exist' do
|
||||
Puppet::Parser::Functions.function('has_ip_in_network').should == 'function_has_ip_in_network'
|
||||
end
|
||||
|
||||
it 'should throw an error on invalid arguments number' do
|
||||
lambda {
|
||||
scope.function_has_ip_in_network(['foo'])
|
||||
}.should(raise_error(ArgumentError))
|
||||
end
|
||||
|
||||
it 'should raise an error if invalid address or network is specified' do
|
||||
lambda {
|
||||
scope.function_has_ip_in_network(['foo', 'bar'])
|
||||
}.should(raise_error(Puppet::ParseError))
|
||||
end
|
||||
|
||||
it 'should return true if IP address is from CIDR' do
|
||||
cidr = '192.168.0.0/16'
|
||||
ipaddr = '192.168.33.66'
|
||||
scope.function_has_ip_in_network([ipaddr, cidr]).should == true
|
||||
end
|
||||
|
||||
it 'should return false if IP address is not from CIDR' do
|
||||
cidr = '192.168.0.0/255.255.0.0'
|
||||
ipaddr = '172.16.0.1'
|
||||
scope.function_has_ip_in_network([ipaddr, cidr]).should == false
|
||||
end
|
||||
|
||||
end
|
|
@ -28,18 +28,18 @@ describe manifest do
|
|||
end
|
||||
|
||||
let (:bind_to_one) {
|
||||
api_ip = Noop.puppet_function 'get_network_role_property', 'swift/api', 'ipaddr'
|
||||
storage_ip = Noop.puppet_function 'get_network_role_property', 'swift/replication', 'ipaddr'
|
||||
api_ip == storage_ip
|
||||
internal_virtual_ip = Noop.hiera_structure 'network_metadata/vips/management/ipaddr'
|
||||
api_net = Noop.puppet_function 'get_network_role_property', 'swift/api', 'network'
|
||||
Noop.puppet_function 'has_ip_in_network', internal_virtual_ip, api_net
|
||||
}
|
||||
|
||||
let (:bm_options) {
|
||||
bm_opt_tail = 'inter 15s fastinter 2s downinter 8s rise 3 fall 3'
|
||||
bind_to_one ? "check port 49001 #{bm_opt_tail}" : "check #{bm_opt_tail}"
|
||||
bind_to_one ? "check #{bm_opt_tail}" : "check port 49001 #{bm_opt_tail}"
|
||||
}
|
||||
|
||||
let (:http_check) {
|
||||
bind_to_one ? 'httpchk' : 'httpchk HEAD /healthcheck HTTP/1.0'
|
||||
bind_to_one ? 'httpchk HEAD /healthcheck HTTP/1.0' : 'httpchk'
|
||||
}
|
||||
|
||||
let(:haproxy_config_opts) do
|
||||
|
|
|
@ -33,6 +33,7 @@ describe manifest do
|
|||
rabbit_user = Noop.hiera_structure('rabbit/user', 'nova')
|
||||
rabbit_password = Noop.hiera_structure('rabbit/password')
|
||||
network_scheme = Noop.hiera_hash 'network_scheme'
|
||||
internal_virtual_ip = Noop.hiera_structure('network_metadata/vips/management/ipaddr')
|
||||
|
||||
if swift_proxies_num < 2
|
||||
ring_replicas = 2
|
||||
|
@ -49,9 +50,8 @@ describe manifest do
|
|||
}
|
||||
|
||||
let (:bind_to_one) {
|
||||
api_ip = Noop.puppet_function 'get_network_role_property', 'swift/api', 'ipaddr'
|
||||
storage_ip = Noop.puppet_function 'get_network_role_property', 'swift/replication', 'ipaddr'
|
||||
api_ip == storage_ip
|
||||
api_net = Noop.puppet_function 'get_network_role_property', 'swift/api', 'network'
|
||||
Noop.puppet_function 'has_ip_in_network', internal_virtual_ip, api_net
|
||||
}
|
||||
|
||||
let(:ssl_hash) { Noop.hiera_hash 'use_ssl' }
|
||||
|
@ -116,10 +116,11 @@ describe manifest do
|
|||
context 'with enabled internal TLS for swift' do
|
||||
swift_endpoint = Noop.hiera_structure 'use_ssl/swift_internal_hostname'
|
||||
it {
|
||||
if bind_to_one
|
||||
unless bind_to_one
|
||||
should contain_class('openstack::swift::status').with(
|
||||
'endpoint' => "https://#{swift_endpoint}:8080",
|
||||
'only_from' => "127.0.0.1 240.0.0.2 #{storage_nets} #{mgmt_nets}",
|
||||
'scan_target' => "#{internal_virtual_ip}:5000",
|
||||
).that_comes_before('Class[swift::dispersion]')
|
||||
else
|
||||
should_not contain_class('openstack::swift::status')
|
||||
|
@ -131,9 +132,10 @@ describe manifest do
|
|||
|
||||
context 'with disabled internal TLS for swift' do
|
||||
it {
|
||||
if bind_to_one
|
||||
unless bind_to_one
|
||||
should contain_class('openstack::swift::status').with(
|
||||
'only_from' => "127.0.0.1 240.0.0.2 #{storage_nets} #{mgmt_nets}",
|
||||
'scan_target' => "#{internal_virtual_ip}:5000",
|
||||
).that_comes_before('Class[swift::dispersion]')
|
||||
else
|
||||
should_not contain_class('openstack::swift::status')
|
||||
|
|
Loading…
Reference in New Issue