(PDB-5351) Fix reports table partition gc bug

This commit fixes a bug that causes reports to get garbage collected
even though they were created within the 'reports-ttl' timeframe. This
was due to a comparision of these two datetime values:
- the 'reports-ttl' derived "cut-off" datetime: exactly 'reports-ttl'
  ago.
- the table partition's datetime: the very beginning of the day in which
  the reports within were created.

In puppetlabs.puppetdb.scf.storage/prune-daily-partitions, a table
partition is deemed expired if the latter datetime is *before* the
former. When GC'ing at 00:05 with a 'reports-ttl' of '1d', the partition
with all of yesterday's reports is deemed expired. The user is left with
reports from only the last 5 minutes.

This fix does a "floor" operation on the cutoff datetime generated from
the 'reports-ttl' value before comparing it to the partition's date.
This means that the "cut-off" datetime is rolled back to 0:00 of its
selected day. This way, in order for a partition to get purged, the
"cut-off" datetime has to be on a day before the partition's datetime.
This approach is already how we handle the 'resource-events-ttl' so this
fix meshes nicely with what's already in place.

The puppetlabs.puppetdb.integration.reports/reports-ttl integration test
is being removed because it is difficult to refactor into a useful test.
It was written before the reports table was partitioned by date and does
not take account how garbage collection currently works (dropping whole
expired table partitions). This test was difficult to modify because the
puppet agent service cannot easily create reports from the past. The
puppetlabs.puppetdb.cli.services-test/correctly-sweep-reports test is
sufficient to verify reports-ttl gc related functionality.

Author: stelcodes

src/puppetlabs/puppetdb/cli/services.clj

@@ -214,8 +214,9 @@
   {:pre [(map? db)
          (period? report-ttl)]}
   (let [rounded-events-ttl (some-> resource-events-ttl rounded-date)
+        rounded-report-ttl (rounded-date report-ttl)
         update-lock-status (partial update-db-lock-status db-lock-status)
-        del-opts (merge {:report-ttl (ago report-ttl)
+        del-opts (merge {:report-ttl rounded-report-ttl
                          :incremental? incremental?
                          :update-lock-status update-lock-status
                          :db db}

test/puppetlabs/puppetdb/cli/services_test.clj

@@ -573,3 +573,33 @@
       (with-redefs [jdbc/pooled-datasource validation-fn]
         (testing "should use connection user"
           (init-write-dbs databases))))))
+
+(deftest correctly-sweep-reports
+  (with-test-db
+    (let [config (-> (create-temp-config)
+                     (assoc :database *db*)
+                     (assoc-in [:database :gc-interval] "0.01"))
+          store-report #(sync-command-post (svc-utils/pdb-cmd-url)
+                                           example-certname
+                                           "store report"
+                                           cmd-consts/latest-report-version
+                                           (change-report-time example-report %))
+          db-lock-status (svcs/database-lock-status)]
+      (call-with-puppetdb-instance
+       config
+       (fn []
+         (testing "A report from 24 hours ago won't be gc'ed with a report-ttl of 1d"
+           (store-report (-> 1 time/days time/ago time/to-string))
+           (svcs/sweep-reports! *db* {:incremental true
+                                      :report-ttl (time/parse-period "1d")
+                                      :resource-events-ttl (time/parse-period "1d")
+                                      :db-lock-status db-lock-status})
+           (is (= 1 (count (jdbc/query ["SELECT * FROM reports"]))))
+           (jdbc/do-commands "DELETE FROM reports"))
+         (testing "A report from 48 hours ago will be gc'ed with a report-ttl of 1d"
+           (store-report (-> 2 time/days time/ago time/to-string))
+           (svcs/sweep-reports! *db* {:incremental true
+                                      :report-ttl (time/parse-period "1d")
+                                      :resource-events-ttl (time/parse-period "1d")
+                                      :db-lock-status db-lock-status})
+           (is (empty? (jdbc/query ["SELECT * FROM reports"])))))))))

test/puppetlabs/puppetdb/integration/reports.clj

@@ -260,35 +260,3 @@
 (defn read-gc-count-metric []
   ;; metrics are global, so...
   (counters/value (:report-purges @pdb-services/admin-metrics)))
-
-(deftest ^:integration report-ttl
-  (with-open [pg (int/setup-postgres)]
-    (with-open [pdb (int/run-puppetdb pg {})
-                ps (int/run-puppet-server [pdb] {})]
-      (testing "Run agent once to populate database"
-        (int/run-puppet-as "ttl-agent" ps pdb "notify { 'irrelevant manifest': }"))
-
-      (testing "Verify we have a report"
-        (is (= 1 (count (int/pql-query pdb "reports { certname = 'ttl-agent' }"))))))
-
-    (testing "Sleep for one second to make sure we have a ttl to exceed"
-      (Thread/sleep 1000))
-
-    (let [initial-gc-count (counters/value (:report-purges @pdb-services/admin-metrics))]
-      (with-open [pdb (int/run-puppetdb pg {:database {:report-ttl "1s"}})]
-        (let [start-time (System/currentTimeMillis)]
-          (loop []
-            (cond
-              (> (- (System/currentTimeMillis) start-time) tu/default-timeout-ms)
-              (throw (ex-info "Timeout waiting for pdb gc to happen" {}))
-
-              (> (read-gc-count-metric) initial-gc-count)
-              true ;; gc happened
-
-              :default
-              (do
-                (Thread/sleep 250)
-                (recur)))))
-
-        (testing "Verify that the report has been deleted"
-          (is (= 0 (count (int/pql-query pdb "reports { certname = 'ttl-agent' }")))))))))