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

add support for journal upload and remote server #482

Merged
merged 5 commits into from
Nov 12, 2024

Conversation

trefzer
Copy link
Contributor

@trefzer trefzer commented Aug 13, 2024

Pull Request (PR) description

add support for uploading journal to a remote server

@trefzer trefzer force-pushed the dev_journal branch 11 times, most recently from 63ef53f to 1b6d0aa Compare August 13, 2024 12:27
@trefzer trefzer force-pushed the dev_journal branch 3 times, most recently from 8da8aa9 to 30089e6 Compare August 13, 2024 12:50
@trefzer trefzer marked this pull request as draft August 13, 2024 13:48
@trefzer
Copy link
Contributor Author

trefzer commented Aug 14, 2024

added journal-remote setting to this merge request to avoid merge conflicts.
This gives the possibility to use combined spec test.

@trefzer trefzer changed the title add support for journal upload to a remote server add support for journal upload and remote server Aug 14, 2024
@trefzer trefzer force-pushed the dev_journal branch 3 times, most recently from 5a5adb1 to bd778d5 Compare August 14, 2024 13:06
@bastelfreak bastelfreak added the enhancement New feature or request label Aug 14, 2024
@trefzer trefzer force-pushed the dev_journal branch 7 times, most recently from 8faafca to e277b93 Compare August 14, 2024 14:51
@trefzer trefzer force-pushed the dev_journal branch 4 times, most recently from b1facf8 to 92caaba Compare August 14, 2024 15:59
@trefzer trefzer marked this pull request as ready for review August 14, 2024 16:03
@kenyon
Copy link
Member

kenyon commented Aug 14, 2024

You'll probably need to change the acceptance tests to use Vagrant instead of the default of Docker for systemd to work correctly.

@trefzer
Copy link
Contributor Author

trefzer commented Aug 14, 2024

@basterfreak thanks, all done

@trefzer
Copy link
Contributor Author

trefzer commented Aug 14, 2024

@kenyon probably, but no idea howto do so !

assert_private()

if $package_name {
stdlib::ensure_packages($package_name)
Copy link
Member

Choose a reason for hiding this comment

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

IMO if the management of the package is already configureable and the package itself is very specific to the module, we don't need to wrap it in a function call.

Copy link
Contributor Author

@trefzer trefzer Nov 12, 2024

Choose a reason for hiding this comment

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

right, but there is currently only one package for upload and remote journal (called systemd-journal-remote) in Debian and RedHat.
This solution gives you the possibility to einer install upload and remote or both.
So I think it's better (and future proof) to keep it like this.

Copy link
Member

Choose a reason for hiding this comment

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

Oh it's the same package. This packaging is so stupid :(
yes in that case it makes sense to stick to stdlib::ensure_packages()

}

service { 'systemd-journal-remote':
ensure => running,
Copy link
Member

Choose a reason for hiding this comment

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

should we also set enable => true, to put it into autostart? usually we do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@bastelfreak bastelfreak merged commit 1fc5d99 into voxpupuli:master Nov 12, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants