CA-399229: Assert no host pending mandatory guidance for pool.join #6007
Conversation
ocaml/xapi/xapi_pool.ml
Outdated
| Client.Host.get_pending_guidances ~rpc ~session_id ~self:master | ||
| in | ||
| if master_pending_mandatory_guidances <> [] then ( | ||
| error "%s: %d mandatory guidances are pending for master %s: [%s]" |
There was a problem hiding this comment.
We should use the term "coordinator" rather than "master" now.
ocaml/xapi/xapi_pool.ml
Outdated
| Repository_helpers.assert_no_host_pending_mandatory_guidance ~__context | ||
| ~host:(Helpers.get_localhost ~__context) ; | ||
| let master = get_master ~rpc ~session_id in | ||
| let master_pending_mandatory_guidances = |
There was a problem hiding this comment.
This only checks pending guidances on the coordinator. Shouldn't the guidances be checked for all hosts instead?
Also, please use the names joiner and remote, as the rest of the code, it helps when reading the code to know what is being checked.
There was a problem hiding this comment.
I think we only need to check the guidances on the coordinator as it is the "joining host" and coordinator whose "software_version", "api versiion" and "db schema" are checked in the pre_join_checks. Outstanding mandatory guidances in other hosts will not affect pool.join.
There was a problem hiding this comment.
I think having any outstanding guidance anywhere will create unpredictable situations. Hosts need to talk to each other for migrations and I don't think you want to create a situation where this is not possible because guidance was ignored.
There was a problem hiding this comment.
But I think outstanding guidances on other pool members will not affect pool join, and you can apply them before or after a new host joins the pool, and with one more host in the pool, it might be helpful to apply some guidance as for example it will be easier for host evacuation.
There was a problem hiding this comment.
True if you look at in isolation. But you are dragging along an undefined situation. It's called mandatory for a reason.
There was a problem hiding this comment.
Yes, agree, outstanding mandatory guidance on any host results in an undefined situation.
But I think it is not only "pool.join" which should avoid operating in such situation, it should be avoided for any pool operation. While we will not check for it in any XAPI API call, we will only check it if the outstanding mandatory guidance will have a direct consequence for the API call, like "host.apply_updates" and "pool.join".
There was a problem hiding this comment.
In current design, I think it is XenCenter's responsibility to guide user to apply all outstanding mandatory guidance after a pool upgrade.
In pre-join check for pool.join, assert that there is no host pending mandatory guidance on the joining host or the pool coordinator. Signed-off-by: Gang Ji <gang.ji@cloud.com>
gangj
force-pushed
the
private/gangj/CA-399229
branch
from
dde70b3 to
4d2134e
Compare
| (* Assert that there is no host pending mandatory guidance on the joiner or | ||
| the remote pool coordinator. | ||
| *) | ||
| Repository_helpers.assert_no_host_pending_mandatory_guidance ~__context |
There was a problem hiding this comment.
Could the check on the joiner be extended to the recommended and full as well? The toolstack on the joiner will be restarted after joining. If there is a pending Toolstack restart (even as a full guidance), it probably means that a newer toolstack (than the one on the coordinator) on the joiner will start after joining.
But usually this is a rare case.
There was a problem hiding this comment.
I think only mandatory guidance is checked is because now RestartToolstack guidance only set as mandatory guidance.
@robhoes Is it possible that RestartToolstack guidance would be set as recommended or full guidance in updates so that we will need to cover all type of guidance here?
There was a problem hiding this comment.
I see it this way: any guidance that must be followed immediately to avoid things breaking must be part of the "mandatory guidance". This includes breaking pool join. And yes, usually we make restarting the Toolstack mandatory if the Toolstack is updated.
There was a problem hiding this comment.
usually we make restarting the Toolstack mandatory if the Toolstack is updated.
Yeah, in this case, it would be fine to check the mandatory guidance only.
In pre-join check for pool.join, assert that there is no host pending mandatory guidance on the joining host or the pool coordinator.