Skip to content

uwsgi: add package with modules (currently cgi and python plugin) [RFC]#9855

Merged
hnyman merged 1 commit into
openwrt:masterfrom
peter-stadler:uwsgi
Jan 1, 2020
Merged

uwsgi: add package with modules (currently cgi and python plugin) [RFC]#9855
hnyman merged 1 commit into
openwrt:masterfrom
peter-stadler:uwsgi

Conversation

@peter-stadler
Copy link
Copy Markdown
Contributor

@peter-stadler peter-stadler commented Aug 28, 2019

Maintainer: Ansuel Smith ansuelsmth@gmail.com
Compile tested: MIPS 74K, Asus RT-N16, master snapshot
Run tested: MIPS 74K, Asus RT-N16, master snapshot, minimal app using uwsgi-python-plugin:

cat <<EOF >foobar.py
def application(env, start_response):
    start_response('200 OK', [('Content-Type','text/html')])
    return [b"Hello World"]
EOF
uwsgi --plugin /usr/lib/uwsgi/python_plugin.so --http-socket 127.0.0.1:9031 --wsgi-file foobar.py &
wget http://127.0.0.1:9031/; rm index.html foobar.py; fg 

Description: This package is a modular build of uwsgi.

It superseeds the uwsgi-cgi package by splitting off the syslog and the cgi plugin. So luci on nginx could use uwsgi-cgi-plugin instead of uwsgi-cgi, see my other PR for nginx-luci and nginx-ssl-luci.

Additional there is the python plugin that would be needed for etesync: See my other PR for etesync-server, too, that will depend on uwsgi-python-plugin.

More plugins can be enabled if another package will use them.

Edit: Right now my other PRs refer to the old version, I will update them afterwards.

Edit 2: For now, include luci-support using the syslog plugin by default, and remove the uwsgi-cgi package entirely.

@peter-stadler peter-stadler changed the title uwsgi: add package with modules (currently cgi and python plugin) uwsgi: add package with modules (currently cgi and python plugin) [RFC] Aug 29, 2019
@peter-stadler
Copy link
Copy Markdown
Contributor Author

I did update this PR to follow the changes of uwsgi-cgi. I will update the other two PRs later.

@peter-stadler peter-stadler marked this pull request as ready for review October 28, 2019 08:37
@peter-stadler
Copy link
Copy Markdown
Contributor Author

(Fixed the dependencies for uwsgi-python3-plugin: it needs python3-light not python3-base.)

@peter-stadler
Copy link
Copy Markdown
Contributor Author

Should I add a CONFLICTS:=uwsgi-cgi and how to make it clear that it would supersede it?

@peter-stadler
Copy link
Copy Markdown
Contributor Author

(nothing changed)

@neheb
Copy link
Copy Markdown
Contributor

neheb commented Nov 10, 2019

ping @Ansuel

@Ansuel
Copy link
Copy Markdown
Member

Ansuel commented Nov 10, 2019

it's all good but we need to first update dependency of luci or we will create package with missing dependency.
(luci on nginx depend on uwsgi-cgi and this commits renames it to uwsgi-plugins-cgi)

@Ansuel
Copy link
Copy Markdown
Member

Ansuel commented Nov 10, 2019

@peter-stadler i think this should be squashed and also add the drop of the uwsgi-cgi package

@Ansuel
Copy link
Copy Markdown
Member

Ansuel commented Nov 10, 2019

@peter-stadler by testing this i have this error
unable to find logger syslog

so i think it should be compiled with syslog support by default ?

OK NO we just need to include the syslog plugin in the emperor

@peter-stadler
Copy link
Copy Markdown
Contributor Author

The syslog plugin is installed by default, in the define Package/uwsgi/install there is
$(INSTALL_BIN) $(PKG_BUILD_DIR)/syslog_plugin.so $(1)/usr/lib/uwsgi/
But, it has to be loaded by plugin = syslog in the ini file.

Should the plugin be embedded in the binary /usr/sbin/uwsgi?

@Ansuel
Copy link
Copy Markdown
Member

Ansuel commented Nov 10, 2019

i think yes...

I don't know why but uwsgi complains if the emperor is loaded with no request plugin (with only the syslog plugin loaded)

So yes, I think we should embed the plugin

@peter-stadler
Copy link
Copy Markdown
Contributor Author

Squashed the comits and:

  • removed uwsgi-cgi package
  • included luci-support
  • embeded the syslog plugin into the uwsgi binary
    But, the warning is still there: The emperor has no request plugin intentionally.

Do we need yaml (I did remove it for now)?

@Ansuel
Copy link
Copy Markdown
Member

Ansuel commented Nov 10, 2019

LGTM but we need to wait for the nginx commit to be ready

@peter-stadler
Copy link
Copy Markdown
Contributor Author

peter-stadler commented Nov 10, 2019

Got rid of the warning by not using the syslog plugin at all, procd allows to log the output to stderr …
I made another commit for comparing it easily and would squash it if you think it would be a good idea.
Edit: I think when using logger = syslog:… there will be spawned a master for the emperor complaining about no request plugin because it checks it before becoming a emperor, which would not need a request plugin. Without master there is not much logging.
Edit 2: Btw: There is still a lot of logging output from the vassals (with and without syslog). Should we add logto = /dev/null for the luci vassals?

@Ansuel
Copy link
Copy Markdown
Member

Ansuel commented Nov 10, 2019

Does logto option affect also output from the application itself ?

This output would be usefull

uwsgi-emperor: luci: accepted login on / for root from 192.168.3.12

@peter-stadler
Copy link
Copy Markdown
Contributor Author

peter-stadler commented Nov 10, 2019

Yes, it affects it, I will look into a log-route.

@peter-stadler peter-stadler force-pushed the uwsgi branch 2 times, most recently from be56313 to 53b14fd Compare November 10, 2019 20:16
@peter-stadler
Copy link
Copy Markdown
Contributor Author

Should work now: All messages that contain "luci:" are logged in the webui vassal :-)

@Ansuel
Copy link
Copy Markdown
Member

Ansuel commented Nov 10, 2019

i notice that the package gets recompiled every time...

@peter-stadler
Copy link
Copy Markdown
Contributor Author

I left out the static parts that are loaded in the same way in both cases.Tested it now also with a Guest window in Chrome -> The same.

@peter-stadler
Copy link
Copy Markdown
Contributor Author

peter-stadler commented Dec 31, 2019

Hmm, did now a reinstall of the virtual machine and it is working. I do not know what is different now, I did a clean install half an hour ago, too.

@peter-stadler
Copy link
Copy Markdown
Contributor Author

Nevermind, it was in an open browser window. After opening a new incognito window, I get the same error as before.

@peter-stadler
Copy link
Copy Markdown
Contributor Author

peter-stadler commented Dec 31, 2019

So the Nginx version is working, when using the cached files of the uhttpd version and there is one more quirk on a clean install:
opkg update && opkg install luci && opkg install luci-nginx
Then it works in a new incognito window; Nginx logs:

error.log: bind() to 0.0.0.0:80 failed (98: Address in use)
access.log: GET /cgi-bin/luci/ -> 403
access.log: … (static files)
error.log: "/www/ubus/index.html" is not found
access.log: GET /ubus/?1577805659919 -> 404
access.log: … (static files)
access.log: POST /cgi-bin/luci/admin/ubus?1577805659971 -> 200

It still works after opening a new incognito window; Nginx logs additionally:

access.log: GET /cgi-bin/luci/ -> 403
access.log: … (static files)
error.log: "/www/ubus/index.html" is not found
access.log: GET /ubus/?1577805659919 -> 404
access.log: … (static files)
access.log: POST /cgi-bin/luci/admin/ubus?1577805659971 -> 200

It fails when opening a new incognito window after restarting Nginx; It adds the following lines to the access.log (nothing to the error.log):

GET /cgi-bin/luci/ -> 403
… (static files)
GET /ubus/?1577805288828 -> 400
… (static files)
POST /ubus/?1577805288966 -> 200
POST /ubus/?1577805288978 -> 200

@peter-stadler
Copy link
Copy Markdown
Contributor Author

peter-stadler commented Dec 31, 2019

We could get a similar log by using location /cgi-bin/luci/admin/ubus in /etc/nginx/luci_uwsgi.conf. Then the difference is only in the size of the last response (to POST /cgi-bin/luci/admin/ubus?1577805659971 -> 200); it is 79 and should be 235.

I will leave it for this year and wish you a good start into the new year.

@Ansuel
Copy link
Copy Markdown
Member

Ansuel commented Dec 31, 2019

So the Nginx version is working, when using the cached files of the uhttpd version and there is one more quirk on a clean install:
opkg update && opkg install luci && opkg install luci-nginx
Then it works in a new incognito window; Nginx logs:

error.log: bind() to 0.0.0.0:80 failed (98: Address in use)
access.log: GET /cgi-bin/luci/ -> 403
access.log: … (static files)
error.log: "/www/ubus/index.html" is not found
access.log: GET /ubus/?1577805659919 -> 404
access.log: … (static files)
access.log: POST /cgi-bin/luci/admin/ubus?1577805659971 -> 200

It still works after opening a new incognito window; Nginx logs additionally:

access.log: GET /cgi-bin/luci/ -> 403
access.log: … (static files)
error.log: "/www/ubus/index.html" is not found
access.log: GET /ubus/?1577805659919 -> 404
access.log: … (static files)
access.log: POST /cgi-bin/luci/admin/ubus?1577805659971 -> 200

It fails when opening a new incognito window after restarting Nginx; It adds the following lines to the access.log (nothing to the error.log):

GET /cgi-bin/luci/ -> 403
… (static files)
GET /ubus/?1577805288828 -> 400
… (static files)
POST /ubus/?1577805288966 -> 200
POST /ubus/?1577805288978 -> 200

i can't get where is the problem in the last log...

the error 400 is intended (used by luci to check if /ubus can be used)
/cgi-bin/luci/admin/ubus
/ubus use the uhttpd ubus integrated module or the nginx ubus integrated module

is just the lua page that use the ubus lua lib

@peter-stadler
Copy link
Copy Markdown
Contributor Author

It hangs with the message Session expired in the browser window. The problem is not shown in the log, maybe if the ubus module in Nginx would be more verbose … Nevertheless it is working if we omit the whole location /ubus section :-)

@Ansuel
Copy link
Copy Markdown
Member

Ansuel commented Jan 1, 2020

I tested with latest commit and nginx + /ubus works correctly o.O

@Ansuel
Copy link
Copy Markdown
Member

Ansuel commented Jan 1, 2020

Anyway there is a dependency recursion problem... make the uwsgi package dependent of the sub package
(+uwsgi -> uwsgi)

@peter-stadler
Copy link
Copy Markdown
Contributor Author

peter-stadler commented Jan 1, 2020

I still have the problem. Can you reproduce it?

  1. Start x86_64.img in qemu:
#!/bin/sh

IMG="openwrt-x86-64-combined-ext4.img"
QEMU="qemu-system-x86_64 -enable-kvm -nographic -m 1G -M q35 \
-drive file=$IMG,id=d0,if=none,bus=0,unit=0 -device ide-hd,drive=d0,bus=ide.0"
MSG="NB: The network needs a minute! tests: ifconfig; nslookup openwrt.org"
if [ $UID -gt 0 ] 
then
    echo To enable LAN and ssh from host run it as root: sudo $0
    $QEMU
else 
    echo Started as root, enabling LAN, connect to it via: ssh root@192.168.1.1
    echo $MSG
    LAN=openwrtx64tap0
    # create tap interface which will be connected to OpenWrt LAN NIC
    ip tuntap add mode tap $LAN && \
    ip link set dev $LAN up && \
    # configure interface with static ip to avoid overlapping routes                         
    ip addr add 192.168.1.123/24 dev $LAN && \
    $QEMU \
        -device virtio-net-pci,netdev=lan \
        -netdev tap,id=lan,ifname=$LAN,script=no,downscript=no \
        -device virtio-net-pci,netdev=wan \
        -netdev user,id=wan 
    # cleanup. delete tap interface created earlier
    ip addr flush dev $LAN
    ip link set dev $LAN down
    ip tuntap del mode tap dev $LAN
fi
  1. Run opkg update && opkg install luci-nginx && service nginx restart in it.
  2. Open a incognito window at http://192.168.1.1/cgi-bin/luci/

I get a message 'Session expired' that repeats when clicking on To login... (the same in Firefox).
Edit: It works before restarting Nginx but then I think it does not use its ubus module.

@peter-stadler
Copy link
Copy Markdown
Contributor Author

I did fix the dependencies, changing also the DEPENDS for nginx-mod-luci/default. Thank you :-)

@Ansuel
Copy link
Copy Markdown
Member

Ansuel commented Jan 1, 2020

made a PR. #10923
This is ready to merge.

@peter-stadler
Copy link
Copy Markdown
Contributor Author

Ah, I see. Sorry I forgot about that change. It is working.
I rebased this PR, tested it and consider it ready to merge :-)

@Ansuel
Copy link
Copy Markdown
Member

Ansuel commented Jan 1, 2020

mh can you remove the nginx changes ?

@peter-stadler
Copy link
Copy Markdown
Contributor Author

Done that, I will make another small PR for Nginx.

@Ansuel
Copy link
Copy Markdown
Member

Ansuel commented Jan 1, 2020

do we really need that?

@peter-stadler
Copy link
Copy Markdown
Contributor Author

I think else it will not be shown in make menuconfig. Or how should I change the uwsgi DEPENDS?

@peter-stadler
Copy link
Copy Markdown
Contributor Author

The draft would be at #10925

@Ansuel
Copy link
Copy Markdown
Member

Ansuel commented Jan 1, 2020

Tested now and yes we need uwsgi in the nginx support package... as we need to change it anyway... At this point better make a mini pr also for nginx and rename the support package here to a better name. Sorry for all the mess.

After this and the mini pr get merged we can start working on nginx draft pr. Is it good for you?

@peter-stadler
Copy link
Copy Markdown
Contributor Author

This is perfect :-)

Provide the minimal applications and plugins for: cgi, filelog, syslog and
python3. More plugins can be added if needed by other packages. Autostart
uwsgi in emperor mode loading vassals on demand.

For now, include luci-support (maybe it will be moved to another package),
which uses the syslog plugin by default.

Signed-off-by: Peter Stadler <peter.stadler@student.uibk.ac.at>
@peter-stadler
Copy link
Copy Markdown
Contributor Author

The above draft PR is a mini PR that can be merged after this one. I changed the name to uwsgi-luci-support ...

@peter-stadler
Copy link
Copy Markdown
Contributor Author

I could also update the Nginx version later on if you think that would be better. And if you have another name for the uwsgi-luci-support I would change that, too ...

@Ansuel
Copy link
Copy Markdown
Member

Ansuel commented Jan 1, 2020

@hnyman good to merge
@peter-stadler convert this to a pr #10925 so it can be merged

@hnyman hnyman merged commit e020c10 into openwrt:master Jan 1, 2020
@peter-stadler peter-stadler deleted the uwsgi branch January 3, 2020 09:57
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