systemd cgroup driver: Split support for unified cgroup2 hierarchy into its own Manager

[filbranden]

Split support for unified cgroup2 hierarchy into its own Manager in the systemd cgroup driver

Detection is done only once, as the manager is instantiated, and then the two implementations are for the most part separate.

This will allow us to iterate on the unified cgroup2 implementation without worrying about compatibility with the legacy implementation.

For now, the unified cgroup2 implementation consists of stubs that need to be filled up with actual implementation.

/cc @giuseppe @rhatdan

I think I'd prefer if we started from a split like this, then added the bits needed for cgroup2 support. What do you think?

[rhatdan]

Works for me. But @giuseppe is doing the work.

[giuseppe]

@filbranden thanks for putting this up!

Split support for unified cgroup2 hierarchy into its own Manager in the systemd cgroup driver

Detection is done only once, as the manager is instantiated, and then the two implementations are for the most part separate.

In the PR I've opened the detection is also done only once (using a sync.Once).

This will allow us to iterate on the unified cgroup2 implementation without worrying about compatibility with the legacy implementation.

For now, the unified cgroup2 implementation consists of stubs that need to be filled up with actual implementation.

I am fine with some higher level refactoring and split the implementation into a new manager.

The biggest part seems to be how to we will handle the subsystems manager. Are we going to have two versions of GetStats() and Apply()?

I thought of splitting it in something like:

ApplyLegacy() and ApplyUnified(), then Apply can be backward compatible and look like:

func (s *CpuGroup) Apply(d *cgroupData) error {
       if cgroups.IsCgroup2UnifiedMode() {
	    return s.ApplyUnified(d)
       } else {
	    return s.ApplyLegacy(d)
       }
}

what do you think?

[filbranden]

I am fine with some higher level refactoring and split the implementation into a new manager.

The biggest part seems to be how to we will handle the subsystems manager. Are we going to have two versions of GetStats() and Apply()?

I feel like reusing the current subsystems with &fs.*Group{} is the wrong approach though.

Why not create a new systemd.UnifiedCpuGroup (and same for other cgroups) and have its Apply() and GetStats() do the "unified" version?

Yes, there's perhaps some opportunity for reuse of code currently in fs and in systemd legacy cgroup, but I guess then just refactoring those bits to export the relevant bits would be good.

Does that make sense?

[giuseppe]

yes thanks, I'll refactor the code and split cgroups v2 stuff in new files under cgroups/fs. I'll add also a new option for fs.Manager to support cgroupvs2. It should be enough to configure the subsystems used.

I won't be available next week, so it might take some time before I submit an updated version.

[filbranden]

Sounds good @giuseppe !

Yes, adding those to cgroup/fs/ makes sense if you want to support the systemd-less case with an fs.UnifiedManager as well.

Whenever you have a new PR, I'm happy to review it.

Cheers!
Filipe

[AkihiroSuda]

nit: you don't need else here. Some lint tool may print a warning.

[AkihiroSuda]

needs rebase

[filbranden]

This has been incorporated in @giuseppe's #2113, so closing now.