[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1025618: cloud-init and firewalld systemd unit files have ordering cycles

On Mon, Dec 12, 2022 at 05:41:46PM -0800, Noah Meyerhans wrote:
> On 12/12/2022 6:44 AM, Sam Hartman wrote:
> >      >> From my quick read: Michael Biebl proposes dropping
> >      >> network-pre.target
> >      Ross> from cloud-init's After=, and replacing it with each of the
> >      Ross> config backends that cloud-init supports.  This sounds pretty
> >      Ross> reasonable, but also like something that upstream should
> >      Ross> address first.
> > 
> > Why wait for upstream?
> > It's a bug affecting Debian users, our systemd maintainer has a solution
> > that you (and I) think is reasonable.
> > The symptom is quite serious.
> > We often make changes before upstream in situations like that,
> > especially when the alternative is:
> > 
> >      Ross> Should we consider adding "Conflicts: firewalld" to cloud-init
> >      Ross> before the freeze?  That's not optimal of course, but it'd
> >      Ross> prevent a user from ending up in this situation for now.
> > 
> > I'd much rather see Debian local changes than conflicts.
> We should simply move this discussion to an upstream pull request rather
> than wait passively for their response. I agree that diverging from upstream
> is preferable to unnecessary conflicts, but it shouldn't be done without
> first consulting with upstream on our proposed solution.

I played with the suggested solution and was unable to get it working:
cloud-init.service doesn't have a /direct/ Before=network-pre.target to remove.
The ordering is implicit in the combination of units.

Probably, I think Michael knew that when he made the suggestion - but I had to
play with it for a few hours first. :)

At a high level the issue is: firewalld.service forces network-pre.target after
sysinit.target, but cloud-init.service forces the other way around.  In detail,
using < to represent Before, the imposed orderings look like:

- from firewalld:
  sysinit.target < dbus.service < firewalld.service < network-pre.target
- from cloud-init:
  cloud-init-local.service < network-pre.target < systemd-networkd-wait-online.service < cloud-init.service < sysinit.target

There's a few approaches to resolving this.  As far as I can tell, the only
immediately viable one (at the bottom) requires users to manually fix this
and accept some trade-offs.  Anyone have any better ideas?

Modify firewalld to run before sysinit.target 

This would let cloud-init and firewalld agree to do network-pre.target before

This is probably not possible since firewalld requires dbus, which starts after
sysinit.target.  There's a thread at [1] about why moving firewalld to be an
early boot service is difficult.

Modify cloud-init to run after sysinit.target

This would let cloud-init and firewalld agree to do network-pre.target after
sysinit.target.  This might not be advisable (see comments in [1] about running
network management services in late boot), but it looks like this is how RHEL
does it [2].

>From [3], I think cloud-init.service added Before=basic.target (which
eventually became Before=sysinit.target) to ensure cloud-init configured block
device mounts were ready early enough in boot process.  The network needs to be
online for this, since some block device config can come from network sources.
So changing this in the Debian package seems risky to me.

Locally override firewalld.service's order

If you need to use both together, create an override unit that removes
Before=network-pre.target.  This eliminates the cycle by allowing cloud-init's
order to win.  But it the network will be up without firewalld for a period.
Unfortunately, dependencies can't be removed in a drop-in - so I think you need
to copy the unit to /etc/systemd/system and modify it.


[1] - https://lists.freedesktop.org/archives/systemd-devel/2022-March/047538.html
[2] - https://github.com/canonical/cloud-init/blob/main/systemd/cloud-init.service.tmpl#L4-L6
[3] - https://github.com/canonical/cloud-init/commit/80f5ec4be0f781b26eca51d90d51abfab396b3f6

Reply to: