Vc/event history pkey (#85)

* alter event history table primary key

When I originally implemented this I envisioned using a single record
for each "workflow" managed by Condition-Orchestrator. For example, if
you did a firmware install you would get results only for the sum of the
install and the following inventory, instead of a record for each
condition we tried.

When I started looking at implementing the corresponding changes in
Condition-API and Condition-Orchestrator, I found that hooking into the
"combination" event was not as natural as providing a history record for
each condition being executed. However, adopting that strategy means
that I needed to change the table schema.

* multiple event history entries with a single id

ConditionAPI and Orchestrator have a concept of "composed conditions"
where several discrete actions are performed on a server. Each of these
actions is tracked individually as a tuple of (condition id, condition
type). Originally, I implemented event histories intending that the
entry would track only the composite data (i.e. the overall success or
failure along with some very limited contextual data). However, within
Condition Orchestrator it is actually much easier to track each
individual condition being applied. This required a change in FleetDB to
remove the uniqueness constraint on event_id, and instead replace it
with (event_id, event_type, target_server).

This pushed some changes into the query API layer, where now querying by
event-id will return all of the conditions applied using that UUID (i.e.
a de-facto timeline of that request). The tests are also now adjusted
to explicitly test the expected behavior of Condition Orchestrator,
creating multiple history entries with different event_types.

* update crdb for lint and test

.github/workflows/lint-test.yml

@@ -21,7 +21,7 @@ jobs:
           go-version-file: go.mod
 
       - name: Install cockroach binary
-        run: curl https://binaries.cockroachdb.com/cockroach-v21.1.21.linux-amd64.tgz | tar -xz && sudo cp -i cockroach-v21.1.21.linux-amd64/cockroach /usr/local/bin/
+        run: curl https://binaries.cockroachdb.com/cockroach-v23.1.23.linux-amd64.tgz | tar -xz && sudo cp -i cockroach-v23.1.23.linux-amd64/cockroach /usr/local/bin/
 
       - name: Start test database
         run: cockroach start-single-node --insecure --background

db/migrations/00006_drop_event_history_idx.sql

@@ -0,0 +1,9 @@
+-- +goose Up
+-- +goose StatementBegin
+DROP INDEX evt_history_target;
+-- +goose StatementEnd
+
+-- +goose Down
+-- +goose StatementBegin
+CREATE INDEX evt_history_target ON public.event_history (target_server ASC) INCLUDE (event_type, event_start, event_end);
+-- +goose StatementEnd

db/migrations/00007_modify_event_history_pkey.sql

@@ -0,0 +1,13 @@
+-- +goose Up
+-- +goose StatementBegin
+ALTER TABLE public.event_history DROP CONSTRAINT event_history_pkey;
+
+ALTER TABLE public.event_history ADD PRIMARY KEY (event_id, event_type, target_server);
+-- +goose StatementEnd
+
+-- +goose Down
+-- +goose StatementBegin
+ALTER TABLE public.event_history DROP CONSTRAINT event_history_pkey;
+
+ALTER TABLE public.event_history ADD PRIMARY KEY (event_id);
+-- +goose StatementEnd

go.mod

@@ -44,14 +44,11 @@ require (
 )
 
 require (
-	cloud.google.com/go/kms v1.15.7 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/bytedance/sonic v1.11.9 // indirect
 	github.com/bytedance/sonic/loader v0.1.1 // indirect
 	github.com/cenkalti/backoff/v4 v4.3.0 // indirect
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
-	github.com/chenzhuoyu/base64x v0.0.0-20230717121745-296ad89f973d // indirect
-	github.com/chenzhuoyu/iasm v0.9.1 // indirect
 	github.com/cloudwego/base64x v0.1.4 // indirect
 	github.com/cloudwego/iasm v0.2.0 // indirect
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect

internal/dbtools/fixtures.go

@@ -98,8 +98,9 @@ var (
 	FixtureBiosConfigComponents []*models.BiosConfigComponent
 	FixtureBiosConfigSettings   [][]*models.BiosConfigSetting
 
-	FixtureEventHistoryServer *models.Server
-	FixtureEventHistories     []*models.EventHistory
+	FixtureEventHistoryServer    *models.Server
+	FixtureEventHistoryRelatedID uuid.UUID
+	FixtureEventHistories        []*models.EventHistory
 )
 
 func addFixtures(t *testing.T) error {
@@ -734,6 +735,8 @@ func setupEventHistoryFixtures(ctx context.Context, db *sqlx.DB) error {
 		return errors.Wrap(err, "event history server fixture")
 	}
 
+	FixtureEventHistoryRelatedID = uuid.New()
+
 	FixtureEventHistories = []*models.EventHistory{
 		{
 			EventID:      uuid.New().String(),
@@ -765,6 +768,36 @@ func setupEventHistoryFixtures(ctx context.Context, db *sqlx.DB) error {
 			FinalState:   "succeeded",
 			FinalStatus:  null.JSONFrom([]byte(`{"status": "some status"}`)),
 		},
+		{
+			EventID:      FixtureEventHistoryRelatedID.String(),
+			EventType:    "test event",
+			EventStart:   time.Now().Add(-1 * time.Hour),
+			EventEnd:     time.Now().Add(-50 * time.Minute),
+			TargetServer: FixtureEventHistoryServer.ID,
+			Parameters:   null.JSONFrom([]byte(`{"msg": "test event"}`)),
+			FinalState:   "succeeded",
+			FinalStatus:  null.JSONFrom([]byte(`{"status": "some status"}`)),
+		},
+		{
+			EventID:      FixtureEventHistoryRelatedID.String(),
+			EventType:    "test event 2",
+			EventStart:   time.Now().Add(-1 * time.Hour),
+			EventEnd:     time.Now().Add(-40 * time.Minute),
+			TargetServer: FixtureEventHistoryServer.ID,
+			Parameters:   null.JSONFrom([]byte(`{"msg": "test event"}`)),
+			FinalState:   "succeeded",
+			FinalStatus:  null.JSONFrom([]byte(`{"status": "some status"}`)),
+		},
+		{
+			EventID:      FixtureEventHistoryRelatedID.String(),
+			EventType:    "test event 3",
+			EventStart:   time.Now().Add(-1 * time.Hour),
+			EventEnd:     time.Now().Add(-50 * time.Minute),
+			TargetServer: FixtureEventHistoryServer.ID,
+			Parameters:   null.JSONFrom([]byte(`{"msg": "test event"}`)),
+			FinalState:   "succeeded",
+			FinalStatus:  null.JSONFrom([]byte(`{"status": "some status"}`)),
+		},
 	}
 
 	for idx, evt := range FixtureEventHistories {

pkg/api/v1/router.go

@@ -147,7 +147,7 @@ func (r *Router) Routes(rg *gin.RouterGroup) {
 
 	srvEvents := rg.Group("/events")
 	{
-		srvEvents.GET("/:evtID", amw.AuthRequired(readScopes("events")), r.getEventByID)
+		srvEvents.GET("/:evtID", amw.AuthRequired(readScopes("events")), r.getHistoryByConditionID)
 		srvEvents.GET("/by-server/:srvID", amw.AuthRequired(readScopes("server")), r.getServerEvents)
 		srvEvents.PUT("/:evtID", amw.AuthRequired(updateScopes("events")), r.updateEvent)
 	}

pkg/api/v1/router_events.go

@@ -30,24 +30,19 @@ type Event struct {
 	FinalStatus json.RawMessage `json:"final_status,omitempty" binding:"-"`
 }
 
-func (r *Router) getEventByID(c *gin.Context) {
+func (r *Router) getHistoryByConditionID(c *gin.Context) {
 	evtID, err := uuid.Parse(c.Param("evtID"))
 	if err != nil {
 		badRequestResponse(c, "failed to parse event id", err)
 		return
 	}
 
-	eh, err := models.EventHistories(
+	// we expect a small number of events with the same id, O(5-10)
+	ehs, err := models.EventHistories(
 		models.EventHistoryWhere.EventID.EQ(evtID.String()),
-	).One(c.Request.Context(), r.DB)
+	).All(c.Request.Context(), r.DB)
 
-	switch {
-	case err == nil:
-	case errors.Is(err, sql.ErrNoRows):
-		msg := fmt.Sprintf("no event for id %s", evtID.String())
-		notFoundResponse(c, msg)
-		return
-	default:
+	if err != nil {
 		metrics.DBError("fetching event history")
 		r.Logger.With(
 			zap.Error(err),
@@ -57,24 +52,38 @@ func (r *Router) getEventByID(c *gin.Context) {
 		return
 	}
 
-	evt := &Event{
-		EventID:    uuid.MustParse(eh.EventID),
-		Type:       eh.EventType,
-		Start:      eh.EventStart,
-		End:        eh.EventEnd,
-		Target:     uuid.MustParse(eh.TargetServer),
-		FinalState: eh.FinalState,
+	if len(ehs) == 0 {
+		msg := fmt.Sprintf("no event for id %s", evtID.String())
+		notFoundResponse(c, msg)
+		return
 	}
 
-	if eh.Parameters.Valid {
-		evt.Parameters = eh.Parameters.JSON
+	events := make([]*Event, 0, len(ehs))
+	for _, eh := range ehs {
+		evt := &Event{
+			EventID:    uuid.MustParse(eh.EventID),
+			Type:       eh.EventType,
+			Start:      eh.EventStart,
+			End:        eh.EventEnd,
+			Target:     uuid.MustParse(eh.TargetServer),
+			FinalState: eh.FinalState,
+		}
+
+		if eh.Parameters.Valid {
+			evt.Parameters = eh.Parameters.JSON
+		}
+
+		if eh.FinalStatus.Valid {
+			evt.FinalStatus = eh.FinalStatus.JSON
+		}
+		events = append(events, evt)
 	}
 
-	if eh.FinalStatus.Valid {
-		evt.FinalStatus = eh.FinalStatus.JSON
+	pd := paginationData{
+		pageCount: len(events),
 	}
 
-	itemResponse(c, evt)
+	listResponse(c, events, pd)
 }
 
 func (r *Router) getServerEvents(c *gin.Context) {
@@ -124,7 +133,7 @@ func (r *Router) getServerEvents(c *gin.Context) {
 
 	ehs, err := models.EventHistories(
 		models.EventHistoryWhere.TargetServer.EQ(srvID.String()),
-		qm.OrderBy("event_end DESC"),
+		qm.OrderBy("event_id, event_end DESC"),
 		qm.Limit(limit),
 		qm.Offset(offset),
 	).All(c.Request.Context(), r.DB)
@@ -201,6 +210,8 @@ func (r *Router) updateEvent(c *gin.Context) {
 	// shortcut if we've seen this event before already
 	existing, err := models.EventHistories(
 		models.EventHistoryWhere.EventID.EQ(evt.EventID.String()),
+		models.EventHistoryWhere.EventType.EQ(evt.Type),
+		models.EventHistoryWhere.TargetServer.EQ(evt.Target.String()),
 	).One(ctx, r.DB)
 
 	switch {