jupyterhub / systemdspawner

Spawn JupyterHub single-user notebook servers with systemd
BSD 3-Clause "New" or "Revised" License
92 stars 45 forks source link

Revert "Check state of service before startup and reset if failed" #47

Closed yuvipanda closed 5 years ago

yuvipanda commented 5 years ago

Reverts jupyterhub/systemdspawner#45

yuvipanda commented 5 years ago

@RohitK89 - unfortunately, your PR seems to fail unit tests for is-failed.

11:11 $ sudo -E $(which py.test) tests/
================================================ test session starts =================================================
platform linux -- Python 3.6.7, pytest-4.4.1, py-1.8.0, pluggy-0.9.0
rootdir: /home/yuvipanda/code/systemdspawner
plugins: asyncio-0.10.0
collected 8 items                                                                                                    

tests/test_systemd.py .F......                                                                                 [100%]

====================================================== FAILURES ======================================================
_____________________________________________ test_service_failed_reset ______________________________________________

    @pytest.mark.asyncio
    async def test_service_failed_reset():
        """
        Test service_failed and reset_service
        """
        unit_name = 'systemdspawner-failed-unittest-' + str(time.time())
        # Running a service with an invalid UID makes it enter a failed state
        await systemd.start_transient_service(
            unit_name,
            ['sleep'],
            ['2000'],
            uid='systemdspawner-unittest-does-not-exist',
            working_dir='/'
        )

        await asyncio.sleep(0.1)

>       assert await systemd.service_failed(unit_name)
E       assert False

tests/test_systemd.py:48: AssertionError
------------------------------------------------ Captured stdout call ------------------------------------------------
inactive
------------------------------------------------ Captured stderr call ------------------------------------------------
Failed to start transient service unit: Invalid User setting: systemdspawner-unittest-does-not-exist

When capturing stdout, it looks like the service is marked as 'inactive' rather than 'failed', causing the unit test to fail.

Thoughts on why this could be happening? What version of systemd were you able to test on? I'm on 239

RohitK89 commented 5 years ago

Hmm. . . I ran the tests on a 16.04 machine. I think maybe the sleep is too low. The state is inactive post-reset, but also pre-start if the check is too early. Could you try by bumping up the sleep to 1 second?

RohitK89 commented 5 years ago

I tested on 229

yuvipanda commented 5 years ago

@RohitK89 I tried with 1, same result now. I wonder if systemd treats failure differently in newer versions?

RohitK89 commented 5 years ago

Hmm, possibly?

Maybe there's a better way to try and create a deliberately failing service. . . Let me look into it a bit

RohitK89 commented 5 years ago

Okay. I think this is a better solution: Change the call in the unit test to:

        await systemd.start_transient_service(
            unit_name,
            ['sleep'],
            ['2000'],
            working_dir='/systemdspawner-unittest-does-not-exist'
        )

I've verified that this works on 229 on an Ubuntu 16.04 host. If you'd like, I can cut a PR

yuvipanda commented 5 years ago

@RohitK89 can confirm that works for me too! Would love a PR.

RohitK89 commented 5 years ago

https://github.com/jupyterhub/systemdspawner/pull/48

yuvipanda commented 5 years ago

@RohitK89 Thank you for the prompt response, debug and PR! <3 hope to see more PRs :)