Merge pull request juju#17030 from jack-w-shaw/3.3-into-3.4

juju#17030

Forward merge:
- juju#16996
- juju#17024
- juju#16984

No conflicts

apiserver/common/networkingcommon/networkconfigapi.go

@@ -149,7 +149,7 @@ func (api *NetworkConfigAPI) getMachineForSettingNetworkConfig(machineTag string
 	}
 
 	m, err := api.getMachine(tag)
-	if errors.IsNotFound(err) {
+	if errors.Is(err, errors.NotFound) {
 		return nil, errors.Trace(apiservererrors.ErrPerm)
 	} else if err != nil {
 		return nil, errors.Trace(err)
@@ -337,6 +337,8 @@ func (o *updateMachineLinkLayerOp) processExistingDeviceAddress(
 
 // processExistingDeviceNewAddresses interrogates the list of incoming
 // addresses and adds any that were not processed as already existing.
+// If there are new address to add then also the subnets are processed
+// to make sure they are updated on the state as well.
 func (o *updateMachineLinkLayerOp) processExistingDeviceNewAddresses(
 	dev LinkLayerDevice, incomingAddrs []state.LinkLayerDeviceAddress,
 ) ([]txn.Op, error) {
@@ -351,6 +353,16 @@ func (o *updateMachineLinkLayerOp) processExistingDeviceNewAddresses(
 			}
 			ops = append(ops, addOps...)
 
+			// Since this is a new address, ensure that we have
+			// discovered all the subnets for the device.
+			if o.discoverSubnets {
+				subNetOps, err := o.processSubnets(dev.Name())
+				if err != nil {
+					return nil, errors.Trace(err)
+				}
+				ops = append(ops, subNetOps...)
+			}
+
 			o.MarkAddrProcessed(dev.Name(), addr.CIDRAddress)
 		}
 	}
@@ -425,7 +437,7 @@ func (o *updateMachineLinkLayerOp) processSubnets(name string) ([]txn.Op, error)
 	for _, cidr := range cidrs {
 		addOps, err := o.st.AddSubnetOps(network.SubnetInfo{CIDR: cidr})
 		if err != nil {
-			if errors.IsAlreadyExists(err) {
+			if errors.Is(err, errors.AlreadyExists) {
 				continue
 			}
 			return nil, errors.Trace(err)

apiserver/common/networkingcommon/networkconfigapi_test.go

@@ -436,6 +436,63 @@ func (s *networkConfigSuite) TestUpdateMachineLinkLayerOpNewSubnetsAdded(c *gc.C
 	c.Check(ops, gc.HasLen, 8)
 }
 
+func (s *networkConfigSuite) TestUpdateMachineLinkLayerAddressOpNewSubnetsAdded(c *gc.C) {
+	ctrl := s.setupMocks(c)
+	defer ctrl.Finish()
+
+	// A machine with one link-layer dev.
+	s.expectMachine()
+	mExp := s.machine.EXPECT()
+
+	// Device eth0 exists with no addresses and will have one added to it.
+	ethMAC := "aa:bb:cc:dd:ee:ff"
+	ethDev := mocks.NewMockLinkLayerDevice(ctrl)
+	ethExp := ethDev.EXPECT()
+	ethExp.Name().Return("eth0").MinTimes(1)
+	ethExp.UpdateOps(state.LinkLayerDeviceArgs{
+		Name:        "eth0",
+		Type:        "ethernet",
+		MACAddress:  ethMAC,
+		IsAutoStart: true,
+		IsUp:        true,
+	}).Return(nil)
+	ethExp.AddAddressOps(state.LinkLayerDeviceAddress{
+		DeviceName:       "eth0",
+		ConfigMethod:     "static",
+		CIDRAddress:      "0.10.0.2/24",
+		GatewayAddress:   "0.10.0.1",
+		IsDefaultGateway: true,
+	}).Return([]txn.Op{{}}, nil)
+
+	mExp.AllLinkLayerDevices().Return([]networkingcommon.LinkLayerDevice{ethDev}, nil)
+	mExp.AllDeviceAddresses().Return(nil, nil)
+
+	// Since there is a new address, then we process (and therefore add)
+	// subnets.
+	s.state.EXPECT().AddSubnetOps(network.SubnetInfo{CIDR: "0.10.0.0/24"}).Return([]txn.Op{{}}, nil)
+
+	op := s.NewUpdateMachineLinkLayerOp(s.machine, network.InterfaceInfos{
+		{
+			InterfaceName: "eth0",
+			InterfaceType: "ethernet",
+			MACAddress:    ethMAC,
+			Addresses: network.ProviderAddresses{
+				network.NewMachineAddress("0.10.0.2", network.WithCIDR("0.10.0.0/24")).AsProviderAddress(),
+			},
+			GatewayAddress:   network.NewMachineAddress("0.10.0.1").AsProviderAddress(),
+			IsDefaultGateway: true,
+		},
+	}, true, s.state)
+
+	ops, err := op.Build(0)
+	c.Assert(err, jc.ErrorIsNil)
+
+	// Expected ops are:
+	// - One for the new device address.
+	// - One for the new subnet.
+	c.Check(ops, gc.HasLen, 2)
+}
+
 func (s *networkConfigSuite) TestUpdateMachineLinkLayerOpBridgedDeviceMovesAddress(c *gc.C) {
 	ctrl := s.setupMocks(c)
 	defer ctrl.Finish()

apiserver/facades/client/application/application.go

@@ -532,7 +532,7 @@ func deployApplication(
 
 	// This check is done early so that errors deeper in the call-stack do not
 	// leave an application deployment in an unrecoverable error state.
-	if err := checkMachinePlacement(backend, args.ApplicationName, args.Placement); err != nil {
+	if err := checkMachinePlacement(backend, model.UUID(), args.ApplicationName, args.Placement); err != nil {
 		return errors.Trace(err)
 	}
 
@@ -783,13 +783,20 @@ type MachinePlacementBackend interface {
 // If the placement scope is for a machine, ensure that the machine exists.
 // If the placement is for a machine or a container on an existing machine,
 // check that the machine is not locked for series upgrade.
-func checkMachinePlacement(backend MachinePlacementBackend, app string, placement []*instance.Placement) error {
+// If the placement scope is model-uuid, replace it with the actual model uuid.
+func checkMachinePlacement(backend MachinePlacementBackend, modelUUID string, app string, placement []*instance.Placement) error {
 	errTemplate := "cannot deploy %q to machine %s"
 
 	for _, p := range placement {
 		if p == nil {
 			continue
 		}
+		// Substitute the placeholder with the actual model uuid.
+		if p.Scope == "model-uuid" {
+			p.Scope = modelUUID
+			continue
+		}
+
 		dir := p.Directive
 
 		toProvisionedMachine := p.Scope == instance.MachineScope

apiserver/facades/client/application/application_unit_test.go

@@ -1365,6 +1365,7 @@ func (s *ApplicationSuite) TestDeployAttachStorage(c *gc.C) {
 
 	ch := s.expectDefaultCharm(ctrl)
 	s.backend.EXPECT().Charm(gomock.Any()).Return(ch, nil).Times(3)
+	s.model.EXPECT().UUID().Return("").AnyTimes()
 
 	args := params.ApplicationsDeploy{
 		Applications: []params.ApplicationDeploy{{
@@ -1401,6 +1402,7 @@ func (s *ApplicationSuite) TestDeployCharmOrigin(c *gc.C) {
 
 	ch := s.expectDefaultCharm(ctrl)
 	s.backend.EXPECT().Charm(gomock.Any()).Return(ch, nil).Times(2)
+	s.model.EXPECT().UUID().Return("").AnyTimes()
 
 	track := "latest"
 	args := params.ApplicationsDeploy{
@@ -1475,6 +1477,7 @@ func (s *ApplicationSuite) TestApplicationDeployWithStorage(c *gc.C) {
 	}}, nil, &charm.Config{})
 	curl := "ch:utopic/storage-block-10"
 	s.backend.EXPECT().Charm(curl).Return(ch, nil)
+	s.model.EXPECT().UUID().Return("")
 
 	storageConstraints := map[string]storage.Constraints{
 		"data": {
@@ -1510,6 +1513,7 @@ func (s *ApplicationSuite) TestApplicationDeployDefaultFilesystemStorage(c *gc.C
 	}}, nil, &charm.Config{})
 	curl := "ch:utopic/storage-filesystem-1"
 	s.backend.EXPECT().Charm(curl).Return(ch, nil)
+	s.model.EXPECT().UUID().Return("")
 
 	args := params.ApplicationDeploy{
 		ApplicationName: "my-app",
@@ -1538,6 +1542,7 @@ func (s *ApplicationSuite) TestApplicationDeployPlacement(c *gc.C) {
 	machine.EXPECT().IsLockedForSeriesUpgrade().Return(false, nil)
 	machine.EXPECT().IsParentLockedForSeriesUpgrade().Return(false, nil)
 	s.backend.EXPECT().Machine("valid").Return(machine, nil)
+	s.model.EXPECT().UUID().Return("")
 
 	placement := []*instance.Placement{
 		{Scope: "deadbeef-0bad-400d-8000-4b1d0d06f00d", Directive: "valid"},
@@ -1559,6 +1564,37 @@ func (s *ApplicationSuite) TestApplicationDeployPlacement(c *gc.C) {
 	c.Assert(s.deployParams["my-app"].Placement, gc.DeepEquals, placement)
 }
 
+func (s *ApplicationSuite) TestApplicationDeployPlacementModelUUIDSubstitute(c *gc.C) {
+	ctrl := s.setup(c)
+	defer ctrl.Finish()
+
+	ch := s.expectDefaultCharm(ctrl)
+	curl := "ch:precise/dummy-42"
+	s.backend.EXPECT().Charm(curl).Return(ch, nil)
+	s.model.EXPECT().UUID().Return("deadbeef-0bad-400d-8000-4b1d0d06f00d")
+
+	placement := []*instance.Placement{
+		{Scope: "model-uuid", Directive: "0"},
+	}
+	args := params.ApplicationDeploy{
+		ApplicationName: "my-app",
+		CharmURL:        curl,
+		CharmOrigin:     createCharmOriginFromURL(curl),
+		NumUnits:        1,
+		Placement:       placement,
+	}
+	results, err := s.api.Deploy(params.ApplicationsDeploy{
+		Applications: []params.ApplicationDeploy{args}},
+	)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(results.Results, gc.HasLen, 1)
+	c.Assert(results.Results[0].Error, gc.IsNil)
+
+	c.Assert(s.deployParams["my-app"].Placement, gc.DeepEquals, []*instance.Placement{
+		{Scope: "deadbeef-0bad-400d-8000-4b1d0d06f00d", Directive: "0"},
+	})
+}
+
 func validCharmOriginForTest() *params.CharmOrigin {
 	return &params.CharmOrigin{Source: "charm-hub"}
 }
@@ -1574,6 +1610,7 @@ func (s *ApplicationSuite) TestApplicationDeployWithPlacementLockedError(c *gc.C
 	containerMachine.EXPECT().IsLockedForSeriesUpgrade().Return(false, nil).MinTimes(1)
 	containerMachine.EXPECT().IsParentLockedForSeriesUpgrade().Return(true, nil).MinTimes(1)
 	s.backend.EXPECT().Machine("0/lxd/0").Return(containerMachine, nil).MinTimes(1)
+	s.model.EXPECT().UUID().Return("").AnyTimes()
 
 	curl := "ch:precise/dummy-42"
 	args := []params.ApplicationDeploy{{
@@ -1660,6 +1697,7 @@ func (s *ApplicationSuite) TestApplicationDeploymentTrust(c *gc.C) {
 	ch := s.expectDefaultCharm(ctrl)
 	curl := "ch:precise/dummy-42"
 	s.backend.EXPECT().Charm(curl).Return(ch, nil).MinTimes(1)
+	s.model.EXPECT().UUID().Return("")
 
 	withTrust := map[string]string{"trust": "true"}
 	args := params.ApplicationDeploy{
@@ -1688,6 +1726,7 @@ func (s *ApplicationSuite) TestClientApplicationsDeployWithBindings(c *gc.C) {
 	ch := s.expectDefaultCharm(ctrl)
 	curl := "ch:focal/riak-42"
 	s.backend.EXPECT().Charm(curl).Return(ch, nil).MinTimes(1)
+	s.model.EXPECT().UUID().Return("").AnyTimes()
 
 	args := []params.ApplicationDeploy{{
 		ApplicationName: "old",
@@ -1754,6 +1793,7 @@ func (s *ApplicationSuite) TestDeployMinDeploymentVersionTooHigh(c *gc.C) {
 		k8sconstants.StorageProviderType,
 		map[string]interface{}{"foo": "bar"}),
 	)
+	s.model.EXPECT().UUID().Return("")
 
 	args := params.ApplicationsDeploy{
 		Applications: []params.ApplicationDeploy{{
@@ -1792,6 +1832,7 @@ func (s *ApplicationSuite) TestDeployCAASModel(c *gc.C) {
 		},
 	)
 	s.backend.EXPECT().Charm(gomock.Any()).Return(ch, nil).Times(4)
+	s.model.EXPECT().UUID().Return("").AnyTimes()
 	s.expectDefaultK8sModelConfig()
 
 	args := params.ApplicationsDeploy{
@@ -1846,6 +1887,7 @@ func (s *ApplicationSuite) TestDeployCAASInvalidServiceType(c *gc.C) {
 
 	ch := s.expectDefaultCharm(ctrl)
 	s.backend.EXPECT().Charm(gomock.Any()).Return(ch, nil)
+	s.model.EXPECT().UUID().Return("")
 
 	curl := "local:foo-0"
 	args := params.ApplicationsDeploy{
@@ -1872,6 +1914,7 @@ func (s *ApplicationSuite) TestDeployCAASBlockStorageRejected(c *gc.C) {
 		Storage: map[string]charm.Storage{"block": {Name: "block", Type: charm.StorageBlock}},
 	}, &charm.Manifest{}, &charm.Config{})
 	s.backend.EXPECT().Charm(gomock.Any()).Return(ch, nil)
+	s.model.EXPECT().UUID().Return("")
 
 	args := params.ApplicationsDeploy{
 		Applications: []params.ApplicationDeploy{{
@@ -1905,6 +1948,7 @@ func (s *ApplicationSuite) TestDeployCAASModelNoOperatorStorage(c *gc.C) {
 		"workload-storage": "k8s-storage",
 	})
 	s.model.EXPECT().ModelConfig().Return(config.New(config.UseDefaults, attrs)).MinTimes(1)
+	s.model.EXPECT().UUID().Return("")
 
 	args := params.ApplicationsDeploy{
 		Applications: []params.ApplicationDeploy{{
@@ -1944,6 +1988,7 @@ func (s *ApplicationSuite) TestDeployCAASModelCharmNeedsNoOperatorStorage(c *gc.
 		"workload-storage": "k8s-storage",
 	})
 	s.model.EXPECT().ModelConfig().Return(config.New(config.UseDefaults, attrs)).MinTimes(1)
+	s.model.EXPECT().UUID().Return("")
 
 	args := params.ApplicationsDeploy{
 		Applications: []params.ApplicationDeploy{{
@@ -1971,6 +2016,7 @@ func (s *ApplicationSuite) TestDeployCAASModelSidecarCharmNeedsNoOperatorStorage
 		"workload-storage": "k8s-storage",
 	})
 	s.model.EXPECT().ModelConfig().Return(config.New(config.UseDefaults, attrs)).MinTimes(1)
+	s.model.EXPECT().UUID().Return("").AnyTimes()
 
 	args := params.ApplicationsDeploy{
 		Applications: []params.ApplicationDeploy{{
@@ -1997,6 +2043,7 @@ func (s *ApplicationSuite) TestDeployCAASModelDefaultOperatorStorageClass(c *gc.
 	s.caasBroker.EXPECT().ValidateStorageClass(gomock.Any()).Return(nil)
 	s.storagePoolManager.EXPECT().Get("k8s-operator-storage").Return(nil, errors.NotFoundf("pool"))
 	s.registry.EXPECT().StorageProvider(storage.ProviderType("k8s-operator-storage")).Return(nil, errors.NotFoundf(`provider type "k8s-operator-storage"`))
+	s.model.EXPECT().UUID().Return("")
 
 	args := params.ApplicationsDeploy{
 		Applications: []params.ApplicationDeploy{{
@@ -2025,6 +2072,7 @@ func (s *ApplicationSuite) TestDeployCAASModelWrongOperatorStorageType(c *gc.C)
 		provider.RootfsProviderType,
 		map[string]interface{}{"foo": "bar"}),
 	)
+	s.model.EXPECT().UUID().Return("")
 
 	args := params.ApplicationsDeploy{
 		Applications: []params.ApplicationDeploy{{
@@ -2055,6 +2103,7 @@ func (s *ApplicationSuite) TestDeployCAASModelInvalidStorage(c *gc.C) {
 		map[string]interface{}{"foo": "bar"}),
 	)
 	s.caasBroker.EXPECT().ValidateStorageClass(gomock.Any()).Return(errors.NotFoundf("storage class"))
+	s.model.EXPECT().UUID().Return("")
 
 	args := params.ApplicationsDeploy{
 		Applications: []params.ApplicationDeploy{{
@@ -2092,6 +2141,7 @@ func (s *ApplicationSuite) TestDeployCAASModelDefaultStorageClass(c *gc.C) {
 		map[string]interface{}{"foo": "bar"}),
 	)
 	s.caasBroker.EXPECT().ValidateStorageClass(gomock.Any()).Return(nil)
+	s.model.EXPECT().UUID().Return("")
 
 	args := params.ApplicationsDeploy{
 		Applications: []params.ApplicationDeploy{{

apiserver/facades/client/application/deployrepository.go

@@ -343,7 +343,7 @@ type deployFromRepositoryValidator struct {
 func (v *deployFromRepositoryValidator) validate(arg params.DeployFromRepositoryArg) (deployTemplate, []error) {
 	errs := make([]error, 0)
 
-	if err := checkMachinePlacement(v.state, arg.ApplicationName, arg.Placement); err != nil {
+	if err := checkMachinePlacement(v.state, v.model.UUID(), arg.ApplicationName, arg.Placement); err != nil {
 		errs = append(errs, err)
 	}
 

apiserver/facades/client/application/deployrepository_test.go

@@ -59,6 +59,7 @@ func (s *validatorSuite) TestValidateSuccess(c *gc.C) {
 	resolvedData := getResolvedData(resultURL, resolvedOrigin)
 	s.repo.EXPECT().ResolveForDeploy(charmID).Return(resolvedData, nil)
 	s.repo.EXPECT().ResolveResources(nil, corecharm.CharmID{URL: resultURL, Origin: resolvedOrigin}).Return(nil, nil)
+	s.model.EXPECT().UUID().Return("")
 
 	// getCharm
 	s.state.EXPECT().ModelConstraints().Return(constraints.Value{Arch: strptr("arm64")}, nil)
@@ -112,6 +113,7 @@ func (s *validatorSuite) testValidateIAASAttachStorage(c *gc.C, argStorage []str
 	resolvedData := getResolvedData(resultURL, resolvedOrigin)
 	s.repo.EXPECT().ResolveForDeploy(charmID).Return(resolvedData, nil)
 	s.repo.EXPECT().ResolveResources(nil, corecharm.CharmID{URL: resultURL, Origin: resolvedOrigin}).Return(nil, nil)
+	s.model.EXPECT().UUID().Return("")
 	// getCharm
 	s.state.EXPECT().ModelConstraints().Return(constraints.Value{Arch: strptr("arm64")}, nil)
 	s.state.EXPECT().Charm(gomock.Any()).Return(nil, errors.NotFoundf("charm"))
@@ -160,6 +162,7 @@ func (s *validatorSuite) TestValidatePlacementSuccess(c *gc.C) {
 	resolvedData := getResolvedData(resultURL, resolvedOrigin)
 	s.repo.EXPECT().ResolveForDeploy(charmID).Return(resolvedData, nil)
 	s.repo.EXPECT().ResolveResources(nil, corecharm.CharmID{URL: resultURL, Origin: resolvedOrigin}).Return(nil, nil)
+	s.model.EXPECT().UUID().Return("")
 
 	// Placement
 	s.state.EXPECT().Machine("0").Return(s.machine, nil).Times(2)
@@ -213,6 +216,7 @@ func (s *validatorSuite) TestValidateEndpointBindingSuccess(c *gc.C) {
 	resolvedData := getResolvedData(resultURL, resolvedOrigin)
 	s.repo.EXPECT().ResolveForDeploy(charmID).Return(resolvedData, nil)
 	s.repo.EXPECT().ResolveResources(nil, corecharm.CharmID{URL: resultURL, Origin: resolvedOrigin}).Return(nil, nil)
+	s.model.EXPECT().UUID().Return("")
 
 	// state bindings
 	endpointMap := map[string]string{"to": "from"}
@@ -948,6 +952,7 @@ func (s *validatorSuite) TestCaasDeployFromRepositoryValidator(c *gc.C) {
 	s.state.EXPECT().ModelConstraints().Return(constraints.Value{
 		Arch: strptr("arm64"),
 	}, nil)
+	s.model.EXPECT().UUID().Return("")
 
 	arg := params.DeployFromRepositoryArg{
 		CharmName: "testcharm",

apiserver/facades/client/machinemanager/upgradebase.go

@@ -524,11 +524,11 @@ func (s charmhubSeriesValidator) ValidateApplications(applications []Application
 	for _, resp := range refreshResp {
 		var found bool
 		for _, base := range resp.Entity.Bases {
-			track, err := corecharm.ChannelTrack(base.Channel)
+			channel, err := corebase.ParseChannel(base.Channel)
 			if err != nil {
 				return errors.Trace(err)
 			}
-			if channelToValidate == track || force {
+			if channelToValidate == channel.Track || force {
 				found = true
 				break
 			}