CLI airflow scheduler -D --pid <PIDFile> fails silently if PIDFile given is a relative path

[XD-DENG]

Apache Airflow version: 2.0.0

Environment: Linux & MacOS, venv

• OS (e.g. from /etc/os-release): Ubuntu 18.04.3 LTS / MacOS 10.15.7
• Kernel (e.g. uname -a):
  • Linux *** 5.4.0-1029-aws example_dags/cron_replacement.py #30~18.04.1-Ubuntu SMP Tue Oct 20 11:09:25 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
  • Darwin *** 19.6.0 Darwin Kernel Version 19.6.0: Thu Oct 29 22:56:45 PDT 2020; root:xnu-6153.141.2.2~1/RELEASE_X86_64 x86_64

What happened:

Say I'm in my home dir, running command airflow scheduler -D --pid test.pid (test.pid is a relative path) is supposed to start the scheduler in daemon mode, and the PID will be stored in the file test.pid (if it doesn't exist, it should be created).

However, the scheduler is NOT started. This can be validated by running ps aux | grep airflow | grep scheduler (no process is shown). In the whole process, I don't see any error message.

However, if I change the pid file path to an absolute path, i.e. airflow scheduler -D --pid ${PWD}/test.pid, it successfully start the scheduler in daemon mode (can be validated via the method above).

What you expected to happen:

Even if the PID file path provided is a relative path, the scheduler should be started properly as well.

How to reproduce it:

Described above

Anything else we need to know:

[XD-DENG]

Fixing this issue is straigth-forward: when a PID file path is provided, always extend it to a full absolute path first. But I would like to have someone to help confirm they can reproduce the error first, meanwhile I will find out the root-cause as well of course.

[potiuk]

Confirmed.

The reason is that default "working_directory" is used in in DaemonContext

airflow/airflow/cli/commands/scheduler_command.py

Line 48 in 97eee35

ctx = daemon.DaemonContext(

By default the "working_directory" is set to "/" which means that the pid file is attempted to be created as "/" - which for non-root user will fail. This works for example in Breeze as we use root user there, but in production setup with non-root user it will fail.

I think we should assume (extending to the code in

airflow/airflow/utils/cli.py

Line 222 in 97eee35

def setup_locations(process, pid=None, stdout=None, stderr=None, log=None):

) than when relative path is specified, it is relative to ${AIRFLOW_HOME}.

[XD-DENG]

Thanks @potiuk for helping identify the root cause.

I have different opinion: if it's a relative path, it should be relative to current directory where the user is running the command. I.e., as I mentioned above, if it's relative path X, extend it to CURRENT_DIR/X (rather than assuming it to be AORFLOW_HOME)

Let me know if you agree?

[potiuk]

I think when you deamonize a process, you actually want to run it in a "specific" directory. The whole idea about running process as a daemon in *NIX is to detach it completely from the originating process/command (that's why for example if you want to make a really detached daemon you should fork it twice https://stackoverflow.com/questions/881388/what-is-the-reason-for-performing-a-double-fork-when-creating-a-daemon to avoid zombies (making sure that the daemon process is parented by init process). That's why also the "DaemonContext' has the "/" as working directory set. This is not a mistake - it is pretty deliberate behaviour.

For example such daemonized process should survive and work if someone decides do delete the whole "current directory" whatever it was. The current working directory is pretty much accidental, so creating an output of "daemon" there is not really what I'd expect.

[XD-DENG]

I'm more thinking from an "expectation" perspective:

Say I'm in directory "/home/xd" (my AIRFLOW_HOME may be /home/airflow_user/airflow), and I run airflow scheduler -D --pid test.pid. When I specify test.pid, I'm expecting the PID to be saved in this PID file in current directory, whose path is /home/xd/test.pid. I don't expect the full path to be /home/airflow_user/airflow/test.pid, which will be a confusion.

On the other hand, when we daemonize this process, the only thing to do with path/directory is this file where PID is stored. We don't run a process in a "specific" directory.

[potiuk]

So maybe it's better to require a non-relative file and fail with error if it is not ?

[XD-DENG]

Haha, I agree that's a "compromise" which can make both of us happy. Why not😉

I will create a PR shortly tonight for this.

[ashb]

The current working directory is pretty much accidental, so creating an output of "daemon" there is not really what I'd expect.

I view it as the opposite. Every* unix program that deals with filenames on the command line treats a non-absolute path input as relative to the current directory. We should behave the same. (The fact the program is deamonized is an implementation detail.)

* every one I can think of

[potiuk]

I think we should take https://linux.die.net/man/1/daemonize as the canonical behaviour.

It supports both - changing the working directory (default as "/") and specifying pidfile at the command line. It is not really specified in the man page what happens if you specify relative path, but maybe we can check and do the same.

[XD-DENG]

But for our case, it may not be a good idea to further expose "working directory" or allow changing it. Making it simple should be better.

I prefer to my very first proposal (if the path is relative, expand it with the current directory) which aligns with @ashb 's suggestion. Meanwhile I'm open to different solution if we come to an agreement by majority.

[potiuk]

I am not sure we should reinvent the wheel. I just wonder what "deamonize" does when you do not specify the working dir. I think we should do exactly the same. Not sure how relative paths are treated by it, but I believe we should do exactly what it does.

[ashb]

What deamonize does is not relevant for the behaviour we want.

We are accepting a file input from the command line: what is the most common pattern, and therefor least friction/most expected to a user.

[XD-DENG]

I start to doubt if we are discussing on the same thing now :-)

[ashb]

Since you asked though - daemonize(1) creates the pid file, then changes directory -- i.e. relative pid file would be relative to the cwd.

https://github.com/bmc/daemonize/blob/master/daemonize.c#L461-L500

[XD-DENG]

Hi guys @ashb @potiuk shall we come to a consensus on this?

[potiuk]

No strong opinion there :). If you want to use relative from cwd, I am fine with it :).

[ashb]

No strong opinion there :). If you want to use relative from cwd, I am fine with it :).

Yes please - let's do that then @XD-DENG

[XD-DENG]

Thanks both @potiuk @ashb . The discussion itself was thorough and concrete.

I will update PR #13232 accordingly.