| Message ID | 20240214150519.13304-1-zajec5@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Rafał Miłecki |
| Headers | show |
| Series | base-files: sysupgrade: include uci-defaults script disabling services | expand |
(Not directly commenting on this change set) Would services not do better to be tracked within uci and its config files in /etc/config? Or do changes to those files there risk triggering other procd actions to the services they dictate? On 2024-02-14 16:05, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > Disabled services should be kept disabled after sysupgrade. This can be > easily handled using a proper uci-defaults script. > > Extend sysupgrade to check for disabled services, generate uci-defaults > script disabling them and include it in backup. > > Cc: Christian Marangi <ansuelsmth@gmail.com> > Cc: Jo-Philipp Wich <jo@mein.io> > Cc: Jonas Gorski <jonas.gorski@gmail.com> > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > --- > package/base-files/files/sbin/sysupgrade | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/package/base-files/files/sbin/sysupgrade b/package/base-files/files/sbin/sysupgrade > index 1e09f65e07..b1ada062ed 100755 > --- a/package/base-files/files/sbin/sysupgrade > +++ b/package/base-files/files/sbin/sysupgrade > @@ -273,6 +273,16 @@ create_backup_archive() { > \) | sed -e 's,.*/,,;s/\.control /\t/' > "$dir/${INSTALLED_PACKAGES}" > fi > > + mkdir -p $dir/etc/uci-defaults/ > + touch $dir/etc/uci-defaults/10_disable_services > + for service in /etc/init.d/*; do > + if ! $service enabled; then > + echo "$service disable" >> $dir/etc/uci-defaults/10_disable_services > + fi > + done > + echo "exit 0" >> $dir/etc/uci-defaults/10_disable_services > + echo "/etc/uci-defaults/10_disable_services" >> "$CONFFILES" > + > v "Saving config files..." > sed -i 's/^\///' "$CONFFILES" # Drop leading slashes > [ "$VERBOSE" -gt 1 ] && TAR_V="v" || TAR_V=""
On 14.02.2024 21:50, Paul D wrote: > Would services not do better to be tracked within uci and its config files in /etc/config? Well, it's a part of a mess we have in our init/config code. It was only last week that Jo was discussing it with Ansuel. In short we have: 1. Packages with no UCI config option for enabling/disabling 2. Packages with "enable" 3. Packages with "enabled" 4. Packages with "disable" 5. Packages with "disabled" I'm pretty sure most users are used to enabling/disabling services using UCI config option but not all of them have such possibility. That is the case my change is meant to deal with. An alternative would be to make sure every package uses "enabled" (or "disabled") UCI option. Should we do that? > Or do changes to those files there risk triggering other procd actions to the services they dictate?
The sender domain has a DMARC Reject/Quarantine policy which disallows sending mailing list messages using the original "From" header. To mitigate this problem, the original message has been wrapped automatically by the mailing list software. On Thursday, February 15th, 2024 at 06:42, Rafał Miłecki <zajec5@gmail.com> wrote: > I'm pretty sure most users are used to enabling/disabling services > using UCI config option but not all of them have such possibility. That > is the case my change is meant to deal with. Well, there was some user discussion on the forum a couple months back and I've always felt 'service X enable/disable' was the right way to do things. https://forum.openwrt.org/t/bug-service-status-is-not-saved-after-sysupgrade/179045 > An alternative would be to make sure every package uses "enabled" (or > "disabled") UCI option. Should we do that? I recently reworked the snort3 package and added "enabled" to its config. I felt then (and still do) that this is sort of a "hack around a bug", and would rather see it addressed as part of sysupgrade's backup/restore process. I'm a definite +1 on your current patch for sysupgrade, it looks like an elegant, unintrusive and robust means for dealing with service state. Eric
On 2024-02-15 15:42, Rafał Miłecki wrote: > On 14.02.2024 21:50, Paul D wrote: >> Would services not do better to be tracked within uci and its config >> files in /etc/config? > > Well, it's a part of a mess we have in our init/config code. It was only > last week that Jo was discussing it with Ansuel. In short we have: > 1. Packages with no UCI config option for enabling/disabling > 2. Packages with "enable" > 3. Packages with "enabled" > 4. Packages with "disable" > 5. Packages with "disabled" > > I'm pretty sure most users are used to enabling/disabling services > using UCI config option but not all of them have such possibility. That > is the case my change is meant to deal with. > > An alternative would be to make sure every package uses "enabled" (or > "disabled") UCI option. Should we do that? > Is 'on' or 'off' in the config perhaps simpler? I think it avoids the linguistic differences of past participle vs present simple. (Sometimes this distinction is important to have.) Otherwise 'en/disabled' is a good paradigm to encourage in configs.
On 15.02.2024 18:46, Paul D wrote: > On 2024-02-15 15:42, Rafał Miłecki wrote: >> On 14.02.2024 21:50, Paul D wrote: >>> Would services not do better to be tracked within uci and its config files in /etc/config? >> >> Well, it's a part of a mess we have in our init/config code. It was only >> last week that Jo was discussing it with Ansuel. In short we have: >> 1. Packages with no UCI config option for enabling/disabling >> 2. Packages with "enable" >> 3. Packages with "enabled" >> 4. Packages with "disable" >> 5. Packages with "disabled" >> >> I'm pretty sure most users are used to enabling/disabling services >> using UCI config option but not all of them have such possibility. That >> is the case my change is meant to deal with. >> >> An alternative would be to make sure every package uses "enabled" (or >> "disabled") UCI option. Should we do that? >> > > Is 'on' or 'off' in the config perhaps simpler? I think it avoids the linguistic differences of past participle vs present simple. (Sometimes this distinction is important to have.) > > Otherwise 'en/disabled' is a good paradigm to encourage in configs. Do you mean "on" and "off" as option values? How would you call such option? "enabled"? :P Something like "status" would be misleading again. Or if you mean "on" and "off" as option names then what value should we assign? uci set foo.config.on=1 ? No, I don't think "on" / "off" is a good option at all.
Hi Rafał, > Extend sysupgrade to check for disabled services, generate uci-defaults > script disabling them and include it in backup. > > Cc: Christian Marangi <ansuelsmth@gmail.com> > Cc: Jo-Philipp Wich <jo@mein.io> > Cc: Jonas Gorski <jonas.gorski@gmail.com> > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> Acked-by: Jo-Philipp Wich <jo@mein.io>
diff --git a/package/base-files/files/sbin/sysupgrade b/package/base-files/files/sbin/sysupgrade index 1e09f65e07..b1ada062ed 100755 --- a/package/base-files/files/sbin/sysupgrade +++ b/package/base-files/files/sbin/sysupgrade @@ -273,6 +273,16 @@ create_backup_archive() { \) | sed -e 's,.*/,,;s/\.control /\t/' > "$dir/${INSTALLED_PACKAGES}" fi + mkdir -p $dir/etc/uci-defaults/ + touch $dir/etc/uci-defaults/10_disable_services + for service in /etc/init.d/*; do + if ! $service enabled; then + echo "$service disable" >> $dir/etc/uci-defaults/10_disable_services + fi + done + echo "exit 0" >> $dir/etc/uci-defaults/10_disable_services + echo "/etc/uci-defaults/10_disable_services" >> "$CONFFILES" + v "Saving config files..." sed -i 's/^\///' "$CONFFILES" # Drop leading slashes [ "$VERBOSE" -gt 1 ] && TAR_V="v" || TAR_V=""