Bug 1198066 - [Staging] yast2-ntp relies on the presence of /sbin/netconfig
[Staging] yast2-ntp relies on the presence of /sbin/netconfig
Status: RESOLVED FIXED
Classification: openSUSE
Product: openSUSE Tumbleweed
Classification: openSUSE
Component: YaST2
Current
Other Other
: P5 - None : Normal (vote)
: ---
Assigned To: Knut Alejandro Anderssen González
Jiri Srain
https://openqa.opensuse.org/tests/228...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2022-04-05 07:41 UTC by Dominique Leuenberger
Modified: 2022-09-26 09:48 UTC (History)
6 users (show)

See Also:
Found By: openQA
Services Priority:
Business Priority:
Blocker: Yes
Marketing QA Status: ---
IT Deployment: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dominique Leuenberger 2022-04-05 07:41:32 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)
Comment 1 Thorsten Kukuk 2022-04-05 08:06:53 UTC
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?
Comment 2 Steffen Winterfeldt 2022-04-05 10:25:41 UTC
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?
Comment 3 Josef Reidinger 2022-04-05 22:17:19 UTC
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.
Comment 4 Thorsten Kukuk 2022-04-06 06:04:25 UTC
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.
Comment 5 Knut Alejandro Anderssen González 2022-05-03 08:26:34 UTC
(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.
Comment 6 Stefan Hundhammer 2022-07-14 08:44:27 UTC
Michal, this is waiting for input from you.
Comment 7 Stefan Hundhammer 2022-07-14 09:23:19 UTC
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.
Comment 8 David Diaz 2022-07-26 11:34:05 UTC
(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.
Comment 9 David Diaz 2022-07-26 11:35:04 UTC
(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.
Comment 11 Knut Alejandro Anderssen González 2022-09-22 09:35:29 UTC
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?
Comment 12 Dominique Leuenberger 2022-09-22 11:39:45 UTC
(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)
Comment 13 Knut Alejandro Anderssen González 2022-09-26 09:48:06 UTC
(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