From ffed2b8a4c939d249fcb446e058b2d6be3d60d96 Mon Sep 17 00:00:00 2001 From: Iain Duncan Date: Tue, 31 Oct 2023 08:46:22 +0000 Subject: [PATCH 1/2] indexer: Remove retry logic The retry logic in the indexer's controller is faulty in that every action that may return a timeout error (the entry path to set retry to true) will also return a Terminal state along with the timeout error. Therefore after pausing briefly in the retry statement it will then break due to the next state being Terminal. I think the correct way to fix this is to just remove the rety logic as it can also cause an empty index report to be written to the database and simplifying the code seems to be the best approach. Signed-off-by: Iain Duncan --- indexer/controller/controller.go | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/indexer/controller/controller.go b/indexer/controller/controller.go index a39103582..198e15482 100644 --- a/indexer/controller/controller.go +++ b/indexer/controller/controller.go @@ -77,8 +77,6 @@ func (s *Controller) Index(ctx context.Context, manifest *claircore.Manifest) (* // Terminal state is encountered. func (s *Controller) run(ctx context.Context) (err error) { var next State - var retry bool - var w time.Duration // As long as there's not an error and the current state isn't Terminal, run // the corresponding function. @@ -97,10 +95,6 @@ func (s *Controller) run(ctx context.Context) (err error) { continue case errors.Is(err, nil): // OK - case errors.Is(err, context.DeadlineExceeded): - // Either the function's internal deadline or the parent's deadline - // was hit. - retry = true case errors.Is(err, context.Canceled): // The parent context was canceled and the stateFunc noticed. // Continuing the loop should drop execution out of it. @@ -122,22 +116,6 @@ func (s *Controller) run(ctx context.Context) (err error) { err = setReportErr break } - if retry { - t := time.NewTimer(w) - select { - case <-ctx.Done(): - case <-t.C: - } - t.Stop() - w = jitter() - retry = false - // Execution is in this conditional because err == - // context.DeadlineExceeded, so reset the err value to make sure the - // loop makes it back to the error handling switch above. If the - // context was canceled, the loop will end there; if not, there will - // be a retry. - err = nil - } // This if statement preserves current behaviour of not setting // currentState to Terminal when it's returned. This should be an // internal detail, but is codified in the tests (for now). From 8af9f439e501fae37d6f1f2c66902e4df305001d Mon Sep 17 00:00:00 2001 From: Iain Duncan Date: Tue, 31 Oct 2023 08:54:20 +0000 Subject: [PATCH 2/2] indexer: Reprocess unfinished index reports If a manifest has scanned with all it's scanners it _should_ end up with a completed index report in the IndexFinished state. If it does not then it will not be possible to query this manifest. To fix an incomplete index report it will need rescanning so trigger a rescan. Signed-off-by: Iain Duncan --- indexer/controller/checkmanifest.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/indexer/controller/checkmanifest.go b/indexer/controller/checkmanifest.go index 839cd0e4a..7d01d4013 100644 --- a/indexer/controller/checkmanifest.go +++ b/indexer/controller/checkmanifest.go @@ -70,6 +70,12 @@ func checkManifest(ctx context.Context, s *Controller) (State, error) { return Terminal, fmt.Errorf("failed to retrieve manifest: %w", err) } s.report = sr + if sr.State != IndexFinished.String() { + // The manifest has been processed with all the Scanners but the index report did not end up in the IndexFinished state. + // This means that something must have gone wrong along the way so reprocess the manifest again. + // Do not do any filtering of the Vscnrs as we do not went wrong during the scanning process. + return FetchLayers, nil + } return Terminal, nil }