quattor/server/config: enable tftpd service on EL9 #239
Conversation
| # Enable tftpd (on EL9 only for backward compatibility) | ||
| include 'components/systemd/config'; | ||
| '/software/components/systemd/unit/tftp' = { | ||
| if ( OS_VERSION_PARAMS['majorversion'] >= '9' ) { |
There was a problem hiding this comment.
'9' should be 9 instead? I suspect string comparison doesn't work with '10' and higher?
There was a problem hiding this comment.
You are right but for discussable reasons this parameter has been defined as a string (to allow number that may have included a letter or dot, e.g. 9.1) and the end result is what you are saying. I'm afraid there are several places to fix in the template library and probably a way of doing it would be to implement a function to compare OS versions that would do the proper massaging... I am not clear whether we should already avoid to propagate the problem here or if we should have an issue to fix it properly (and if possible using the same approach) everywhere this test is present in the template library... I'm open!
There was a problem hiding this comment.
Would pkg_compare_version not do the job?
There was a problem hiding this comment.
I'm not familiar with OS_VERSION_PARAMS and complete agree that overall versioning needs to take things like 8.8 into account. However I would assume that "major version" would only be the first part as an int? i.e. in MS we have similar fields and they are constructed by selecting out the first int before a dot or other character from the overall version.
Assuming this isn't trivially fixable in the templates then maybe a quick hack here would be (untested):
if( to_long(OS_VERSION_PARAMS['majorversion']) >= 9 ) {
There was a problem hiding this comment.
@ned21 I agree with you. The proposed hack would work as major version is currently only something that can be converted to a number. It may not work if at some point we decide to put something that cannot be converted. Thus my proposal to implement a function like get_os_major_version() that would return a number after doing whatever is necessary with OS_VERSION_PARAMS['majorversion']...
There was a problem hiding this comment.
Proposal open as an issue: #242. I suggest to merge this PR as it is and to update the template (and others affected) once the function is available (and before RHEL 10!).
|
|
|
#238 is merged now |
jouvin
force-pushed
the
aii_server_el9_enable_tftpd
branch
from
a37b865 to
e3e59e9
Compare
jouvin
force-pushed
the
aii_server_el9_enable_tftpd
branch
from
e3e59e9 to
0841f16
Compare
This change should have been upstreamed before 24.10 release. Required for a working AII server on EL9.