Makes sense (from your description). I was slightly confused if both After= and WantedBy= could point to multi-user.target, but apparently that's possible; https://unix.stackexchange.com/questions/503679/systemd-unit-file-wantedby-and-after

While it's ok to merge this change here, note that the systemd unit file that's included in the packages is taken from this repository; https://github.com/docker/docker-ce-packaging/tree/master/systemd (we should probably bring those back in this repository, after reviewing the changes between them, but that's something still on my list 😅)

Could you open a pull request there as well?

Thanks for the review @thaJeztah! This is my first time doing this, so I'm a little unsure how it all goes down, but i'm exited to contribute. I'll go put in a pull request on the docker-ce-packaging repo.

So far you're doing great (thanks!) 👍 Commit message has a DCO sign-off (which people often forget).

One thing I noticed is that you opened this pull request from your own master branch; I recommend creating a branch for each pull request; branches are "cheap" when working with Git, and having your own "master" branch clean, and each "feature / PR" you're working on in its own branch can save you a lot of headaches.

Separate branches allow you to "switch" between features/fixes you're working on; very useful if you have more pull requests open (and some of those take longer to be reviewed, or need more work). It also saves some headaches if you're asked to "rebase" and/or "squash" commits.

We do have a contributors guide, but it was moved between repositories, and has gone outdated unfortunately; it might still have some useful tips though; https://github.com/moby/moby/tree/master/docs/contributing

I'm a little hesitant here because I can imagine some users expecting Docker to start as soon as possible during boot (especially if they're running what they consider to be "system" service inside Docker), and this reorders that.

If this is causing trouble, perhaps our inclusion of WantedBy=multi-user.target should actually be something else? Is that the bit that's causing multi-user.target to think it needs to wait for docker.service to be up and running before it considers itself resolved? (I think we want the start-on-boot behavior that WantedBy=multi-user.target implies, but we do not want the dependency relationship that it implies. 🙈)

Looking at the output of systemctl list-units --type target on my local (Debian) system, I don't see a better suggestion, so this LGTM

$ sudo systemctl list-units --type target
UNIT                  LOAD   ACTIVE SUB    DESCRIPTION              
basic.target          loaded active active Basic System             
bluetooth.target      loaded active active Bluetooth                
cryptsetup.target     loaded active active Local Encrypted Volumes  
getty.target          loaded active active Login Prompts            
graphical.target      loaded active active Graphical Interface      
local-fs-pre.target   loaded active active Local File Systems (Pre) 
local-fs.target       loaded active active Local File Systems       
machines.target       loaded active active Containers               
multi-user.target     loaded active active Multi-User System        
network-online.target loaded active active Network is Online        
network.target        loaded active active Network                  
paths.target          loaded active active Paths                    
remote-fs-pre.target  loaded active active Remote File Systems (Pre)
remote-fs.target      loaded active active Remote File Systems      
slices.target         loaded active active Slices                   
sockets.target        loaded active active Sockets                  
sound.target          loaded active active Sound Card               
swap.target           loaded active active Swap                     
sysinit.target        loaded active active System Initialization    
time-sync.target      loaded active active System Time Synchronized 
timers.target         loaded active active Timers                   

LOAD   = Reflects whether the unit definition was properly loaded.
ACTIVE = The high-level unit activation state, i.e. generalization of SUB.
SUB    = The low-level unit activation state, values depend on unit type.

21 loaded units listed. Pass --all to see loaded but inactive units, too.
To show all installed unit files use 'systemctl list-unit-files'.

@tianon good to go, or want more eyes on it?

I think it's good to go -- it'd be great to get more eyes on it (especially those that know systemd deeper than I do), but it seems perfectly sane for now (and trivial for users to override themselves if it does turn out to be a problem, and then we'll get more data 😈).

OK, let's get this in for the next (20.0x) release 👍

Thanks for reverting.
This change apparently causes a regression and breaks all systemd scripts with "After=docker.service" and WantedBy=multi-user.target.
This seems a sane use case for scripts that depend on e.g. docker0 network.

systemd-analyze verify multi-user.target says:

multi-user.target: Found ordering cycle on test.service/start
multi-user.target: Found dependency on docker.service/start
multi-user.target: Found dependency on multi-user.target/start
multi-user.target: Job test.service/start deleted to break ordering cycle starting with multi-user.target/start

To test:
e.g. /lib/systemd/system/test.service

[Unit]
Description=Starts script after docker
After=docker.service
Wants=docker.service

[Service]
Type=simple
ExecStartPre=systemctl is-active sys-devices-virtual-net-docker0.device
ExecStart=sleep 3600
Restart=always
RestartSec=60

[Install]
WantedBy=multi-user.target

Result:

 test.service - Starts script after docker
   Loaded: loaded (/lib/systemd/system/test.service; enabled; vendor preset: enabled)
   Active: inactive (dead)

Before considering this change in the future - bear in mind that cloud-init typically occurs before multi-user.target, and this would ensure that docker interaction could not occur within userdata or runcmd blocks.