Added a concurrency policy option for scheduledScan CRD#1749
Added a concurrency policy option for scheduledScan CRD#1749Ilyesbdlala merged 5 commits intomainfrom
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
0bdc038 to
689ec78
Compare
J12934
left a comment
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.
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>
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