Skip to content

ROX-32848: Emit both ACK messages from Central#18488

Merged
vikin91 merged 3 commits intomasterfrom
piotr/ROX-32316-new-ack-central
Mar 5, 2026
Merged

ROX-32848: Emit both ACK messages from Central#18488
vikin91 merged 3 commits intomasterfrom
piotr/ROX-32316-new-ack-central

Conversation

@vikin91
Copy link
Copy Markdown
Contributor

@vikin91 vikin91 commented Jan 14, 2026 •
edited
Loading

Description

We have currently two ACK messages from Central to Sensor:

  1. Old central.MsgToSensor_NodeInventoryAck - added for node scanning v2. Supported in all Sensors since version 4.1.
  2. New central.MsgToSensor_SensorAck named more generic so that it is not bound to node scanning. Supported in Sensor 4.10 and newer.

For the transition period (sensor-central compatibility) of both types of ACK messages being used, we should send the new ACK messages in parallel to the old ones from Central.

This is applied here for Node Inventory and Node Index.

Based that Sensor 4.10 already supports MsgToSensor_SensorAck, the MsgToSensor_NodeInventoryAck can be removed in 4.12.

User-facing documentation

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

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

  • CI
  • On a cluster

Central

➜ logs-central | grep ACK
sensor/service/pipeline/virtualmachineindex: 2026/01/28 09:35:37.463428 pipeline.go:231: Debug: Sent VM index report ACK for VM 7fd3b708-b253-4aff-95b4-addb4cf86496 in cluster 62b2bdc5-6894-4cce-bbfc-99e73da78ea1 (reason="")
sensor/service/pipeline/virtualmachineindex: 2026/01/28 09:35:37.538591 pipeline.go:231: Debug: Sent VM index report ACK for VM 7fd3b708-b253-4aff-95b4-addb4cf86496 in cluster 62b2bdc5-6894-4cce-bbfc-99e73da78ea1 (reason="")
sensor/service/pipeline/virtualmachineindex: 2026/01/28 09:35:37.553424 pipeline.go:231: Debug: Sent VM index report ACK for VM 7fd3b708-b253-4aff-95b4-addb4cf86496 in cluster 62b2bdc5-6894-4cce-bbfc-99e73da78ea1 (reason="")
sensor/service/pipeline/virtualmachineindex: 2026/01/28 09:35:37.570931 pipeline.go:231: Debug: Sent VM index report ACK for VM 7fd3b708-b253-4aff-95b4-addb4cf86496 in cluster 62b2bdc5-6894-4cce-bbfc-99e73da78ea1 (reason="")
sensor/service/pipeline/virtualmachineindex: 2026/01/28 09:35:37.584307 pipeline.go:231: Debug: Sent VM index report ACK for VM 7fd3b708-b253-4aff-95b4-addb4cf86496 in cluster 62b2bdc5-6894-4cce-bbfc-99e73da78ea1 (reason="")

Sensor

# HELP rox_sensor_virtual_machine_index_report_acks_received_total Total number of ACK/NACK responses received from Central for VM index reports
# TYPE rox_sensor_virtual_machine_index_report_acks_received_total counter
rox_sensor_virtual_machine_index_report_acks_received_total{action="ACK"} 140

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Jan 14, 2026

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

@vikin91
Copy link
Copy Markdown
Contributor Author

vikin91 commented Jan 14, 2026 •
edited
Loading

This change is part of the following stack:

Change managed by git-spice.

This was referenced Jan 14, 2026
Closed
Merged
Closed
@rhacs-bot
Copy link
Copy Markdown
Contributor

rhacs-bot commented Jan 14, 2026 •
edited
Loading

Images are ready for the commit at 5db0e06.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.11.x-237-g5db0e0684f.

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 14, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 74.35897% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.62%. Comparing base (141b82d) to head (5db0e06).
⚠️ Report is 19 commits behind head on master.

Files with missing lines Patch % Lines
.../sensor/service/pipeline/nodeinventory/pipeline.go 72.72% 4 Missing and 2 partials ⚠️
...tral/sensor/service/pipeline/nodeindex/pipeline.go 76.47% 2 Missing and 2 partials ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #18488   +/-   ##
=======================================
  Coverage   49.62%   49.62%           
=======================================
  Files        2680     2680           
  Lines      202195   202233   +38     
=======================================
+ Hits       100332   100360   +28     
- Misses      94386    94391    +5     
- Partials     7477     7482    +5
Flag Coverage Δ
go-unit-tests 49.62% <74.35%> (+<0.01%) ⬆️

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.

@vikin91 vikin91 force-pushed the piotr/ROX-32316-sensor-new-ack branch from d9ebf5d to 0380835 Compare January 23, 2026 08:20
@vikin91 vikin91 force-pushed the piotr/ROX-32316-new-ack-central branch from 6a069e3 to af9d192 Compare January 23, 2026 08:24
@vikin91 vikin91 force-pushed the piotr/ROX-32316-sensor-new-ack branch from 0380835 to 4909e9e Compare January 23, 2026 08:25
@vikin91 vikin91 force-pushed the piotr/ROX-32316-new-ack-central branch from af9d192 to 993e68d Compare January 23, 2026 08:25
@vikin91 vikin91 force-pushed the piotr/ROX-32316-sensor-new-ack branch 2 times, most recently from 8340b7e to 166d103 Compare January 27, 2026 08:22
Base automatically changed from piotr/ROX-32316-sensor-new-ack to master January 27, 2026 15:27
@vikin91 vikin91 force-pushed the piotr/ROX-32316-new-ack-central branch from 993e68d to b8fbaeb Compare January 27, 2026 15:29
@vikin91 vikin91 marked this pull request as ready for review January 27, 2026 15:56
@vikin91 vikin91 requested a review from a team January 27, 2026 16:33
@vikin91 vikin91 changed the title ROX-32316: Emit both ACK messages from Central ROX-32848: Emit both ACK messages from Central Jan 28, 2026
@vikin91 vikin91 force-pushed the piotr/ROX-32316-new-ack-central branch from b8fbaeb to 1443f69 Compare March 3, 2026 14:34
@vikin91 vikin91 force-pushed the piotr/ROX-32316-new-ack-central branch from 1443f69 to 5db0e06 Compare March 4, 2026 12:03
@vikin91
Copy link
Copy Markdown
Contributor Author

vikin91 commented Mar 5, 2026

/test gke-nongroovy-e2e-tests
/test gke-ui-e2e-tests
/test gke-upgrade-tests

@vikin91 vikin91 enabled auto-merge (squash) March 5, 2026 16:55
@vikin91 vikin91 merged commit 4e8b5b6 into master Mar 5, 2026
159 of 187 checks passed
@vikin91 vikin91 deleted the piotr/ROX-32316-new-ack-central branch March 5, 2026 17:58
ksurabhi91 pushed a commit that referenced this pull request Mar 12, 2026
@vikin91 @ksurabhi91
ROX-32848: Emit both ACK messages from Central (#18488)
27c51db
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rhybrillou rhybrillou rhybrillou approved these changes

Assignees

No one assigned

Labels

area/central

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vikin91 @rhacs-bot @rhybrillou