-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adopt reconciler logic for orchestrator #320
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chrisgacsal
force-pushed
the
refactor-scan-mgmt
branch
30 times, most recently
from
May 30, 2023 11:00
6c93b47
to
6fdec0b
Compare
chrisgacsal
force-pushed
the
refactor-scan-mgmt
branch
from
June 5, 2023 13:24
7aa984e
to
a55d685
Compare
chrisgacsal
changed the title
refactor: adopt reconciler logic for orchestrator
Adopt reconciler logic for orchestrator
Jun 5, 2023
ghost
previously approved these changes
Jun 5, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, all comments as minor that can be addressed in follow ups. I think the AWS provider can probably be cleaned up a lot now that the interface is simpler (remove all the wrapping interfaces for instance/snapshot etc)
FrimIdan
reviewed
Jun 6, 2023
ghost
reviewed
Jun 6, 2023
FrimIdan
reviewed
Jun 6, 2023
ghost
approved these changes
Jun 6, 2023
FrimIdan
approved these changes
Jun 6, 2023
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The main purpose for this PR to adopt the reconciler (from the
runtime_scan/orchestrator/common
package)based workflow for managing API objects such as ScanConfig, Scan, Target, ScanResult.
This also required to refactor the Provider as all operations must be idempotent as the Scan life-cycle is broken up
into multiple stages where controllers handle a single transition state during a reconciling event.
Orchestrator (and Scanner)
The Orchestrator is responsible for managing controllers for each API object type: ScanConfig, Scan, ScanResult.
ScanConfigWatcher
The
ScanConfigWatcher
is responsible for scheduling/handlingScans
based onScanConfig
.When the
ScanConfig
has both theSchedule.operationTime
andSchedule.CronLine
defined and the latter representsa single point in time (Quartz), the latter is used for defining the actual
operationTime
for newScans
.ScanWatcher
The
ScanWatcher
periodically polls the API to detect if theScan
objects are scheduled to be run. In case there isnew
Scan
scheduled, it invokes the Provider to get theTargets
in scope. WhenTargets
are available it createsScanResult
object for each. It monitors in-progressScans
and updates their Summary based on the informationgathered from the corresponding
ScanResult
objects.ScanResultWatcher
The
ScanResultWatcher
monitors the API to findScanResult
objects which are in transient state based ontheir
status.General.State
andresourceCleanup
fields. It is responsible for invokingRunTargetScan
method of theProvider
in order to set up the scanner instance and start the scan job. TheScanResult
is marked asDONE
with errors(it is considered failed) if the
Provider
was unable to set up the scanner instance due to a permanent error.It also does perform cleanup of the resources created for scan job by invoking the
RemoveTargetScan
method of the provideraccording to the
DeleteJobPolicy
configuration.Provider
Interface
The
Provider
interface has been split into two (Discoverer
andScanner
) in order to separate responsibilities fordiscovering scan scopes and actually performing scans.
Both
Discoverer
andScanner
interfaces have been reworked in order to have better separation of concerns betweenthe
Provider
and the caller (Orchestrator
), like leaking provider specific data back to the caller.The new
Scanner
interface also requires any implementation to be idempotent.VMClarity-Scanner (aka cli)
The
vmclarity-scanner
does not have timeout for waiting for volume attachment anymore, instead it monitorsthe corresponding
ScanResult
and raises an error if the state forScanResult
is changed toDONE
meaning thatscanning cannot be performed due to external (mostly due to provider) error.
Bugfixes/Improvements
/dev
mount forvmclarity-scanner
container which prevented it to detect when the scanner volume is mountedprovider/aws
implementation conform with the newProvider
interface including the requirement for idempotencyIssues
Type of Change
[x] Bug Fix
[x] New Feature
[ ] Breaking Change
[x] Refactor
[ ] Documentation
[ ] Other (please describe)
Checklist