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

Fix: crmadmin: return error if DC is not elected #2902 #3606 #3716

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

Conversation

aleksei-burlakov
Copy link

@aleksei-burlakov aleksei-burlakov commented Nov 2, 2024

If the DC is not yet elected, the crmadmin will return an error. (This change complements #3606).

@aleksei-burlakov aleksei-burlakov force-pushed the fix-crmadmin-dclookup-return-undetermined branch from a35d796 to aac55a2 Compare November 2, 2024 12:43
Copy link
Contributor

@kgaillot kgaillot left a comment

Choose a reason for hiding this comment

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

thanks

@@ -234,8 +234,7 @@ designated_controller_event_cb(pcmk_ipc_api_t *controld_api,
}

reply = (const pcmk_controld_api_reply_t *) event_data;
out->message(out, "dc", reply->host_from);
data->rc = pcmk_rc_ok;
data->rc = out->message(out, "dc", reply->host_from);
Copy link
Contributor

Choose a reason for hiding this comment

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

A message's return value should indicate only whether the message could be successfully output. We can set data->rc directly based on whether host_from is NULL.

If I follow correctly, this would change crmadmin's exit status from CRM_EX_OK to CRM_EX_INDETERMINATE ("Could not determine status")? If so, I think CRM_EX_NOSUCH ("No such object") might be better, since we could definitively determine that there is no DC. Alternatively, we could create a new rc and exit code for "DC not elected yet".

@aleksei-burlakov aleksei-burlakov force-pushed the fix-crmadmin-dclookup-return-undetermined branch from aac55a2 to 2f31653 Compare November 11, 2024 13:40
…sterLabs#3606

If the DC is not yet elected, the crmadmin will return an error.
(This change complements ClusterLabs#3606).
@aleksei-burlakov aleksei-burlakov force-pushed the fix-crmadmin-dclookup-return-undetermined branch from 2f31653 to f0471b5 Compare November 11, 2024 13:45
Copy link
Contributor

@kgaillot kgaillot left a comment

Choose a reason for hiding this comment

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

This looks great, thanks

@@ -273,6 +273,7 @@ typedef enum crm_exit_e {
CRM_EX_NOT_YET_IN_EFFECT = 111, //!< Requested item is not in effect
CRM_EX_INDETERMINATE = 112, //!< Could not determine status
CRM_EX_UNSATISFIED = 113, //!< Requested item does not satisfy constraints
CRM_EX_DC_NOT_ELECTED_YET = 114, //!< DC is not yet elected, e.g. right after cluster restart
Copy link
Contributor

Choose a reason for hiding this comment

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

For brevity, let's use CRM_EX_NO_DC

@@ -235,7 +235,7 @@ designated_controller_event_cb(pcmk_ipc_api_t *controld_api,

reply = (const pcmk_controld_api_reply_t *) event_data;
out->message(out, "dc", reply->host_from);
data->rc = pcmk_rc_ok;
data->rc = reply->host_from ? pcmk_rc_ok : CRM_EX_DC_NOT_ELECTED_YET;
Copy link
Contributor

Choose a reason for hiding this comment

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

Whoops, I'm a little slow this morning ... data->rc is a return code, not an exit code. It will get mapped to an exit code by crmadmin.

We need a new pcmk_rc_no_dc return code in enum pcmk_rc_e in include/crm/common/results.h, and then pcmk_rc2exitc() can map it to CRM_EX_NO_DC.

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.

2 participants