cloud/bmaas/bmdb: correctly handle installation report
Previously we ignored the result of an installation report.
The bmdb does now store the result and correctly triggers
a recovery flow of the installation fails.
Change-Id: Ie8445cf9178ba84c6362b61ef8fa47208ab690be
Reviewed-on: https://review.monogon.dev/c/monogon/+/1865
Reviewed-by: Serge Bazanski <serge@monogon.tech>
Tested-by: Jenkins CI
diff --git a/cloud/bmaas/bmdb/model/migrations/1687875953_add_installation_report.down.sql b/cloud/bmaas/bmdb/model/migrations/1687875953_add_installation_report.down.sql
new file mode 100644
index 0000000..6c2cb40
--- /dev/null
+++ b/cloud/bmaas/bmdb/model/migrations/1687875953_add_installation_report.down.sql
@@ -0,0 +1,5 @@
+ALTER TABLE machine_os_installation_report
+DROP COLUMN os_installation_result,
+DROP COLUMN os_installation_report_raw;
+
+DROP TYPE machine_os_installation_result;
diff --git a/cloud/bmaas/bmdb/model/migrations/1687875953_add_installation_report.up.sql b/cloud/bmaas/bmdb/model/migrations/1687875953_add_installation_report.up.sql
new file mode 100644
index 0000000..4f25589
--- /dev/null
+++ b/cloud/bmaas/bmdb/model/migrations/1687875953_add_installation_report.up.sql
@@ -0,0 +1,10 @@
+CREATE TYPE machine_os_installation_result AS ENUM (
+ 'Success',
+ 'Error'
+ );
+
+-- Add column for storing the serialized cloud.bmaas.server.api.OSInstallationReport
+-- also add a column to display if the installation was successful or not.
+ALTER TABLE machine_os_installation_report
+ ADD COLUMN os_installation_result machine_os_installation_result NOT NULL,
+ ADD COLUMN os_installation_report_raw BYTEA NOT NULL;
\ No newline at end of file
diff --git a/cloud/bmaas/bmdb/model/queries_tags.sql b/cloud/bmaas/bmdb/model/queries_tags.sql
index ddb6f19..58e2d96 100644
--- a/cloud/bmaas/bmdb/model/queries_tags.sql
+++ b/cloud/bmaas/bmdb/model/queries_tags.sql
@@ -45,11 +45,13 @@
-- name: MachineSetOSInstallationReport :exec
INSERT INTO machine_os_installation_report (
- machine_id, generation
+ machine_id, generation, os_installation_result, os_installation_report_raw
) VALUES (
- $1, $2
+ $1, $2, $3, $4
) ON CONFLICT (machine_id) DO UPDATE SET
- generation = $2
+ generation = $2,
+ os_installation_result = $3,
+ os_installation_report_raw = $4
;
diff --git a/cloud/bmaas/bmdb/model/queries_workflows.sql b/cloud/bmaas/bmdb/model/queries_workflows.sql
index 854c15d..8400d4e 100644
--- a/cloud/bmaas/bmdb/model/queries_workflows.sql
+++ b/cloud/bmaas/bmdb/model/queries_workflows.sql
@@ -45,7 +45,7 @@
INNER JOIN machine_agent_started ON machines.machine_id = machine_agent_started.machine_id
LEFT JOIN machine_agent_heartbeat ON machines.machine_id = machine_agent_heartbeat.machine_id
LEFT JOIN machine_os_installation_request ON machines.machine_id = machine_os_installation_request.machine_id
-LEFT JOIN machine_os_installation_report ON machines.machine_id = machine_os_installation_report.machine_id
+LEFT JOIN machine_os_installation_report ON machines.machine_id = machine_os_installation_report.machine_id AND machine_os_installation_report.os_installation_result = 'Success'
WHERE
-- Do not recover machines that have a fulfilled OS installation request.
(
diff --git a/cloud/bmaas/bmdb/queries_test.go b/cloud/bmaas/bmdb/queries_test.go
index facc54f..96e8dbe 100644
--- a/cloud/bmaas/bmdb/queries_test.go
+++ b/cloud/bmaas/bmdb/queries_test.go
@@ -109,8 +109,9 @@
// remain a candidate.
if err = session.Transact(ctx, func(q *model.Queries) error {
if err := q.MachineSetOSInstallationReport(ctx, model.MachineSetOSInstallationReportParams{
- MachineID: machine.MachineID,
- Generation: 9,
+ MachineID: machine.MachineID,
+ Generation: 9,
+ OsInstallationResult: model.MachineOsInstallationResultSuccess,
}); err != nil {
return err
}
@@ -124,8 +125,9 @@
// be a candidate anymore.
if err = session.Transact(ctx, func(q *model.Queries) error {
if err := q.MachineSetOSInstallationReport(ctx, model.MachineSetOSInstallationReportParams{
- MachineID: machine.MachineID,
- Generation: 10,
+ MachineID: machine.MachineID,
+ Generation: 10,
+ OsInstallationResult: model.MachineOsInstallationResultSuccess,
}); err != nil {
return err
}
@@ -229,8 +231,9 @@
}
if scenario.reportGeneration != 0 {
if err := q.MachineSetOSInstallationReport(ctx, model.MachineSetOSInstallationReportParams{
- MachineID: machine.MachineID,
- Generation: scenario.reportGeneration,
+ MachineID: machine.MachineID,
+ Generation: scenario.reportGeneration,
+ OsInstallationResult: model.MachineOsInstallationResultSuccess,
}); err != nil {
return fmt.Errorf("MachineSetOSInstallationReport: %w", err)
}
diff --git a/cloud/bmaas/bmdb/reflection/schema.go b/cloud/bmaas/bmdb/reflection/schema.go
index 30ce6c4..d497f5d 100644
--- a/cloud/bmaas/bmdb/reflection/schema.go
+++ b/cloud/bmaas/bmdb/reflection/schema.go
@@ -78,6 +78,7 @@
var knownProtoFields = map[string]proto.Message{
"hardware_report_raw": &api.AgentHardwareReport{},
"os_installation_request_raw": &api.OSInstallationRequest{},
+ "os_installation_report_raw": &api.OSInstallationReport{},
}
// HumanType returns a human-readable representation of the field's type. This is
diff --git a/cloud/bmaas/scruffy/hw_stats_test.go b/cloud/bmaas/scruffy/hw_stats_test.go
index ffb572c..dbcab2d 100644
--- a/cloud/bmaas/scruffy/hw_stats_test.go
+++ b/cloud/bmaas/scruffy/hw_stats_test.go
@@ -10,9 +10,10 @@
"google.golang.org/protobuf/proto"
aapi "source.monogon.dev/cloud/agent/api"
+ "source.monogon.dev/cloud/bmaas/server/api"
+
"source.monogon.dev/cloud/bmaas/bmdb"
"source.monogon.dev/cloud/bmaas/bmdb/model"
- "source.monogon.dev/cloud/bmaas/server/api"
"source.monogon.dev/cloud/lib/component"
)
@@ -197,8 +198,9 @@
}
if m.installationReportGeneration != nil {
err = q.MachineSetOSInstallationReport(ctx, model.MachineSetOSInstallationReportParams{
- MachineID: mach.MachineID,
- Generation: *m.installationReportGeneration,
+ MachineID: mach.MachineID,
+ Generation: *m.installationReportGeneration,
+ OsInstallationResult: model.MachineOsInstallationResultSuccess,
})
if err != nil {
return err
diff --git a/cloud/bmaas/server/agent_callback_service.go b/cloud/bmaas/server/agent_callback_service.go
index 27ce9af..97636f7 100644
--- a/cloud/bmaas/server/agent_callback_service.go
+++ b/cloud/bmaas/server/agent_callback_service.go
@@ -14,8 +14,9 @@
"google.golang.org/protobuf/proto"
"k8s.io/klog/v2"
- "source.monogon.dev/cloud/bmaas/bmdb/model"
apb "source.monogon.dev/cloud/bmaas/server/api"
+
+ "source.monogon.dev/cloud/bmaas/bmdb/model"
"source.monogon.dev/metropolis/node/core/rpc"
)
@@ -81,10 +82,18 @@
}
}
+ var installRaw []byte
+ if req.InstallationReport != nil {
+ installRaw, err = proto.Marshal(req.InstallationReport)
+ if err != nil {
+ return nil, status.Errorf(codes.InvalidArgument, "could not serialize installation report: %v", err)
+ }
+ }
+
// Upsert heartbeat time and hardware report.
err = session.Transact(ctx, func(q *model.Queries) error {
// Upsert hardware report if submitted.
- if hwraw != nil {
+ if len(hwraw) != 0 {
err = q.MachineSetHardwareReport(ctx, model.MachineSetHardwareReportParams{
MachineID: machineId,
HardwareReportRaw: hwraw,
@@ -94,10 +103,21 @@
}
}
// Upsert os installation report if submitted.
- if req.InstallationReport != nil {
+ if len(installRaw) != 0 {
+ var result model.MachineOsInstallationResult
+ switch req.InstallationReport.Result.(type) {
+ case *apb.OSInstallationReport_Success_:
+ result = model.MachineOsInstallationResultSuccess
+ case *apb.OSInstallationReport_Error_:
+ result = model.MachineOsInstallationResultError
+ default:
+ return fmt.Errorf("unknown installation report result: %T", req.InstallationReport.Result)
+ }
err = q.MachineSetOSInstallationReport(ctx, model.MachineSetOSInstallationReportParams{
- MachineID: machineId,
- Generation: req.InstallationReport.Generation,
+ MachineID: machineId,
+ Generation: req.InstallationReport.Generation,
+ OsInstallationResult: result,
+ OsInstallationReportRaw: installRaw,
})
}
return q.MachineSetAgentHeartbeat(ctx, model.MachineSetAgentHeartbeatParams{
diff --git a/cloud/bmaas/server/agent_callback_service_test.go b/cloud/bmaas/server/agent_callback_service_test.go
index 320bb68..3bd3df9 100644
--- a/cloud/bmaas/server/agent_callback_service_test.go
+++ b/cloud/bmaas/server/agent_callback_service_test.go
@@ -11,9 +11,10 @@
"google.golang.org/grpc"
"google.golang.org/protobuf/proto"
+ apb "source.monogon.dev/cloud/bmaas/server/api"
+
"source.monogon.dev/cloud/bmaas/bmdb"
"source.monogon.dev/cloud/bmaas/bmdb/model"
- apb "source.monogon.dev/cloud/bmaas/server/api"
"source.monogon.dev/cloud/lib/component"
"source.monogon.dev/metropolis/node/core/rpc"
)
@@ -215,7 +216,7 @@
}
// Submit a report, expect no more request.
- hbr, err = heartbeat(machine.MachineID, &apb.OSInstallationReport{Generation: 123})
+ hbr, err = heartbeat(machine.MachineID, &apb.OSInstallationReport{Generation: 123, Result: &apb.OSInstallationReport_Success_{}})
if err != nil {
t.Fatalf("heartbeat: %v", err)
}