Enable/disable hooks.

[atmaxinger]

@markus2330 has identified the need for a user to manualy enable and/or disable certain hooks. Below are two insightful comments on this topic.

This issue shall serve as a focused place for discussion about how we solve this.

---

kdb cache already has support for disabling a global plugin, this feature should be available (in the same way) for all global plugins.

Important is that the user can always fix the problem somehow (e.g. disable the broken plugin). If we already fail in kdbOpen, then e.g. kdb cache disable probably will also fail, as it tries to write to KDB (and obviously needs to open it first).

So I think broken global plugins can only lead to a warning, not to an error.

Originally posted by @markus2330 in #4471 (comment)

---

it will now return -1 if any hooks fail to initialized

What I meant was that there is already a failure case (couldn't load gopts plugin), but the function doesn't actually return an error code for that.

this feature should be available (in the same way) for all global plugins.

I agree and disagree... The flag in the KDB doesn't make sense for all hooks. Especially for gopts it isn't the right solution. gopts can only be enabled via the kdbOpen contract. Therefore having a permanent flag in the KDB makes zero sense. It makes no sense to "enable gopts by default" and if the flag is used to override the contract it will just break applications that expect gopts to run.

For cache the flag makes sense of course. For spec I could see the flag as well, but in this case it is probably more about switching from errors to warnings. I don't really see a case where completely disabling spec would help. Finally, there is the notification stuff. AFAIK for internalnotification the situation is the same was with gopts. You enable it only via contract and therefore a flag in the KDB is useless. For dbus and zeromq enabling/disabling via the KDB would probably make sense again.

system:/elektra/hook/cache/enabled

Yes, I'd do that too. But as stated above, I'd tailor the actual config to the individual hook. So for spec it might be something like system:/elektra/hook/spec/loglevel.

we can set all hooks to be 1 via a installation script,

We could also switch from cache/enabled to cache/disabled which automatically switches the default to "cache enabled".

Originally posted by @kodebach in #4471 (comment)

[atmaxinger]

My proposal would be to let the user disable every hook, even if it does not appear to make sense right now.

system:/elektra/hook/<hook name>/disabled should be set to a truthy value.

One problem, however, with this approach is that that user would have to manually disable all hooks that are not installed.

[kodebach]

I think a general system:/elektra/hook/<hook name>/disabled is not the best solution.

Only hooks that are enabled by default should use a disabled key. The ones that are enabled by default also ship with the standard install so there is no problem for the user. If a hook isn't enabled by default, it could be shipped as separate package and it should use an enabled key.

In addition to enabling/disabling via the KDB, I think a similar option should exist in the kdbOpen contract. Then for example kdb cache disable could use that option before writing the config to the KDB.

So I think broken global plugins can only lead to a warning, not to an error.

I think we should have an option for the kdbOpen contract to only load the system:/elektra bootstrap backend without loading or validating any part of system:/elektra/mountpoints. If this option is used, the parentKey for kdbGet/kdbSet must be system:/elektra. With any other parentKey we immediately return an error in kdbGet/kdbSet for such KDB instances.

This option could be used to repair most broken KDBs. If the bootstrap backend also breaks only manual file editing can help.

Importantly, having such an option allows us to properly return errors from kdbOpen instead of creating half-working KDB instances that will error out later.

[markus2330]

I don't think that we should invert the logic. Off on "nothing" is the more robust choice, e.g. on incomplete/missing installations.

Instead we should check at installation time which hooks are actually installed and then set system:/elektra/hook/<hook name>/enabled = 1 if unset, see #4446. (And again unset if purged.)

This option could be used to repair most broken KDBs.

How to change this option if KDB is already broken? I think it is necessary to make kdbOpen/kdbGet as robust as possible, so even on failures regarding hooks we only yield warnings (simply disabling broken hooks).

instances that will error out later.

Why would a missing hook have errors later?

[kodebach]

Instead we should check at installation time which hooks are actually installed and then set system:/elektra/hook/<hook name>/enabled = 1 if unset

I'm not a fan of this. The default should be correct and working without needing to execute code after install. Also what is "at installation time"? The script would need to run, every time a new hook plugin is installed. But then, why not just default to "enabled"?

If we really want to set system:/elektra/hook/<hook name>/enabled = 1 I would actually create a special backend plugin for system:/elektra/hook. The plugin would work like a normal file-based backend hardcoded to use resolver and dump and without any other plugins (for this it could delegate to backend). The special behaviour is during kdbGet where the plugin also checks which plugins are actually installed and sets missing system:/elektra/hook/<hook name>/enabled = 1 and removes any system:/elektra/hook/<hook name>/enabled = 1 where no plugin exists and issues a warning.

How to change this option if KDB is already broken?

The option is part of the contract for kdbOpen. There could be a mode for kdb that uses this option. The problem is how this mode would be enabled, because a command-line option wouldn't work since that would now also use gopts and therefore kdbOpen must be called before we know about command-line args.

We could create a separate kdb-rescue executable. It would always use the "bootstrap only" mode and simply create a backup of system:/elektra either in e.g. system:/elektra/rescue/backup or by copying the file and then reset the rest of system:/elektra to default state. Afterwards you can use the normal kdb to inspect the backup and try to rebuild your KDB.

so even on failures regarding hooks we only yield warnings (simply disabling broken hooks).

At the very least, there should be an error, if a hook (e.g. gopts) was explicitly configured via the contract. Just producing a KDB without gopts and issuing a warning would be very weird behaviour.

Why would a missing hook have errors later?

Well for example, if gopts is missing kdb won't do anything, since command-line args are not processed. Similarly, most applications won't work if spec doesn't do it's job.

But that can be fixed, if we still produce errors for hooks explicitly enabled via the contract. Then any application that requires a hook can simply add the option to the contract and kdbOpen will fail if something is missing.

---

I propose this logic:

1. If there is no .so file for a hook it is always disabled
2. If a hook is configured via the contract for kdbOpen it is always enabled, if loading the hook plugin fails for any reason we produce an error and return `NULL.
3. If system:/elektra/hook/<hook name>/disabled is set a hook is disabled
4. Otherwise the hook is enabled, if loading the plugin fails we issue a warning and disable also any related hooks (e.g. if there is a dependency between hooks)

Alternatively, we use the backend plugin described above and do:

1. If system:/elektra/hook/<hook name>/enabled = 1 is set the hook is enabled, warning if loading fails
2. If a hook is configured via the contract for kdbOpen it is always enabled, if loading the hook plugin fails for any reason we produce an error and return `NULL.

[markus2330]

But then, why not just default to "enabled"?

Because then all would be enabled, and not the ones which are installed.

We could create a separate kdb-rescue executable.

Makes sense, but is a far-away feature.

At the very least, there should be an error, if a hook (e.g. gopts) was explicitly configured via the contract.

Yes, for contracts there can be an error in kdbOpen.

If there is no .so file for a hook it is always disabled

How you want to do that check?

[kodebach]

Because then all would be enabled, and not the ones which are installed.

I meant default to "enabled" when there is an .so file, like I listed in the steps at the bottom of the comment.

How you want to do that check?

Just try to load the plugin (just the module loading without calling the open function). If it works continue, if it fails consider the hook disabled. Since plugin names for hooks are hardcoded this should work.

Specifically, instead of just doing elektraPluginOpen we would do:

elektraPluginFactory factory = elektraModuleLoad (modules, name, errorKey);
if (factory == NULL) {
    // plugin not installed -> consider hook disabled
   return;
}

Plugin * plugin = elektraPluginOpen (name, modules, config, errorKey);

For the special backend plugin I would also just call elektraModuleLoad to see if the plugin for a hook is installed.

[kodebach]

Another idea using a special plugin:

• Create mountpoint with same config as bootstrap at system:/elektra/hook
• Add additional plugin hookcheck to get/poststorage and set/prestorage.
• When the hookcheck plugin called (both in get and set) it calls elektraModuleLoad for every hook and set's system:/elektra/hook/<hook name>/installed to 0/1 when the result is NULL/non-NULL (replacing any existing values).
• Enable hook if system:/elektra/hook/<hook name>/installed = 1 AND system:/elektra/hook/<hook name>/disabled = 0 (or unset), i.e. automatically enabled after install unless disabled by user.

I think this is probably the best solution and the solution that most follows Elektra's principles. It also makes it possible to check whether a hook is installed by doing:

kdb get "system:/elektra/hook/<hook name>/installed"

[markus2330]

Just try to load the plugin

This is a bad idea as you cannot distinguish something broken vs. missing. For broken at lest a warning must be issued.

I've set this low priority, as the general hook framework must be decided first.

[kodebach]

This is a bad idea as you cannot distinguish something broken vs. missing. For broken at lest a warning must be issued.

Does it matter in this case? If elektraModulesLoad doesn't work, we consider the plugin "not (correctly) installed". If that's the case we disable the hook.

For informing the user: elektraModuleLoad does add errors/warnings to errorKey. So we just need to propagate those.

I've set this low priority, as the general hook framework must be decided first.

If you think something is undecided, please open an issue and concretely list the things where you want clarification. IMO @atmaxinger and I are on the same page and I see the "general framework" as decided. Some details might not be fully worked out, but the details can always change until we have a working implementation that shows any assumptions were correct.

[markus2330]

Of course it matters. There are plenty of different possible errors.

So we just need to propagate those.

But we should not propagate warnings/errors if everything is as intended (hook not installed).

I still don't see any other way as the one I described above (mark at installation what was installed).

[kodebach]

Of course it matters. There are plenty of different possible errors.

The question is really: Does it matter, if the application did not explicitly request the hook plugin? IMO it doesn't, because if the application/user doesn't request it, it shouldn't matter if the hook is enabled or disabled.

However, let's assume it does matter, we actually do want to try to enabled everything that's available.

But we should not propagate warnings/errors if everything is as intended (hook not installed).

You could have been more explicit, if you wanted to say this. But only after googling the dlopen interface did I understand the problem. dlopen doesn't differentiate between errors at all and dlerror only provides a human-readable string. So there is no way to differentiate between "library not found" and any other type of error.

I still don't see any other way as the one I described above (mark at installation what was installed).

My issue is still with the "mark at installation". This seems far too brittle. If it's stored in /etc/kdb/default.ecf or /etc/kdb/elektra.ecf, we need a way to restore this data, when the file is delete (e.g. to reset the KDB). Then we need to detect installed hook plugins anyway, so why not do dynamically detect them all the time?

One option would be do the same thing as kdb plugin-list. This would be a limitation, because kdb plugin-list finds fewer plugins than elektraModuleLoad can theoretically load.

Another option would be to create marker files e.g. /etc/kdb/hook/spec that tell us whether a hook is installed. If the file is present we try to load the hook. The checking for the file, would be done by a special hard coded read-only backend plugin. I prefer this way over putting the information in the default or the bootstrap backend, because it is more resilient. It also significantly simplifies the installation process. Instead of needing to run a script (*), we just need to copy additional files.

Finally, a variation of the previous option would be using symlinks instead of empty marker files. So /etc/kdb/hook/spec would be a symlink to e.g. /usr/lib/elektra/libelektra-spec.so. We would then use the absolute path /etc/kdb/hook/spec directly to load the hook plugin. Like above, the presence of /etc/kdb/hook/spec defines whether or not the spec hook plugin is installed. But in this version it is also clear, that the hook plugin can be replaced with another implementation, because we directly point at the library to use.

(*) even worse, the script most likely would have to use kdb to mark the hooks as installed. So kdb must already be installed and functional. This for example makes it impossible, to have separate package for kdb and the gopts plugin and make kdb depend on gopts.

[github-actions]

I mark this stale as it did not have any activity for one year. I'll close it in two weeks if no further activity occurs. If you want it to be alive again, ping by writing a message here or create a new issue with the remainder of this issue.
Thank you for your contributions 💖

[github-actions]

I closed this now because it has been inactive for more than one year. If I closed it by mistake, please do not hesitate to reopen it or create a new issue with the remainder of this issue.
Thank you for your contributions 💖