Skip to content

Commit

Permalink
set dying only if juju api don't fail
Browse files Browse the repository at this point in the history
  • Loading branch information
SimoneDutto committed Dec 6, 2024
1 parent 10111ef commit f29457e
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 26 deletions.
16 changes: 11 additions & 5 deletions internal/jimm/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -1165,15 +1165,21 @@ func (j *JIMM) DestroyModel(ctx context.Context, user *openfga.User, mt names.Mo
zapctx.Info(ctx, string(op))

err := j.doModelAdmin(ctx, user, mt, func(m *dbmodel.Model, api API) error {
if err := api.DestroyModel(ctx, mt, destroyStorage, force, maxWait, timeout); err != nil {
return err
}
m.Life = state.Dying.String()
if err := j.Database.UpdateModel(ctx, m); err != nil {
// If the database fails to update don't worry too much the
// monitor should catch it.
zapctx.Error(ctx, "failed to store model change", zaputil.Error(err))
return err
}
if err := api.DestroyModel(ctx, mt, destroyStorage, force, maxWait, timeout); err != nil {
zapctx.Error(ctx, "failed to call destroy juju api", zaputil.Error(err))
// this is a manual way of restoring the life state to alive if the JUJU api fails.
m.Life = state.Alive.String()
if err := j.Database.UpdateModel(ctx, m); err != nil {
zapctx.Error(ctx, "failed to store model change", zaputil.Error(err))
}
return err
}

return nil
})
if err != nil {
Expand Down
49 changes: 28 additions & 21 deletions internal/jimm/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3141,6 +3141,7 @@ var destroyModelTests = []struct {
timeout *time.Duration
expectError string
expectErrorCode errors.Code
expectedLife string
}{{
name: "NotFound",
env: destroyModelTestEnv,
Expand Down Expand Up @@ -3182,30 +3183,34 @@ var destroyModelTests = []struct {
force: newBool(false),
maxWait: newDuration(time.Second),
timeout: newDuration(time.Second),
expectedLife: "dying",
}, {
name: "SuperuserSuccess",
env: destroyModelTestEnv,
destroyModel: func(_ context.Context, _ names.ModelTag, _, _ *bool, _, _ *time.Duration) error {
return nil
},
username: "[email protected]",
uuid: "00000002-0000-0000-0000-000000000001",
username: "[email protected]",
uuid: "00000002-0000-0000-0000-000000000001",
expectedLife: "dying",
}, {
name: "DialError",
env: destroyModelTestEnv,
dialError: errors.E("dial error"),
username: "[email protected]",
uuid: "00000002-0000-0000-0000-000000000001",
expectError: `dial error`,
name: "DialError",
env: destroyModelTestEnv,
dialError: errors.E("dial error"),
username: "[email protected]",
uuid: "00000002-0000-0000-0000-000000000001",
expectError: `dial error`,
expectedLife: "alive",
}, {
name: "APIError",
env: destroyModelTestEnv,
destroyModel: func(_ context.Context, _ names.ModelTag, _, _ *bool, _, _ *time.Duration) error {
return errors.E("api error")
},
username: "[email protected]",
uuid: "00000002-0000-0000-0000-000000000001",
expectError: `api error`,
username: "[email protected]",
uuid: "00000002-0000-0000-0000-000000000001",
expectError: `api error`,
expectedLife: "alive",
}}

func TestDestroyModel(t *testing.T) {
Expand Down Expand Up @@ -3249,18 +3254,20 @@ func TestDestroyModel(t *testing.T) {
if test.expectErrorCode != "" {
c.Check(errors.ErrorCode(err), qt.Equals, test.expectErrorCode)
}
return
} else {
c.Assert(err, qt.IsNil)
}
c.Assert(err, qt.IsNil)
m := dbmodel.Model{
UUID: sql.NullString{
String: test.uuid,
Valid: true,
},
if test.expectedLife != "" {
m := dbmodel.Model{
UUID: sql.NullString{
String: test.uuid,
Valid: true,
},
}
err = j.Database.GetModel(ctx, &m)
c.Assert(err, qt.IsNil)
c.Assert(m.Life, qt.Equals, test.expectedLife)
}
err = j.Database.GetModel(ctx, &m)
c.Assert(err, qt.IsNil)
c.Check(m.Life, qt.Equals, state.Dying.String())
})
}
}
Expand Down

0 comments on commit f29457e

Please sign in to comment.