Merge pull request juju#17135 from jack-w-shaw/fix_model_migration

juju#17135 Restore upgrade steps v2 facade Model migration from 3.5 to 4.0 is broken because in a previous PR we bumped the version of the UpgradeSteps facade The only change between v2 -> v3 was a KVM method was dropped because kvm container type is not supported in 4.0 Restore this method, to a no-op for non-KVM containers, and return an error for kvm container types ## Checklist - [x] Code style: imports ordered, good names, simple structure, etc - [x] Comments saying why design decisions were made - [x] Go unit tests, with comments saying what you're testing - [ ] [Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing - [ ] [doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages ## QA steps Download a local charm with ``` $ juju download ubuntu Fetching charm "ubuntu" revision 24 using "stable" channel and base "amd64/ubuntu/22.04" Install the "ubuntu" charm with: juju deploy ./ubuntu_r24.charm ``` ### Model migrations Repeat the following steps bootstrapping controllers: 1) `lxd-src` from this PR; `lxd-dst` from this PR 2) `lxd-src` from 3.5; `lxd-dst` from this PR ``` $ juju add-model m $ juju deploy ./ubuntu_r24.charm ubu-local $ juju deploy ubuntu ubu-ch $ juju status Model Controller Cloud/Region Version Timestamp m lxd-src localhost/localhost 4.0-beta3.1 15:08:57+01:00 App Version Status Scale Charm Channel Rev Exposed Message ubu-ch 22.04 active 1 ubuntu stable 24 no ubu-local 22.04 active 1 ubuntu 0 no Unit Workload Agent Machine Public address Ports Message ubu-ch/0* active idle 0 10.219.211.127 ubu-local/0* active idle 1 10.219.211.111 Machine State Address Inst id Base AZ Message 0 started 10.219.211.127 juju-975167-0 [email protected] Running 1 started 10.219.211.111 juju-975167-1 [email protected] Running $ juju migrate m lxd-dst Migration started with ID "355633ec-fc28-47a0-8a86-ae3dcd975167:0" $ juju status ERROR Model "admin/m" has been migrated to controller "lxd-dst". To access it run 'juju switch lxd-dst:admin/m'. $ juju switch lxd-dst:m $ juju status Model Controller Cloud/Region Version Timestamp m lxd-dst localhost/localhost 4.0-beta3.1 15:10:10+01:00 App Version Status Scale Charm Channel Rev Exposed Message ubu-ch 22.04 active 1 ubuntu latest/stable 24 no ubu-local 22.04 active 1 ubuntu 0 no Unit Workload Agent Machine Public address Ports Message ubu-ch/0* active idle 0 10.219.211.127 ubu-local/0* active idle 1 10.219.211.111 Machine State Address Inst id Base AZ Message 0 started 10.219.211.127 juju-975167-0 [email protected] Running 1 started 10.219.211.111 juju-975167-1 [email protected] Running $ juju add-unit ubu-local $ juju add-unit ubu-ch $ juju status Model Controller Cloud/Region Version Timestamp m lxd-dst localhost/localhost 4.0-beta3.1 15:34:30+01:00 App Version Status Scale Charm Channel Rev Exposed Message ubu-ch active 2 ubuntu latest/stable 24 no ubu-local active 2 ubuntu 0 no Unit Workload Agent Machine Public address Ports Message ubu-ch/0* active idle 0 10.219.211.127 ubu-ch/1 active idle 2 10.219.211.3 ubu-local/0* active idle 1 10.219.211.111 ubu-local/1 active idle 3 10.219.211.40 Machine State Address Inst id Base AZ Message 0 started 10.219.211.127 juju-975167-0 [email protected] Running 1 started 10.219.211.111 juju-975167-1 [email protected] Running 2 started 10.219.211.3 juju-975167-2 [email protected] Running 3 started 10.219.211.40 juju-975167-3 [email protected] Running ```
manadart · Apr 5, 2024 · 78b14c2 · 78b14c2
2 parents e86a0e8 + e0bcf75
commit 78b14c2
Show file tree

Hide file tree

Showing 4 changed files with 114 additions and 10 deletions.
diff --git a/apiserver/facades/agent/upgradesteps/register.go b/apiserver/facades/agent/upgradesteps/register.go
@@ -7,6 +7,8 @@ import (
 	"context"
 	"reflect"
 
+	"github.com/juju/errors"
+
 	"github.com/juju/juju/apiserver/facade"
 )
 
@@ -15,6 +17,18 @@ func Register(registry facade.FacadeRegistry) {
 	registry.MustRegister("UpgradeSteps", 3, func(stdCtx context.Context, ctx facade.ModelContext) (facade.Facade, error) {
 		return newFacadeV3(ctx)
 	}, reflect.TypeOf((*UpgradeStepsAPI)(nil)))
+	registry.MustRegister("UpgradeSteps", 2, func(stdCtx context.Context, ctx facade.ModelContext) (facade.Facade, error) {
+		return newFacadeV2(ctx)
+	}, reflect.TypeOf((*UpgradeStepsAPIV2)(nil)))
+}
+
+// newFacadeV2 is used for API registration.
+func newFacadeV2(ctx facade.ModelContext) (*UpgradeStepsAPIV2, error) {
+	api, err := newFacadeV3(ctx)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+	return &UpgradeStepsAPIV2{UpgradeStepsAPI: api}, nil
 }
 
 // newFacadeV3 is used for API registration.

diff --git a/apiserver/facades/agent/upgradesteps/upgradesteps.go b/apiserver/facades/agent/upgradesteps/upgradesteps.go
@@ -16,19 +16,13 @@ import (
 	"github.com/juju/juju/state"
 )
 
-// UpgradeStepsV3 defines the methods on the version 2 facade for the
-// upgrade steps API endpoint.
-type UpgradeSteps interface {
-	WriteAgentState(context.Context, params.SetUnitStateArgs) (params.ErrorResults, error)
-}
-
 // Logger represents the logging methods used by the upgrade steps.
 type Logger interface {
 	Criticalf(string, ...any)
 	Warningf(string, ...any)
 }
 
-// UpgradeStepsAPI implements version 2 of the Upgrade Steps API,
+// UpgradeStepsAPI implements version 3 of the Upgrade Steps API,
 // which adds WriteUniterState.
 type UpgradeStepsAPI struct {
 	st                 UpgradeStepsState
@@ -40,8 +34,15 @@ type UpgradeStepsAPI struct {
 	logger             Logger
 }
 
-// UpgradeStepsAPIV1 implements version 1 of the Upgrade Steps API.
-type UpgradeStepsAPIV1 struct {
+// UpgradeStepsAPIV2 implements version 2 of the Uppgrade Steps API.
+// We need this gavade for migrations from 3.x. This api includes the method
+// ResetKVMMachineModificationStatusIdle dropped in v3. KVM is not supported
+// in Juju 4.x, so this method is a simple no-op for non-KVM, and errors out
+// for KVM
+//
+// TODO(jack-w-shaw): As soon as we no longer need to support migrations
+// from 3.x, drop this facade
+type UpgradeStepsAPIV2 struct {
 	*UpgradeStepsAPI
 }
 
@@ -69,6 +70,30 @@ func NewUpgradeStepsAPI(
 	}, nil
 }
 
+// ResetKVMMachineModificationStatusIdle is a legacy method required to support
+// UpgradeSteps facade v2. Either no-op for non-KVM machines, or error out
+func (api *UpgradeStepsAPIV2) ResetKVMMachineModificationStatusIdle(ctx context.Context, arg params.Entity) (params.ErrorResult, error) {
+	var result params.ErrorResult
+
+	mTag, err := names.ParseMachineTag(arg.Tag)
+	if err != nil {
+		return result, errors.Trace(err)
+	}
+	entity, err := api.st.FindEntity(mTag)
+	if err != nil {
+		return result, errors.Trace(err)
+	}
+	machine, ok := entity.(Machine)
+	if !ok {
+		return result, errors.NotValidf("machine entity")
+	}
+	if machine.ContainerType() != "kvm" {
+		// no-op
+		return result, nil
+	}
+	return result, errors.NotSupportedf("kvm container type")
+}
+
 // WriteAgentState writes the agent state for the set of units provided. This
 // call presently deals with the state for the unit agent.
 func (api *UpgradeStepsAPI) WriteAgentState(ctx context.Context, args params.SetUnitStateArgs) (params.ErrorResults, error) {

diff --git a/apiserver/facades/agent/upgradesteps/upgradesteps_test.go b/apiserver/facades/agent/upgradesteps/upgradesteps_test.go
@@ -15,6 +15,7 @@ import (
 
 	"github.com/juju/juju/apiserver/facades/agent/upgradesteps"
 	"github.com/juju/juju/controller"
+	"github.com/juju/juju/core/instance"
 	"github.com/juju/juju/rpc/params"
 	"github.com/juju/juju/state"
 	jujutesting "github.com/juju/juju/testing"
@@ -194,3 +195,67 @@ type dummyOp struct {
 
 func (d dummyOp) Build(attempt int) ([]txn.Op, error) { return nil, nil }
 func (d dummyOp) Done(_ error) error                  { return nil }
+
+type kvmMachineUpgradeStepsSuite struct {
+	upgradeStepsSuite
+
+	tag1    names.Tag
+	machine *MockMachine
+}
+
+var _ = gc.Suite(&kvmMachineUpgradeStepsSuite{})
+
+func (s *kvmMachineUpgradeStepsSuite) SetUpTest(c *gc.C) {
+	s.upgradeStepsSuite.SetUpTest(c)
+	s.tag1 = names.NewMachineTag("0")
+}
+
+func (s *kvmMachineUpgradeStepsSuite) setup(c *gc.C) *gomock.Controller {
+	ctlr := s.upgradeStepsSuite.setup(c)
+
+	s.expectAuthCalls()
+	s.machine = NewMockMachine(ctlr)
+	return ctlr
+}
+
+func (s *kvmMachineUpgradeStepsSuite) TestResetKVMMachineModificationStatusIdleNoop(c *gc.C) {
+	defer s.setup(c).Finish()
+
+	s.expectAuthCalls()
+	s.setupFacadeAPI(c)
+	api := &upgradesteps.UpgradeStepsAPIV2{s.api}
+
+	s.expectFindEntityMachine(instance.LXD)
+
+	result, err := api.ResetKVMMachineModificationStatusIdle(context.Background(), params.Entity{Tag: s.tag1.String()})
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(result.Error, gc.IsNil)
+}
+
+func (s *kvmMachineUpgradeStepsSuite) TestResetKVMMachineModificationStatusIdleKVMUnsupported(c *gc.C) {
+	defer s.setup(c).Finish()
+
+	s.expectAuthCalls()
+	s.setupFacadeAPI(c)
+	api := &upgradesteps.UpgradeStepsAPIV2{s.api}
+
+	s.expectFindEntityMachine("kvm")
+
+	_, err := api.ResetKVMMachineModificationStatusIdle(context.Background(), params.Entity{Tag: s.tag1.String()})
+	c.Assert(err, jc.ErrorIs, errors.NotSupported)
+}
+
+func (s *kvmMachineUpgradeStepsSuite) expectFindEntityMachine(t instance.ContainerType) {
+	s.machine.EXPECT().ContainerType().Return(t)
+	m := machineEntityShim{
+		Machine: s.machine,
+		Entity:  s.entity,
+	}
+
+	s.state.EXPECT().FindEntity(s.tag1.(names.MachineTag)).Return(m, nil)
+}
+
+type machineEntityShim struct {
+	upgradesteps.Machine
+	state.Entity
+}
diff --git a/apiserver/facades/schema.json b/apiserver/facades/schema.json
@@ -45553,7 +45553,7 @@
     },
     {
         "Name": "UpgradeSteps",
-        "Description": "UpgradeStepsAPI implements version 2 of the Upgrade Steps API,\nwhich adds WriteUniterState.",
+        "Description": "UpgradeStepsAPI implements version 3 of the Upgrade Steps API,\nwhich adds WriteUniterState.",
         "Version": 3,
         "AvailableTo": [
             "controller-machine-agent",