Keep an Android host's disk space when a re-enrollment reports none - #52639
Keep an Android host's disk space when a re-enrollment reports none#52639Dhvanit41 wants to merge 1 commit into
Conversation
AMAPI only sends memory events on status reports, so an enrollment payload calculates as "storage measurement not supported" and updateHost wrote the -1 sentinel over a measurement Fleet already had. A host that re-enrolled showed "Not supported" on Host details, with its total disk space still correct, until the next status report arrived. Skip the assignment when the calculation comes back not supported. The fields stay zero, so UpdateAndroidHost's guard skips host_disks and the stored measurement survives. A device that genuinely never measures still records the sentinel when it first enrolls, which is a different path, and a later status report with a measurement still updates.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. WalkthroughAndroid host updates now preserve existing storage metrics when enrollment data lacks measured memory events. The update path writes storage values only when calculated available space is non-negative. Tests add re-enrollment setup and cover payloads both without measured events and with measured events. Merge Risk: ⚪ Minimal · up to Android re-enrollment no longer replaces stored disk-space measurements with “Not supported” when a payload lacks storage metrics, and later measured reports still update those values. No current merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #52639 +/- ##
=======================================
Coverage 75.91% 75.92%
=======================================
Files 4102 4102
Lines 247984 247987 +3
Branches 14100 14100
=======================================
+ Hits 188266 188283 +17
+ Misses 59542 59528 -14
Partials 176 176
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@noahtalerman |
1 participant
Related issue: Resolves #52605
After an Android host re-enrolls, its disk space on Host details changes from a real value to "Not supported," while total disk space stays correct.
AMAPI only sends memory events on status reports, so an enrollment payload carries none.
calculateAndroidStorageMetricsreads that as "measurement not supported" and returns its-1sentinel, andupdateHostwrites that over a measurement Fleet already had.The fix skips the assignment when the calculation comes back not supported. The fields stay zero, so
UpdateAndroidHost's existing!= 0guard skipshost_disksand the stored measurement survives until a report brings a new one. A device that genuinely never measures still records the sentinel when it first enrolls — that is the create path, which isn't touched.What this trades away
host_disksis written as one row, so preserving the available space means skipping the whole write. Two consequences, both confined to hosts that report no measurement:EXTERNAL_STORAGE_DETECTEDno longer refreshes its total on later reports. Measured against the same scenario,mainmoves a 110.29 GB total to 160.29 GB when 50 GB of external storage appears; with this change it stays at 110.29 GB. Such a host already displays "Not supported" for free space.Keeping the total fresh while preserving free space would mean teaching the datastore to update the columns independently. That seemed like the wrong trade for a released bug where the visible symptom is a correct value being replaced by a wrong one, but I'm happy to take it further if you'd rather have it.
Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.Testing
TestAndroidStorageExtractiongains two update-path cases: a re-enrollment payload with no memory events must leave the stored values alone, and one that does carry measurements must still update them. Its three existing cases all cover the create path and are unchanged.Checked by reverting the fix: the first new case then fails on
Should be zero, but was -1, and everything else passes either way.go test ./server/mdm/android/... -raceis green.Manual QA
Local Fleet server with Android MDM enabled and an enrolled Android host, driving the Pub/Sub endpoint with AMAPI enrollment and status-report payloads. Same database and build flags for both columns; every push carries a fresh
messageIdso nothing is dropped by the duplicate check.EXTERNAL_STORAGE_DETECTEDRow 2 is the reported bug. The rest are the checks that the fix is narrow: a later measurement still lands, a genuinely full disk still records
0rather than being mistaken for "no data," and a device that truly can't measure still reports "Not supported" instead of having the symptom hidden. Values read fromhost_disks.Summary by CodeRabbit