CA-299944: Cross pool update apply failed because HTTP connection not enabled in CC mode #3739
Conversation
Rather than going over http over the network to fetch the RPMs and yum metadata, connect to the local xapi and have the requests forwarded over stunnel. This behaviour is activated by calling the (undocumented) Pool_update.attach API with the new parameter 'use_localhost_proxy' set to true. The absence of the parameter is treated as 'false' and xapi will fall back to the old behaviour. Signed-off-by: Jon Ludlam <jonathan.ludlam@citrix.com>
Signed-off-by: Min Li <min.li1@citrix.com>
1.Adding more log 2.Catch exception for invalid UUID and close the fd. Signed-off-by: Min Li <min.li1@citrix.com>
|
I have refined the comments, mainly trying to handle the invalid host uuid scenario. |
1.Add naples_release. 2.Do not use 'get_localhost_uuid'. Signed-off-by: Min Li <min.li1@citrix.com>
ocaml/xapi/xapi_pool_update.ml
Outdated
| |None -> | ||
| debug "Caught exception while get Host by uuid %s" host_uuid; | ||
| Http_svr.response_badrequest ~req s; | ||
| req.Request.close <- true |
There was a problem hiding this comment.
In spite of the indentation, this line is part of the | None -> case (only). I think that rather than having it here, we should put this line near the top of pool_update_download_handler, because we always want to handle just one request, not matter what the response will be, so we never want to keep the connection alive after the response.
Even before this patch, we don't actually close the connection on the path that calls Http_svr.response_forbidden, which looks wrong to me. Moving the line to the top would fix that as well.
Searching for close <- true reveals that many similar handlers do it this way.
There was a problem hiding this comment.
For both 'forbidden' and 'badrequest' we instruct the client to close the connection, which should cause us to close too (see https://github.com/xapi-project/xen-api-libs-transitional/blob/master/http-svr/http.ml#L30-L33 and https://github.com/xapi-project/xen-api-libs-transitional/blob/master/http-svr/http.ml#L57-L60). However, it's good form for us to close proactively so we should set req.Request.close to true in all cases, as @robhoes suggests.
There was a problem hiding this comment.
Note that the we may end up handling multiple requests on the one connection - since after the initial headers we proxy all the traffic, any subsequent requests will also be proxied, until either the client or remote server closes the connection.
There was a problem hiding this comment.
Thanks, @robhoes @jonludlam
Finally I move the req.Request.close <- true near the top of the handler as Rob suggested, I found other handlers do in this way.
- Put 'req.Request.close <- true' on the top of the handler Signed-off-by: Min Li <min.li1@citrix.com>
|
Code updated, please help to take a look again and merge if no other concern. Many thanks, Rob. @robhoes |
7 participants
Job 2288870 passed.
Signed-off-by: Min Li min.li1@citrix.com