Skip to content

allow the status-section to be updated on a crm_shadow commit with an explicit switch #4106

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.

Sign up for GitHub

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

Open
wenningerk wants to merge 2 commits into
base: main
Choose a base branch
from
+25 −0
Open

allow the status-section to be updated on a crm_shadow commit with an explicit switch #4106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5387893
Feature: crm_shadow: allow to commit a CIB including status-section
wenningerk Jun 11, 2026
09bbbfc
Feature: cibadmin: allow altering status section without triggering r…
wenningerk Jun 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions include/crm/cib/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,8 @@ cib__client_triggers_refresh(const char *name)
"crm_attribute",
"crm_node",
"crm_resource",
"crm_shadow_status",

@nrwahl2 nrwahl2 May 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need the commit message to reference RHEL-70283, and to explain why we're doing this (since future contributors may not be able to access this RHEL Jira issue).

We've encountered numerous issues when commit messages don't explain the reasoning for the changes in the commit. We often end up, much later, removing or changing behavior that we depend on. At best, we spend a long time trying to figure out why things are the way they are, and we're often not confident in the conclusions we reach.

@wenningerk wenningerk Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As it doesn't fix the issue right away I can add that being able to alter the status section is a pre-requisite for RHEL-70283 and that this commit gives us back this possibility.

@wenningerk wenningerk Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Of course I would have loved to get some pcs feedback regarding the approach.
Which is why I put it here for now as something generically useful. But the reference to what triggered the implementation is probably still a good idea.

@wenningerk wenningerk Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a bit of data that should help understand why the feature was added including a reference to the Jira Issue.

"cibadmin_status",
"crm_ticket",
NULL);
}
Expand Down
11 changes: 11 additions & 0 deletions tools/cibadmin.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ static struct {
gboolean input_stdin;
gboolean allow_create;
gboolean force;
gboolean update_status;
gboolean get_node_path;
gboolean no_children;
gboolean score_update;
Expand Down Expand Up @@ -433,6 +434,11 @@ cibadmin_handle_command(pcmk__output_t *out,
"entire CIB", options.cib_section);
}

if (options.update_status) {
free(crm_system_name);
crm_system_name = strdup("cibadmin_status");
}

rc = cib__create_signon(&cib_conn);
if (rc != pcmk_rc_ok) {
exit_code = pcmk_rc2exitc(rc);
Expand Down Expand Up @@ -807,6 +813,11 @@ static GOptionEntry addl_entries[] = {
"result",
NULL },

{ "update-status", 'u', G_OPTION_FLAG_NONE, G_OPTION_ARG_NONE,
&options.update_status,
"(Advanced) Apply CIB update without triggering a controller refresh",
NULL },

// @COMPAT Deprecated since 3.0.0
{ "local", 'l', G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_NONE, &options.local,
"(deprecated)", NULL },
Expand Down
12 changes: 12 additions & 0 deletions tools/crm_shadow.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ static struct {
gboolean force;
gboolean batch;
gboolean full_upload;
gboolean update_status;
gchar *validate_with;
} options = {
.cmd_options = cib_sync_call,
Expand Down Expand Up @@ -679,6 +680,12 @@ commit_shadow_file(GError **error)
return;
}

if (options.update_status) {

@nrwahl2 nrwahl2 May 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't understand how this would fix the issue. Based on RHEL-70283, it appears that the issue is that adding devices to a fence_scsi or fence_mpath resource using pcs stonith update causes resource restarts. However, pcs doesn't use crm_shadow at all. So updating the crm_shadow CLI tool doesn't seem like it would affect the behavior of pcs.

@wenningerk wenningerk Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It doesn't fix the issue right away. There is already RHELHA-1011 to take care of this from the pcs side. Just after your commit there was no low-level-tool left you could use to update the status section. Both for getting an atomic update and for the interface crm_shadow seems like the reasonable answer.

options.full_upload = TRUE;
free(crm_system_name);
crm_system_name = strdup("crm_shadow_status");
}

filename = get_shadow_file(options.instance);
if (check_file_exists(filename, true, error) != pcmk_rc_ok) {
goto done;
Expand Down Expand Up @@ -1139,6 +1146,11 @@ static GOptionEntry addl_entries[] = {
&options.validate_with,
"(Advanced) Create an older configuration version", NULL },

{ "update-status", 'u', G_OPTION_FLAG_NONE, G_OPTION_ARG_NONE,
&options.update_status,
"(Advanced) Upload entire CIB with --commit, without triggering a "
"controller refresh", NULL },

{ NULL }
};

Expand Down