LGTM, thanks!

On Monday, September 12, 2016 at 5:31:42 PM UTC+1, Brian Foley wrote:
>
> This defaults to 20 -- the same as the current hardcoded option. 
>
> Signed-off-by: Brian Foley <[email protected]> 
> --- 
>  lib/http/server.py   | 11 ++++++++--- 
>  lib/server/noded.py  | 22 ++++++++++++++++------ 
>  lib/server/rapi.py   | 16 ++++++++++++---- 
>  man/ganeti-noded.rst | 10 ++++++++-- 
>  man/ganeti-rapi.rst  |  7 ++++++- 
>  5 files changed, 50 insertions(+), 16 deletions(-) 
>
> diff --git a/lib/http/server.py b/lib/http/server.py 
> index 9a4563e..b4a41c8 100644 
> --- a/lib/http/server.py 
> +++ b/lib/http/server.py 
> @@ -479,9 +479,8 @@ class HttpServer(http.HttpBase, asyncore.dispatcher): 
>    """Generic HTTP server class 
>   
>    """ 
> -  MAX_CHILDREN = 20 
>   
> -  def __init__(self, mainloop, local_address, port, handler, 
> +  def __init__(self, mainloop, local_address, port, max_clients, handler, 
>                 ssl_params=None, ssl_verify_peer=False, 
>                 request_executor_class=None, ssl_verify_callback=None): 
>      """Initializes the HTTP server 
> @@ -492,6 +491,11 @@ class HttpServer(http.HttpBase, asyncore.dispatcher): 
>      @param local_address: Local IP address to bind to 
>      @type port: int 
>      @param port: TCP port to listen on 
> +    @type max_clients: int 
> +    @param max_clients: maximum number of client connections 
> +        open simultaneously. 
> +    @type handler: HttpServerHandler 
> +    @param handler: Request handler object 
>      @type ssl_params: HttpSslParams 
>      @param ssl_params: SSL key and certificate 
>      @type ssl_verify_peer: bool 
> @@ -524,6 +528,7 @@ class HttpServer(http.HttpBase, asyncore.dispatcher): 
>      self._children = [] 
>      self.set_socket(self.socket) 
>      self.accepting = True 
> +    self.max_clients = max_clients 
>      mainloop.RegisterSignal(self) 
>   
>    def Start(self): 
> @@ -549,7 +554,7 @@ class HttpServer(http.HttpBase, asyncore.dispatcher): 
>      """ 
>      if not quick: 
>        # Don't wait for other processes if it should be a quick check 
> -      while len(self._children) > self.MAX_CHILDREN: 
> +      while len(self._children) > self.max_clients: 
>          try: 
>            # Waiting without a timeout brings us into a potential DoS 
> situation. 
>            # As soon as too many children run, we'll not respond to new 
> diff --git a/lib/server/noded.py b/lib/server/noded.py 
> index a5e05dd..9375fa0 100644 
> --- a/lib/server/noded.py 
> +++ b/lib/server/noded.py 
> @@ -1283,7 +1283,7 @@ class 
> NodeRequestHandler(http.server.HttpServerHandler): 
>      return backend.CleanupImportExport(params[0]) 
>   
>   
> -def CheckNoded(_, args): 
> +def CheckNoded(options, args): 
>    """Initial checks whether to run or exit with a failure. 
>   
>    """ 
> @@ -1291,6 +1291,12 @@ def CheckNoded(_, args): 
>      print >> sys.stderr, ("Usage: %s [-f] [-d] [-p port] [-b ADDRESS]" % 
>                            sys.argv[0]) 
>      sys.exit(constants.EXIT_FAILURE) 
> + 
> +  if options.max_clients < 1: 
> +    print >> sys.stderr, ("%s --max-clients argument must be >= 1" % 
> +                          sys.argv[0]) 
> +    sys.exit(constants.EXIT_FAILURE) 
> + 
>    try: 
>      codecs.lookup("string-escape") 
>    except LookupError: 
> @@ -1404,11 +1410,11 @@ def PrepNoded(options, _): 
>    handler = NodeRequestHandler() 
>   
>    mainloop = daemon.Mainloop() 
> -  server = \ 
> -    http.server.HttpServer(mainloop, options.bind_address, options.port, 
> -                           handler, ssl_params=ssl_params, 
> ssl_verify_peer=True, 
> -                           request_executor_class=request_executor_class, 
> -                           ssl_verify_callback=SSLVerifyPeer) 
> +  server = http.server.HttpServer( 
> +      mainloop, options.bind_address, options.port, options.max_clients, 
> +      handler, ssl_params=ssl_params, ssl_verify_peer=True, 
> +      request_executor_class=request_executor_class, 
> +      ssl_verify_callback=SSLVerifyPeer) 
>    server.Start() 
>   
>    return (mainloop, server) 
> @@ -1437,6 +1443,10 @@ def Main(): 
>    parser.add_option("--no-mlock", dest="mlock", 
>                      help="Do not mlock the node memory in ram", 
>                      default=True, action="store_false") 
> +  parser.add_option("--max-clients", dest="max_clients", 
> +                    default=20, type="int", 
> +                    help="Number of simultaneous connections accepted" 
> +                    " by noded") 
>   
>    daemon.GenericMain(constants.NODED, parser, CheckNoded, PrepNoded, 
> ExecNoded, 
>                       default_ssl_cert=pathutils.NODED_CERT_FILE, 
> diff --git a/lib/server/rapi.py b/lib/server/rapi.py 
> index 9782ada..fc1d27c 100644 
> --- a/lib/server/rapi.py 
> +++ b/lib/server/rapi.py 
> @@ -318,6 +318,11 @@ def CheckRapi(options, args): 
>                            sys.argv[0]) 
>      sys.exit(constants.EXIT_FAILURE) 
>   
> +  if options.max_clients < 1: 
> +    print >> sys.stderr, ("%s --max-clients argument must be >= 1" % 
> +                          sys.argv[0]) 
> +    sys.exit(constants.EXIT_FAILURE) 
> + 
>    ssconf.CheckMaster(options.debug) 
>   
>    # Read SSL certificate (this is a little hackish to read the cert as 
> root) 
> @@ -344,10 +349,9 @@ def PrepRapi(options, _): 
>   
>    users.Load(pathutils.RAPI_USERS_FILE) 
>   
> -  server = \ 
> -    http.server.HttpServer(mainloop, options.bind_address, options.port, 
> -                           handler, 
> -                           ssl_params=options.ssl_params, 
> ssl_verify_peer=False) 
> +  server = http.server.HttpServer( 
> +      mainloop, options.bind_address, options.port, options.max_clients, 
> +      handler, ssl_params=options.ssl_params, ssl_verify_peer=False) 
>    server.Start() 
>   
>    return (mainloop, server) 
> @@ -377,6 +381,10 @@ def Main(): 
>                      default=False, action="store_true", 
>                      help=("Disable anonymous HTTP requests and require" 
>                            " authentication")) 
> +  parser.add_option("--max-clients", dest="max_clients", 
> +                    default=20, type="int", 
> +                    help="Number of simultaneous connections accepted" 
> +                    " by ganeti-rapi") 
>   
>    daemon.GenericMain(constants.RAPI, parser, CheckRapi, PrepRapi, 
> ExecRapi, 
>                       default_ssl_cert=pathutils.RAPI_CERT_FILE, 
> diff --git a/man/ganeti-noded.rst b/man/ganeti-noded.rst 
> index 3321cb8..df725e4 100644 
> --- a/man/ganeti-noded.rst 
> +++ b/man/ganeti-noded.rst 
> @@ -9,8 +9,9 @@ ganeti-noded - Ganeti node daemon 
>  Synopsis 
>  -------- 
>   
> -**ganeti-noded** [-f] [-d] [-p *PORT*] [-b *ADDRESS*] [-i *INTERFACE*] 
> -[--no-mlock] [--syslog] [--no-ssl] [-K *SSL_KEY_FILE*] [-C 
> *SSL_CERT_FILE*] 
> +| **ganeti-noded** [-f] [-d] [-p *PORT*] [-b *ADDRESS*] [-i *INTERFACE*] 
> +| [\--max-clients *CLIENTS*] [\--no-mlock] [\--syslog] [\--no-ssl] 
> +| [-K *SSL_KEY_FILE*] [-C *SSL_CERT_FILE*] 
>   
>  DESCRIPTION 
>  ----------- 
> @@ -38,6 +39,11 @@ option.  The ``-b`` option can be used to specify the 
> address to bind 
>  to (defaults to ``0.0.0.0``); alternatively, the ``-i`` option can be 
>  used to specify the interface to bind do. 
>   
> +The maximum number of simultaneous client connections may be configured 
> +with the ``--max-clients`` option. This defaults to 20. Connections 
> +above this count are accepted, but no responses are sent until enough 
> +connections are closed. 
> + 
>  Ganeti noded communication is protected via SSL, with a key 
>  generated at cluster init time. This can be disabled with the 
>  ``--no-ssl`` option, or a different SSL key and certificate can be 
> diff --git a/man/ganeti-rapi.rst b/man/ganeti-rapi.rst 
> index 3cded7b..2111f82 100644 
> --- a/man/ganeti-rapi.rst 
> +++ b/man/ganeti-rapi.rst 
> @@ -10,7 +10,7 @@ Synopsis 
>  -------- 
>   
>  | **ganeti-rapi** [-d] [-f] [-p *PORT*] [-b *ADDRESS*] [-i *INTERFACE*] 
> -| [\--no-ssl] [-K *SSL_KEY_FILE*] [-C *SSL_CERT_FILE*] 
> +| [\--max-clients] [\--no-ssl] [-K *SSL_KEY_FILE*] [-C *SSL_CERT_FILE*] 
>  | [\--require-authentication] 
>   
>  DESCRIPTION 
> @@ -34,6 +34,11 @@ it will not be able to reach the RAPI interface and 
> will attempt to 
>  restart it all the time. Alternatively to setting the IP with ``--b``, 
>  the ``-i`` option can be used to specify the interface to bind do. 
>   
> +The maximum number of simultaneous client connections may be configured 
> +with the ``--max-clients`` option. This defaults to 20. Connections 
> +above this count are accepted, but no responses are sent until enough 
> +connections are closed. 
> + 
>  See the *Ganeti remote API* documentation for further information. 
>   
>  Requests are logged to ``@LOCALSTATEDIR@/log/ganeti/rapi-daemon.log``, 
> -- 
> 2.8.0.rc3.226.g39d4020 
>
>

Reply via email to