Skip to content

Commit beee17c

Browse files
aarongableaarongable
authored
Janitor: refactor to be controlled by config (letsencrypt#5195)
Previously, configuration of the boulder-janitor was split into two places: the actual json config file (which controlled which jobs would be enabled, and what their rate limits should be), and the janitor code itself (which controlled which tables and columns those jobs should query). This resulted in significant duplicated code, as most of the jobs were identical except for their table and column names. This change abstracts away the query which jobs use to find work. Instead of having each job type parse its own config and produce its own work query (in Go code), now each job supplies just a few key values (the table name and two column names) in its JSON config, and the Go code assembles the appropriate query from there. We are able to delete all of the files defining individual job types, and replace them with a single slightly smarter job constructor. This enables further refactorings, namely: * Moving all of the logic code into its own module; * Ensuring that the exported interface of that module is safe (i.e. that a client cannot create and run jobs without them being valid, because the only exposed methods ensure validity); * Collapsing validity checks into a single location; * Various renamings.
1 parent 5ca0c34 commit beee17c

File tree

17 files changed

+377
-615
lines changed

17 files changed

+377
-615
lines changed

‎cmd/boulder-janitor/certs.go‎

Lines changed: 0 additions & 35 deletions
This file was deleted.

‎cmd/boulder-janitor/certsPerName.go‎

Lines changed: 0 additions & 38 deletions
This file was deleted.

‎cmd/boulder-janitor/certstatus.go‎

Lines changed: 0 additions & 34 deletions
This file was deleted.

‎cmd/boulder-janitor/config.go‎

Lines changed: 0 additions & 102 deletions
This file was deleted.

‎cmd/boulder-janitor/config_test.go‎

Lines changed: 0 additions & 68 deletions
This file was deleted.

‎cmd/boulder-janitor/janitor/handlers.go‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package janitor
2+
3+
import (
4+
"context"
5+
"fmt"
6+
7+
"github.com/letsencrypt/boulder/db"
8+
)
9+
10+
// deleteHandlers is a map of json-usable strings to actual functions, so that
11+
// configs can specify a delete handler by name.
12+
var deleteHandlers = map[string]func(*batchedDBJob, int64) error{
13+
"default": deleteDefault,
14+
"deleteOrder": deleteOrder,
15+
}
16+
17+
// deleteDefault performs a delete of the given ID from the batchedDBJob's
18+
// table or returns an error. It does not use a transaction and assumes there
19+
// are no foreign key constraints or referencing rows in other tables to manage.
20+
func deleteDefault(j *batchedDBJob, id int64) error {
21+
// NOTE(@cpu): We throw away the sql.Result here without checking the rows
22+
// affected because the query is always specific to the ID auto-increment
23+
// primary key. If there are multiple rows with the same primary key MariaDB
24+
// has failed us deeply.
25+
query := fmt.Sprintf(`DELETE FROM %s WHERE id = ?`, j.table)
26+
if _, err := j.db.Exec(query, id); err != nil {
27+
return err
28+
}
29+
j.log.Debugf("deleted ID %d in table %q", id, j.table)
30+
deletedStat.WithLabelValues(j.table).Inc()
31+
return nil
32+
}
33+
34+
// deleteOrder performs a delete of the given ID from the batchedDBJob's `orders`
35+
// table or returns an error. It also deletes corresponding rows from three
36+
// other tables which reference the `orders` table by foreign key.
37+
func deleteOrder(j *batchedDBJob, orderID int64) error {
38+
ctx := context.Background()
39+
// Perform a multi-table delete inside of a transaction using the order ID.
40+
// Either all of the rows associated with the order ID will be deleted or the
41+
// transaction will be rolled back.
42+
_, err := db.WithTransaction(ctx, j.db, func(txWithCtx db.Executor) (interface{}, error) {
43+
// Delete table rows in the childTables that reference the order being deleted.
44+
childTables := []string{"requestedNames", "orderFqdnSets", "orderToAuthz2"}
45+
for _, t := range childTables {
46+
query := fmt.Sprintf(`DELETE FROM %s WHERE orderID = ?`, t)
47+
res, err := txWithCtx.Exec(query, orderID)
48+
if err != nil {
49+
return nil, err
50+
}
51+
affected, err := res.RowsAffected()
52+
if err != nil {
53+
return nil, err
54+
}
55+
deletedStat.WithLabelValues(t).Add(float64(affected))
56+
}
57+
// Finally delete the order itself
58+
if _, err := txWithCtx.Exec(`DELETE FROM orders WHERE id = ?`, orderID); err != nil {
59+
return nil, err
60+
}
61+
deletedStat.WithLabelValues("orders").Inc()
62+
j.log.Debugf("deleted order ID %d and associated rows", orderID)
63+
return nil, nil
64+
})
65+
return err
66+
}

cmd/boulder-janitor/orders_test.go renamed to cmd/boulder-janitor/janitor/handlers_test.go

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package main
1+
package janitor
22

33
import (
44
"context"
@@ -73,11 +73,15 @@ func TestDeleteOrder(t *testing.T) {
7373
test.AssertNotError(t, err, "error creating test order")
7474

7575
// Create a cleanup config for the Orders job
76-
config := CleanupConfig{
77-
WorkSleep: cmd.ConfigDuration{Duration: time.Second},
78-
BatchSize: 1,
79-
MaxDPS: 1,
80-
Parallelism: 1,
76+
config := JobConfig{
77+
Enabled: true,
78+
Table: "orders",
79+
ExpiresColumn: "expires",
80+
WorkSleep: cmd.ConfigDuration{Duration: time.Second},
81+
BatchSize: 1,
82+
MaxDPS: 1,
83+
Parallelism: 1,
84+
DeleteHandler: "deleteOrder",
8185
}
8286

8387
// Create a dbMap for the janitor user. We don't want to use the SA dbMap
@@ -86,9 +90,11 @@ func TestDeleteOrder(t *testing.T) {
8690
janitorDbMap, err := sa.NewDbMap("janitor@tcp(boulder-mysql:3306)/boulder_sa_test", 0)
8791
test.AssertNotError(t, err, "error creating db map")
8892

89-
// Create an Orders job and delete the mock order by its ID
90-
j := newOrdersJob(janitorDbMap, log, fc, config)
91-
err = j.deleteHandler(testOrder.Id)
93+
// Create an Orders job
94+
j := newJob(config, janitorDbMap, log, fc)
95+
96+
// Delete the mock order by its ID
97+
err = j.deleteHandler(j, testOrder.Id)
9298
// It should not error
9399
test.AssertNotError(t, err, "error calling deleteHandler")
94100

0 commit comments

Comments
 (0)