Skip to content
This repository has been archived by the owner on Aug 29, 2018. It is now read-only.

BZ1199901, BZ1199903, BZ1199904 creating and deleting monitors #6092

Merged
merged 1 commit into from Mar 13, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions routing-daemon/bin/oo-admin-ctl-routing
Expand Up @@ -135,7 +135,7 @@ list-monitors
create-monitor <name> <path> <up-code>
Create a new monitor with the specified name that queries <path>
and expects to receive <up-code> to indicate that the pool is up.
delete-monitor <name>
delete-monitor <name> <type>
Delete the specified monitor.
list-pool-members <pool>
List all members of the specified pool.
Expand Down Expand Up @@ -205,8 +205,8 @@ argvs.each do |argv|
lb.create_monitor *argv

when 'delete-monitor'
raise ArgumentError.new "Requires a pool name." unless 1 == argv.length
puts "Deleting monitor #{argv[0]}."
raise ArgumentError.new "Requires a pool name and monitor type." unless 2 == argv.length
puts "Deleting monitor #{argv[0]} of type #{argv[1]}."
lb.delete_monitor *argv

when 'list-pool-members'
Expand Down
3 changes: 1 addition & 2 deletions routing-daemon/conf/routing-daemon.conf
Expand Up @@ -32,8 +32,7 @@ HTTP_PORT=80
#MONITOR_NAME=monitor_ose_%a_%n
#MONITOR_PATH=/health_check.php
#MONITOR_UP_CODE=1
MONITOR_TYPE=http-ecv
#MONITOR_TYPE=https-ecv
MONITOR_TYPE=http
#MONITOR_INTERVAL=10
#MONITOR_TIMEOUT=5

Expand Down
Expand Up @@ -342,13 +342,13 @@ def create_monitor monitor_name, path, up_code, type, interval, timeout
monitors.push monitor_name
end

def delete_monitor monitor_name, pool_name=nil
def delete_monitor monitor_name, pool_name, type
raise LBControllerException.new "Monitor not found: #{monitor_name}" unless monitors.include? monitor_name

# :delete_monitor blocks
# if the corresponding pool is being deleted (if one is specified) or
# if the monitor is being created.
queue_op Operation.new(:delete_monitor, [monitor_name]), @ops.select {|op| (op.type == :create_monitor && op.operands[0] == monitor_name) || (pool_name && op.type == :delete_pool && op.operands[0] == pool_name)}
queue_op Operation.new(:delete_monitor, [monitor_name, pool_name, type]), @ops.select {|op| (op.type == :create_monitor && op.operands[0] == monitor_name) || (pool_name && op.type == :delete_pool && op.operands[0] == pool_name)}

monitors.delete monitor_name
end
Expand Down
5 changes: 3 additions & 2 deletions routing-daemon/lib/openshift/routing/controllers/batched.rb
Expand Up @@ -104,10 +104,11 @@ def create_monitor monitor_name, path, up_code, type, interval, timeout
@monitors.push monitor_name
end

def delete_monitor monitor_name, pool_name=nil
def delete_monitor monitor_name, pool_name, type
raise LBControllerException.new "Monitor not found: #{monitor_name}" unless monitors.include? monitor_name

@lb_model.delete_monitor monitor_name if monitors.include? monitor_name
# we assume the types won't be modified at runtime
@lb_model.delete_monitor monitor_name, type if monitors.include? monitor_name
Copy link
Contributor

Choose a reason for hiding this comment

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

This line shows that things are kind of tricky now. Is it safe to assume that there will never be two monitors with the same name but different types? If not, we'll need to change monitors from an array of monitor names to an array of tuples of monitor name and monitor type and then modify this check accordingly.

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'm not sure.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably a safe assumption given that the type is set in the configuration file and won't be changing at runtime at least.

Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth adding a comment somewhere to that effect.


@monitors.delete monitor_name
end
Expand Down
Expand Up @@ -101,12 +101,12 @@ def delete_pool pool_name
def create_monitor monitor_name, path, up_code, type, interval, timeout
end

# delete_monitor :: String, String -> undefined
# delete_monitor :: String, String, String -> undefined
# Note: The pool_name is optional but is required for some backends so that
# the daemon can specify an associated pool that must be deleted first. In
# particular, the asynchronous controller used with the LBaaS model will
# make delete_monitor operations block on related delete_pool operations.
def delete_monitor monitor_name, pool_name=nil
def delete_monitor monitor_name, pool_name, type
end

# Push pending pool add_member and delete_member operations to the
Expand Down
4 changes: 2 additions & 2 deletions routing-daemon/lib/openshift/routing/controllers/simple.rb
Expand Up @@ -102,10 +102,10 @@ def create_monitor monitor_name, path, up_code, type, interval, timeout
monitors.push monitor_name
end

def delete_monitor monitor_name, pool_name=nil
def delete_monitor monitor_name, pool_name, type
raise LBControllerException.new "Monitor not found: #{monitor_name}" unless monitors.include? monitor_name

@lb_model.delete_monitor monitor_name
@lb_model.delete_monitor monitor_name, type

monitors.delete monitor_name
end
Expand Down
3 changes: 2 additions & 1 deletion routing-daemon/lib/openshift/routing/daemon.rb
Expand Up @@ -326,12 +326,13 @@ def delete_application app_name, namespace
# having the application's name and namespace in the monitor's name).
if @monitor_name_format && @monitor_name_format.match(/%a/) && @monitor_name_format.match(/%n/)
monitor_name = generate_monitor_name app_name, namespace
monitor_path = generate_monitor_path app_name, namespace
unless monitor_name.nil? or monitor_name.empty? or monitor_path.nil? or monitor_path.empty?
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, that was a weird defect. Good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thx!

@logger.info "Deleting unused monitor: #{monitor_name}"
# We pass pool_name to delete_monitor because some backends need the
# name of the pool so that they will block the delete_monitor
# operation until any corresponding delete_pool operation completes.
@lb_controller.delete_monitor monitor_name, pool_name
@lb_controller.delete_monitor monitor_name, pool_name, @monitor_type
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions routing-daemon/lib/openshift/routing/models/dummy.rb
Expand Up @@ -35,8 +35,8 @@ def create_monitor monitor_name, path, up_code, type, interval, timeout
[] # If using AsyncLoadBalancerController, return an array of jobids.
end

def delete_monitor monitor_name
@logger.debug "delete monitor #{monitor_name}"
def delete_monitor monitor_name, type
@logger.debug "delete monitor #{monitor_name} of type #{type}"
[] # If using AsyncLoadBalancerController, return an array of jobids.
end

Expand Down
11 changes: 5 additions & 6 deletions routing-daemon/lib/openshift/routing/models/f5-icontrol-rest.rb
Expand Up @@ -102,20 +102,19 @@ def get_monitor_names

def create_monitor monitor_name, path, up_code, type, interval, timeout
type = type == 'https-ecv' ? 'https' : 'http'
post(url: "https://#{@host}/mgmt/tm/ltm/monitor/#{type}/#{monitor_name}",
post(url: "https://#{@host}/mgmt/tm/ltm/monitor/#{type}",
payload: {
"name" => monitor_name,
"interval" => interval,
"recv" => up_code,
"send" => "HEAD #{path} HTTP/1.0\\r\\n\\r\\n",
"timeout" => timeout,
"upInterval" => interval,
})
}.to_json)
end

def delete_monitor monitor_name
# TODO: delete_monitor needs a 'type' parameter for the REST API.
delete(url: "https://#{@host}/mgmt/tm/ltm/monitor/http/#{monitor_name}")
#delete(url: "https://#{@host}/mgmt/tm/ltm/monitor/#{type}/#{monitor_name}")
def delete_monitor monitor_name, type
delete(url: "https://#{@host}/mgmt/tm/ltm/monitor/#{type}/#{monitor_name}")
end

def get_pool_members pool_name
Expand Down
2 changes: 1 addition & 1 deletion routing-daemon/lib/openshift/routing/models/lbaas.rb
Expand Up @@ -138,7 +138,7 @@ def create_monitor monitor_name, path, up_code, type, interval, timeout
end

# Returns [String] of job ids.
def delete_monitor monitor_name
def delete_monitor monitor_name, type
response = RestClient.delete("http://#{@host}/loadbalancers/tenant/#{@tenant}/monitors/#{monitor_name}",
:content_type => :json,
:accept => :json,
Expand Down
4 changes: 2 additions & 2 deletions routing-daemon/lib/openshift/routing/models/load_balancer.rb
Expand Up @@ -42,8 +42,8 @@ def get_monitor_names
def create_monitor monitor_name, path, up_code, type, interval, timeout
end

# delete_monitor :: String -> undefined
def delete_monitor monitor_name
# delete_monitor :: String, String -> undefined
def delete_monitor monitor_name, type
end

def get_pool_certificates pool_name
Expand Down
2 changes: 1 addition & 1 deletion routing-daemon/lib/openshift/routing/models/nginx.rb
Expand Up @@ -65,7 +65,7 @@ def get_monitor_names
def create_monitor monitor_name, path, up_code, type, interval, timeout
end

def delete_monitor monitor_name
def delete_monitor monitor_name, type
end

def get_pool_members pool_name
Expand Down