Page MenuHomePhabricator

Uwsgi webservices have strange routing behavior when tool name and first component of route path are exact matches
Closed, ResolvedPublicBUG REPORT

Description

The Python flask application https://gitlab.wikimedia.org/toolforge-repos/ifttt behaves differently when deployed as the "ifttt" tool vs any other toolname. Specifically the /ifttt/v1/... routes in the application return an unexpected 404 response when deployed as https://ifttt.toolforge.org/ifttt/v1/.... Changing the called URL to https://ifttt.toolforge.org//ifttt/v1/... (double solidus to start the path) produces the expected responses from the application.

When the tool name is anything other than "ifttt", for example "ifttt-bd808" or "ifttt-dev", the URLs work as expected. It took me a couple of days to convince myself that this is a bug outside of the code and configuration I have been working on for T294448: <Break-Fix> IFTTT integration: fix or sunset?.

I think this behavior is related to the --mount "/$TOOL=$HOME/www/python/src/app.py" cli arguments added to the uwsgi command that is executed inside of the kubernetes container. The documentation is a bit confusing, but I think this line was needed prior to T234617: Toolforge. introduce new domain toolforge.org and is now unnecessary. The fact that things work as generally expected when the $TOOL is not the same as the first path component of the route may be related to https://github.com/unbit/uwsgi/issues/1935

Details

Related Changes in Gerrit:
Related Changes in GitLab:
TitleReferenceAuthorSource BranchDest Branch
python: Replace --mount with --wsgi-filerepos/cloud/toolforge/webservice-cli!1raymond-ndibebryan_replace_mount_with_wsgi_filemaster
Customize query in GitLab

Event Timeline

Testing a fix for this is a bit tricky as the code that needs to change is inside of the container image as the /usr/bin/webservice-runner command. I think I can rig up a testing scenario by creating a manual Deployment that mounts an alternate webservice-runner into to the container.

bd808 triaged this task as Medium priority.
bd808 added a project: cloud-services-team.

I think these changes in the webservice-runner script used in the python3.9 container will fix the bug:

--- webservice-runner   2023-05-31 23:17:47.840616786 +0000
+++ webservice-runner-no-mount  2023-05-31 23:04:58.194231033 +0000
@@ -4,12 +4,6 @@
 # Make sure $USER is always set
 USER=${USER:-$(whoami)}

-# Use awk to split by period and return the second item (tool names can't have
-# periods in them)
-# tools.foo -> foo
-# toolsbeta.foo -> foo
-TOOL=$(echo $USER | awk -F. '{print $2}')
-
 if [ -d "$HOME/www/python/venv" ]; then
   venv="--venv $HOME/www/python/venv"
 else
@@ -34,7 +28,6 @@
     --callable app \
     --manage-script-name \
     --workers 4 \
-    --mount "/$TOOL=$HOME/www/python/src/app.py" \
     --die-on-term \
     --strict \
     --master \

I am going to try manually applying this inside the Pod for ifttt.toolforge.org by patching its deployment.apps/ifttt:

--- Deployment.old      2023-05-31 23:21:56.669147626 +0000
+++ Deployment  2023-05-31 23:25:12.367916463 +0000
@@ -55,9 +55,17 @@
           requests:
             cpu: 125m
             memory: 256Mi
+        volumeMounts:
+        - mountPath: /usr/bin/webservice-runner
+          name: webservice-runner
         terminationMessagePath: /dev/termination-log
         terminationMessagePolicy: File
         workingDir: /data/project/ifttt/
+      volumes:
+      - name: webservice-runner
+        hostPath:
+          path: /data/project/ifttt/webservice-runner-no-mount
+          type: File
       dnsPolicy: ClusterFirst
       restartPolicy: Always
       schedulerName: default-scheduler

My guess in T337897#8894062 was almost correct. It turns out that --wsgi-file app.py is needed to replace the prior --mount command. The full path is not needed because we have an existing --chdir $HOME/www/python/src argument in the command.

Change 925097 had a related patch set uploaded (by BryanDavis; author: Bryan Davis):

[operations/software/tools-webservice@master] python: Replace --mount with --wsgi-file

https://gerrit.wikimedia.org/r/925097

Change 925099 had a related patch set uploaded (by BryanDavis; author: Bryan Davis):

[operations/docker-images/toollabs-images@master] python: Replace --mount with --wsgi-file in webservice-runner

https://gerrit.wikimedia.org/r/925099

Change 925099 merged by jenkins-bot:

[operations/docker-images/toollabs-images@master] python: Replace --mount with --wsgi-file in webservice-runner

https://gerrit.wikimedia.org/r/925099

Mentioned in SAL (#wikimedia-cloud) [2023-06-08T20:21:37Z] <bd808> Rebuilding container images (T337897)

Change 925097 abandoned by BryanDavis:

[operations/software/tools-webservice@master] python: Replace --mount with --wsgi-file

Reason:

Moved to https://gitlab.wikimedia.org/repos/ci-tools/commit-message-validator/-/merge_requests/2

https://gerrit.wikimedia.org/r/925097