CA-392887: set_tls_config immediately after enabling clustering
#5644
Conversation
Vincent-lau
force-pushed
the
private/shul2/tls-config
branch
from
6909565 to
68e4040
Compare
ocaml/xapi/xapi_cluster_host.ml
Outdated
| @@ -239,15 +239,15 @@ let resync_host ~__context ~host = | |||
| (Ref.string_of self) ; | |||
| Xapi_clustering.Daemon.enable ~__context ; | |||
| maybe_switch_cluster_stack_version ~__context ~self ~cluster_stack ; | |||
| let verify = Stunnel_client.get_verify_by_default () in | |||
| set_tls_config ~__context ~self ~verify ; | |||
| (* Note that join_internal and enable both use the clustering lock *) | |||
| Client.Client.Cluster_host.enable ~rpc ~session_id ~self | |||
| ) ; | |||
There was a problem hiding this comment.
There is an implicit else branch here for the "joined and not enabled" case, which now excludes set_tls_config as well. This is correct?
There was a problem hiding this comment.
I think this is a good point. If the cluster daemon is not enabled, then in theory xapi-clusterd service itself should not be running, hence any requests to xapi-clusterd should fail. That means not only should we do the set_tls_config inside enabled case, but the observer initialisation should be moved into the enabled path as well. Need to verify that....
There was a problem hiding this comment.
I believe there is no need to do anything if xapi-clusterd is not enabled, I have resturctured the code for this
Vincent-lau
force-pushed
the
private/shul2/tls-config
branch
from
68e4040 to
3e8cd41
Compare
|
requesting a review from @lindig since he originally wrote this code to make sure this refactor is valid |
|
xrt tests pending, let's see wait for the tests before merging this |
|
@Vincent-lau how are the tests? Can we merge this? |
|
Let's draft this for now as it might take some time to triage the xrt issues |
Vincent-lau
force-pushed
the
private/shul2/tls-config
branch
from
3e8cd41 to
a869550
Compare
Previously xapi calls `set_tls_config` regardless of whether a host has joined or enabled, which will restart the remote server of xapi-clusterd. In the meantime, another xapi-clusterd might also be joining, which causes `distribute_state` to be called while the remote server is restarting. Now remove the `set_tls_config`, this is because `join_internal` already creates a tls_config and passes it to xapi-clusterd, but xapi-clusterd does not store that tls_config in its db, it just starts the http server with that tls config. Modifying xapi-clusterd to store that config will be done in a separate PR. Moreover, `cluster_host.enable` also calls `set_tls_config`, which means there is no need to call `set_tls_config` if the cluster host is joined but not enabled. Also move the observer and watcher creation into the not joined case, since cluster_host.enable already calls them and there is no need to call them if the host is not enabled. This does not, however, solve the whole problem. For that, we need to make sure that `distribute_state` and `set_tls_config` cannot happen at the same time. More generally, any remote calls cannot happen while `tls_config` is running. Hence we need them to hold the same lock. This will be done in xapi-clusterd. Signed-off-by: Vincent Liu <shuntian.liu2@cloud.com>
Vincent-lau
force-pushed
the
private/shul2/tls-config
branch
from
a869550 to
5975562
Compare
|
A few small refactoring to fix bugs in xrt, should be ready to go now, accompanied by a xapi-clusterd PR |
Previously xapi calls
set_tls_configafter the join call, which will restart the remote server of xapi-clusterd. In the meantime, another xapi-clusterd might also be joining, which causesdistribute_stateto be called while the remote server is restarting.Now move the
set_tls_configimmediately after the cluster daemon is enabled to reduce the chance of race. Moreover,join_internalalready creates a tls_config and passes it to xapi-clusterd, there is no point resetting the tls config immediately afterwards.This does not, however, solve the whole problem. For that, we need to make sure that
distribute_stateandset_tls_configcannot happen at the same time. More generally, any remote calls cannot happen whiletls_configis running. Hence we need them to hold the same lock. This will be done in xapi-clusterd.