systemd: Write rounded CPU quota to cgroupfs

[hshiina]

This is a carry of opencontainers/runc#4639.

When CPU quota is updated, the value is converted to CPUQuotaPerSecUSec property for passing to systemd. The value can be rounded in the following cases:

• The value is rounded up to the nearest 10ms.
• Depending on CPU period, the value may be rounded during division.

Because of this rounding, systemd and systemd driver may write different values to cgroupfs. In order to avoid this inconsistency, this fix makes systemd driver write the same rounded value to cgroupfs by calculating the value from CPUQuotaPerSecUSec.

Even if systemd writes CPU quota and CPU period to cgroupfs, systemd driver still needs to write to cgroupfs for a case where CPU period is updated to less than the minimum value. In this case, systemd accepts this value by adjusting CPU period while direct update by systemd driver fails. In order to keep this case invalid, systemd driver still needs to update cgroupfs.

Fixes opencontainers/runc#4622

[kolyshkin]

1. Why don't we just do

r = adjustResources(r)

at the very beginning of Set methods? This way we won't have to call convertToCpuQuotaPerSecUsec twice.

2. We don't have CI in this repo yet, so this have to wait (until at least #3 is merged).

[kolyshkin]

I would also add a comment to adjustResources to explain why it is needed.

[rata]

@hshiina IIUC this didn't address the first comment here. Can you please address it? Or am I missing something?

[hshiina]

@hshiina IIUC this didn't address the first comment here. Can you please address it? Or am I missing something?

At the first revision, I introduced adjustResources() to update resources before writing values to cgroupfs. Now, I removed adjustResources() in order to avoid calling convertToCpuQuotaPerSecUsec() twice. In the current approach, all required calculations are performed in addCPUQuota().

[hshiina]

1. Why don't we just do

r = adjustResources(r)

at the very beginning of Set methods? This way we won't have to call convertToCpuQuotaPerSecUsec twice.

I removed adjustResources(). Now, r.CpuQuota is updated in addCpuQuota().

[rata]

Cool, sorry I missed it, I have not been following this PR.

@kolyshkin PTAL :)

[kolyshkin]

LGTM

[rata]

@AkihiroSuda friendly ping? This fixes a bug people is hitting in Kubernetes

[AkihiroSuda]

Two LGTMs, but the merge button seems unclickable w/o checking "bypass rules" 🤔

[AkihiroSuda]

Let me uncheck "Require branches to be up to date before merging" in https://github.com/opencontainers/cgroups/settings/branch_protection_rules/60411775

[rata]

Do we need a release of this to include it in runc 1.3?

[kolyshkin]

I can make a release but I'd like other PRs to be merged first.

[rata]

Sure, this can be part of a patch release in 1.3 if we can't release this before final 1.3.0 :)

[rata]

I think all code-related PRs are merged (open PRs are more about CI tweaks). Do I smell a release soon? :-D