Merge pull request #5617 from larsewi/lastseen

ENT-11838: Fixed perpetual warnings that a deleted host has checked in

cf-check/backup.c

@@ -70,8 +70,9 @@ const char *create_backup_dir()
         return NULL;
     }
 
-    const int n =
-        snprintf(backup_dir, PATH_MAX, "%s%jd/", backup_root, (intmax_t)ts);
+    int n =
+        snprintf(backup_dir, PATH_MAX - 1, // trailing slash for later
+                 "%s%jd-XXXXXX", backup_root, (intmax_t)ts);
     if (n >= PATH_MAX)
     {
         Log(LOG_LEVEL_ERR,
@@ -81,7 +82,7 @@ const char *create_backup_dir()
         return NULL;
     }
 
-    if (mkdir(backup_dir, 0700) != 0)
+    if (mkdtemp(backup_dir) == NULL)
     {
         Log(LOG_LEVEL_ERR,
             "Could not create directory '%s' (%s)",
@@ -90,6 +91,10 @@ const char *create_backup_dir()
         return NULL;
     }
 
+    // Add trailing forward slash
+    backup_dir[n++] = FILE_SEPARATOR;
+    backup_dir[n] = '\0';
+
     return backup_dir;
 }
 
@@ -102,6 +107,11 @@ int backup_files_copy(Seq *filenames)
     assert_or_return(length > 0, 1);
 
     const char *backup_dir = create_backup_dir();
+    if (backup_dir == NULL)
+    {
+        // Error already logged
+        return -1;
+    }
 
     Log(LOG_LEVEL_INFO, "Backing up to '%s'", backup_dir);
 

cf-check/db_structs.h

@@ -17,6 +17,7 @@
 // Struct used for quality entries in /var/cfengine/state/cf_lastseen.lmdb:
 typedef struct
 {
+    bool acknowledged;
     time_t lastseen;
     QPoint Q;
 } KeyHostSeen; // Keep in sync with lastseen.h

cf-check/dump.c

@@ -107,6 +107,7 @@ static void print_struct_lastseen_quality(
         memcpy(&quality, value.mv_data, sizeof(quality));
         const time_t lastseen = quality.lastseen;
         const QPoint Q = quality.Q;
+        bool acknowledged = quality.acknowledged;
 
         JsonElement *q_json = JsonObjectCreate(4);
         JsonObjectAppendReal(q_json, "q", Q.q);
@@ -115,6 +116,7 @@ static void print_struct_lastseen_quality(
         JsonObjectAppendReal(q_json, "dq", Q.dq);
 
         JsonElement *top_json = JsonObjectCreate(2);
+        JsonObjectAppendBool(top_json, "acknowledged", acknowledged);
         JsonObjectAppendInteger(top_json, "lastseen", lastseen);
         JsonObjectAppendObject(top_json, "Q", q_json);
 

libpromises/dbm_api.c

@@ -426,17 +426,23 @@ bool OpenDBInstance(DBHandle **dbp, dbid id, DBHandle *handle)
                 }
             }
 
-            DBPathUnLock(&lock);
-        }
-
-        if (handle->priv)
-        {
-            if (!DBMigrate(handle, id))
+            if (handle->priv != NULL)
             {
-                DBPrivCloseDB(handle->priv);
-                handle->priv = NULL;
-                handle->open_tstamp = -1;
+                if (!DBMigrate(handle, id))
+                {
+                    /* Migration failed. The best we can do is to move the
+                     * broken DB to the side and start fresh. */
+                    DBPrivCloseDB(handle->priv);
+                    DBPathMoveBroken(handle->filename);
+                    handle->priv = DBPrivOpenDB(handle->filename, id);
+                    if (handle->priv == DB_PRIV_DATABASE_BROKEN)
+                    {
+                        handle->priv = NULL;
+                    }
+                }
             }
+
+            DBPathUnLock(&lock);
         }
     }
 

libpromises/dbm_lmdb.c

@@ -69,6 +69,7 @@ struct DBCursorPriv_
     MDB_cursor *mc;
     MDB_val delkey;
     void *curkv;
+    size_t curks;
     bool pending_delete;
 };
 
@@ -1290,6 +1291,7 @@ bool DBPrivAdvanceCursor(
         }
         cursor->curkv = xmalloc(keybuf_size + data.mv_size);
         memcpy(cursor->curkv, mkey.mv_data, mkey.mv_size);
+        cursor->curks = mkey.mv_size;
         *key = cursor->curkv;
         *key_size = mkey.mv_size;
         *value_size = data.mv_size;
@@ -1355,7 +1357,7 @@ bool DBPrivWriteCursorEntry(
     {
         MDB_val curkey;
         curkey.mv_data = cursor->curkv;
-        curkey.mv_size = sizeof(cursor->curkv);
+        curkey.mv_size = cursor->curks;
 
         rc = mdb_cursor_put(cursor->mc, &curkey, &data, MDB_CURRENT);
         CheckLMDBUsable(rc, cursor->db->env);

libpromises/dbm_migration.c

@@ -26,11 +26,12 @@
 
 #include <lastseen.h>
 #include <string_lib.h>
+#include <backup.h>
 
 extern const DBMigrationFunction dbm_migration_plan_lastseen[];
 
 
-#ifdef LMDB
+#ifndef LMDB
 bool DBMigrate(ARG_UNUSED DBHandle *db, ARG_UNUSED  dbid id)
 {
     return true;
@@ -64,6 +65,11 @@ bool DBMigrate(DBHandle *db, dbid id)
         {
             if (step_version == DBVersion(db))
             {
+                Seq *seq = SeqNew(1, free);
+                SeqAppend(seq, DBIdToPath(id));
+                backup_files_copy(seq);
+                SeqDestroy(seq);
+
                 if (!(*step)(db))
                 {
                     return false;

libpromises/dbm_migration_lastseen.c

@@ -26,6 +26,7 @@
 
 #include <lastseen.h>
 #include <logging.h>
+#include <string_lib.h>
 
 typedef struct
 {
@@ -34,6 +35,12 @@ typedef struct
     double var;
 } QPoint0;
 
+typedef struct
+{
+    time_t lastseen;
+    QPoint Q; // Average time between connections (rolling weighted average)
+} KeyHostSeen1;
+
 #define QPOINT0_OFFSET 128
 
 /*
@@ -48,142 +55,95 @@ typedef struct
 
 static bool LastseenMigrationVersion0(DBHandle *db)
 {
-    bool errors = false;
-    DBCursor *cursor;
+    /* For some reason DB migration for LMDB was disabled in 2014 (in commit
+     * 8611970bfa33be7b3cf0724eb684833e08582850). Unfortunately there is no
+     * mention as to why this was done. Maybe it was not working?
+     *
+     * However, we're re-enabling it now (10 years later). Since this
+     * migration function has not been active for the last 10 years, the
+     * safest thing is to remove the migration logic, and only update the
+     * version number.
+     *
+     * If you have not upgraded CFEngine in the last 10 years, this will be
+     * the least of your problems.
+     */
+    return WriteDB(db, "version", "1", sizeof("1"));
+}
 
+static bool LastseenMigrationVersion1(DBHandle *db)
+{
+    DBCursor *cursor;
     if (!NewDBCursor(db, &cursor))
     {
+        Log(LOG_LEVEL_ERR,
+            "Unable to create database cursor during lastseen migration");
         return false;
     }
 
     char *key;
     void *value;
-    int ksize, vsize;
+    int key_size, value_size;
 
-    while (NextDB(cursor, &key, &ksize, &value, &vsize))
+    // Iterate through all key-value pairs
+    while (NextDB(cursor, &key, &key_size, &value, &value_size))
     {
-        if (ksize == 0)
+        if (key_size == 0)
         {
-            Log(LOG_LEVEL_INFO, "LastseenMigrationVersion0: Database structure error -- zero-length key.");
+            Log(LOG_LEVEL_WARNING,
+                "Found zero-length key during lastseen migration");
             continue;
         }
 
-        /* Only look for old [+-]kH -> IP<QPoint> entries */
-        if ((key[0] != '+') && (key[0] != '-'))
+        // Only look for old KeyHostSeen entries
+        if (key[0] != 'q')
         {
-            /* Warn about completely unexpected keys */
-
-            if ((key[0] != 'q') && (key[0] != 'k') && (key[0] != 'a'))
+            // Warn about completely unexpected keys
+            if ((key[0] != 'k') && (key[0] != 'a') && !StringEqualN(key, "version", key_size))
             {
-                Log(LOG_LEVEL_INFO, "LastseenMigrationVersion0: Malformed key found '%s'", key);
+                Log(LOG_LEVEL_WARNING,
+                    "Found unexpected key '%s' during lastseen migration. "
+                    "Only expecting 'version' or 'k', 'a' and 'q[i|o]' prefixed keys.",
+                    key);
             }
-
-            continue;
-        }
-
-        bool incoming = key[0] == '-';
-        const char *hostkey = key + 1;
-
-        /* Only migrate sane data */
-        if (vsize != QPOINT0_OFFSET + sizeof(QPoint0))
-        {
-            Log(LOG_LEVEL_INFO,
-                "LastseenMigrationVersion0: invalid value size for key '%s', entry is deleted",
-                key);
-            DBCursorDeleteEntry(cursor);
-            continue;
-        }
-
-        /* Properly align the data */
-        const char *old_data_address = (const char *) value;
-        QPoint0 old_data_q;
-        memcpy(&old_data_q, (const char *) value + QPOINT0_OFFSET,
-               sizeof(QPoint0));
-
-        char hostkey_key[CF_BUFSIZE];
-        snprintf(hostkey_key, CF_BUFSIZE, "k%s", hostkey);
-
-        if (!WriteDB(db, hostkey_key, old_data_address, strlen(old_data_address) + 1))
-        {
-            Log(LOG_LEVEL_INFO, "Unable to write version 1 lastseen entry for '%s'", key);
-            errors = true;
-            continue;
-        }
-
-        char address_key[CF_BUFSIZE];
-        snprintf(address_key, CF_BUFSIZE, "a%s", old_data_address);
-
-        if (!WriteDB(db, address_key, hostkey, strlen(hostkey) + 1))
-        {
-            Log(LOG_LEVEL_INFO, "Unable to write version 1 reverse lastseen entry for '%s'", key);
-            errors = true;
-            continue;
-        }
-
-        char quality_key[CF_BUFSIZE];
-        snprintf(quality_key, CF_BUFSIZE, "q%c%s", incoming ? 'i' : 'o', hostkey);
-
-        /*
-           Ignore malformed connection quality data
-        */
-
-        if ((!isfinite(old_data_q.q))
-            || (old_data_q.q < 0)
-            || (!isfinite(old_data_q.expect))
-            || (!isfinite(old_data_q.var)))
-        {
-            Log(LOG_LEVEL_INFO, "Ignoring malformed connection quality data for '%s'", key);
-            DBCursorDeleteEntry(cursor);
             continue;
         }
 
-        KeyHostSeen data = {
-            .lastseen = (time_t)old_data_q.q,
-            .Q = {
-                /*
-                   Previously .q wasn't stored in database, but was calculated
-                   every time as a difference between previous timestamp and a
-                   new timestamp. Given we don't have this information during
-                   the database upgrade, just assume that last reading is an
-                   average one.
-                */
-                .q = old_data_q.expect,
-                .dq = 0,
-                .expect = old_data_q.expect,
-                .var = old_data_q.var,
-            }
+        KeyHostSeen1 *old_value = value;
+        KeyHostSeen new_value = {
+            .acknowledged = true, // We don't know, assume yes
+            .lastseen = old_value->lastseen,
+            .Q = old_value->Q,
         };
 
-        if (!WriteDB(db, quality_key, &data, sizeof(data)))
-        {
-            Log(LOG_LEVEL_INFO, "Unable to write version 1 connection quality key for '%s'", key);
-            errors = true;
-            continue;
-        }
-
-        if (!DBCursorDeleteEntry(cursor))
+        // This will overwrite the entry
+        if (!DBCursorWriteEntry(cursor, &new_value, sizeof(new_value)))
         {
-            Log(LOG_LEVEL_INFO, "Unable to delete version 0 lastseen entry for '%s'", key);
-            errors = true;
+            Log(LOG_LEVEL_ERR,
+                "Unable to write version 2 of entry for key '%s' during lastseen migration.",
+                key);
+            return false;
         }
     }
 
-    if (DeleteDBCursor(cursor) == false)
+    if (!DeleteDBCursor(cursor))
     {
-        Log(LOG_LEVEL_ERR, "LastseenMigrationVersion0: Unable to close cursor");
-        errors = true;
+        Log(LOG_LEVEL_ERR, "Unable to close cursor during lastseen migration");
+        return false;
     }
 
-    if ((!errors) && (!WriteDB(db, "version", "1", sizeof("1"))))
+    if (!WriteDB(db, "version", "2", sizeof("2")))
     {
-        errors = true;
+        Log(LOG_LEVEL_ERR, "Failed to update version number during lastseen migration");
+        return false;
     }
 
-    return !errors;
+    Log(LOG_LEVEL_INFO, "Migrated lastseen database from version 1 to 2");
+    return true;
 }
 
 const DBMigrationFunction dbm_migration_plan_lastseen[] =
 {
     LastseenMigrationVersion0,
+    LastseenMigrationVersion1,
     NULL
 };