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

routing-daemon: Resynchronize with load-balancer #6405

Merged

Conversation

Miciah
Copy link
Contributor

@Miciah Miciah commented Jul 25, 2016

Resynchronize the routing daemon with the load balancer when the daemon attempts to perform on operation on an unrecognized pool or pool member.

The intention of this commit is to improve the daemon's reliability when something other than the current instance of the daemon changes the load balancer's state, for example the system administrator or clustered routing daemons.

AsyncLoadBalancerController::Pool, BatchedLoadBalancerController::Pool, LoadBalancerController::Pool, SimpleLoadBalancerController::Pool: Add synch method.

AsyncLoadBalancerController, BatchedLoadBalancerController, LoadBalancerController, SimpleLoadBalancerController: Add synch_pools method.

RoutingDaemon: Add with_pool method, which checks whether a pool exists, resynchronizes the daemon with the load balancer if it is not found (using the new synch_pools controller method), optionally creates the pool if it is still not found, and invokes the supplied block if the pool is found.

RoutingDaemon#create_application: Take pool_name as a parameter rather than generating the value itself since callers will already have generated the value themselves.

RoutingDaemon#delete_application, RoutingDaemon#add_endpoint, RoutingDaemon#remove_endpoint, RoutingDaemon#add_alias, RoutingDaemon#remove_alias, RoutingDaemon#add_ssl, RoutingDaemon#remove_ssl: Use the daemon's new with_pool method in order to check whether the pool exists, esynchronize the daemon with the load balancer if an operation is attempted on an unrecognized pool, and (in the case of add methods) create the pool if it does not exist.

RoutingDaemon#remove_endpoint: Use the controller's new synch method for pools to resynchronize the daemon with the load balancer if the operation is attempted on an unknown pool member.

Resynchronize the routing daemon with the load balancer when the daemon
attempts to perform on operation on an unrecognized pool or pool member.

The intention of this commit is to improve the daemon's reliability when
something other than the current instance of the daemon changes the load
balancer's state, for example the system administrator or clustered routing
daemons.

AsyncLoadBalancerController::Pool, BatchedLoadBalancerController::Pool,
LoadBalancerController::Pool, SimpleLoadBalancerController::Pool: Add
synch method.

AsyncLoadBalancerController, BatchedLoadBalancerController,
LoadBalancerController, SimpleLoadBalancerController: Add synch_pools
method.

RoutingDaemon: Add with_pool method, which checks whether a pool exists,
resynchronizes the daemon with the load balancer if it is not found (using
the new synch_pools controller method), optionally creates the pool if it
is still not found, and invokes the supplied block if the pool is found.

RoutingDaemon#create_application: Take pool_name as a parameter rather than
generating the value itself since callers will already have generated the
value themselves.

RoutingDaemon#delete_application, RoutingDaemon#add_endpoint,
RoutingDaemon#remove_endpoint, RoutingDaemon#add_alias,
RoutingDaemon#remove_alias, RoutingDaemon#add_ssl,
RoutingDaemon#remove_ssl: Use the daemon's new with_pool method in order to
check whether the pool exists, resynchronize the daemon with the load
balancer if an operation is attempted on an unrecognized pool, and (in the
case of add methods) create the pool if it does not exist.

RoutingDaemon#remove_endpoint: Use the controller's new synch method for
pools to resynchronize the daemon with the load balancer if the operation
is attempted on an unknown pool member.
pool_name = generate_pool_name app_name, namespace
@logger.info "Adding new alias #{alias_str} to pool #{pool_name}"
@lb_controller.pools[pool_name].add_alias alias_str
with_pool app_name, namespace do |pool|
Copy link
Member

Choose a reason for hiding this comment

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

Should this be passing create_if_nil=false, since it was not creating the application previously?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It makes sense to me that all the add_ methods should as a rule create the pool if it does not exist. Using the default of create_if_nil=true here does indeed change the behaviour, but I see no reason for add_alias to be an exception to that rule. There is a risk if events are handled out of order (for example, by clustered routing-daemon instances, each picking event notifications off a queue), and in taking this approach, the result of handling operations out of order could be that we create an extra pool (for example, if we get a create-application event, an add-alias event, and a delete-application event and handle the last two out of order). On the other hand, if we specified create_if_nil=false, then if corresponding create-application and add-alias events were swapped, the result would be that we would fail to create the pool and alias when we should have done so (the pool would be created by any subsequent add-public-endpoint event, but the alias would never be created).

@tiwillia
Copy link
Member

@Miciah Looks good to me sir, besides the one small comment I made.

I looked over the RoutingDaemon with these changes in mind too and I don't see it affecting anything there. Fairly confident that there aren't any other issues, although I havn't tested it myself.

@Miciah
Copy link
Contributor Author

Miciah commented Jul 27, 2016

Thanks!

@tiwillia
Copy link
Member

[merge] please!

@openshift-bot
Copy link

openshift-bot commented Jul 27, 2016

Online Merge Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/test_pull_requests/9346/) (Image: devenv_5808)

@openshift-bot
Copy link

Evaluated for online merge up to d3465c7

@openshift-bot openshift-bot merged commit f020953 into openshift:master Jul 27, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants