Judge a PC's backup by its stalest kind, not its newest

plugins/geenforce/api/routes.py

@@ -1079,10 +1079,52 @@ def _attach_backup_state(facts, hostnames):
     rows.sort(key=lambda revision: (revision.lastseenat is not None,
                                     revision.lastseenat or _EPOCH),
               reverse=True)
+
+    # Reduce to the FRESHEST revision per (host, kind), then judge a host by its
+    # STALEST kind.
+    #
+    # Until part markers, a PC reported exactly one kind and "newest wins" was
+    # the same thing as "the truth". A marker bay reports two - ntlars for the
+    # machine's DNC settings and partmarker for the controller's patterns - and
+    # newest-wins then means the daily ntlars post keeps the badge green while
+    # the marker backup has been failing for a month. The failure would show
+    # only on that marker's own asset page, which is not where anyone looks to
+    # find out that something stopped.
+    #
+    # So: a host is healthy when EVERYTHING it backs up is current, and the
+    # badge names the kind that is not. Freshest-per-kind first, because
+    # lastseenat only moves on the newest revision of a chain - an older
+    # revision of the same kind keeps its original value and would otherwise
+    # read as stale for ever.
+    #
+    # KNOWN CONSEQUENCE: a device retired from a PC leaves its last revision
+    # behind, so that host stays stale until the revision is pruned or removed.
+    # That is the honest failure direction - it complains about a backup that
+    # genuinely is not running any more, rather than staying quiet about one.
+    freshest = {}
     for revision in rows:
         key = (revision.sourcehostname or '').lower()
+        if key not in facts:
+            continue
+        kindkey = (key, revision.backupkind)
+        if kindkey not in freshest:
+            # rows are already newest-confirmed-first, so the first is freshest.
+            freshest[kindkey] = revision
+
+    stalest = {}
+    for (key, _kind), revision in freshest.items():
+        current = stalest.get(key)
+        if current is None:
+            stalest[key] = revision
+            continue
+        # None (never confirmed) is worse than any date.
+        if revision.lastseenat is None:
+            stalest[key] = revision
+        elif current.lastseenat is not None and revision.lastseenat < current.lastseenat:
+            stalest[key] = revision
+
+    for key, revision in stalest.items():
         entry = facts.get(key)
-        # First row per host wins.
         if entry and entry['backuplastseen'] is None:
             entry['backupkind'] = revision.backupkind
             entry['backuplastseen'] = (

tests/test_plugins/test_geenforce_reporting.py

@@ -486,3 +486,80 @@ def test_the_capture_date_is_reported_separately_from_the_check(client, db, app,
     assert row['backupcollectedat'] is not None
     assert row['backupcollectedat'] < row['backuplastseen'], \
         'captured before it was last confirmed'
+
+
+def _extra_revision(db, hostname, kind, ageda, contenthash=None):
+    """A second backup kind for a host that already exists.
+
+    _backup_revision creates the asset and the Computer row, so it cannot be
+    called twice for one host.
+    """
+    from datetime import timedelta
+    from plugins.backups.models import BackupRevision
+    from plugins.geenforce.service import _utcnow
+    from plugins.computers.models import Computer
+
+    computer = Computer.query.filter_by(hostname=hostname).first()
+    db.session.add(BackupRevision(
+        assetid=computer.assetid, backupkind=kind, sourcehostname=hostname,
+        contenthash=(contenthash or (kind[0] * 64))[:64],
+        collectedat=_utcnow() - timedelta(days=ageda + 30),
+        lastseenat=_utcnow() - timedelta(days=ageda)))
+    db.session.commit()
+
+
+def test_a_stale_kind_is_not_hidden_by_a_healthy_one(client, db, app, auth_headers):
+    """A marker bay reports TWO kinds, and the badge must not report the better.
+
+    ntlars posts daily for the machine's DNC settings; partmarker posts the
+    controller's patterns. Judging a host by its newest revision meant a
+    marker backup that had been failing for a week still showed green, because
+    ntlars kept the badge fed. The failure was then visible only on the
+    marker's own asset page, which is not where anyone looks to discover that
+    something stopped.
+    """
+    _seed_and_publish(app)
+    secret = _token(client, auth_headers, ['geenforce.report'])
+    _backup_revision(db, 'WJBAK10', 'ntlars', ageda=1)
+    _extra_revision(db, 'WJBAK10', 'partmarker', ageda=9)
+    client.post('/api/geenforce/report', json={
+        'hostname': 'WJBAK10', 'scopename': 'gea-shopfloor-cmm', 'counts': {}},
+        headers={'X-API-Key': secret})
+
+    row = _row_for(client, auth_headers, 'WJBAK10')
+    assert row['backupok'] is False
+    # And it names the kind that is actually broken, not the healthy one.
+    assert row['backupkind'] == 'partmarker'
+
+
+def test_a_host_is_good_only_when_every_kind_is_current(client, db, app, auth_headers):
+    _seed_and_publish(app)
+    secret = _token(client, auth_headers, ['geenforce.report'])
+    _backup_revision(db, 'WJBAK11', 'ntlars', ageda=1)
+    _extra_revision(db, 'WJBAK11', 'partmarker', ageda=1)
+    client.post('/api/geenforce/report', json={
+        'hostname': 'WJBAK11', 'scopename': 'gea-shopfloor-cmm', 'counts': {}},
+        headers={'X-API-Key': secret})
+
+    row = _row_for(client, auth_headers, 'WJBAK11')
+    assert row['backupok'] is True
+
+
+def test_an_older_revision_of_a_kind_does_not_make_it_look_stale(client, db, app, auth_headers):
+    """lastseenat only ever moves on the NEWEST revision of a chain.
+
+    Older revisions keep the value they had when they were superseded, so
+    judging on every row rather than the freshest per kind would report a
+    healthy, frequently changing config as permanently stale.
+    """
+    _seed_and_publish(app)
+    secret = _token(client, auth_headers, ['geenforce.report'])
+    _backup_revision(db, 'WJBAK12', 'ntlars', ageda=1)
+    _extra_revision(db, 'WJBAK12', 'ntlars', ageda=40, contenthash='f' * 64)
+    client.post('/api/geenforce/report', json={
+        'hostname': 'WJBAK12', 'scopename': 'gea-shopfloor-cmm', 'counts': {}},
+        headers={'X-API-Key': secret})
+
+    row = _row_for(client, auth_headers, 'WJBAK12')
+    assert row['backupok'] is True
+    assert row['backupkind'] == 'ntlars'