PR#49822: Issue 49640 - Cleanup plugin bootstrap logging - 389-ds-base

389-ds-base

#49822 Issue 49640 - Cleanup plugin bootstrap logging

Closed 3 years ago by spichugi. Opened 5 years ago by spichugi.

spichugi/389-ds-base plugin_error_fix into master

Issue 49640 - Cleanup plugin bootstrap logging

Simon Pichugin • 5 years ago

2fa0408

ldap/servers/slapd/config.c

file modified

+11 -3

		`@@ -526,10 +526,18 @@`
		`if (e == NULL) {`
		`continue;`
		`}`
		`- if (plugin_setup(e, 0, 0, 1, returntext) != 0) {`
		`- slapi_log_err(SLAPI_LOG_TRACE, "slapd_bootstrap_config", "Application of plugin failed, maybe already there?\n");`
		`- } else {`
		`+ rc = plugin_setup(e, 0, 0, 1, returntext);`
		`+ if (rc == 0) {`
		`slapi_log_err(SLAPI_LOG_TRACE, "slapd_bootstrap_config", "Application of plugin SUCCESS\n");`
		`+ } else if (rc == 1) {`
		`+ /* It means that the plugin entry already exists */`
		`+ slapi_log_err(SLAPI_LOG_TRACE, "slapd_bootstrap_config",`
		`+ "The plugin entry [%s] in the configfile %s was invalid. %s\n",`
		`+ slapi_entry_get_dn(e), configfile, returntext);`
mreynolds commented 5 years ago Is this logging line redundant from what you did in plugin.c?
		`+ } else {`
		`+ slapi_log_err(SLAPI_LOG_ERR, "slapd_bootstrap_config",`
		`+ "The plugin entry [%s] in the configfile %s was invalid. %s\n",`
		`+ slapi_entry_get_dn(e), configfile, returntext);`
		`}`
		`slapi_entry_free(e);`
		`}`

ldap/servers/slapd/plugin.c

file modified

+6 -4

		`@@ -2786,12 +2786,12 @@`
		`}`

		`if ((existname = plugin_exists(slapi_entry_get_sdn_const(plugin_entry))) != NULL) {`
		`- slapi_log_err(SLAPI_LOG_ERR, "plugin_setup", "The plugin named %s already exists, "`
		`- "or is already setup.\n",`
		`+ slapi_log_err(SLAPI_LOG_TRACE, "plugin_setup", "The plugin named %s already exists, "`
		`+ "or is already setup.\n",`
		`existname);`
		`PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE,`
		`"the plugin named %s already exists, or is already setup.", existname);`
		`- return -1;`
		`+ return 1;`
		`}`

		`/*`
		`@@ -3016,7 +3016,9 @@`
		`}`

		`if (!status) {`
		`- status = plugin_add_descriptive_attributes(plugin_entry, plugin);`
		`+ if (plugin_add_descriptive_attributes(plugin_entry, plugin) != 0) {`
		`+ status = -1;`
		`+ }`
		`}`

		`slapi_ch_free((void **)&value);`

spichugi commented 5 years ago

Bug Description: We add PBKDF2_SHA256 password storage schema two times. During:
1. the dse.ldif parsing;
2. the bootstrap plugin operation.
It causes the error to appear during the startup.

Fix Description: Make plugin_setup() function report the error to TRACE log level
if the plugin already exists. We will report the error in ERR log level during
the config bootstrap anyway (code path for the 1st option from bug description).
For 2nd option, report the error to TRACE if it is 'already exist' issue
and to ERR if it is any other case.

Make the plugin_setup returns more consistent.

https://pagure.io/389-ds-base/issue/49640

Reviewed by: ?

rebased onto 7ecfefe478266fae0fb6dd792342a2df50a5597f

5 years ago

mreynolds commented on line 15 of ldap/servers/slapd/config.c 5 years ago

Is this logging line redundant from what you did in plugin.c?

spichugi commented 5 years ago

Is this logging line redundant from what you did in plugin.c

I was thinking that it can potentially help with the debugging.
It points out the function name 'slapd_bootstrap_config' and the configfile where the issue is happening. So we can narrow down the problem if it would happen.

But we use TRACE log level rarely because it's really noisy... So if you think it is too much redundant, I'll remove it.

mreynolds commented 5 years ago

Good point, ack

rebased onto 2fa0408

5 years ago

Pull-Request has been merged by spichugi

5 years ago

spichugi commented 3 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/2881

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

3 years ago

Metadata

Assignee

None

Tags

No Tags

Changes Summary 2

+11 -3

file changed

ldap/servers/slapd/config.c

+6 -4

file changed

ldap/servers/slapd/plugin.c

389-ds-base

Source Code

#49822 Issue 49640 - Cleanup plugin bootstrap logging Closed 3 years ago by spichugi. Opened 5 years ago by spichugi. spichugi/389-ds-base plugin_error_fix into master

Metadata

Changes Summary 2

#49822 Issue 49640 - Cleanup plugin bootstrap logging

Closed 3 years ago by spichugi. Opened 5 years ago by spichugi.

spichugi/389-ds-base plugin_error_fix into master