diff --git a/apiserver/common/networkingcommon/networkconfigapi.go b/apiserver/common/networkingcommon/networkconfigapi.go index e30062d08f9..637ba17ef53 100644 --- a/apiserver/common/networkingcommon/networkconfigapi.go +++ b/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) diff --git a/apiserver/common/networkingcommon/networkconfigapi_test.go b/apiserver/common/networkingcommon/networkconfigapi_test.go index 5f131e55a03..cb6d0cafec0 100644 --- a/apiserver/common/networkingcommon/networkconfigapi_test.go +++ b/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() diff --git a/apiserver/facades/client/application/application.go b/apiserver/facades/client/application/application.go index 1badec59ed4..27ca2f64bfb 100644 --- a/apiserver/facades/client/application/application.go +++ b/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 diff --git a/apiserver/facades/client/application/application_unit_test.go b/apiserver/facades/client/application/application_unit_test.go index b9411aa23c4..94a4e8466ed 100644 --- a/apiserver/facades/client/application/application_unit_test.go +++ b/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 ¶ms.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{{ diff --git a/apiserver/facades/client/application/deployrepository.go b/apiserver/facades/client/application/deployrepository.go index 21a2895d52c..95c4c410554 100644 --- a/apiserver/facades/client/application/deployrepository.go +++ b/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) } diff --git a/apiserver/facades/client/application/deployrepository_test.go b/apiserver/facades/client/application/deployrepository_test.go index 1e41c26c419..9c4a1dc2614 100644 --- a/apiserver/facades/client/application/deployrepository_test.go +++ b/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", diff --git a/apiserver/facades/client/machinemanager/upgradebase.go b/apiserver/facades/client/machinemanager/upgradebase.go index f34b8ed7a49..b672ab56b58 100644 --- a/apiserver/facades/client/machinemanager/upgradebase.go +++ b/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 } diff --git a/charmhub/refresh.go b/charmhub/refresh.go index 56211b09363..178419b5941 100644 --- a/charmhub/refresh.go +++ b/charmhub/refresh.go @@ -429,7 +429,10 @@ func constructRefreshBase(base RefreshBase) (transport.Base, error) { if err == nil { channel = potential } else { - channel = base.Channel + channel, err = sanitiseChannel(base.Channel) + if err != nil { + return transport.Base{}, logAndReturnError(errors.Trace(err)) + } } } @@ -440,6 +443,21 @@ func constructRefreshBase(base RefreshBase) (transport.Base, error) { }, nil } +// sanitiseChannel returns a channel, sanitised for charmhub +// +// Sometimes channels we receive include a risk, which charmhub +// cannot understand. So ensure any risk is dropped. +func sanitiseChannel(channel string) (string, error) { + if channel == "" { + return channel, nil + } + ch, err := corebase.ParseChannel(channel) + if err != nil { + return "", errors.Trace(err) + } + return ch.Track, nil +} + // validateBase ensures that we do not pass "all" as part of base. // This function is to help find programming related failures. func validateBase(rp RefreshBase) error { diff --git a/charmhub/refresh_test.go b/charmhub/refresh_test.go index ed122a66bf7..e1ee2becab2 100644 --- a/charmhub/refresh_test.go +++ b/charmhub/refresh_test.go @@ -466,6 +466,39 @@ func (s *RefreshConfigSuite) TestRefreshOneBuild(c *gc.C) { }) } +func (s *RefreshConfigSuite) TestRefreshOneWithBaseChannelRiskBuild(c *gc.C) { + id := "foo" + config, err := RefreshOne("instance-key", id, 1, "latest/stable", RefreshBase{ + Name: "ubuntu", + Channel: "20.04/stable", + Architecture: arch.DefaultArchitecture, + }) + c.Assert(err, jc.ErrorIsNil) + + req, err := config.Build() + c.Assert(err, jc.ErrorIsNil) + c.Assert(req, gc.DeepEquals, transport.RefreshRequest{ + Context: []transport.RefreshRequestContext{{ + InstanceKey: "instance-key", + ID: "foo", + Revision: 1, + Base: transport.Base{ + Name: "ubuntu", + Channel: "20.04", + Architecture: arch.DefaultArchitecture, + }, + TrackingChannel: "latest/stable", + }}, + Actions: []transport.RefreshRequestAction{{ + Action: "refresh", + InstanceKey: "instance-key", + ID: &id, + }}, + Fields: expRefreshFields, + }) + +} + func (s *RefreshConfigSuite) TestRefreshOneBuildInstanceKeyCompatibility(c *gc.C) { id := "foo" config, err := RefreshOne("", id, 1, "latest/stable", RefreshBase{ diff --git a/core/base/channel_test.go b/core/base/channel_test.go index 6ce7f0b9e27..ac991f9d738 100644 --- a/core/base/channel_test.go +++ b/core/base/channel_test.go @@ -22,6 +22,9 @@ func (s *ChannelSuite) TestParse(c *gc.C) { ch, err = ParseChannel("22.04/edge") c.Assert(err, jc.ErrorIsNil) c.Assert(ch, jc.DeepEquals, Channel{Track: "22.04", Risk: "edge"}) + ch, err = ParseChannel("all") + c.Assert(err, jc.ErrorIsNil) + c.Assert(ch, jc.DeepEquals, Channel{Track: "all"}) } func (s *ChannelSuite) TestParseError(c *gc.C) { diff --git a/core/charm/origin.go b/core/charm/origin.go index 423ce32e42e..ee5d019396c 100644 --- a/core/charm/origin.go +++ b/core/charm/origin.go @@ -192,17 +192,3 @@ func (p Platform) String() string { return path } - -func ChannelTrack(channel string) (string, error) { - // Base channel can be found as either just the version `20.04` (focal) - // or as `20.04/latest` (focal latest). We should future proof ourself - // for now and just drop the risk on the floor. - ch, err := charm.ParseChannel(channel) - if err != nil { - return "", errors.Trace(err) - } - if ch.Track == "" { - return "", errors.NotValidf("channel track") - } - return ch.Track, nil -} diff --git a/core/charm/origin_test.go b/core/charm/origin_test.go index 73395c39c3e..85ca55ecb10 100644 --- a/core/charm/origin_test.go +++ b/core/charm/origin_test.go @@ -139,35 +139,3 @@ func (s platformSuite) TestString(c *gc.C) { c.Assert(platform.String(), gc.DeepEquals, test.Expected) } } - -type channelTrackSuite struct { - testing.IsolationSuite -} - -var _ = gc.Suite(&channelTrackSuite{}) - -func (*channelTrackSuite) TestChannelTrack(c *gc.C) { - tests := []struct { - channel string - result string - }{{ - channel: "20.10", - result: "20.10", - }, { - channel: "focal", - result: "focal", - }, { - channel: "20.10/stable", - result: "20.10", - }, { - channel: "focal/stable", - result: "focal", - }} - - for i, test := range tests { - c.Logf("test %d - %s", i, test.channel) - got, err := charm.ChannelTrack(test.channel) - c.Assert(err, jc.ErrorIsNil) - c.Assert(got, gc.Equals, test.result) - } -} diff --git a/core/charm/repository/charmhub.go b/core/charm/repository/charmhub.go index 7d2f36d56f2..777520e6ab2 100644 --- a/core/charm/repository/charmhub.go +++ b/core/charm/repository/charmhub.go @@ -1006,12 +1006,13 @@ func (c *CharmHubRepository) composeSuggestions(releases []transport.Release, or base corebase.Base err error ) - track, err := corecharm.ChannelTrack(release.Base.Channel) + + channel, err := corebase.ParseChannel(release.Base.Channel) if err != nil { c.logger.Errorf("invalid base channel %v: %s", release.Base.Channel, err) continue } - if track == "all" || release.Base.Name == "all" { + if channel.Track == "all" || release.Base.Name == "all" { base, err = corebase.ParseBase(origin.Platform.OS, origin.Platform.Channel) } else { base, err = corebase.ParseBase(release.Base.Name, release.Base.Channel) diff --git a/core/charm/repository/charmhub_test.go b/core/charm/repository/charmhub_test.go index 08f71acbf62..e359fbe0466 100644 --- a/core/charm/repository/charmhub_test.go +++ b/core/charm/repository/charmhub_test.go @@ -1005,7 +1005,8 @@ func (refreshConfigSuite) TestRefreshByChannel(c *gc.C) { func (refreshConfigSuite) TestRefreshByChannelVersion(c *gc.C) { name := "wordpress" - platform := corecharm.MustParsePlatform("amd64/ubuntu/20.10/latest") + // 'mistakenly' include a risk in the platform + platform := corecharm.MustParsePlatform("amd64/ubuntu/20.10/stable") channel := corecharm.MustParseChannel("latest/stable").Normalize() origin := corecharm.Origin{ Platform: platform, @@ -1028,7 +1029,7 @@ func (refreshConfigSuite) TestRefreshByChannelVersion(c *gc.C) { Channel: &ch, Base: &transport.Base{ Name: "ubuntu", - Channel: "20.10/latest", + Channel: "20.10", Architecture: "amd64", }, }}, diff --git a/tests/suites/deploy/deploy_charms.sh b/tests/suites/deploy/deploy_charms.sh index 49ae47f47c7..65d16d37ed0 100644 --- a/tests/suites/deploy/deploy_charms.sh +++ b/tests/suites/deploy/deploy_charms.sh @@ -14,6 +14,21 @@ run_deploy_charm() { destroy_model "test-deploy-charm" } +run_deploy_charm_placement_directive() { + echo + + file="${TEST_DIR}/test-deploy-charm-placement-directive.log" + + ensure "test-deploy-charm-placement-directive" "${file}" + + juju add-machine --base ubuntu@20.04 + juju deploy jameinel-ubuntu-lite --to 0 + wait_for "ubuntu-lite" "$(idle_condition "ubuntu-lite")" + + destroy_model "test-deploy-charm-placement-directive" + +} + run_deploy_charm_unsupported_series() { # Test trying to deploy a charmhub charm to an operating system # never supported in the specified channel. It should fail. @@ -318,6 +333,7 @@ test_deploy_charms() { cd .. || exit run "run_deploy_charm" + run "run_deploy_charm_placement_directive" run "run_deploy_specific_series" run "run_resolve_charm" run "run_deploy_charm_unsupported_series"