kvp daemon first

[rjschwei]

Ensure the kvp data is populated in Azure, run after the hv_kvp_daemon.

[Moustafa-Moustafa]

We first need to make sure that hv_kvp_daemon.service doesn't have the Requires and After directives on network-online.target. otherwise we will risk having circular dependencies.
cloud-init.service After cloud-init-local.service After hv_kvp_daemon.service After network-online.target After cloud-init.service

The hv_kvp_daemon.service in the current latest sles-15-sp1 image (SUSE:sles-15-sp1:gen1:2020.06.10) has both the Requires and After directives on network-online.target.

[rjschwei]

We first need to make sure that hv_kvp_daemon.service doesn't have the Requires and After directives on network-online.target. otherwise we will risk having circular dependencies.
cloud-init.service After cloud-init-local.service After hv_kvp_daemon.service After network-online.target After cloud-init.service

The hv_kvp_daemon.service in the current latest sles-15-sp1 image (SUSE:sles-15-sp1:gen1:2020.06.10) has both the Requires and After directives on network-online.target.

That's a packaging and release timing exercise. How distributions implement the kvp daemon should not gate upstream changes.

[trstringer]

Another consideration for packaging for the KVP daemon is the handling of the vmbus device (having a udev rules to tag it for systemd management) as well as having the KVP daemon with a dependency on that device.

[trstringer]

Another thing worth considering is that not all KVP services are named the same. This name in the PR hv-kvp-daemon.service is the same for Ubuntu as well, but Debian has the service named hyperv-daemons.hv-kvp-daemon.service.

Perhaps the right answer is to work with the distro package maintainers for the hyperv daemons package to add an Alias=hv-kvp-daemon.service directive to Install.

[OddBloke]

Another thing worth considering is that not all KVP services are named the same. This name in the PR hv-kvp-daemon.service is the same for Ubuntu as well, but Debian has the service named hyperv-daemons.hv-kvp-daemon.service.

After= only affects the ordering of services that are being started at the same time; it does not cause systemd to start a listed service if it would not otherwise be started (that's what Wants=/Requires= are for). It follows that we can list all namings of this service here without problem; the directive is only relevant if something else is pulling in the named service, and nothing should pull those named services in on systems where they don't exist.

You can, in fact, see that we already do this for networking:

Before=NetworkManager.service
Before=network-pre.target

and this is fine on an Ubuntu server system:

$ grep NetworkManager.service /lib/systemd/system/cloud-init-local.service 
Before=NetworkManager.service
$ systemctl status NetworkManager.service
Unit NetworkManager.service could not be found.

Perhaps the right answer is to work with the distro package maintainers for the hyperv daemons package to add an Alias=hv-kvp-daemon.service directive to Install.

We don't need to do this to be able to add After= for the existing names of services, and we probably want to list out those names anyway, because there will be circumstances where systems don't have the new daemon package with its alias but do have a new cloud-init. (That said, the alias sounds like it could be helpful for users, so I think it's worth pursuing in parallel, but it is perhaps orthogonal to this PR.)

[github-actions]

Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close.

If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging mitechie, and he will ensure that someone takes a look soon.

(If the pull request is closed, please do feel free to reopen it if you wish to continue working on it.)

[rjschwei]

@OddBloke Anything for me to do or can this be merged?

[mitechie]

@rjschwei he went on holiday and we've been working to get to the new release so reviewers are scarce atm. I'll try to get it looked at here soon.

[OddBloke]

This LGTM, thanks!