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

Krill version not set properly #1222

Open
timbru opened this issue Jul 4, 2024 · 0 comments
Open

Krill version not set properly #1222

timbru opened this issue Jul 4, 2024 · 0 comments

Comments

@timbru
Copy link
Contributor

timbru commented Jul 4, 2024

Hi!

Apologies, this bug was produced by yours truly....

It looks like the KrillVersion is not set properly inside of the PropertiesManager in start_krill_daemon in src/daemon/http/server.rs. Its .init() function is only called in case there is an actual upgrade.

This can lead to issues with future updates as the upgrade_versions function will not be able to work out what the version was for an existing instance.

One fix would be to add the following after the conditional finalise_data_migration is called:

    if !properties_manager.is_initialized() {
        // This looks like a new instance. Initialise the version so it can be
        // used in future to determine whether an update is needed.
        properties_manager.init(KrillVersion::code_version())?;
    }

But... it would be better to also detect existing situations where the version was not set. If disk is used (and postgresql cannot be used in practice, so this case can be ignored) this can be recognized by the fact that a properties directory exists but it is not initialized. But... this is not entirely trivial because for new installations this PropertyManager instance is created before checking what the current version is, because Krill tries to use it to check if it has been initialized - that act uses a lock and creates the directory. So, probably what would be needed to handle this gracefully is some code in upgrade_versions so that instead of this:

if properties_manager.is_initialized() {
        // The properties manager was introduced in Krill 0.14.0.
        // If it's initialised then it MUST have a Krill Version.
        let current = properties_manager.current_krill_version()?;
        UpgradeVersions::for_current(current)
    } else { ...

Where the .is_initialized creates the directory and lock files, it is checked first whether this directory exists and if:

  • the directory exists
  • but the manages is subsequently NOT initialized

Then conclude that this must be a 0.14.x installation that never initialized the manager - and therefore never stored the version.

Of course, feel free to fix the bug I introduced in a different way ;) But I hope this helps, and please let me know if this is not clear.

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

No branches or pull requests

1 participant