[boot] Start ntp-config service after all Docker containers are started

[jleveque]

Docker containers in SONiC rely on supervisord to manage running processes. However, supervisord currently has the following issue: If the system clock rolls backwards (e.g., due to NTP correcting system time) after supervisor has marked a process as being in the STARTING state but before it has entered the RUNNING state, when supervisor goes to change the process' state to RUNNING, supervisor will crash if the current system time precedes the timestamp when it marked the process as STARTING.

Until supervisor is made to work nicely when the system clock rolls backward, this PR will wait to configure and restart the NTP service until after all services which start Docker containers have started.

[nikos-github]

Why does supervisord care about what the clock is doing because of ntp? Is it not using a monotonic clock?

If it's not using a monotonic clock, I don't see how we can keep using supervisord. What if the clock shifts again and supervisord crashes later on or gives some undesirable side effects?

Starting ntp in the system after all the containers are started and correcting the clock afterwards, is creating a dependency that isn't acceptable.

[jleveque]

Why does supervisord care about what the clock is doing because of ntp? Is it not using a monotonic clock?

No. Supervisor currently does not use a monotonic clock. I have asked about moving to monotonic time here. The author has reservations due to the varying availability of monotonic time in different versions of Python that Supervisor supports.

[nikos-github]

No. Supervisor currently does not use a monotonic clock. I have asked about moving to monotonic time here. The author has reservations due to the varying availability of monotonic time in different versions of Python that Supervisor supports.

Thanks for the pointer to the thread. This makes supervisord quite unstable in my opinion. Seems to me we should consider pushing forward for the suggestion in Supervisor/supervisor#468 (comment) to be implemented if supporting older than Python 3 versions is important and an absolute must. Python 3 has been around for 10 years now.

I am also concerned about supervisord's progress in addressing critical issues such as this one. It appears this has been known for 5+ yrs now yet hasn't been addressed. Perfectly fine for a home network but not good enough for something such as sonic.

[lguohan]

@nikos-github , after is different than required. it is just ordering dependency. it does not matter whether service fail to start or start.

[lguohan]

@jleveque , I actually think we should add before= in all other services. In such case, we can manage the ordering between ntp and individual service separately. Especially, when we later introduce new service, we just need to add before in the new service unit file.