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

Remove nojournal parameter; Cleanup journal management #730

Merged

Conversation

h-haaks
Copy link
Contributor

@h-haaks h-haaks commented Apr 4, 2024

Pull Request (PR) description

Removing duplicate param to manage storage.journal.enabled
Usage of nojournal => true must be changed to journal => false

In addition storage.journal.enabled is removed in mongodb 7

@h-haaks
Copy link
Contributor Author

h-haaks commented Apr 4, 2024

Resolves mongodb 7 issues in #728

Comment on lines 383 to 404
if $journal != undef {
if ($mongodb::globals::manage_package_repo and versioncmp($mongodb::globals::repo_version, '7.0') >= 0)
or ($package_ensure =~ /\./ and versioncmp($package_ensure, '7.0.0') >= 0 ) {
fail('`journal` parameter is only supported for MongoDB < 7.0')
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that mongodb::globals::repo_version needs to be set or the package_ensure variable needs to be set to a version for this validation to work? The validation is just skipped when someone manages their own repo, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The validation is just skipped when someone manages their own repo

That was my intention. We can't validate something we don't know about.
I added the version constraint in param doc just for this.

But I now see that there is a small bug.
When we bump the default to 7.0 this will start to fail even if repo_locationis set.
I'll check that repo_location is undef as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed and enhanced by looking for a version match in provided repo_location as well.
Added a few more tests.

@h-haaks h-haaks force-pushed the cleanup-journaling-config branch 2 times, most recently from 97c701a to 866e298 Compare April 4, 2024 23:14
@h-haaks h-haaks merged commit 4c5f8af into voxpupuli:master Apr 5, 2024
24 checks passed
@h-haaks h-haaks deleted the cleanup-journaling-config branch April 5, 2024 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants