From f29457e0addb8e1d5e362b0780a08901a4da0d1c Mon Sep 17 00:00:00 2001
From: SimoneDutto <simone.dutto@canonical.com>
Date: Fri, 6 Dec 2024 12:10:46 +0100
Subject: [PATCH] set dying only if juju api don't fail

---
 internal/jimm/model.go      | 16 ++++++++----
 internal/jimm/model_test.go | 49 +++++++++++++++++++++----------------
 2 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/internal/jimm/model.go b/internal/jimm/model.go
index d3122b67a..4e5d7d751 100644
--- a/internal/jimm/model.go
+++ b/internal/jimm/model.go
@@ -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 {
diff --git a/internal/jimm/model_test.go b/internal/jimm/model_test.go
index f7d76265c..111a9378e 100644
--- a/internal/jimm/model_test.go
+++ b/internal/jimm/model_test.go
@@ -3141,6 +3141,7 @@ var destroyModelTests = []struct {
 	timeout         *time.Duration
 	expectError     string
 	expectErrorCode errors.Code
+	expectedLife    string
 }{{
 	name:            "NotFound",
 	env:             destroyModelTestEnv,
@@ -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: "charlie@canonical.com",
-	uuid:     "00000002-0000-0000-0000-000000000001",
+	username:     "charlie@canonical.com",
+	uuid:         "00000002-0000-0000-0000-000000000001",
+	expectedLife: "dying",
 }, {
-	name:        "DialError",
-	env:         destroyModelTestEnv,
-	dialError:   errors.E("dial error"),
-	username:    "alice@canonical.com",
-	uuid:        "00000002-0000-0000-0000-000000000001",
-	expectError: `dial error`,
+	name:         "DialError",
+	env:          destroyModelTestEnv,
+	dialError:    errors.E("dial error"),
+	username:     "alice@canonical.com",
+	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:    "charlie@canonical.com",
-	uuid:        "00000002-0000-0000-0000-000000000001",
-	expectError: `api error`,
+	username:     "charlie@canonical.com",
+	uuid:         "00000002-0000-0000-0000-000000000001",
+	expectError:  `api error`,
+	expectedLife: "alive",
 }}
 
 func TestDestroyModel(t *testing.T) {
@@ -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())
 		})
 	}
 }