scan estimation orchestrator #560
Conversation
|
@fishkerez if you rebase this PR onto the API server one and leave this in draft there is no need for a feature branch, and we can review just the top commits for each part of the commit chain. That will make all the CI work and the top of the chain testable, and allow us to merge things directly to main without having to have a second pull request to merge it all in later. |
| @@ -97,6 +97,10 @@ func (c Client) Kind() models.CloudProvider { | |||
| return models.Azure | |||
| } | |||
|
|
|||
| func (c *Client) Estimate(ctx context.Context, stats models.AssetScanStats, asset *models.Asset, assetScanTemplate *models.AssetScanTemplate) (*models.Estimation, error) { | |||
| return &models.Estimation{}, nil | |||
There was a problem hiding this comment.
I would return a FatalErrorf("Not Implemented") here for now
| Cost: utils.PointerTo(float32(10.0)), | ||
| Size: utils.PointerTo(500), | ||
| Time: utils.PointerTo(10), | ||
| }, nil |
There was a problem hiding this comment.
ditto for fatal Not Implemented error
| @@ -67,6 +67,10 @@ func (c Client) Kind() models.CloudProvider { | |||
| return models.External | |||
| } | |||
|
|
|||
| func (c *Client) Estimate(ctx context.Context, stats models.AssetScanStats, asset *models.Asset, assetScanTemplate *models.AssetScanTemplate) (*models.Estimation, error) { | |||
| return &models.Estimation{}, nil | |||
| @@ -88,6 +88,10 @@ func (c Client) Kind() models.CloudProvider { | |||
| return models.GCP | |||
| } | |||
|
|
|||
| func (c *Client) Estimate(ctx context.Context, stats models.AssetScanStats, asset *models.Asset, assetScanTemplate *models.AssetScanTemplate) (*models.Estimation, error) { | |||
| return &models.Estimation{}, nil | |||
|
|
||
| if numOfErrs > 0 { | ||
| return fmt.Errorf("failed to create %d AssetScanEstimation(s) for ScanEstimation. ScanEstimationID=%s: %w", numOfErrs, *scanEstimation.Id, assetErrs[0]) | ||
| } |
There was a problem hiding this comment.
As we don't know how many Assets will match the scope we probably should avoid using buffered channels here to prevent huge memory spikes. Instead we can create an array and a consuming go routine and then wrap it the final array using errors.Join:
errs := make([]error)
errChan := make(chan error)
go func() {
for e := range errChan {
errs = append(errs, err)
}
}()
for _, id := range *scanEstimation.AssetIDs {
....
}
wg.Wait()
close(errChan) # This stops the go routine that is consuming on errChan
err := errors.Join(errs...)
if err != nil {
return fmt.Errorf("failed to create %d AssetScanEstimation(s) for ScanEstimation. ScanEstimationID=%s: %w", len(errs), *scanEstimation.Id, err)
}
There was a problem hiding this comment.
Took it from scanWatcher reconcile logic.
| ScansCount: asset.ScansCount, | ||
| Summary: asset.Summary, | ||
| AssetInfo: asset.AssetInfo, | ||
| } |
There was a problem hiding this comment.
This should be a relationship to an asset in the system, we should not be duplicating any asset information in the AssetScanEstimation.
| Asset: assetWriteable, | ||
| AssetScanTemplate: scanEstimation.ScanTemplate.AssetScanTemplate, | ||
| Estimation: nil, | ||
| Id: nil, |
There was a problem hiding this comment.
No need to specify these fields, their empty value will be nil.
|
Moved to #603 |
Description
Resolve #586
Discussion: #465
Continuing the apiserver PR: #559
Adds 2 new reconcile controllers for ScanEstimation and AssetScanEstimation.
The cost calculation implementation for AWS will be added soon.
Type of Change
[ ] Bug Fix
[* ] New Feature
[ ] Breaking Change
[ ] Refactor
[ ] Documentation
[ ] Other (please describe)