| Message ID | 20210320195734.4061364-1-hauke@hauke-m.de |
|---|---|
| State | Accepted |
| Delegated to: | Hauke Mehrtens |
| Headers | show |
| Series | [v2,1/2] uhttpd: Reload config after uhttpd-mod-ubus was added | expand |
Hi Hauke, On 20.03.2021 20:57, Hauke Mehrtens wrote: > Without this change the config is only committed, but the uhttpd daemon > is not reloaded. This reload is needed to apply the config. Without the > reload of uhttpd, the ubus server is not available over http and returns > a Error 404. > > This caused problems when installing luci on the snapshots and > accessing it without reloading uhttpd. > > Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> this looks wrong to me. Calling init.d scripts from uci-defaults script may result (and it results here) in starting uhttpd too early. We need all uci-defaults script to do their updates before we call any init.d. I hit this incorrect behaviour as I have my own uci-defaults script switching uhttpd from port 80 to another one. If you need some changes applied after *installing* a package perhaps postinst should be used? On another note it doesn't make sense to ship package with wrong config and then always update it with uci-defaults script. Those scripts should be used only for dealing with outdated configs. If we always set ubus_prefix to "/ubus" then make it also a default value in the uhttpd.config.
On 6.01.2022 22:50, Rafał Miłecki wrote: > On 20.03.2021 20:57, Hauke Mehrtens wrote: >> Without this change the config is only committed, but the uhttpd daemon >> is not reloaded. This reload is needed to apply the config. Without the >> reload of uhttpd, the ubus server is not available over http and returns >> a Error 404. >> >> This caused problems when installing luci on the snapshots and >> accessing it without reloading uhttpd. >> >> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> > > this looks wrong to me. Calling init.d scripts from uci-defaults script > may result (and it results here) in starting uhttpd too early. We need > all uci-defaults script to do their updates before we call any init.d. > > I hit this incorrect behaviour as I have my own uci-defaults script > switching uhttpd from port 80 to another one. > > If you need some changes applied after *installing* a package perhaps > postinst should be used? > > On another note it doesn't make sense to ship package with wrong config > and then always update it with uci-defaults script. Those scripts should > be used only for dealing with outdated configs. > > If we always set ubus_prefix to "/ubus" then make it also a default > value in the uhttpd.config. Another comment: we don't want any "uci commit"s in uci-defaults scripts. That is executed by /etc/init.d/boot after all scripts get called.
On 6.01.2022 22:50, Rafał Miłecki wrote: > Hi Hauke, > > On 20.03.2021 20:57, Hauke Mehrtens wrote: >> Without this change the config is only committed, but the uhttpd daemon >> is not reloaded. This reload is needed to apply the config. Without the >> reload of uhttpd, the ubus server is not available over http and returns >> a Error 404. >> >> This caused problems when installing luci on the snapshots and >> accessing it without reloading uhttpd. >> >> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> > > this looks wrong to me. Calling init.d scripts from uci-defaults script > may result (and it results here) in starting uhttpd too early. We need > all uci-defaults script to do their updates before we call any init.d. > > I hit this incorrect behaviour as I have my own uci-defaults script > switching uhttpd from port 80 to another one. > > If you need some changes applied after *installing* a package perhaps > postinst should be used? Hint: rpcd: reload rpcd on installation of rpcd-mod-* https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=32ba52e2174d0c12fa476c3295daca12a864e547
On 1/6/22 22:52, Rafał Miłecki wrote: > On 6.01.2022 22:50, Rafał Miłecki wrote: >> On 20.03.2021 20:57, Hauke Mehrtens wrote: >>> Without this change the config is only committed, but the uhttpd daemon >>> is not reloaded. This reload is needed to apply the config. Without the >>> reload of uhttpd, the ubus server is not available over http and returns >>> a Error 404. >>> >>> This caused problems when installing luci on the snapshots and >>> accessing it without reloading uhttpd. >>> >>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> >> >> this looks wrong to me. Calling init.d scripts from uci-defaults script >> may result (and it results here) in starting uhttpd too early. We need >> all uci-defaults script to do their updates before we call any init.d. >> >> I hit this incorrect behaviour as I have my own uci-defaults script >> switching uhttpd from port 80 to another one. >> >> If you need some changes applied after *installing* a package perhaps >> postinst should be used? >> >> On another note it doesn't make sense to ship package with wrong config >> and then always update it with uci-defaults script. Those scripts should >> be used only for dealing with outdated configs. >> >> If we always set ubus_prefix to "/ubus" then make it also a default >> value in the uhttpd.config. > > Another comment: we don't want any "uci commit"s in uci-defaults > scripts. That is executed by /etc/init.d/boot after all scripts get > called. Hi Rafal, Thanks for pointing this out, I think you are right. We should remove the "uci commit" and "/etc/init.d/uhttpd reload" from the init script. As far as I understand this script should be used to migrate old configurations when you upgrade OpenWrt. We should also set ubus_prefix in package/network/services/uhttpd/files/uhttpd.config directly, then no migration is needed for a fresh install. Hauke
On 1/11/22 00:03, Hauke Mehrtens wrote: > On 1/6/22 22:52, Rafał Miłecki wrote: >> On 6.01.2022 22:50, Rafał Miłecki wrote: >>> On 20.03.2021 20:57, Hauke Mehrtens wrote: >>>> Without this change the config is only committed, but the uhttpd daemon >>>> is not reloaded. This reload is needed to apply the config. Without the >>>> reload of uhttpd, the ubus server is not available over http and >>>> returns >>>> a Error 404. >>>> >>>> This caused problems when installing luci on the snapshots and >>>> accessing it without reloading uhttpd. >>>> >>>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> >>> >>> this looks wrong to me. Calling init.d scripts from uci-defaults script >>> may result (and it results here) in starting uhttpd too early. We need >>> all uci-defaults script to do their updates before we call any init.d. >>> >>> I hit this incorrect behaviour as I have my own uci-defaults script >>> switching uhttpd from port 80 to another one. >>> >>> If you need some changes applied after *installing* a package perhaps >>> postinst should be used? >>> >>> On another note it doesn't make sense to ship package with wrong config >>> and then always update it with uci-defaults script. Those scripts should >>> be used only for dealing with outdated configs. >>> >>> If we always set ubus_prefix to "/ubus" then make it also a default >>> value in the uhttpd.config. >> >> Another comment: we don't want any "uci commit"s in uci-defaults >> scripts. That is executed by /etc/init.d/boot after all scripts get >> called. > > Hi Rafal, > > Thanks for pointing this out, I think you are right. > > We should remove the "uci commit" and "/etc/init.d/uhttpd reload" from > the init script. As far as I understand this script should be used to > migrate old configurations when you upgrade OpenWrt. > We should also set ubus_prefix in > package/network/services/uhttpd/files/uhttpd.config directly, then no > migration is needed for a fresh install. > > Hauke Hi, I had a second look at this. ubus.default is installed by the optional package uhttpd-mod-ubus. We can not set ubus_prefix by default in the uhttpd configuration. We can try to do the modification of the configuration and the reload in a postinst part of uhttpd-mod-ubus. I will try this tomorrow. Hauke
diff --git a/package/network/services/uhttpd/Makefile b/package/network/services/uhttpd/Makefile index 796eb6129849..28a817d2e0d6 100644 --- a/package/network/services/uhttpd/Makefile +++ b/package/network/services/uhttpd/Makefile @@ -8,7 +8,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=uhttpd -PKG_RELEASE:=1 +PKG_RELEASE:=2 PKG_SOURCE_PROTO:=git PKG_SOURCE_URL=$(PROJECT_GIT)/project/uhttpd.git diff --git a/package/network/services/uhttpd/files/ubus.default b/package/network/services/uhttpd/files/ubus.default index ca9e72a3150a..b218d3f85d11 100644 --- a/package/network/services/uhttpd/files/ubus.default +++ b/package/network/services/uhttpd/files/ubus.default @@ -3,11 +3,13 @@ if [ -z "$(uci -q get uhttpd.main.ubus_prefix)" ]; then uci set uhttpd.main.ubus_prefix=/ubus uci commit uhttpd + /etc/init.d/uhttpd reload fi [ "$(uci -q get uhttpd.main.ubus_socket)" = "/var/run/ubus.sock" ] && { uci set uhttpd.main.ubus_socket='/var/run/ubus/ubus.sock' uci commit uhttpd + /etc/init.d/uhttpd reload } exit 0
Without this change the config is only committed, but the uhttpd daemon is not reloaded. This reload is needed to apply the config. Without the reload of uhttpd, the ubus server is not available over http and returns a Error 404. This caused problems when installing luci on the snapshots and accessing it without reloading uhttpd. Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> --- package/network/services/uhttpd/Makefile | 2 +- package/network/services/uhttpd/files/ubus.default | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)