kms: Add semaphore to limit concurrency

[lentzi90]

generateCipher is memory heavy, so to avoid OOM situations, a semaphore is added to limit concurrency here.

Fixes: #3472

NOTE: I think the original issue is already resolved (see discussion in the issue). I pushed this anyway because I had already done the work and hoping that it can be useful in other situations or just for discussion. Feel free to close!

---

Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

• /retest ci/centos/<job-name>: retest the <job-name> after unrelated
  failure (please report the failure too!)
• /retest all: run this in case the CentOS CI failed to start/report any test
  progress or results

[nixpanic]

I think this is good to have, it adds to the stability of ceph-csi when multiple volumes are created at the same time. Volume creation is not a critical part of a workflow when running applications, a slight delay while generating the cipher should be acceptable for increased stability.

[nixpanic]

nit, this could be part of the above require block

[nixpanic]

commitlint fails because kms is not a valid prefix for commits. Use util instead?

[trociny]

@lentzi90 Is there any estimate how "heavy" the generateCipher may be?

Because from the comment [1] of the related issue, the oom-killer was invoked by cryptsetup process, which is called by EncryptVolume, i.e. after generateCipher. And it looks like the memory limit was reached due to spawning many cryptsetup processes simultaneously and this patch will not prevent this (although it might throttle it a little and make the problem more difficult to reproduce).

It seems to me that a solution could be to limit the number of NodeStageVolume concurrent calls. Looking at the code, I suppose it is in this loop [2], where we spawn all NodeStageVolume calls concurrently [3] and then just wait for them to complete. I think it could do it in configurable chunks, e.g. not more than 10 calls simultaneously.

[1] #3472 (comment)
[2]

ceph-csi/internal/rbd/rbd_healer.go

Line 179 in 991c21f

for i := range val.Items {

[3]

ceph-csi/internal/rbd/rbd_healer.go

Line 219 in 991c21f

// run multiple NodeStageVolume calls concurrently

[lentzi90]

@trociny like I mentioned on the issue already, I think this is a different issue. I just pushed the code that I already had for the original issue (which seems to be fixed already).
If you (or anyone else) would like to take this over and push a new PR I would be very happy. Unfortunately I won't have time to work on this in the near time.

[mergify]

This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in two weeks if no further activity occurs. Thank you for your contributions.

[github-actions]

This pull request has been automatically closed due to inactivity. Please re-open if these changes are still required.