Fix destroy-model/destroy-controller to handle --force better.

--force and --timeout/--model-timeout now properly control
the destruction of models so that --force with --timeout
properly progresses despite the status of the model/environment.

--force without --timeout now only propagates force to the
cleanup of model entities, rather than the model removal
itself.

Since model destruction cannot be stopped once it has started,
--timeout no longer makes sense to be passed for non-forceful
model destruction.

Graceful model destroys now wait indefinitely
for the entities to cleanup and the cloud resources to cleanup
before removing the model from state. Due to this change it
is now required to be able to change the model destruction
parameters while a model destroy is being processed by the
undertaker. The undertaker will now restart itself when
ForceDestroy or Timeout changes.

api/client/modelmanager/modelmanager.go

@@ -394,7 +394,7 @@ func (c *Client) DumpModelDB(model names.ModelTag) (map[string]interface{}, erro
 // DestroyModel puts the specified model into a "dying" state, which will
 // cause the model's resources to be cleaned up, after which the model will
 // be removed.
-func (c *Client) DestroyModel(tag names.ModelTag, destroyStorage, force *bool, maxWait *time.Duration, timeout time.Duration) error {
+func (c *Client) DestroyModel(tag names.ModelTag, destroyStorage, force *bool, maxWait *time.Duration, timeout *time.Duration) error {
 	var args interface{}
 	if c.BestAPIVersion() < 4 {
 		if destroyStorage == nil || !*destroyStorage {
@@ -409,7 +409,7 @@ func (c *Client) DestroyModel(tag names.ModelTag, destroyStorage, force *bool, m
 		if c.BestAPIVersion() > 6 {
 			arg.Force = force
 			arg.MaxWait = maxWait
-			arg.Timeout = &timeout
+			arg.Timeout = timeout
 		}
 		args = params.DestroyModelsParams{Models: []params.DestroyModelParams{arg}}
 	}

api/client/modelmanager/modelmanager_test.go

@@ -216,7 +216,7 @@ func (s *modelmanagerSuite) testDestroyModel(c *gc.C, v int, destroyStorage, for
 		),
 	}
 	client := modelmanager.NewClient(apiCaller)
-	err := client.DestroyModel(coretesting.ModelTag, destroyStorage, force, maxWait, timeout)
+	err := client.DestroyModel(coretesting.ModelTag, destroyStorage, force, maxWait, &timeout)
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(called, jc.IsTrue)
 }
@@ -245,15 +245,17 @@ func (s *modelmanagerSuite) TestDestroyModelV3(c *gc.C) {
 	)
 	client := modelmanager.NewClient(apiCaller)
 	destroyStorage := true
-	err := client.DestroyModel(coretesting.ModelTag, &destroyStorage, nil, nil, time.Minute)
+	timeout := time.Minute
+	err := client.DestroyModel(coretesting.ModelTag, &destroyStorage, nil, nil, &timeout)
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(called, jc.IsTrue)
 }
 
 func (s *modelmanagerSuite) TestDestroyModelV3DestroyStorageNotTrue(c *gc.C) {
 	client := modelmanager.NewClient(basetesting.BestVersionCaller{})
 	for _, destroyStorage := range []*bool{nil, new(bool)} {
-		err := client.DestroyModel(coretesting.ModelTag, destroyStorage, nil, nil, time.Minute)
+		timeout := time.Minute
+		err := client.DestroyModel(coretesting.ModelTag, destroyStorage, nil, nil, &timeout)
 		c.Assert(err, gc.ErrorMatches, "this Juju controller requires destroyStorage to be true")
 	}
 }

api/controller/undertaker/undertaker.go

@@ -8,6 +8,8 @@ import (
 	"github.com/juju/names/v4"
 
 	"github.com/juju/juju/api/base"
+	"github.com/juju/juju/api/common"
+	"github.com/juju/juju/api/common/cloudspec"
 	"github.com/juju/juju/core/status"
 	"github.com/juju/juju/core/watcher"
 	"github.com/juju/juju/rpc/params"
@@ -18,6 +20,8 @@ type NewWatcherFunc func(base.APICaller, params.NotifyWatchResult) watcher.Notif
 
 // Client provides access to the undertaker API
 type Client struct {
+	*cloudspec.CloudSpecAPI
+	*common.ModelWatcher
 	modelTag   names.ModelTag
 	caller     base.FacadeCaller
 	newWatcher NewWatcherFunc
@@ -29,10 +33,13 @@ func NewClient(caller base.APICaller, newWatcher NewWatcherFunc) (*Client, error
 	if !ok {
 		return nil, errors.New("undertaker client is not appropriate for controller-only API")
 	}
+	facadeCaller := base.NewFacadeCaller(caller, "Undertaker")
 	return &Client{
-		modelTag:   modelTag,
-		caller:     base.NewFacadeCaller(caller, "Undertaker"),
-		newWatcher: newWatcher,
+		modelTag:     modelTag,
+		caller:       facadeCaller,
+		newWatcher:   newWatcher,
+		CloudSpecAPI: cloudspec.NewCloudSpecAPI(facadeCaller, modelTag),
+		ModelWatcher: common.NewModelWatcher(facadeCaller),
 	}, nil
 }
 
@@ -89,7 +96,24 @@ func (c *Client) WatchModelResources() (watcher.NotifyWatcher, error) {
 	if err != nil {
 		return nil, err
 	}
+	if len(results.Results) != 1 {
+		return nil, errors.Errorf("expected 1 result, got %d", len(results.Results))
+	}
+	result := results.Results[0]
+	if result.Error != nil {
+		return nil, result.Error
+	}
+	w := c.newWatcher(c.caller.RawAPICaller(), result)
+	return w, nil
+}
 
+// WatchModel starts a watcher for changes to the model.
+func (c *Client) WatchModel() (watcher.NotifyWatcher, error) {
+	var results params.NotifyWatchResults
+	err := c.entityFacadeCall("WatchModel", &results)
+	if err != nil {
+		return nil, err
+	}
 	if len(results.Results) != 1 {
 		return nil, errors.Errorf("expected 1 result, got %d", len(results.Results))
 	}

apiserver/facades/client/modelupgrader/shims.go

@@ -42,6 +42,7 @@ type Model interface {
 	Name() string
 	MigrationMode() state.MigrationMode
 	Type() state.ModelType
+	Life() state.Life
 }
 
 type statePoolShim struct {

apiserver/facades/client/modelupgrader/upgrader.go

@@ -23,6 +23,7 @@ import (
 	"github.com/juju/juju/environs/config"
 	"github.com/juju/juju/environs/context"
 	"github.com/juju/juju/rpc/params"
+	"github.com/juju/juju/state"
 	"github.com/juju/juju/upgrades/upgradevalidation"
 )
 
@@ -157,6 +158,11 @@ func (m *ModelUpgraderAPI) UpgradeModel(arg params.UpgradeModelParams) (result p
 		return result, errors.Trace(err)
 	}
 
+	if model.Life() != state.Alive {
+		result.Error = apiservererrors.ServerError(errors.NotValidf("model is not alive"))
+		return result, nil
+	}
+
 	agentVersion, err := model.AgentVersion()
 	if err != nil {
 		return result, errors.Trace(err)
@@ -315,12 +321,6 @@ func (m *ModelUpgraderAPI) validateModelUpgrade(
 			continue
 		}
 
-		cloudspec, err := m.environscloudspecGetter(names.NewModelTag(modelUUID))
-		if err != nil {
-			return errors.Trace(err)
-		}
-		validators := upgradevalidation.ValidatorsForControllerUpgrade(false, targetVersion, cloudspec)
-
 		st, err := m.statePool.Get(modelUUID)
 		if err != nil {
 			return errors.Trace(err)
@@ -330,6 +330,18 @@ func (m *ModelUpgraderAPI) validateModelUpgrade(
 		if err != nil {
 			return errors.Trace(err)
 		}
+
+		if model.Life() != state.Alive {
+			logger.Tracef("skipping upgrade check for dying/dead model %s", modelUUID)
+			continue
+		}
+
+		cloudspec, err := m.environscloudspecGetter(names.NewModelTag(modelUUID))
+		if err != nil {
+			return errors.Trace(err)
+		}
+		validators := upgradevalidation.ValidatorsForControllerUpgrade(false, targetVersion, cloudspec)
+
 		checker := upgradevalidation.NewModelUpgradeCheck(modelUUID, m.statePool, st, model, validators...)
 		blockersForModel, err := checker.Validate()
 		if err != nil {

apiserver/facades/client/modelupgrader/upgrader_test.go

@@ -243,6 +243,7 @@ func (s *modelUpgradeSuite) assertUpgradeModelForControllerModelJuju3(c *gc.C, d
 		// 2. Check other models.
 		s.statePool.EXPECT().Get(model1ModelUUID.String()).Return(state1, nil),
 		state1.EXPECT().Model().Return(model1, nil),
+		model1.EXPECT().Life().Return(state.Alive),
 		// - check agent version;
 		model1.EXPECT().AgentVersion().Return(version.MustParse("2.9.1"), nil),
 		//  - check if model migration is ongoing;
@@ -309,6 +310,135 @@ func (s *modelUpgradeSuite) TestUpgradeModelForControllerModelJuju3DryRun(c *gc.
 	s.assertUpgradeModelForControllerModelJuju3(c, true)
 }
 
+func (s *modelUpgradeSuite) TestUpgradeModelForControllerDyingHostedModelJuju3(c *gc.C) {
+	ctrl, api := s.getModelUpgraderAPI(c)
+	defer ctrl.Finish()
+
+	s.PatchValue(&upgradevalidation.MinMajorUpgradeVersions, map[int]version.Number{
+		3: version.MustParse("2.9.1"),
+	})
+
+	server := upgradevalidationmocks.NewMockServer(ctrl)
+	serverFactory := upgradevalidationmocks.NewMockServerFactory(ctrl)
+	s.PatchValue(&upgradevalidation.NewServerFactory,
+		func(httpClient *http.Client) lxd.ServerFactory {
+			return serverFactory
+		},
+	)
+
+	ctrlModelTag := coretesting.ModelTag
+	model1ModelUUID, err := utils.NewUUID()
+	c.Assert(err, jc.ErrorIsNil)
+	ctrlModel := mocks.NewMockModel(ctrl)
+	model1 := mocks.NewMockModel(ctrl)
+	ctrlModel.EXPECT().IsControllerModel().Return(true)
+
+	ctrlState := mocks.NewMockState(ctrl)
+	state1 := mocks.NewMockState(ctrl)
+	ctrlState.EXPECT().Release().AnyTimes()
+	ctrlState.EXPECT().Model().Return(ctrlModel, nil)
+	state1.EXPECT().Release()
+
+	s.statePool.EXPECT().Get(ctrlModelTag.Id()).Return(ctrlState, nil)
+	var agentStream string
+	assertions := []*gomock.Call{
+		s.blockChecker.EXPECT().ChangeAllowed().Return(nil),
+
+		// Decide/validate target version.
+		ctrlModel.EXPECT().AgentVersion().Return(version.MustParse("2.9.1"), nil),
+		s.toolsFinder.EXPECT().FindTools(params.FindToolsParams{MajorVersion: 3, MinorVersion: -1}).Return(
+			params.FindToolsResult{
+				List: []*coretools.Tools{
+					{Version: version.MustParseBinary("3.9.99-ubuntu-amd64")},
+				},
+			}, nil,
+		),
+		ctrlModel.EXPECT().Type().Return(state.ModelTypeIAAS),
+		ctrlModel.EXPECT().Name().Return("controller"),
+
+		// 1. Check controller model.
+		// - check agent version;
+		ctrlModel.EXPECT().AgentVersion().Return(version.MustParse("2.9.1"), nil),
+		// - check mongo status;
+		ctrlState.EXPECT().MongoCurrentStatus().Return(&replicaset.Status{
+			Members: []replicaset.MemberStatus{
+				{
+					Id:      1,
+					Address: "1.1.1.1",
+					State:   replicaset.PrimaryState,
+				},
+				{
+					Id:      2,
+					Address: "2.2.2.2",
+					State:   replicaset.SecondaryState,
+				},
+				{
+					Id:      3,
+					Address: "3.3.3.3",
+					State:   replicaset.SecondaryState,
+				},
+			},
+		}, nil),
+		// - check mongo version;
+		s.statePool.EXPECT().MongoVersion().Return("4.4", nil),
+		// - check if the model has win machines;
+		ctrlState.EXPECT().MachineCountForSeries(
+			"win10", "win2008r2", "win2012", "win2012hv", "win2012hvr2", "win2012r2",
+			"win2016", "win2016hv", "win2019", "win7", "win8", "win81",
+		).Return(nil, nil),
+		// - check if the model has deprecated ubuntu machines;
+		ctrlState.EXPECT().MachineCountForSeries(
+			"artful",
+			"bionic",
+			"cosmic",
+			"disco",
+			"eoan",
+			"groovy",
+			"hirsute",
+			"impish",
+			"kinetic",
+			"precise",
+			"quantal",
+			"raring",
+			"saucy",
+			"trusty",
+			"utopic",
+			"vivid",
+			"wily",
+			"xenial",
+			"yakkety",
+			"zesty",
+		).Return(nil, nil),
+		// - check LXD version.
+		serverFactory.EXPECT().RemoteServer(s.cloudSpec).Return(server, nil),
+		server.EXPECT().ServerVersion().Return("5.2"),
+
+		ctrlState.EXPECT().AllModelUUIDs().Return([]string{ctrlModelTag.Id(), model1ModelUUID.String()}, nil),
+
+		// 2. Check other models.
+		s.statePool.EXPECT().Get(model1ModelUUID.String()).Return(state1, nil),
+		state1.EXPECT().Model().Return(model1, nil),
+		model1.EXPECT().Life().Return(state.Dying), // Skip this dying model.
+
+		// s.statePool.EXPECT().Get(ctrlModelTag.Id()).Return(ctrlState, nil),
+		ctrlState.EXPECT().SetModelAgentVersion(version.MustParse("3.9.99"), nil, false).Return(nil),
+	}
+	gomock.InOrder(assertions...)
+
+	result, err := api.UpgradeModel(
+		params.UpgradeModelParams{
+			ModelTag:      ctrlModelTag.String(),
+			TargetVersion: version.MustParse("3.9.99"),
+			AgentStream:   agentStream,
+			DryRun:        false,
+		},
+	)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(result, gc.DeepEquals, params.UpgradeModelResult{
+		ChosenVersion: version.MustParse("3.9.99"),
+	})
+}
+
 func (s *modelUpgradeSuite) TestUpgradeModelForControllerModelJuju3Failed(c *gc.C) {
 	ctrl, api := s.getModelUpgraderAPI(c)
 	defer ctrl.Finish()
@@ -418,6 +548,7 @@ func (s *modelUpgradeSuite) TestUpgradeModelForControllerModelJuju3Failed(c *gc.
 		// 2. Check other models.
 		s.statePool.EXPECT().Get(model1ModelUUID.String()).Return(state1, nil),
 		state1.EXPECT().Model().Return(model1, nil),
+		model1.EXPECT().Life().Return(state.Alive),
 		// - check agent version;
 		model1.EXPECT().AgentVersion().Return(version.MustParse("2.9.0"), nil),
 		//  - check if model migration is ongoing;

apiserver/facades/controller/undertaker/mock_test.go

@@ -76,10 +76,6 @@ func (m *mockState) Model() (undertaker.Model, error) {
 	return m.model, nil
 }
 
-func (m *mockState) ModelConfig() (*config.Config, error) {
-	return &config.Config{}, nil
-}
-
 func (m *mockState) FindEntity(tag names.Tag) (state.Entity, error) {
 	if tag.Kind() == names.ModelTagKind && tag.Id() == m.model.UUID() {
 		return m.model, nil
@@ -98,12 +94,13 @@ func (m *mockState) ModelUUID() string {
 // mockModel implements Model interface and allows inspection of called
 // methods.
 type mockModel struct {
-	owner   names.UserTag
-	life    state.Life
-	name    string
-	uuid    string
-	forced  bool
-	timeout *time.Duration
+	owner       names.UserTag
+	life        state.Life
+	name        string
+	uuid        string
+	forced      bool
+	timeout     *time.Duration
+	modelConfig config.Config
 
 	status     status.Status
 	statusInfo string
@@ -152,6 +149,18 @@ func (m *mockModel) SetStatus(sInfo status.StatusInfo) error {
 	return nil
 }
 
+func (m *mockModel) WatchForModelConfigChanges() state.NotifyWatcher {
+	return nil
+}
+
+func (m *mockModel) Watch() state.NotifyWatcher {
+	return nil
+}
+
+func (m *mockModel) ModelConfig() (*config.Config, error) {
+	return &m.modelConfig, nil
+}
+
 type mockWatcher struct {
 	state.NotifyWatcher
 	changes chan struct{}