Merge pull request juju#16995 from SimonRichardson/remove-kvm-broker

juju#16995

This removes the KVM broker in favour of the LXD virtual-machine support. As LXD VMs are based on QEMU, which you get via libvirt, they're analogous to each other. Although, fundamentally, the LXD VMs have a more opinionated setup; based on a more modern layout (Q35) with UEFI and SecureBoot by default and driven by a simpler experience. All the LXD VMs are virtio based, the same as KVM.

It's expected that the LXD broker will take over from the KVM broker for VM support. This will be a breaking change for 4.0, so it is expected that models with KVM VMs will be redeployed to LXD VMs before finally migrating to 4.0. Additional work could be done to make this more painless, for now though, an extra hop will be required.

## 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

## QA steps

This is removal of code, I'd just expect that bootstrapping works.

## Documentation changes

Dropping support for KVM broker.

## Links

**Jira card:** JUJU-

acceptancetests/assess

@@ -199,7 +199,7 @@ def parse_args():
                               default="jammy")
     pass_through.add_argument("--machine-type", "--virt", dest="machine_type",
                               help="Virtualisation technology for machine (only relevant for container_networking, constraints tests) [default: lxd]",
-                              choices=["lxc", "lxd", "kvm"], default="lxd")
+                              choices=["lxc", "lxd"], default="lxd")
     pass_through.add_argument("--space-constraint", metavar="C",
                               help="Adds --constraints spaces=C to the underlying hypervisor (only relevant for container_networking test). ")
     pass_through.add_argument("--secondary-env", metavar="ENV",
@@ -454,9 +454,6 @@ def main():
         testrun_argv.extend(["--machine-type", args.machine_type])
         if args.space_constraint:
             testrun_argv.extend(["--space-constraint", args.space_constraint])
-    elif assess == "constraints":
-        if args.machine_type == "kvm":
-            testrun_argv.append("--with-virttype-kvm")
     elif assess == "cross_model_relations":
         if args.secondary_env:
             testrun_argv.extend(["--secondary-env", args.secondary_env])

agent/agentbootstrap/bootstrap_test.go

@@ -72,9 +72,6 @@ func (s *bootstrapSuite) TestInitializeState(c *gc.C) {
 				"10.0.4.1",
 				"10.0.4.4",
 			}, nil
-		} else if name == network.DefaultKVMBridge {
-			// claim we don't have a virbr0 bridge
-			return nil, nil
 		}
 		c.Fatalf("unknown bridge in testing: %v", name)
 		return nil, nil

api/agent/provisioner/provisioner_test.go

@@ -665,15 +665,15 @@ func (s *provisionerSuite) TestSetSupportedContainers(c *gc.C) {
 	args := params.MachineContainersParams{
 		Params: []params.MachineContainers{{
 			MachineTag:     "machine-666",
-			ContainerTypes: []instance.ContainerType{"lxd", "kvm"},
+			ContainerTypes: []instance.ContainerType{"lxd"},
 		}},
 	}
 	results := params.ErrorResults{
 		Results: []params.ErrorResult{{}},
 	}
 	s.expectCall(caller, "SetSupportedContainers", args, results)
 
-	err := machine.SetSupportedContainers([]instance.ContainerType{"lxd", "kvm"}...)
+	err := machine.SetSupportedContainers([]instance.ContainerType{"lxd"}...)
 	c.Assert(err, jc.ErrorIsNil)
 
 }
@@ -688,14 +688,14 @@ func (s *provisionerSuite) TestSupportedContainers(c *gc.C) {
 		Entities: []params.Entity{{Tag: "machine-666"}},
 	}
 	results := params.MachineContainerResults{
-		Results: []params.MachineContainerResult{{ContainerTypes: []instance.ContainerType{"lxd", "kvm"}, Determined: true}},
+		Results: []params.MachineContainerResult{{ContainerTypes: []instance.ContainerType{"lxd"}, Determined: true}},
 	}
 
 	s.expectCall(caller, "SupportedContainers", args, results)
 
 	result, determined, err := machine.SupportedContainers()
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(result, jc.SameContents, []instance.ContainerType{"lxd", "kvm"})
+	c.Assert(result, jc.SameContents, []instance.ContainerType{"lxd"})
 	c.Assert(determined, jc.IsTrue)
 }
 

api/agent/upgradesteps/upgradesteps_test.go

@@ -27,30 +27,6 @@ func (s *upgradeStepsSuite) SetUpTest(c *gc.C) {
 	s.BaseSuite.SetUpTest(c)
 }
 
-func (s *upgradeStepsSuite) TestResetKVMMachineModificationStatusIdle(c *gc.C) {
-	defer s.setupMocks(c).Finish()
-	mTag := names.NewMachineTag("0/kvm/0")
-	resetArg := params.Entity{Tag: mTag.String()}
-
-	s.expectResetKVMMachineModificationStatusIdleSuccess(resetArg)
-
-	client := upgradesteps.NewClientFromFacade(s.fCaller)
-	err := client.ResetKVMMachineModificationStatusIdle(mTag)
-	c.Assert(err, jc.ErrorIsNil)
-}
-
-func (s *upgradeStepsSuite) TestResetKVMMachineModificationStatusIdleError(c *gc.C) {
-	defer s.setupMocks(c).Finish()
-	mTag := names.NewMachineTag("0/kvm/0")
-	resetArg := params.Entity{Tag: mTag.String()}
-
-	s.expectResetKVMMachineModificationStatusIdleError(resetArg)
-
-	client := upgradesteps.NewClientFromFacade(s.fCaller)
-	err := client.ResetKVMMachineModificationStatusIdle(mTag)
-	c.Assert(err, gc.ErrorMatches, "did not find")
-}
-
 func (s *upgradeStepsSuite) TestWriteAgentState(c *gc.C) {
 	defer s.setupMocks(c).Finish()
 
@@ -95,23 +71,6 @@ func (s *upgradeStepsSuite) setupMocks(c *gc.C) *gomock.Controller {
 	return ctrl
 }
 
-func (s *upgradeStepsSuite) expectResetKVMMachineModificationStatusIdleSuccess(resetArg params.Entity) {
-	fExp := s.fCaller.EXPECT()
-	resultSource := params.ErrorResult{}
-	fExp.FacadeCall(gomock.Any(), "ResetKVMMachineModificationStatusIdle", resetArg, gomock.Any()).SetArg(3, resultSource)
-}
-
-func (s *upgradeStepsSuite) expectResetKVMMachineModificationStatusIdleError(resetArg params.Entity) {
-	fExp := s.fCaller.EXPECT()
-	resultSource := params.ErrorResult{
-		Error: &params.Error{
-			Code:    params.CodeNotFound,
-			Message: "did not find",
-		},
-	}
-	fExp.FacadeCall(gomock.Any(), "ResetKVMMachineModificationStatusIdle", resetArg, gomock.Any()).SetArg(3, resultSource)
-}
-
 func (s *upgradeStepsSuite) expectWriteAgentStateSuccess(c *gc.C, args params.SetUnitStateArgs) {
 	fExp := s.fCaller.EXPECT()
 	resultSource := params.ErrorResults{}

api/facadeversions.go

@@ -127,7 +127,7 @@ var facadeVersions = facades.FacadeVersions{
 	"Uniter":                       {19},
 	"Upgrader":                     {1},
 	"UpgradeSeries":                {3},
-	"UpgradeSteps":                 {2},
+	"UpgradeSteps":                 {3},
 	"UserManager":                  {3},
 	"VolumeAttachmentsWatcher":     {2},
 	"VolumeAttachmentPlansWatcher": {1},

apiserver/facades/agent/provisioner/provisioner_test.go

@@ -708,7 +708,7 @@ func (s *withoutControllerSuite) TestWatchContainers(c *gc.C) {
 
 	args := params.WatchContainers{Params: []params.WatchContainer{
 		{MachineTag: s.machines[0].Tag().String(), ContainerType: string(instance.LXD)},
-		{MachineTag: s.machines[1].Tag().String(), ContainerType: string(instance.KVM)},
+		{MachineTag: s.machines[1].Tag().String(), ContainerType: string(instance.LXD)},
 		{MachineTag: "machine-42", ContainerType: ""},
 		{MachineTag: "unit-foo-0", ContainerType: ""},
 		{MachineTag: "application-bar", ContainerType: ""},
@@ -1475,11 +1475,12 @@ func (s *provisionerSuite) getManagerConfig(c *gc.C, typ instance.ContainerType)
 }
 
 func (s *withoutControllerSuite) TestContainerManagerConfigDefaults(c *gc.C) {
-	cfg := s.getManagerConfig(c, instance.KVM)
+	cfg := s.getManagerConfig(c, instance.LXD)
 	c.Assert(cfg, jc.DeepEquals, map[string]string{
 		container.ConfigModelUUID:        coretesting.ModelTag.Id(),
 		config.ContainerImageStreamKey:   "released",
 		config.ContainerNetworkingMethod: config.ConfigDefaults()[config.ContainerNetworkingMethod].(string),
+		config.LXDSnapChannel:            "5.0/stable",
 	})
 }
 
@@ -1664,7 +1665,7 @@ func (s *withoutControllerSuite) TestSetSupportedContainers(c *gc.C) {
 		ContainerTypes: []instance.ContainerType{instance.LXD},
 	}, {
 		MachineTag:     "machine-1",
-		ContainerTypes: []instance.ContainerType{instance.LXD, instance.KVM},
+		ContainerTypes: []instance.ContainerType{instance.LXD},
 	}}}
 	results, err := s.provisioner.SetSupportedContainers(context.Background(), args)
 	c.Assert(err, jc.ErrorIsNil)
@@ -1682,7 +1683,7 @@ func (s *withoutControllerSuite) TestSetSupportedContainers(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 	containers, ok = m1.SupportedContainers()
 	c.Assert(ok, jc.IsTrue)
-	c.Assert(containers, gc.DeepEquals, []instance.ContainerType{instance.LXD, instance.KVM})
+	c.Assert(containers, gc.DeepEquals, []instance.ContainerType{instance.LXD})
 }
 
 func (s *withoutControllerSuite) TestSetSupportedContainersPermissions(c *gc.C) {
@@ -1731,7 +1732,7 @@ func (s *withoutControllerSuite) TestSupportedContainers(c *gc.C) {
 		ContainerTypes: []instance.ContainerType{instance.LXD},
 	}, {
 		MachineTag:     "machine-1",
-		ContainerTypes: []instance.ContainerType{instance.LXD, instance.KVM},
+		ContainerTypes: []instance.ContainerType{instance.LXD},
 	}}}
 	_, err := s.provisioner.SetSupportedContainers(context.Background(), setArgs)
 	c.Assert(err, jc.ErrorIsNil)

apiserver/facades/agent/reboot/reboot_test.go

@@ -102,7 +102,7 @@ func (s *rebootSuite) SetUpTest(c *gc.C) {
 	container, err := st.AddMachineInsideMachine(template, machine.Id(), instance.LXD)
 	c.Assert(err, jc.ErrorIsNil)
 
-	nestedContainer, err := st.AddMachineInsideMachine(template, container.Id(), instance.KVM)
+	nestedContainer, err := st.AddMachineInsideMachine(template, container.Id(), instance.LXD)
 	c.Assert(err, jc.ErrorIsNil)
 
 	s.machine = s.setUpMachine(c, machine)

apiserver/facades/agent/uniter/newlxdprofile.go

@@ -301,9 +301,6 @@ func (u *LXDProfileAPIv2) getOneCanApplyLXDProfile(machine LXDProfileMachineV2,
 		return true, nil
 	}
 	switch machine.ContainerType() {
-	case instance.KVM:
-		// charm profiles cannot be applied to KVM containers.
-		return false, nil
 	case instance.LXD:
 		return true, nil
 	}

apiserver/facades/agent/uniter/newlxdprofile_test.go

@@ -179,22 +179,6 @@ func (s *newLxdProfileSuite) TestCanApplyLXDProfileCAAS(c *gc.C) {
 	s.testCanApplyLXDProfile(c, false)
 }
 
-func (s *newLxdProfileSuite) TestCanApplyLXDProfileIAASMAASNotManualKVM(c *gc.C) {
-	// model type: IAAS
-	// provider type: maas
-	// manual: false
-	// container: KVM
-	defer s.setupMocks(c).Finish()
-	s.expectUnitAndMachine()
-	s.expectModel()
-	s.expectModelTypeIAAS()
-	s.expectProviderType(c, "maas")
-	s.expectManual(false)
-	s.expectContainerType(instance.KVM)
-
-	s.testCanApplyLXDProfile(c, false)
-}
-
 func (s *newLxdProfileSuite) TestCanApplyLXDProfileIAASMAASNotManualLXD(c *gc.C) {
 	// model type: IAAS
 	// provider type: maas

apiserver/facades/agent/upgradesteps/register.go

@@ -12,13 +12,13 @@ import (
 
 // Register is called to expose a package of facades onto a given registry.
 func Register(registry facade.FacadeRegistry) {
-	registry.MustRegister("UpgradeSteps", 2, func(stdCtx context.Context, ctx facade.ModelContext) (facade.Facade, error) {
-		return newFacadeV2(ctx)
+	registry.MustRegister("UpgradeSteps", 3, func(stdCtx context.Context, ctx facade.ModelContext) (facade.Facade, error) {
+		return newFacadeV3(ctx)
 	}, reflect.TypeOf((*UpgradeStepsAPI)(nil)))
 }
 
-// newFacadeV2 is used for API registration.
-func newFacadeV2(ctx facade.ModelContext) (*UpgradeStepsAPI, error) {
+// newFacadeV3 is used for API registration.
+func newFacadeV3(ctx facade.ModelContext) (*UpgradeStepsAPI, error) {
 	return NewUpgradeStepsAPI(
 		ctx.State(),
 		ctx.ServiceFactory().ControllerConfig(),

apiserver/facades/agent/upgradesteps/upgradesteps.go

@@ -12,25 +12,16 @@ import (
 	"github.com/juju/juju/apiserver/common"
 	apiservererrors "github.com/juju/juju/apiserver/errors"
 	"github.com/juju/juju/apiserver/facade"
-	"github.com/juju/juju/core/instance"
-	"github.com/juju/juju/core/status"
 	"github.com/juju/juju/rpc/params"
 	"github.com/juju/juju/state"
 )
 
-// UpgradeStepsV2 defines the methods on the version 2 facade for the
+// UpgradeStepsV3 defines the methods on the version 2 facade for the
 // upgrade steps API endpoint.
-type UpgradeStepsV2 interface {
-	UpgradeStepsV1
+type UpgradeSteps interface {
 	WriteAgentState(context.Context, params.SetUnitStateArgs) (params.ErrorResults, error)
 }
 
-// UpgradeStepsV1 defines the methods on the version 2 facade for the
-// upgrade steps API endpoint.
-type UpgradeStepsV1 interface {
-	ResetKVMMachineModificationStatusIdle(context.Context, params.Entity) (params.ErrorResult, error)
-}
-
 // Logger represents the logging methods used by the upgrade steps.
 type Logger interface {
 	Criticalf(string, ...any)
@@ -54,8 +45,6 @@ type UpgradeStepsAPIV1 struct {
 	*UpgradeStepsAPI
 }
 
-var _ UpgradeStepsV2 = (*UpgradeStepsAPI)(nil)
-
 func NewUpgradeStepsAPI(
 	st UpgradeStepsState,
 	ctrlConfigGetter ControllerConfigGetter,
@@ -80,44 +69,6 @@ func NewUpgradeStepsAPI(
 	}, nil
 }
 
-// ResetKVMMachineModificationStatusIdle sets the modification status
-// of a kvm machine to idle if it is in an error state before upgrade.
-// Related to lp:1829393.
-func (api *UpgradeStepsAPI) ResetKVMMachineModificationStatusIdle(ctx context.Context, arg params.Entity) (params.ErrorResult, error) {
-	var result params.ErrorResult
-	canAccess, err := api.getMachineAuthFunc()
-	if err != nil {
-		return result, errors.Trace(err)
-	}
-
-	mTag, err := names.ParseMachineTag(arg.Tag)
-	if err != nil {
-		return result, errors.Trace(err)
-	}
-	m, err := api.getMachine(canAccess, mTag)
-	if err != nil {
-		return result, errors.Trace(err)
-	}
-
-	if m.ContainerType() != instance.KVM {
-		// noop
-		return result, nil
-	}
-
-	modStatus, err := m.ModificationStatus()
-	if err != nil {
-		result.Error = apiservererrors.ServerError(err)
-		return result, nil
-	}
-
-	if modStatus.Status == status.Error {
-		err = m.SetModificationStatus(status.StatusInfo{Status: status.Idle})
-		result.Error = apiservererrors.ServerError(err)
-	}
-
-	return result, nil
-}
-
 // 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) {
@@ -170,24 +121,6 @@ func (api *UpgradeStepsAPI) WriteAgentState(ctx context.Context, args params.Set
 	return results, nil
 }
 
-func (api *UpgradeStepsAPI) getMachine(canAccess common.AuthFunc, tag names.MachineTag) (Machine, error) {
-	if !canAccess(tag) {
-		return nil, apiservererrors.ErrPerm
-	}
-	entity, err := api.st.FindEntity(tag)
-	if err != nil {
-		return nil, err
-	}
-	// The authorization function guarantees that the tag represents a
-	// machine.
-	var machine Machine
-	var ok bool
-	if machine, ok = entity.(Machine); !ok {
-		return nil, errors.NotValidf("machine entity")
-	}
-	return machine, nil
-}
-
 func (api *UpgradeStepsAPI) getUnit(canAccess common.AuthFunc, tag names.UnitTag) (Unit, error) {
 	if !canAccess(tag) {
 		api.logger.Criticalf("getUnit kind=%q, name=%q", tag.Kind(), tag.Id())