-
Notifications
You must be signed in to change notification settings - Fork 2
Sync with xenserver-release-8.4.0-18 #33
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Yann Dirson <[email protected]>
Signed-off-by: Samuel Verschelde <[email protected]>
- Sync with xenserver-release-8.4.0-18 - Add 0004-Sync-with-xenserver-release-v8.4.0-17.tar.gz.patch - Add 0005-etc-rsyslog.d-xenserver.conf-mention-XCP-ng-in-addit.patch - *** Upstream changelog *** * Thu Mar 13 2025 Gerald Elder-Vass <[email protected]> - 8.4.0-18 - CA-407370: Separate rsyslog configs for xenserver and customer * Thu Mar 13 2025 Deli Zhang <[email protected]> - 8.4.0-17 - CP-50340: Revert "Obsolete unused packages for OpenSSL 3 upgrade" * Fri Feb 07 2025 Deli Zhang <[email protected]> - 8.4.0-16 - CA-401322: Ensure openssl-libs, openssl and xenserver-release-config rpms to be updated in order - CP-50340: Obsolete unused packages for OpenSSL 3 upgrade - CP-50298: Move ssh config files to openssh.spec Signed-off-by: Samuel Verschelde <[email protected]>
86e6fdf to
b46d6f0
Compare
| # CA-401322: /sbin/update-issue calls openssl command which must link to | ||
| # compatible (or same version) openssl-libs. Here specify the require to | ||
| # ensure openssl-libs, openssl and %%name-config rpms to be updated in order. | ||
| Requires: openssl >= 3.0.9 | ||
| Requires(pre): sed | ||
| Requires(pre): coreutils |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how this gets the packages updated in the order they want, but I haven't given it much thought.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment only apply to the 1st dep at:
27fd57a#diff-ff0cb122a768fd67f0299827b8f65762c64492394b3936e0cc649b64b556668aR115
Maybe a newline is missing just after SSL3
ydirson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The upstream tgz is kept in the merge.
I'm a bit wary of making such changes now that 8.3 is LTS. We would likely want to make some rather extensive tests, but we don't have access to every type of change our users may have done. I feel almost certain that this is going to break on some machines.
SOURCES/0005-etc-rsyslog.d-xenserver.conf-mention-XCP-ng-in-addit.patch
Outdated
Show resolved
Hide resolved
| %pre config | ||
| # On first upgrade extract any log forwarding rules into remote.conf | ||
| if [ "$1" -eq "2" ] && [ -f %{_sysconfdir}/rsyslog.d/xenserver.conf ] && ! [ -f %{_sysconfdir}/rsyslog.d/remote.conf ] ; then | ||
| sed -n '/\*\.\*.*@/p' %{_sysconfdir}/rsyslog.d/xenserver.conf > %{_sysconfdir}/rsyslog.d/remote.conf | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That "upgrade" logic is to create a rsyslog.d/remote.conf if one does not exist, and it will be empty for any case where no remote logging was configured. We may want to avoid the user removing it "just because it's empty" by prepending a comment in it.
| %pre config | |
| # On first upgrade extract any log forwarding rules into remote.conf | |
| if [ "$1" -eq "2" ] && [ -f %{_sysconfdir}/rsyslog.d/xenserver.conf ] && ! [ -f %{_sysconfdir}/rsyslog.d/remote.conf ] ; then | |
| sed -n '/\*\.\*.*@/p' %{_sysconfdir}/rsyslog.d/xenserver.conf > %{_sysconfdir}/rsyslog.d/remote.conf | |
| fi | |
| %pre config | |
| # On first upgrade extract any log forwarding rules into remote.conf | |
| if [ "$1" -eq "2" ] && [ -f %{_sysconfdir}/rsyslog.d/xenserver.conf ] && ! [ -f %{_sysconfdir}/rsyslog.d/remote.conf ] ; then | |
| cat > %{_sysconfdir}/rsyslog.d/remote.conf <<EOF | |
| # This file is intended to contain any log forwarding to a remote syslog. | |
| # Do not remove, it will be recreated on next xcp-ng-release-config upgrade. | |
| EOF | |
| sed -n '/\*\.\*.*@/p' %{_sysconfdir}/rsyslog.d/xenserver.conf >> %{_sysconfdir}/rsyslog.d/remote.conf | |
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We must also check whether XAPI cares if remote.conf exists or not and might get confused by an empty remote.conf. I doubt it, but let's check.
CC @gthvn1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like XAPI doesn't read the file at all and only (over)writes it (or deletes it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, a non-existing remote.conf is expected when no remote is configured. We don't need to create an empty file, but the script does so anyway. If the user removes it, no problem. When XAPI writes it, it does put a header telling you not to modify, so, although it would be nice to add a header for the initial remote.conf contents when migrated from rsyslog.conf, I think we can do without and remain in sync with XenServer's spec file on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What looks wrong, though, is that on every update of the RPM, the empty file will get re-created if it doesn't exist... And XAPI does delete it if you remove the remote forwarding configuration... This is a mess...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the best would be for XAPI not to delete the file when one removes the forwarding rule, but to empty it instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the best would be for XAPI not to delete the file when one removes the forwarding rule, but to empty it instead.
Yes I agrea, so I proposed this change upstream:
That said it should not be a blocker for x-n-r, should it be ?
Anyway et me also add a similar disclaimer (file "will be used for forwarding logging and not supposed to be edited") our side to enable a smooth transition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That said it should not be a blocker for x-n-r, should it be ?
I don't think it's a blocker indeed.
|
Pre-build towards v8.3-u-stormi2: https://koji.xcp-ng.org/taskinfo?taskID=91533 |
| # xcp-rrdd-plugins (info and above) to local0 | ||
| $outchannel xcp_rrdd_log,/var/log/xcp-rrdd-plugins.log,104857600,/etc/cron.daily/logrotate | ||
| local0.info :omfile:$xcp_rrdd_log | ||
| - | ||
| -# ignore default rules | ||
| -*.* ~ | ||
| - | ||
| -- | ||
| 2.30.2 | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implies that rules ordering in the config is indeed significant in interpretation of the config and the impact of the change on ordering has not been been explained.
Analyzing this requires the full HTML doc matching the 8yo version we use (from rsyslogd-doc.rpm) as upstream website only seems to publish the latest, and manpage does not even mention $IncludeConfig.
There are "legacy" and "RainerScript" ways of configuring, the HTML doc seems to confirm my worries, as the config files we use essentially use legacy syntax:
RainerScript is easier to read and maintain, avoids side effects with include files
Be warned that legacy action format is hard to get right. It is recommended to use RainerScript-Style action format whenever possible!
To avoid any side effects, do a $ResetConfigVariables after the $IncludeConfig. It may also be a good idea to do a $ResetConfigVariables right at the start of the include, so that the module knows exactly what it does. Of course, one might specifically NOT do this to inherit parameters from the main file. As always, use it as it best fits...
how this config works
There are several kind of lines in those files:
- manpage mentions:
- Global directives (start with
$) - Templates (we have none)
- Output channels
- Rules (selector + action), most of those we use being in legacy syntax
- Global directives (start with
- then HTML introduces
- rulesets (set of rules, but the example shows only actions, no selector - latest doc has one though), but we use none
My understanding is that global directives are "last value written to a given directive wins", and output channel are "all apply, until a discard (~) action is triggered".
$IncludeConfig doc does not specify in which order the included files are processed.
before this change
we have the following files on a host:
rsyslog.conf, includingrsyslog.d/*.conf- we ship
listen.conf(fromsystemd)xenserver.conf, which finishes with a global discard rule, effectively neutralizing all rules inrsyslog.conffollowing the directive, and any coming from a file that would be included after it. That effectively neutralizes all of the directives in the defaultrsyslog.conf, but users could have uncommented some global directives there, and those would not be impacted by the global discard, as it is a Rules thing.
after an upgrade including this change
- no more rule in
rsyslog.conf - any global directive modified by the user after the
$IncludeConfiggets lost - any rule coming from a file that would be included after
xenserver.confwould get de-neutralized by the removal of the discard
also to consider
Current host-installer (and all versions published today for 8.x and 9.x) does not preserve any of those config files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All in all, this change seems to:
- allow a user config to include rules even if it gets included after
xenserver.conf, neutralizing the effect the discard rule had on those files. But at the same time user configs do not seem to be promoted to "preserved on upgrade" status. - create that empty
remote.conffile which Samuel showed is a dirty thing
So I'm not sure we really want that change after all 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous situation is not a lot better. We have a xenserver.conf which can be modified by XAPI to add remote servers, but all changes are lost if any modification is made to the file in the RPM, as it is not %config(noreplace). It seems positive to me to move the contents that is under XAPI's responsibility to a separate remote.conf file.
If we were to refuse these changes in xcp-ng-release, then we would have to patch XAPI to come back to its old behaviour, which is not really good.
any global directive modified by the user after the $IncludeConfig gets lost
Except definitions of remote servers, which are migrated to remote.conf in %pre. And anyway I don't think xenserver.conf was ever supposed to be modified by users, given how . Thankfully, users rarely alter the rsyslog configuration for anything but remote forwarding, as far as I know.
So I'm not sure we really want that change after all 🤷
I agree that there are issues with the change, but there are also issues with not applying it, and I think the issues are going to be rare / easy to overcome. The worst I can foresee is someone losing changes made directly to rsyslog.conf, which is not supported.
But at the same time user configs do not seem to be promoted to "preserved on upgrade" status.
This is a good point that deserves considering for the upgrade to XCP-ng 9 (which will likely have a different configuration structure anyway for logging?), but not a new one given that xenserver.conf is not preserved either as far as I can see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ydirson > $IncludeConfig doc does not specify in which order the included files are processed.
According to:
https://www.rsyslog.com/doc/configuration/global/options/rsconf1_includeconfig.html
"if multiple files are included, they are processed in ascending sort order of the file name"
We can use number prefixes to config files to force the ordering, and eventually propose users to use a range eg: [90-99]-*.conf ?
It looks like this feature is also available in the version shipped by xcp-ng (rsyslog-8.24.0-52.el7_8.2.x86_64)
[11:12 xcp-ng-pcl-11 ~]# lynx -dump /usr/share/doc/rsyslog-8.24.0/html/configuration/global/options/rsconf1_includeconfig.html | grep -2 'if multiple files are included'
the main file. As always, use it as it best fits...
Note: if multiple files are included, they are processed in ascending
sort order of the file name. We use the “glob()” C library function for
Extra note about loosing user changes in /etc, what about adding etc-keeper to distro (this tool track all changes in /etc in a local /etc/.git repo, I use it on most of my systems but it can grows over time, unsure there is a autosquash/garbage collector feature https://etckeeper.branchable.com/forum/_Any_suggestion_to_avoid_infinite_growth_of_.git___63__/ )
|
Converting to draft. There is no urgent change here and we'd like to better manage the rsyslog configuration management update. The associated commit in XAPI was reverted. |
Xenserver's changes around rsyslog configuration have downsides and the packaging prefers the previous behaviour because there's no time to properly fix the behaviour before the next release. For the original change, see xapi-project#6328 Discussions on why the original change is dubious: xcp-ng-rpms/xcp-ng-release#33 Reverts commit 468eb75. Signed-off-by: Pau Ruiz Safont <[email protected]>
For the record this has been verified by enabling debug mode
# /etc/sysconfig/rsyslog: Enable logs with
SYSLOGD_OPTIONS="-d"
RSYSLOG_DEBUG="Debug"
RSYSLOG_DEBUGLOG="/tmp/rsyslog.log"
# Create dummy files
touch /etc/rsyslog.d/aa-before-listen.conf
touch /etc/rsyslog.d/uu-before-xenserver.conf
touch /etc/rsyslog.d/zz-after-xenserver.conf
# Reparse
sudo systemctl restart rsyslog && grep '\.conf' /tmp/rsyslog.log
: config parser: pushed file /etc/rsyslog.conf on top of stack
: requested to include config file '/etc/rsyslog.d/zz-after-xenserver.conf'
: config parser: pushed file /etc/rsyslog.d/zz-after-xenserver.conf on top of stack
: requested to include config file '/etc/rsyslog.d/xenserver.conf'
: config parser: pushed file /etc/rsyslog.d/xenserver.conf on top of stack
: requested to include config file '/etc/rsyslog.d/uu-before-xenserver.conf'
: config parser: pushed file /etc/rsyslog.d/uu-before-xenserver.conf on top of stack
: requested to include config file '/etc/rsyslog.d/listen.conf'
: config parser: pushed file /etc/rsyslog.d/listen.conf on top of stack
: requested to include config file '/etc/rsyslog.d/aa-before-listen.conf'
: config parser: pushed file /etc/rsyslog.d/aa-before-listen.conf on top of stack
: config parser: reached end of file /etc/rsyslog.d/aa-before-listen.conf
: config parser: resume parsing of file /etc/rsyslog.d/listen.conf at line 1
: config parser: reached end of file /etc/rsyslog.d/listen.conf
: config parser: resume parsing of file /etc/rsyslog.d/uu-before-xenserver.conf at line 1
: config parser: reached end of file /etc/rsyslog.d/uu-before-xenserver.conf
: config parser: resume parsing of file /etc/rsyslog.d/xenserver.conf at line 1
: config parser: reached end of file /etc/rsyslog.d/xenserver.conf
: config parser: resume parsing of file /etc/rsyslog.d/zz-after-xenserver.conf at line 1
: config parser: reached end of file /etc/rsyslog.d/zz-after-xenserver.conf
: config parser: resume parsing of file /etc/rsyslog.conf at line 36
: config parser: reached end of file /etc/rsyslog.conf
Relate-to: xcp-ng-rpms/xcp-ng-release#33 (comment)
Signed-off-by: Philippe Coval <[email protected]>
Xenserver's changes around rsyslog configuration have downsides and the packaging prefers the previous behaviour because there's no time to properly fix the behaviour before the next release. For the original change, see xapi-project#6328 Discussions on why the original change is dubious: xcp-ng-rpms/xcp-ng-release#33 Reverts commit 468eb75. Signed-off-by: Pau Ruiz Safont <[email protected]>
Where exactly ? in new included files ? or in xenserver.conf (eg: removal of discarded rules) ? For changes in rsylogs after last includes i am less concerned ( https://github.com/xcp-ng-rpms/xcp-ng-release/pull/33#discussion_r2570927813# backup) Can you suggest any scenario to be tested ? |
Uh oh!
There was an error while loading. Please reload this page.