Skip to content
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

luci-base: conversion of ucitrack handling from uci to json #7056

Merged
merged 33 commits into from
Apr 16, 2024

Conversation

feckert
Copy link
Member

@feckert feckert commented Apr 11, 2024

Description:

The ucitrack configuration under /etc/config defines which service must be restarted on a LuCI change. This uci file defines how all this works together. Additionally there are uci-defaults scripts in the various applications that add additional configurations parameters to this ucitrack file if a service must be reloaded/restarted if a uci change was made in this application.

There are some problems with the current implementation:

  • The uci should be used to configure the system and not for this kind of reload/restart dependency handling on LuCI changes.
  • On a system update with configuration restore of the uci file ucitrack the new behavior on LuCI reload/restart could not take into account because the new file is not be used.

In these two repositories, the change must then also be made after the merge.
openwrt/packages#23879
openwrt/routing#1062

  • This PR is not from my main or master branch 💩, but a separate branch ✅
  • Each commit has a valid ✒️ Signed-off-by: <my@email.address> row (via git commit --signoff)
  • Each commit and PR title has a valid 📝 <package name>: title first line subject for packages
  • ( Preferred ) Mention: @jow-
  • Description: (describe the changes proposed in this PR)

@feckert feckert requested a review from jow- April 11, 2024 12:59
@jow-
Copy link
Contributor

jow- commented Apr 11, 2024

In general this seems fine. My gut feeling is that a lot of these are redundant nowadays, every package with a procd enabled init script that installs a reload handler or does uci to command line conversion should be covered by generic logic already. Only ucitrack entries for servies having legacy init scripts or no init scripts at all should remain (or preferably, the underlying packages should be ported to procd) (I overlooked that you already removed quite a bunch)

In your commit messages please change deletet to deleted.

The 'ucitrack' configuration under '/etc/config' defines which service must
be restarted on a LuCI change. This uci file defines how all this works
together. Additionally there are 'uci-defaults' scripts in the various
applications that add additional configurations parameters to this ucitrack
file if a service must be reloaded/restarted on a LuCI change.

There are some problems with the current implementation:

* The uci should be used to configure the system and not for this kind of
  reload/restart dependency handling on LuCI changes.
* On a system update with configuration restore of the 'ucitrack' file
  the new behavior on LuCI reload/restart could not take into account
  because the new file is *not* used.

This commit converts the handling from uci to json.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
This script is no longer needed and can therefore be deleted.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
The 'ucitrack' file is not available anymore this changes is not needed.
Therefore, this 'uci-defaults' script is deleted.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
The 'ucitrack' file is not available anymore this changes are not needed.
Therefore, this 'uci-defaults' script is deleted.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
The 'ucitrack' file is not available anymore this changes are not needed.
Therefore, this 'uci-defaults' script is deleted.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
The 'ucitrack' file is not available anymore this changes are not needed.
Therefore, this 'uci-defaults' script is deleted.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Conversion of the 'uci-defaults' script for ucitrack handling to the new
json processing.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Conversion of the 'uci-defaults' script for ucitrack handling to the new
json processing.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Conversion of the 'uci-defaults' script for ucitrack handling to the new
json processing.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
… json

Conversion of the 'uci-defaults' script for ucitrack handling to the new
json processing.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
…e json

Conversion of the 'uci-defaults' script for ucitrack handling to the new
json processing.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
…json

Conversion of the 'uci-defaults' script for ucitrack handling to the new
json processing.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
… json

Conversion of the 'uci-defaults' script for ucitrack handling to the new
json processing.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Conversion of the 'uci-defaults' script for ucitrack handling to the new
json processing.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Conversion of the 'uci-defaults' script for ucitrack handling to the new
json processing.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
The 'ucitrack' file is not available anymore this changes are not needed.
Therefore, this 'uci-defaults' script is deleted.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
The 'ucitrack' file is not available anymore this changes are not needed.
Therefore, this 'uci-defaults' script is deleted.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
In the whole OpenWrt there is no httpd uci config, this must be a leftover
from before uhttpd and can therefore be deleted.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
In the whole OpenWrt there is no ntpclient uci config, this must be a
leftover and can therefore be deleted.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
In the whole OpenWrt there is no olsr uci config, this must be a leftover
and can therefore be deleted.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Move the json file to where it belongs.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Move the json file to where it belongs.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Move the json file to where it belongs.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Move the json file to where it belongs.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Move the json file to where it belongs.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Move the json file to where it belongs.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Move the json file to where it belongs.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Move the json file to where it belongs.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Move the json file to where it belongs.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Move the json file to where it belongs.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Move the json file to where it belongs.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Move the json file to where it belongs.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
Move the json file to where it belongs.

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
@systemcrash
Copy link
Contributor

@feckert

For those out there (posterity, and googlers who find this) who have PRs in the works, they just migrate from ucitrack to json file and everything just 'works'?

Delete this:

#!/bin/sh

uci -q batch <<-EOF >/dev/null
	delete ucitrack.@app[-1]
	add ucitrack app
	set ucitrack.@app[-1].init=appd
	commit ucitrack
EOF

exit 0

Do this:

{
	"config": "appd",
	"init": "appd"
}

(How) Does one handle the following in JSON? e.g.

#!/bin/sh

rm -f /tmp/luci-indexcache
exit 0

@systemcrash
Copy link
Contributor

I also did not understand this bit:

the new behavior on LuCI reload/restart could not take into account because the new file is not be used

@feckert
Copy link
Member Author

feckert commented Apr 12, 2024

@systemcrash

(How) Does one handle the following in JSON? e.g.

#!/bin/sh

rm -f /tmp/luci-indexcache
exit 0

This is no longer needed. We already add this to every package in luci.mk in general. See the following lines in luci.mk that get called as a post script during packages installation.

luci/luci.mk

Lines 227 to 230 in 291dd24

[ -n "$${IPKG_INSTROOT}" ] || { \
rm -f /tmp/luci-indexcache.*
rm -rf /tmp/luci-modulecache/
killall -HUP rpcd 2>/dev/null

I also did not understand this bit:

the new behavior on LuCI reload/restart could not take into account because the new file is not be used

This means that if I keep the /etc/config/ucitrack file during a system upgrade and this file has changed, the new file will not be used. And the LuCI will not react as intended to a LuCI change.

@feckert feckert merged commit d5c413d into openwrt:master Apr 16, 2024
5 checks passed
@feckert feckert deleted the pr/20240411-ucitrack branch April 16, 2024 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants