diff mbox series

[v3] procd: add procd json output to init

Message ID 20201217084008.19774-1-fe@dev.tdt.de
State Rejected
Headers show
Series [v3] procd: add procd json output to init | expand

Commit Message

Florian Eckert Dec. 17, 2020, 8:40 a.m. UTC
By adding the extra command `procd` it is now possible to retrieve all
relevant data from a procd started service directly via the init script.

Until now, you have to query the ubus to get the information with the
following command.

`ubus call service list '{"name":"<xxx>","verbose":true}'`

With this change, the init script is now extend with the command to get
this information easier.

`/etc/init.d/<xxx> procd`

Signed-off-by: Florian Eckert <fe@dev.tdt.de>
---
v2:
- Remove duplicate json service string

v3:
- Change init call from `info` to `procd`
- Update PKG_RELEASE for base-files and procd package

 package/base-files/Makefile            |  2 +-
 package/base-files/files/etc/rc.common |  5 +++++
 package/system/procd/Makefile          |  2 +-
 package/system/procd/files/procd.sh    | 14 ++++++++++++++
 4 files changed, 21 insertions(+), 2 deletions(-)

Comments

Petr Štetiar Dec. 17, 2020, 9:12 a.m. UTC | #1
Florian Eckert <fe@dev.tdt.de> [2020-12-17 09:40:08]:

Hi,

> By adding the extra command `procd` it is now possible to retrieve all
> relevant data from a procd started service directly via the init script.
> 
> Until now, you have to query the ubus to get the information with the
> following command.
> 
> `ubus call service list '{"name":"<xxx>","verbose":true}'`
> 
> With this change, the init script is now extend with the command to get
> this information easier.

I still lack the information about your use case, how do you use this output
of this command.

BTW it looks like you're doing something similar to what we're already doing
in the `status` command.

Maybe you just want to implement `status_verbose` (or such) command with all
the details you would like to get, but in human readable format?

I see the service command as CLI mainly for end users, so in my oppinion it
should provide human readable output.

> +	json_init
> +	json_add_string name "$service"
> +	json_add_boolean verbose "1"
> +	json_add_string name "$service"

Duplicate json_add_string line.

-- ynezz
Florian Eckert Dec. 17, 2020, 9:34 a.m. UTC | #2
Hello Petr

On 2020-12-17 10:12, Petr Štetiar wrote:
> Florian Eckert <fe@dev.tdt.de> [2020-12-17 09:40:08]:
>> With this change, the init script is now extend with the command to 
>> get
>> this information easier.
> 
> I still lack the information about your use case, how do you use this 
> output
> of this command.

Should I write this in the commit messsage?
If yes, then I will do that.
I just thought that this is clearly evident to simplify the input.
So that I do not always have to type the whole string.

> BTW it looks like you're doing something similar to what we're already 
> doing
> in the `status` command.

Yes, but that only returns `running`.
But I want to see more information.

> Maybe you just want to implement `status_verbose` (or such) command 
> with all
> the details you would like to get, but in human readable format?

The command `status_verbose` would be good as a command.
But the whole json is quit complex.
I do not think we could convert this to a non json output.

> I see the service command as CLI mainly for end users, so in my 
> oppinion it
> should provide human readable output.

I didn't realize it that way, that only human readable output are 
allowed.
The different service in the openwrt, which use the procd add also 
extra_commands.
I'm not sure if these are always human readable outputs.

>> +	json_init
>> +	json_add_string name "$service"
>> +	json_add_boolean verbose "1"
>> +	json_add_string name "$service"
> 
> Duplicate json_add_string line.

Damn, that slipped back in.
That's what happens when you don't update the thing right away!
My fault -> sorry

Best regards

Florian
Petr Štetiar Dec. 17, 2020, 9:56 a.m. UTC | #3
Florian Eckert <fe@dev.tdt.de> [2020-12-17 10:34:25]:

> So that I do not always have to type the whole string.

Well, you don't need to.

root@OpenWrt:/# cat ~/.shinit
procd_service_list() {
        ubus call service list "{'name':\"$1\",'verbose':true}"
}

root@OpenWrt:/# procd_service_list urngd
{
        "urngd": {
                "instances": {
                        "instance1": {
                                "running": true,
                                "pid": 500,
                                "command": [
                                        "/sbin/urngd"
                                ],
                                "term_timeout": 5
                        }
                }
        }
}

Then just put this into your files/root/.shinit file and you've it in all your
images.

> I didn't realize it that way, that only human readable output are allowed.

allowed != expected

Cheers,

Petr
diff mbox series

Patch

diff --git a/package/base-files/Makefile b/package/base-files/Makefile
index 0c612b73ca..fbcb694592 100644
--- a/package/base-files/Makefile
+++ b/package/base-files/Makefile
@@ -12,7 +12,7 @@  include $(INCLUDE_DIR)/version.mk
 include $(INCLUDE_DIR)/feeds.mk
 
 PKG_NAME:=base-files
-PKG_RELEASE:=239
+PKG_RELEASE:=240
 PKG_FLAGS:=nonshared
 
 PKG_FILE_DEPENDS:=$(PLATFORM_DIR)/ $(GENERIC_PLATFORM_DIR)/base-files/
diff --git a/package/base-files/files/etc/rc.common b/package/base-files/files/etc/rc.common
index f39b69464e..1a4ee2929b 100755
--- a/package/base-files/files/etc/rc.common
+++ b/package/base-files/files/etc/rc.common
@@ -121,6 +121,7 @@  extra_command "enabled" "Check if service is started on boot"
 	extra_command "running" "Check if service is running"
 	extra_command "status" "Service status"
 	extra_command "trace" "Start with syscall trace"
+	extra_command "procd" "Show procd info as json"
 
 	. $IPKG_INSTROOT/lib/functions/procd.sh
 	basescript=$(readlink "$initscript")
@@ -173,6 +174,10 @@  extra_command "enabled" "Check if service is started on boot"
 			_procd_status "$(basename ${basescript:-$initscript})" "$1"
 		fi
 	}
+
+	procd() {
+		_procd_info "$(basename ${basescript:-$initscript})"
+	}
 }
 
 ALL_COMMANDS="${ALL_COMMANDS} ${EXTRA_COMMANDS}"
diff --git a/package/system/procd/Makefile b/package/system/procd/Makefile
index ed73587056..ad0ead2346 100644
--- a/package/system/procd/Makefile
+++ b/package/system/procd/Makefile
@@ -8,7 +8,7 @@ 
 include $(TOPDIR)/rules.mk
 
 PKG_NAME:=procd
-PKG_RELEASE:=2
+PKG_RELEASE:=3
 
 PKG_SOURCE_PROTO:=git
 PKG_SOURCE_URL=$(PROJECT_GIT)/project/procd.git
diff --git a/package/system/procd/files/procd.sh b/package/system/procd/files/procd.sh
index d86b7219da..1969cdf973 100644
--- a/package/system/procd/files/procd.sh
+++ b/package/system/procd/files/procd.sh
@@ -474,6 +474,20 @@  _procd_status() {
 	fi
 }
 
+_procd_info() {
+	local service="$1"
+	local data
+
+	data=$(_procd_ubus_call list | jsonfilter -e '@["'"$service"'"]')
+	[ -z "$data" ] && { echo "not found"; return 3; }
+
+	json_init
+	json_add_string name "$service"
+	json_add_boolean verbose "1"
+	json_add_string name "$service"
+	_procd_ubus_call list
+}
+
 procd_open_data() {
 	local name="$1"
 	json_set_namespace procd __procd_old_cb