Added a concurrency policy option for scheduledScan CRD #1749
Conversation
🦙 MegaLinter status:
|
| Descriptor | Linter | Files | Fixed | Errors | Elapsed time |
|---|---|---|---|---|---|
| golangci-lint | 5 | 1 | 1.99s |
See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff
MegaLinter is graciously provided by
Ilyesbdlala
force-pushed
the
feature/concurrency-policy
branch
from
0bdc038 to
689ec78
Compare
There was a problem hiding this comment.
Works great 👍
One small note about a timeout value.
Also I think it would be good if we started attaching events to the ScheduledScans in Replace / Forbid cases to really communicate / logs to the users what's happening.
The kubernetes cronjob controller does this (in the Replace case) by attaching an event to the cronjob each time it creates / deletes a job:
| // check concurrency policy | ||
| if scheduledScan.Spec.ConcurrencyPolicy == executionv1.ForbidConcurrent && len(InProgressScans) > 0 { | ||
| log.V(8).Info("concurrency policy blocks concurrent runs, skipping", "num active", len(InProgressScans)) | ||
| return ctrl.Result{RequeueAfter: 5 * time.Minute}, nil |
There was a problem hiding this comment.
5m seems quite long. Especially as both interval and cron based scans can go lower than that.
Maybe rather use 5s? Should not be too much of a resource drain.
There was a problem hiding this comment.
My reasoning was that it's basically waiting on a scan to finish i.e go from in progress to Done or Errored State. On second thought, 5 min is a bit too much. I would set it to 1 min, unless you know the resource drain is basically negligible.
There was a problem hiding this comment.
I'd think the resource drain to be minimal, especially as this reque only happens when the Forbid case happens. So hopefully only rarely. 1m would also be ok for me.
Ilyesbdlala
force-pushed
the
feature/concurrency-policy
branch
from
7e5d220 to
463d4c0
Compare
This is done as a compromise between speed and performance see discussion: #1749 (comment) Signed-off-by: Ilyes Ben Dlala <ilyes.bendlala@iteratec.com>
Signed-off-by: Ilyes Ben Dlala <ilyes.bendlala@iteratec.com>
Signed-off-by: Ilyes Ben Dlala <ilyes.bendlala@iteratec.com>
Signed-off-by: Ilyes Ben Dlala <ilyes.bendlala@iteratec.com>
This is done as a compromise between speed and performance see discussion: #1749 (comment) Signed-off-by: Ilyes Ben Dlala <ilyes.bendlala@iteratec.com>
Ilyesbdlala
force-pushed
the
feature/concurrency-policy
branch
from
fd48eeb to
4e7c8dc
Compare
… after rebase Signed-off-by: Ilyes Ben Dlala <ilyes.bendlala@iteratec.com>
This is done as a compromise between speed and performance see discussion: #1749 (comment) Signed-off-by: Ilyes Ben Dlala <ilyes.bendlala@iteratec.com>
Description
closes #275
Checklist