ROX-33939: virtual machine v2 reconciliation improvements by dashrews78 · Pull Request #19904 · stackrox/stackrox

dashrews78 · 2026-04-08T19:13:14Z

Description

Reconciliation only needs IDs, not full VM objects. Use Search() +
ResultsToIDSet() — the same pattern all other pipelines use — to
avoid deserializing full rows and the manual ID-extraction loop.

Partially generated by AI.

User-facing documentation

CHANGELOG.md is updated OR update is not needed
documentation PR is created and is linked above OR is not needed

Testing and quality

the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
CI results are inspected

Automated testing

How I validated my change

change me!

…ciliation Reconciliation only needs IDs, not full VM objects. Use Search() + ResultsToIDSet() — the same pattern all other pipelines use — to avoid deserializing full rows and the manual ID-extraction loop. Partially generated by AI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

openshift-ci · 2026-04-08T19:13:20Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

github-actions · 2026-04-08T19:25:28Z

🚀 Build Images Ready

Images are ready for commit 7275fcd. To use with deploy scripts:

export MAIN_IMAGE_TAG=4.11.x-645-g7275fcd626

codecov · 2026-04-08T19:48:39Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.56%. Comparing base (39d15cc) to head (7828320).
⚠️ Report is 44 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #19904      +/-   ##
==========================================
- Coverage   49.60%   49.56%   -0.05%     
==========================================
  Files        2766     2764       -2     
  Lines      208567   208342     -225     
==========================================
- Hits       103454   103259     -195     
+ Misses      97436    97430       -6     
+ Partials     7677     7653      -24

Flag	Coverage Δ
go-unit-tests	`49.56% <100.00%> (-0.05%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai · 2026-04-09T14:46:25Z

📝 Walkthrough

Summary by CodeRabbit

Refactor
- Improved internal virtual machine reconciliation implementation for better code consistency and maintainability.

Walkthrough

The reconciliation logic in the VM pipeline is refactored to use the Search API instead of SearchRawVirtualMachines, with results converted to an ID set via search.ResultsToIDSet() for deletion operations. Corresponding test mocks are updated to match the new API signature.

Changes

Cohort / File(s)	Summary
VM Pipeline Search API Migration `central/sensor/service/pipeline/virtualmachines/pipeline.go`, `central/sensor/service/pipeline/virtualmachines/pipeline_test.go`	Refactored `reconcileV2` to call `Search(ctx, query)` instead of `SearchRawVirtualMachines` with explicit `QueryPagination`, and convert results to ID set via `search.ResultsToIDSet()`. Updated test mocks to return `[]search.Result` instead of `[]*storage.VirtualMachineV2`, and added `github.com/stackrox/rox/pkg/search` import.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The description explains the technical rationale but leaves critical validation sections incomplete, with 'change me!' placeholder remaining in the 'How I validated my change' section.	Complete the 'How I validated my change' section explaining the testing approach or why validation was deferred to CI; clarify which testing checkboxes apply.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: a refactoring of the virtual machine v2 reconciliation pipeline to use a more efficient search pattern.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch dashrews/virt-reconciliation-33939

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@central/sensor/service/pipeline/virtualmachines/pipeline.go`:
- Around line 89-90: reconcileV2 is calling p.virtualMachineV2Store.Search with
a query built by search.NewQueryBuilder().AddExactMatches(...).ProtoQuery()
which relies on the default 100-result page size and thus misses VMs past the
first page; modify reconcileV2 to set explicit pagination on the query (either a
sufficiently large PageSize or, preferably, implement paging by setting/reading
PageToken and looping calls to p.virtualMachineV2Store.Search until no more
results) so you collect the complete set of VMs for the cluster before
reconciling/cleaning up.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: e7c9e062-aafa-4d2d-8080-83bc2c387d41

📥 Commits

Reviewing files that changed from the base of the PR and between 39d15cc and 1fd38ea.

📒 Files selected for processing (2)

central/sensor/service/pipeline/virtualmachines/pipeline.go
central/sensor/service/pipeline/virtualmachines/pipeline_test.go

…ct here.

dashrews78 · 2026-04-10T10:57:32Z

/test gke-nongroovy-e2e-tests

dashrews78 · 2026-04-10T18:05:26Z

/test gke-nongroovy-e2e-tests

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

dashrews78 and others added 2 commits April 8, 2026 15:06

X-Smart-Branch-Parent: master

6c6f5b9

openshift-ci bot added the do-not-merge/work-in-progress label Apr 8, 2026

github-actions bot added the area/central label Apr 8, 2026

dashrews78 marked this pull request as ready for review April 9, 2026 14:37

openshift-ci bot removed the do-not-merge/work-in-progress label Apr 9, 2026

dashrews78 requested a review from ajheflin April 9, 2026 14:38

dashrews78 added the coderabbit-review label Apr 9, 2026

coderabbitai bot reviewed Apr 9, 2026

View reviewed changes

Comment thread central/sensor/service/pipeline/virtualmachines/pipeline.go

ajheflin approved these changes Apr 9, 2026

View reviewed changes

coderabbit had a point that the default paging of search is not corre…

7828320

…ct here.

dashrews78 merged commit 7275fcd into master Apr 14, 2026
95 checks passed

dashrews78 deleted the dashrews/virt-reconciliation-33939 branch April 14, 2026 14:30

vikin91 pushed a commit that referenced this pull request Apr 16, 2026

ROX-33939: virtual machine v2 reconciliation improvements (#19904)

8476353

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ROX-33939: virtual machine v2 reconciliation improvements#19904

ROX-33939: virtual machine v2 reconciliation improvements#19904
dashrews78 merged 3 commits intomasterfrom
dashrews/virt-reconciliation-33939

dashrews78 commented Apr 8, 2026

Uh oh!

openshift-ci bot commented Apr 8, 2026

Uh oh!

github-actions bot commented Apr 8, 2026 •

edited

Loading

Uh oh!

codecov bot commented Apr 8, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Apr 9, 2026

Summary by CodeRabbit

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning, 1 inconclusive)

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

dashrews78 commented Apr 10, 2026

Uh oh!

dashrews78 commented Apr 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dashrews78 commented Apr 8, 2026

Description

User-facing documentation

Testing and quality

Automated testing

How I validated my change

Uh oh!

openshift-ci bot commented Apr 8, 2026

Uh oh!

github-actions bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🚀 Build Images Ready

Uh oh!

codecov bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai bot commented Apr 9, 2026

Summary by CodeRabbit

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning, 1 inconclusive)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dashrews78 commented Apr 10, 2026

Uh oh!

dashrews78 commented Apr 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Apr 8, 2026 •

edited

Loading

codecov bot commented Apr 8, 2026 •

edited

Loading