Bugzilla – Bug 1198066
[Staging] yast2-ntp relies on the presence of /sbin/netconfig
Last modified: 2022-09-26 09:48:06 UTC
## Observation openQA test in scenario microos-Staging:F-Staging-DVD-x86_64-microos@uefi-staging fails in [await_install](https://openqa.opensuse.org/tests/2280485/modules/await_install/steps/26) This error popped up after changing NetworkManager to no longer require, but rather only recommend sysconfig-netconfig (NetworkManager will likely lose this dep completely, as it does not really require them. The wetwork stack is being reworked to some degree and netconfig might get eliminated completely) Now, the problem here is that on MicroOS, the system, is installed without recommends (which explains why this error was only seen in the MicroOS staging tests, but not the TW/KGDE and GNOME tests for example) Tracked the error message down to (sadly no log files on the test) https://github.com/yast/yast-ntp-client/blob/756c366fb67cf4801d71584844db148c0ab305c2/src/modules/NtpClient.rb#L831 This error happens if sbin/netconfig fails - which is 'clear' in this case as it's not even installed. if yast2-ntp relies that much on it, it should require the dependency (not using it would be even nice thourg) ## Reproducible Fails since (at least) Build [F.636.1](https://openqa.opensuse.org/tests/2279282) ## Expected result Last good: [F.635.4](https://openqa.opensuse.org/tests/2270401) (or more recent) ## Further details Always latest result in this scenario: [latest](https://openqa.opensuse.org/tests/latest?arch=x86_64&distri=microos&flavor=Staging-DVD&machine=uefi-staging&test=microos&version=Staging%3AF)
Looking at the code it looks like to me, that YaST is running in the inst-sys, but calls netconfig in the installed system. This doesn't make any sense to me, as there is no network daemon nor a ntp daemon running in the installed system. I'm not sure, but could it be that yast2-ntp should either call the netconfig from the inst-sys if it wants to update the running chronyd or shouldn't call netconfig at all during fresh installation?
It updates the target system's config and then runs netconfig. So it makes sense to run it in the target system, in my view. In that case, a package dependency would not make much sense, as yast need not be available in the target system. Josef, Knut, could you comment?
Well, what YaST should do in that case is that it should require installation of required tools. Looks like ntp do something with it at https://github.com/yast/yast-ntp-client/blob/756c366fb67cf4801d71584844db148c0ab305c2/src/clients/ntp-client_proposal.rb#L510 but sadly it looks like it is missing netconfig there and only chrony is required. https://github.com/yast/yast-ntp-client/blob/756c366fb67cf4801d71584844db148c0ab305c2/src/modules/NtpClient.rb#L54 So if we need netconfig on target system, we should require its installation same as chrony.
Big question: is netconfig really required here? By looking at the netconfig ntp code: it creates a new temporary NTP server list (does it have access to all input files if it is running inside the new system, while everything else is running in the inst-sys? Or are we have now an incomplete list?) and updates the currently running chronyd from the inst-sys with this list. 1. does this really do what is expected? 2. why do we need to "update" the running ntp server list of the currently running chronyd from the inst-sys at the end of the installation? 3. why do we run something, which calls many other tools and modifies running daemons in the inst-sys from a chroot system? Is this really safe? Since this all works fine without netconfig (that's really a bad documented scripting mess) and we clearly want to move to use only native NetworkManager (why limiting ourself and customers by having netconfig in the middle, which means we have to re-implement and maintain all the supporting scripts a second time at our own and miss all new upstream features?). My suggestion would be: check for the existence of netconfig, and don't call it if it does not exist. Else you have to touch the code again in a few weeks.
(In reply to Thorsten Kukuk from comment #4) > Big question: is netconfig really required here? > > By looking at the netconfig ntp code: it creates a new temporary NTP server > list (does it have access to all input files if it is running inside the new > system, while everything else is running in the inst-sys? Or are we have now > an incomplete list?) and updates the currently running chronyd from the > inst-sys with this list. > > 1. does this really do what is expected? > 2. why do we need to "update" the running ntp server list of the currently > running chronyd from the inst-sys at the end of the installation? > 3. why do we run something, which calls many other tools and modifies > running daemons in the inst-sys from a chroot system? Is this really safe? > > Since this all works fine without netconfig (that's really a bad documented > scripting mess) and we clearly want to move to use only native > NetworkManager (why limiting ourself and customers by having netconfig in > the middle, which means we have to re-implement and maintain all the > supporting scripts a second time at our own and miss all new upstream > features?). > > My suggestion would be: check for the existence of netconfig, and don't call > it if it does not exist. Else you have to touch the code again in a few > weeks. I completely agree, BTW mchf was checking the current state of ntp configuration since we moved to chronyd so I guess these checks could be part of his current work.
Michal, this is waiting for input from you.
Knut just pointed me to https://trello.com/c/lAINPFUm/2663-3-osdistribution-p5-1188980-multiple-pool-entries-in-chrony-configuration-at-install as a fix for bug #1188980. He wrote that it should fix this bug as well.
(In reply to Stefan Hundhammer from comment #7) > He wrote that it should fix this bug as well. Then, I'll drop the need info here and link that issue report to the mentioned Trello card.
(In reply to David Diaz from comment #8) > (In reply to Stefan Hundhammer from comment #7) > > He wrote that it should fix this bug as well. > > Then, I'll drop the need info here and link that issue report to the > mentioned Trello card. Ah, already done. Changing the status to confirmed.
With this PR https://github.com/yast/yast-ntp-client/pull/174 we will avoid to call netconfig when it is not present. BTW, it looks like there is no error anymore in the test scenario so I guess the change recommending netconfig was reverted and now it is again required. Is there already some test already failing where we could test our changes?
(In reply to Knut Alejandro Anderssen González from comment #11) > With this PR https://github.com/yast/yast-ntp-client/pull/174 we will avoid > to call netconfig when it is not present. > > BTW, it looks like there is no error anymore in the test scenario so I guess > the change recommending netconfig was reverted and now it is again required. > > Is there already some test already failing where we could test our changes? Indeed, the pending NM change was revoked as we did not want to block GNOME 43 for too long. I can recreate that submission though and get it into a staging (currently there is some space available)
(In reply to Dominique Leuenberger from comment #12) > (In reply to Knut Alejandro Anderssen González from comment #11) > > With this PR https://github.com/yast/yast-ntp-client/pull/174 we will avoid > > to call netconfig when it is not present. > > > > BTW, it looks like there is no error anymore in the test scenario so I guess > > the change recommending netconfig was reverted and now it is again required. > > > > Is there already some test already failing where we could test our changes? > > Indeed, the pending NM change was revoked as we did not want to block GNOME > 43 for too long. > > I can recreate that submission though and get it into a staging (currently > there is some space available) Already branched the package and modified to recommend syconfig-netconfig instead of require it. https://build.opensuse.org/package/show/home:teclator:branches:openSUSE:Factory/NetworkManager So created a dud with this package and also with the proposed fix (https://github.com/yast/yast-ntp-client/pull/174) successfully. Therefore, the bug should be fixed with yast2-ntp-client-4.5.1