Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CA-399229: Assert no host pending mandatory guidance for pool.join #6007

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gangj
Copy link
Contributor

@gangj gangj commented Sep 19, 2024

In pre-join check for pool.join, assert that there is no host pending mandatory guidance on the joining host or the pool coordinator.

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]"
Copy link
Member

Choose a reason for hiding this comment

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

We should use the term "coordinator" rather than "master" now.

Copy link
Member

@robhoes robhoes left a comment

Choose a reason for hiding this comment

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

Minor comment below, otherwise this is fine.

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 =
Copy link
Member

@psafont psafont Sep 19, 2024

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@gangj gangj Sep 19, 2024

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

True if you look at in isolation. But you are dragging along an undefined situation. It's called mandatory for a reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants