Skip to content

Commit

Permalink
Merge pull request juju#18747 from manadart/dqlite-empty-relation-state
Browse files Browse the repository at this point in the history
juju#18747

This fixes a panic where unit relation state is non-nil, but empty. 

We were attempting to prepare a Dqlite statement with the zero index of an empty slice. Now we just skip the insert operation altogether.

This is code slated for rework under a new `hook` domain, to make it transactional along with ports, secrets etc. This patch constitutes a quality-of life improvement in the interim.

## QA steps

Bootstrap, add a model, deploy a unit.
  • Loading branch information
jujubot authored Jan 29, 2025
2 parents aa725c2 + 7e66888 commit bb7d0a7
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 8 deletions.
18 changes: 10 additions & 8 deletions domain/unitstate/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,19 +182,21 @@ func (st *State) SetUnitStateRelation(ctx domain.AtomicContext, uuid string, sta

keyVals := makeUnitRelationStateKeyVals(uuid, state)

q = "INSERT INTO unit_state_relation(*) VALUES ($unitRelationStateKeyVal.*)"
iStmt, err := st.Prepare(q, keyVals[0])
if err != nil {
return errors.Errorf("preparing relation state insert query: %w", err)
}

return domain.Run(ctx, func(ctx context.Context, tx *sqlair.TX) error {
if err := tx.Query(ctx, dStmt, id).Run(); err != nil {
return errors.Errorf("deleting unit relation state: %w", err)
}

if err := tx.Query(ctx, iStmt, keyVals).Run(); err != nil {
return errors.Errorf("setting unit relation state: %w", err)
if len(keyVals) != 0 {
q = "INSERT INTO unit_state_relation(*) VALUES ($unitRelationStateKeyVal.*)"
iStmt, err := st.Prepare(q, keyVals[0])
if err != nil {
return errors.Errorf("preparing relation state insert query: %w", err)
}

if err := tx.Query(ctx, iStmt, keyVals).Run(); err != nil {
return errors.Errorf("setting unit relation state: %w", err)
}
}
return nil
})
Expand Down
36 changes: 36 additions & 0 deletions domain/unitstate/state/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,42 @@ func (s *stateSuite) TestUpdateUnitStateRelation(c *gc.C) {
c.Check(gotState, gc.DeepEquals, expState)
}

func (s *stateSuite) TestUpdateUnitStateRelationEmptyMap(c *gc.C) {
st := NewState(s.TxnRunnerFactory())
ctx := context.Background()

// Set some initial state. This should be overwritten.
err := s.TxnRunner().StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error {
q := "INSERT INTO unit_state_relation VALUES (?, 1, 'one-val')"
_, err := tx.ExecContext(ctx, q, s.unitUUID)
return err
})
c.Assert(err, jc.ErrorIsNil)

err = st.RunAtomic(ctx, func(ctx domain.AtomicContext) error {
return st.SetUnitStateRelation(ctx, s.unitUUID, map[int]string{})
})
c.Assert(err, jc.ErrorIsNil)

var rowCount int
err = s.TxnRunner().StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error {
q := "SELECT key, value FROM unit_state_relation WHERE unit_uuid = ?"
rows, err := tx.QueryContext(ctx, q, s.unitUUID)
if err != nil {
return err
}
defer func() { _ = rows.Close() }()

for rows.Next() {
rowCount++
}
return rows.Err()
})
c.Assert(err, jc.ErrorIsNil)

c.Check(rowCount, gc.DeepEquals, 0)
}

func (s *stateSuite) TestGetUnitState(c *gc.C) {
st := NewState(s.TxnRunnerFactory())

Expand Down

0 comments on commit bb7d0a7

Please sign in to comment.