Keep unreported Android app installs pending instead of failing them - #52632
Keep unreported Android app installs pending instead of failing them#52632Dhvanit41 wants to merge 1 commit into
Conversation
verifyDeviceSoftware marked an app failed when the device status report mentioned it in neither ApplicationReports nor NonComplianceDetails. Absence is not a failure signal: AMAPI reports a real install failure as a non-compliance detail, and an app still settling is simply omitted. verification_failed_at is terminal, so the row leaves the pending set and a later INSTALLED report can no longer correct it. Setup experience apps that had installed successfully were left showing Failed. Leave those apps out of the verified/failed sets so they stay pending for a later report, which is what the PENDING/IN_PROGRESS branch above already does.
|
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. Walkthrough
Merge Risk: ⚪ Minimal · up to Android VPP apps omitted from a status report now remain pending and can be verified by a later installed report, while explicit failures retain their existing behavior. The covered change presents no remaining merge-blocking risk. 🚥 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 #52632 +/- ##
=======================================
Coverage 75.91% 75.91%
=======================================
Files 4102 4102
Lines 247984 247982 -2
Branches 14261 14261
=======================================
+ Hits 188262 188265 +3
+ Misses 59545 59540 -5
Partials 177 177
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 |
Related issue: Resolves #52617
Apps installed during the Android setup experience could show as "Failed" in Fleet while the device had them installed and running.
verifyDeviceSoftwaremarks a pending app failed when the device's status report mentions it in neitherApplicationReportsnorNonComplianceDetails. But absence isn't a failure signal: AMAPI reports a real failure as a non-compliance detail, and an app that is still settling is simply left out of both lists. The guess is also unrecoverable, becauseverification_failed_attakes the row out ofListHostMDMAndroidVPPAppsPendingInstallWithVersion, so the later report that saysINSTALLEDnever gets a chance to correct it.The fix leaves those apps out of both the verified and the failed set, so they stay pending for a later report. That's the same thing the
PENDING/IN_PROGRESSbranch a few lines up already does, and the loop below already skips apps missing frommarkVerified("ignore those not in markVerified, as they will enter a final state in a future pub-sub message"). The production change is one removed assignment and a log line dropped from error to debug.Trade-off
An install that fails silently — no non-compliance report, ever — now stays pending instead of being failed. I went with option (a) from the issue because the two errors aren't equal: a stuck Pending is still correctable by the next report, while a wrong Failed is terminal and contradicts what the user sees on the device. Android app installs don't go through
upcoming_activities, so a pending row doesn't hold up anything else on the host.Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.Testing
TestStatusReportAppInstallVerificationgains a case where the device applies the policy and says nothing at all about a pending app: it must stay pending, and a laterINSTALLEDreport must still verify it. The existingmultiple apps in various statescase encoded the bug as expected behaviour ("app3 not reported at all so failed"), so its expectation moves from failed to pending.Both were checked by reverting the fix: the new case then fails on
Should be empty, but was [a]and the updated case on the failed-set contents, while the other six cases pass either way.go test ./server/mdm/android/... -raceis green.Manual QA
Ran a local Fleet server with Android MDM enabled and a fake enrolled Android host, and pushed AMAPI status reports at the Pub/Sub endpoint. Same database and same build flags for both columns — the only difference is
pubsub.go.failed_installpending_installINSTALLEDfailed_installinstalledfailed_installfailed_installPENDING/IN_PROGRESSpending_installpending_installRow 1 is the reported bug, row 2 shows it was unrecoverable, and rows 3 and 4 confirm genuine failures and in-progress installs are unaffected. Statuses are from
GET /api/latest/fleet/hosts/:id/software.Summary by CodeRabbit