Repository: opencontainers/runc
File: libcontainer/process_linux.go (Go)

Code before:
		sub = p.process.SubCgroupPaths[""]
	}
	cgroup := path.Join(base, sub)
	if !strings.HasPrefix(cgroup, base) {
		return nil, fmt.Errorf("bad sub cgroup path: %s", sub)
	}

	fd, err := cgroups.OpenFile(base, sub, openFlags)
	if err != nil {
		if p.rootlessCgroups {
			return nil, nil
		}
		// Failed to open the configured cgroup, fall back to container init's cgroup,
		// unless sub-cgroup is explicitly requested.
		if sub == "" {
			if initCg := p.initProcessCgroupPath(); initCg != "" {
				logrus.Debugf("failed to open configured cgroup %q: %v, try using container init's cgroup %q",
					cgroup, err, initCg)
				// We do not freeze the container so cgroup is not guaranteed to exist.
				fd, err = cgroups.OpenFile(initCg, "", openFlags)
				cgroup = initCg
			}
		}
		if err != nil {
			return nil, fmt.Errorf("can't open cgroup: %w", err)
		}
	}

	logrus.Debugf("using CLONE_INTO_CGROUP %q", cgroup)
	if p.cmd.SysProcAttr == nil {
		p.cmd.SysProcAttr = &syscall.SysProcAttr{}
	}
	p.cmd.SysProcAttr.UseCgroupFD = true
	p.cmd.SysProcAttr.CgroupFD = int(fd.Fd())

	return fd, nil
}

// startWithCgroupFD starts a process via clone3 with CLONE_INTO_CGROUP,
// with a fallback if it fails (e.g. not available).
func (p *setnsProcess) startWithCgroupFD() error {
	// Close the child side of the pipes.
	defer p.comm.closeChild()

	fd, err := p.prepareCgroupFD()
	if err != nil {
		return err
	}
	if fd != nil {
		defer fd.Close()
	}

Review comment (refactor):
I don't like this last `if err != nil` handling the two error cases: the sub != "" case as well as the initCg open file. It seems more tricky than needed.

It seems clearer for me if we change it to be like:
```
if sub != "" {
   return nil, fmt.Errorf(...).
} 
if initCg := p.initProcess... {
   ....
   fd, err := cgroups.OpenFile(initCg, ...)
  if err != nil {
     return ...
  }
}
```

The err here catching both the `if sub != ""` case as well as the fallback to open the initCg seems fragile. Like very easy to make a mistake in a code refactor, trickier to reason about.

Let's just handle the errors as soon as they happen. I'm fine also if you want to change code slightly before this diff and do like:
```
if err != nil && p.rootlessCgroup { 
   return nil, nil
}
if err != nil && sub != "" {
   return ...
}
if err != nil {
   ... The code we have now 
}
```

OH, there is more! It is also catching the case that `initCg == ""`. I think we need to make the code more explicit. I think we should do an explicit check for `initCg == ""` and return in that case too.

Code after:
	const openFlags = unix.O_PATH | unix.O_DIRECTORY | unix.O_CLOEXEC

	if !cgroups.IsCgroup2UnifiedMode() {
		return nil, nil
	}

	base := p.manager.Path("")
	if base == "" { // No cgroup to join.
		return nil, nil
	}
	sub := ""
	if p.process.SubCgroupPaths != nil {
		sub = p.process.SubCgroupPaths[""]
	}
	cgroup := path.Join(base, sub)
	if !strings.HasPrefix(cgroup, base) {
		return nil, fmt.Errorf("bad sub cgroup path: %s", sub)
	}

	fd, err := cgroups.OpenFile(base, sub, openFlags)
	if err == nil {
		goto success
	}
	// Failed to open the configured cgroup. Fall back to container init's cgroup
	// unless sub-cgroup is explicitly requested. The fallback logic should be
	// the same as in addIntoCgroupV2.
	if sub != "" {
		goto fail
	}
	cgroup = p.initProcessCgroupPath()
	if cgroup == "" {
		goto fail
	}
	logrus.Debugf("failed to open configured cgroup (%v), will open container init cgroup %q", err, cgroup)
	// NOTE: path is not guaranteed to exist because we didn't pause the container.
	fd, err = cgroups.OpenFile(cgroup, "", openFlags)
	if err != nil {
		goto fail
	}

success:
	logrus.Debugf("using CLONE_INTO_CGROUP %q", cgroup)
	if p.cmd.SysProcAttr == nil {
		p.cmd.SysProcAttr = &syscall.SysProcAttr{}
	}
	p.cmd.SysProcAttr.UseCgroupFD = true
	p.cmd.SysProcAttr.CgroupFD = int(fd.Fd())

	return fd, nil

fail: