distributed icon indicating copy to clipboard operation
distributed copied to clipboard

test_bad_local_directory fails if run with root permissions on Linux.

Open detrout opened this issue 4 years ago • 3 comments

What happened:

Under some test running conditions on at least Debian, test_bad_local_directory fails, because the test assumes that it shouldn't have write permissions to /, but if you're running as root. you can write to /.

What you expected to happen:

It would be nice if the test didn't generate an error when run as root.

Anything else we need to know?:

Environment:

  • Dask version: 2021.08.1, 2021.09.1
  • Python version: 3.9.2
  • Operating System: Debian bookworm
  • Install method (conda, pip, source): Debian package.

So what do you think of this patch (against 2021.09.1) as an alternate solution? I think it would also handle Windows more cleanly than the current try/except/else.

--- a/distributed/tests/test_worker.py
+++ b/distributed/tests/test_worker.py
@@ -1730,6 +1730,8 @@
     assert "Heartbeat to scheduler failed" in logger.getvalue()
 
 
[email protected](not os.access("/", os.W_OK),
+                    reason="skip if we have elevated privileges")
 @gen_cluster(nthreads=[])
 async def test_bad_local_directory(s):
     try:

detrout avatar Oct 09 '21 02:10 detrout

Thanks for reporting @detrout. Are you able to post the traceback you're seeing? I'm wondering where things are going wrong in this test

jrbourbeau avatar Oct 12 '21 20:10 jrbourbeau

Well the problem is more a the lack of stack trace, this is what it looks like when the test fails.

For whatever reason Debian CI test runner defaults to running the application as root in test environment, and root does have the ability to create /not/a/valid-directory so the IOException never gets called, and there are no error messages in the log.

While getting the package tests working it occurred to me the ability to create a directory in the root of a file system explains the else: assert WINDOWS branch of the code to skip the test and that asking python if your process has write access to "/" might be a more general solution.

Here's the full log of just that one test.

(unstable-amd64-sbuild)root@amarana:/tmp/autopkgtest.b1BCxx/autopkgtest_tmp# py.test-3 ../../autopkgtest.b1BCxx/build.BWD/src/distributed/tests/test_worker.py::test_bad_local_directory
========================================================= test session starts ==========================================================
platform linux -- Python 3.9.7, pytest-6.2.5, py-1.10.0, pluggy-0.13.0 -- /usr/bin/python3
cachedir: .pytest_cache
rootdir: /tmp/autopkgtest.b1BCxx/build.BWD/src, configfile: setup.cfg
plugins: timeout-2.0.1
timeout: 300.0s
timeout method: thread
timeout func_only: False
collected 1 item                                                                                                                       

../build.BWD/src/distributed/tests/test_worker.py::test_bad_local_directory FAILED                                               [100%]

=============================================================== FAILURES ===============================================================
_______________________________________________________ test_bad_local_directory _______________________________________________________

s = <Scheduler: "tcp://127.0.0.1:38955" workers: 0 cores: 0, tasks: 0>

    @gen_cluster(nthreads=[])
    async def test_bad_local_directory(s):
        try:
            async with Worker(s.address, local_directory="/not/a/valid-directory"):
                pass
        except OSError:
            # On Linux: [Errno 13] Permission denied: '/not'
            # On MacOSX: [Errno 30] Read-only file system: '/not'
            pass
        else:
>           assert WINDOWS
E           assert False

../build.BWD/src/distributed/tests/test_worker.py:1743: AssertionError
--------------------------------------------------------- Captured stderr call ---------------------------------------------------------
distributed.http.proxy - INFO - To route to workers diagnostics web server please install jupyter-server-proxy: python -m pip install jupyter-server-proxy
distributed.scheduler - INFO - Clear task state
distributed.scheduler - INFO -   Scheduler at:     tcp://127.0.0.1:38955
distributed.scheduler - INFO -   dashboard at:           127.0.0.1:34119
distributed.worker - INFO -       Start worker at:      tcp://127.0.0.1:33447
distributed.worker - INFO -          Listening to:      tcp://127.0.0.1:33447
distributed.worker - INFO -          dashboard at:            127.0.0.1:41861
distributed.worker - INFO - Waiting to connect to:      tcp://127.0.0.1:38955
distributed.worker - INFO - -------------------------------------------------
distributed.worker - INFO -               Threads:                          4
distributed.worker - INFO -                Memory:                  15.52 GiB
distributed.worker - INFO -       Local Directory: /not/a/valid-directory/dask-worker-space/worker-e84uws0z
distributed.worker - INFO - -------------------------------------------------
distributed.scheduler - INFO - Register worker <WorkerState 'tcp://127.0.0.1:33447', name: tcp://127.0.0.1:33447, memory: 0, processing: 0>
distributed.scheduler - INFO - Starting worker compute stream, tcp://127.0.0.1:33447
distributed.core - INFO - Starting established connection
distributed.worker - INFO -         Registered to:      tcp://127.0.0.1:38955
distributed.worker - INFO - -------------------------------------------------
distributed.worker - INFO - Stopping worker at tcp://127.0.0.1:33447
distributed.core - INFO - Starting established connection
distributed.scheduler - INFO - Remove worker <WorkerState 'tcp://127.0.0.1:33447', name: tcp://127.0.0.1:33447, memory: 0, processing: 0>
distributed.core - INFO - Removing comms to tcp://127.0.0.1:33447
distributed.scheduler - INFO - Lost all workers
distributed.scheduler - INFO - Scheduler closing...
distributed.scheduler - INFO - Scheduler closing all comms
=========================================================== warnings summary ===========================================================
../build.BWD/src/distributed/tests/test_worker.py:1099
  /tmp/autopkgtest.b1BCxx/build.BWD/src/distributed/tests/test_worker.py:1099: PytestUnknownMarkWarning: Unknown pytest.mark.flaky - is this a typo?  You can register custom marks to avoid this warning - for details, see https://docs.pytest.org/en/stable/mark.html
    @pytest.mark.flaky(reruns=10, reruns_delay=5)

../build.BWD/src/distributed/tests/test_worker.py:1509
  /tmp/autopkgtest.b1BCxx/build.BWD/src/distributed/tests/test_worker.py:1509: PytestUnknownMarkWarning: Unknown pytest.mark.asyncio - is this a typo?  You can register custom marks to avoid this warning - for details, see https://docs.pytest.org/en/stable/mark.html
    @pytest.mark.asyncio

../build.BWD/src/distributed/tests/test_worker.py:1541
  /tmp/autopkgtest.b1BCxx/build.BWD/src/distributed/tests/test_worker.py:1541: PytestUnknownMarkWarning: Unknown pytest.mark.asyncio - is this a typo?  You can register custom marks to avoid this warning - for details, see https://docs.pytest.org/en/stable/mark.html
    @pytest.mark.asyncio

../build.BWD/src/distributed/tests/test_worker.py:1555
  /tmp/autopkgtest.b1BCxx/build.BWD/src/distributed/tests/test_worker.py:1555: PytestUnknownMarkWarning: Unknown pytest.mark.asyncio - is this a typo?  You can register custom marks to avoid this warning - for details, see https://docs.pytest.org/en/stable/mark.html
    @pytest.mark.asyncio

../build.BWD/src/distributed/tests/test_worker.py:1574
  /tmp/autopkgtest.b1BCxx/build.BWD/src/distributed/tests/test_worker.py:1574: PytestUnknownMarkWarning: Unknown pytest.mark.asyncio - is this a typo?  You can register custom marks to avoid this warning - for details, see https://docs.pytest.org/en/stable/mark.html
    @pytest.mark.asyncio

-- Docs: https://docs.pytest.org/en/stable/warnings.html
========================================================= slowest 20 durations =========================================================
0.04s call     distributed/tests/test_worker.py::test_bad_local_directory

(2 durations < 0.005s hidden.  Use -vv to show these durations.)
======================================================= short test summary info ========================================================
FAILED ../build.BWD/src/distributed/tests/test_worker.py::test_bad_local_directory - assert False
==================================================== 1 failed, 5 warnings in 0.24s =====================================================

detrout avatar Oct 13 '21 06:10 detrout

So what do you think of this patch (against 2021.09.1) as an alternate solution? I think it would also handle Windows more cleanly than the current try/except/else.

A fix to the patch - the logic was accidentally inverted:

--- a/distributed/tests/test_worker.py
+++ b/distributed/tests/test_worker.py
@@ -1730,6 +1730,8 @@
     assert "Heartbeat to scheduler failed" in logger.getvalue()
 
 
[email protected](os.access("/", os.W_OK),
+                    reason="skip if we have elevated privileges")
 @gen_cluster(nthreads=[])
 async def test_bad_local_directory(s):
     try:

juliangilbey avatar Aug 22 '24 10:08 juliangilbey