Skip to content

Commit

Permalink
Merge pull request juju#16101 from jack-w-shaw/JUJU-4465_use_base_in_…
Browse files Browse the repository at this point in the history 
…NewCharmAtPath

juju#16101

Replace it with BaseSelector

This is part of the ongoing task to replace series with base

In most places, BaseSelector and SeriesSelector behave the same. There are a few notable exceptions. BaseSelector prefers the latest LTS over the charm's default, which results in some unit test failures.

Also, as a flyby, change NewCharmAtPath to use a base instead of a series

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

Ensure all unit tests pass

```
./main.sh -v -c aws -p ec2 deploy
./main.sh -v -c aws -p ec2 refresh
```

```
$ juju deploy ubuntu ubu1
Deployed "ubu1" from charm-hub charm "ubuntu", revision 24 in channel stable on ubuntu@22.04/stable
$ juju deploy ubuntu ubu2 --base ubuntu@20.04
Deployed "ubu2" from charm-hub charm "ubuntu", revision 24 in channel stable on ubuntu@20.04/stable
$ juju model-config default-base=ubuntu@20.04
$ juju deploy ubuntu ubu3
Deployed "ubu3" from charm-hub charm "ubuntu", revision 24 in channel stable on ubuntu@20.04/stable
```

Deploy some bundles as follows:
```
$ cat bundle1.yaml
applications:
 ubu1:
 charm: ubuntu
 num_units: 1

$ juju deploy ./bundle1.yaml
Located charm "ubuntu" in charm-hub, channel stable
Executing changes:
- upload charm ubuntu from charm-hub with architecture=amd64
- deploy application ubu1 from charm-hub using ubuntu
- add unit ubu1/0 to new machine 1
Deploy of bundle completed.

$ juju status
Model Controller Cloud/Region Version SLA Timestamp
m2 lxd localhost/localhost 3.3-beta2.1 unsupported 12:38:46+01:00

App Version Status Scale Charm Channel Rev Exposed Message
ubu1 waiting 0/1 ubuntu stable 24 no waiting for machine

Unit Workload Agent Machine Public address Ports Message
ubu1/0 waiting allocating 1 waiting for machine

Machine State Address Inst id Base AZ Message
1 pending pending ubuntu@22.04 
```

```
$ cat bundle2.yaml
applications:
 ubu1:
 charm: ubuntu
 base: ubuntu@20.04
 num_units: 1

$ juju deploy ./bundle2.yaml
Located charm "ubuntu" in charm-hub, channel stable
Executing changes:
- upload charm ubuntu from charm-hub for base ubuntu@20.04/stable with architecture=amd64
- deploy application ubu1 from charm-hub on ubuntu@20.04/stable using ubuntu
- add unit ubu1/0 to new machine 0
Deploy of bundle completed.
```

```
$ cat bundle3.yaml
default-base: ubuntu@20.04
applications:
 ubu1:
 charm: ubuntu
 num_units: 1

$ juju deploy ./bundle3.yaml
Located charm "ubuntu" in charm-hub, channel stable
Executing changes:
- upload charm ubuntu from charm-hub for base ubuntu@20.04/stable with architecture=amd64
- deploy application ubu1 from charm-hub on ubuntu@20.04/stable using ubuntu
- add unit ubu1/0 to new machine 0
Deploy of bundle completed.
```
  • Loading branch information
@jujubot
jujubot authored Aug 25, 2023
2 parents 7ad2ad4 + a6b4368 commit 7a9eb97
Show file tree
Hide file tree
Showing 42 changed files with 466 additions and 1,017 deletions.
2 changes: 1 addition & 1 deletion cmd/juju/application/deploy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ when refreshing the application in the future.
A local charm may be deployed by giving the path to its directory:
juju deploy /path/to/charm
juju deploy /path/to/charm ---base ubuntu@22.04
juju deploy /path/to/charm --base ubuntu@22.04
You will need to be explicit if there is an ambiguity between a local and a
remote charm:
Expand Down
128 changes: 42 additions & 86 deletions cmd/juju/application/deploy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/juju/cmd/v3"
"github.com/juju/cmd/v3/cmdtesting"
"github.com/juju/collections/set"
"github.com/juju/collections/transform"
"github.com/juju/errors"
"github.com/juju/gnuflag"
"github.com/juju/loggo"
Expand Down Expand Up @@ -148,16 +149,6 @@ func (s *DeploySuiteBase) SetUpTest(c *gc.C) {
}
s.RepoSuite.SetUpTest(c)

// TODO: remove this patch once we removed all the old series from tests in current package.
s.PatchValue(&deployer.SupportedJujuSeries,
func(time.Time, string, string) (set.Strings, error) {
return set.NewStrings(
"centos7", "centos9", "genericlinux", "kubernetes",
"jammy", "focal", "jammy", "xenial",
), nil
},
)

s.CmdBlockHelper = coretesting.NewCmdBlockHelper(s.APIState)
c.Assert(s.CmdBlockHelper, gc.NotNil)
s.AddCleanup(func(*gc.C) { s.CmdBlockHelper.Close() })
Expand Down Expand Up @@ -366,9 +357,15 @@ func (s *DeploySuite) TestDeployFromPathOldCharm(c *gc.C) {
}

func (s *DeploySuite) TestDeployFromPathOldCharmMissingSeries(c *gc.C) {
path := testcharms.RepoWithSeries("bionic").ClonedDirPath(c.MkDir(), "dummy")
path := testcharms.RepoWithSeries("bionic").ClonedDirPath(c.MkDir(), "dummy-no-series")
err := s.runDeploy(c, path, "--base", "ubuntu@20.04")
c.Assert(err, gc.ErrorMatches, "charm does not define any bases, not valid")
}

func (s *DeploySuite) TestDeployFromPathOldCharmMissingSeriesNoBase(c *gc.C) {
path := testcharms.RepoWithSeries("bionic").ClonedDirPath(c.MkDir(), "dummy-no-series")
err := s.runDeploy(c, path)
c.Assert(err, gc.ErrorMatches, "series not specified and charm does not define any")
c.Assert(err, gc.ErrorMatches, "charm does not define any bases, not valid")
}

func (s *DeploySuite) TestDeployFromPathOldCharmMissingSeriesUseDefaultSeries(c *gc.C) {
Expand Down Expand Up @@ -415,31 +412,27 @@ func (s *DeploySuite) TestDeployFromPath(c *gc.C) {
}

func (s *DeploySuite) TestDeployFromPathUnsupportedSeriesHaveOverlap(c *gc.C) {
// Donot remove this because we want to test: series supported by the charm and series supported by Juju have overlap.
s.PatchValue(&deployer.SupportedJujuSeries,
func(time.Time, string, string) (set.Strings, error) {
return set.NewStrings(
"jammy", "focal",
), nil
},
)
// Do not remove this because we want to test: bases supported by the charm and bases supported by Juju have overlap.
s.PatchValue(&deployer.SupportedJujuBases, func(time.Time, corebase.Base, string) ([]corebase.Base, error) {
return transform.SliceOrErr([]string{"ubuntu@22.04", "ubuntu@20.04", "ubuntu@12.10"}, corebase.ParseBaseFromString)
})

path := testcharms.RepoWithSeries("bionic").ClonedDirPath(c.MkDir(), "multi-series")
err := s.runDeploy(c, path, "--base", "ubuntu@12.10")
c.Assert(err, gc.ErrorMatches, `series "quantal" is not supported, supported series are: focal,jammy`)
c.Assert(err, gc.ErrorMatches, `base "ubuntu@12.10/stable" is not supported, supported bases are: .*`)
}

func (s *DeploySuite) TestDeployFromPathUnsupportedSeriesHaveNoOverlap(c *gc.C) {
// Donot remove this because we want to test: series supported by the charm and series supported by Juju have NO overlap.
s.PatchValue(&deployer.SupportedJujuSeries,
func(time.Time, string, string) (set.Strings, error) {
return set.NewStrings("kinetic"), nil
func (s *DeploySuite) TestDeployFromPathUnsupportedBaseHaveNoOverlap(c *gc.C) {
// Do not remove this because we want to test: bases supported by the charm and bases supported by Juju have NO overlap.
s.PatchValue(&deployer.SupportedJujuBases,
func(time.Time, corebase.Base, string) ([]corebase.Base, error) {
return []corebase.Base{corebase.MustParseBaseFromString("ubuntu@22.10")}, nil
},
)

path := testcharms.RepoWithSeries("bionic").ClonedDirPath(c.MkDir(), "multi-series")
err := s.runDeploy(c, path, "--base", "ubuntu@12.10")
c.Assert(err, gc.ErrorMatches, `multi-series is not available on the following series: quantal`)
err := s.runDeploy(c, path)
c.Assert(err, gc.ErrorMatches, `the charm defined bases ".*" not supported`)
}

func (s *DeploySuite) TestDeployFromPathUnsupportedSeriesForce(c *gc.C) {
Expand All @@ -448,14 +441,10 @@ func (s *DeploySuite) TestDeployFromPathUnsupportedSeriesForce(c *gc.C) {
withLocalCharmDeployable(s.fakeAPI, curl, charmDir, false)
withCharmDeployable(s.fakeAPI, curl, corebase.MustParseBaseFromString("ubuntu@20.04"), charmDir.Meta(), charmDir.Metrics(), false, false, 1, nil, nil)

// TODO: remove this patch once we removed all the old series from tests in current package.
s.PatchValue(&deployer.SupportedJujuSeries,
func(time.Time, string, string) (set.Strings, error) {
return set.NewStrings(
"jammy", "focal", "jammy", "xenial", "quantal",
), nil
},
)
// TODO remove this patch once we removed all the old bases from tests in current package.
s.PatchValue(&deployer.SupportedJujuBases, func(time.Time, corebase.Base, string) ([]corebase.Base, error) {
return transform.SliceOrErr([]string{"ubuntu@22.04", "ubuntu@20.04", "ubuntu@18.04", "ubuntu@12.10"}, corebase.ParseBaseFromString)
})

err := s.runDeployForState(c, charmDir.Path, "--base", "ubuntu@12.10", "--force")
c.Assert(err, jc.ErrorIsNil)
Expand All @@ -468,14 +457,10 @@ func (s *DeploySuite) TestDeployFromPathUnsupportedLXDProfileForce(c *gc.C) {
withLocalCharmDeployable(s.fakeAPI, curl, charmDir, false)
withCharmDeployable(s.fakeAPI, curl, corebase.MustParseBaseFromString("ubuntu@20.04"), charmDir.Meta(), charmDir.Metrics(), false, true, 1, nil, nil)

// TODO: remove this patch once we removed all the old series from tests in current package.
s.PatchValue(&deployer.SupportedJujuSeries,
func(time.Time, string, string) (set.Strings, error) {
return set.NewStrings(
"jammy", "focal", "jammy", "xenial", "quantal",
), nil
},
)
// TODO remove this patch once we removed all the old bases from tests in current package.
s.PatchValue(&deployer.SupportedJujuBases, func(time.Time, corebase.Base, string) ([]corebase.Base, error) {
return transform.SliceOrErr([]string{"ubuntu@22.04", "ubuntu@20.04", "ubuntu@18.04", "ubuntu@12.10"}, corebase.ParseBaseFromString)
})

err := s.runDeployForState(c, charmDir.Path, "--base", "ubuntu@12.10", "--force")
c.Assert(err, jc.ErrorIsNil)
Expand Down Expand Up @@ -1037,16 +1022,6 @@ func (s *CAASDeploySuiteBase) expectDeployer(c *gc.C, cfg deployer.DeployerConfi
}

func (s *CAASDeploySuiteBase) SetUpTest(c *gc.C) {

// TODO: remove this patch once we removed all the old series from tests in current package.
s.PatchValue(&deployer.SupportedJujuSeries,
func(time.Time, string, string) (set.Strings, error) {
return set.NewStrings(
"centos7", "centos9", "genericlinux", "kubernetes",
"jammy", "focal", "jammy", "xenial",
), nil
},
)
cookiesFile := filepath.Join(c.MkDir(), ".go-cookies")
s.PatchEnvironment("JUJU_COOKIEFILE", cookiesFile)

Expand Down Expand Up @@ -1217,9 +1192,9 @@ func (s *CAASDeploySuite) TestDevices(c *gc.C) {

func (s *DeploySuite) TestDeployStorageFailContainer(c *gc.C) {
charmDir := testcharms.RepoWithSeries("bionic").ClonedDir(c.MkDir(), "multi-series")
curl := charm.MustParseURL("local:focal/multi-series-1")
curl := charm.MustParseURL("local:jammy/multi-series-1")
withLocalCharmDeployable(s.fakeAPI, curl, charmDir, false)
withCharmDeployable(s.fakeAPI, curl, corebase.MustParseBaseFromString("ubuntu@20.04"), charmDir.Meta(), charmDir.Metrics(), false, false, 1, nil, nil)
withCharmDeployable(s.fakeAPI, curl, corebase.MustParseBaseFromString("ubuntu@22.04"), charmDir.Meta(), charmDir.Metrics(), false, false, 1, nil, nil)

machine, err := s.State.AddMachine(state.UbuntuBase("22.04"), state.JobHostUnits)
c.Assert(err, jc.ErrorIsNil)
Expand Down Expand Up @@ -1446,24 +1421,20 @@ func (s *DeploySuite) TestDeployLocalWithSeriesMismatchReturnsError(c *gc.C) {

_, _, err := s.runDeployWithOutput(c, charmDir.Path, "--base", "ubuntu@12.10")

c.Check(err, gc.ErrorMatches, `terms1 is not available on the following series: quantal not supported`)
c.Check(err, gc.ErrorMatches, `terms1 is not available on the following base: ubuntu@12.10/stable`)
}

func (s *DeploySuite) TestDeployLocalWithSeriesAndForce(c *gc.C) {
// TODO remove this patch once we removed all the old bases from tests in current package.
s.PatchValue(&deployer.SupportedJujuBases, func(time.Time, corebase.Base, string) ([]corebase.Base, error) {
return transform.SliceOrErr([]string{"ubuntu@22.04", "ubuntu@20.04", "ubuntu@18.04", "ubuntu@12.10"}, corebase.ParseBaseFromString)
})

charmDir := testcharms.RepoWithSeries("quantal").ClonedDir(c.MkDir(), "terms1")
curl := charm.MustParseURL("local:quantal/terms1-1")
withLocalCharmDeployable(s.fakeAPI, curl, charmDir, true)
withCharmDeployable(s.fakeAPI, curl, defaultBase, charmDir.Meta(), charmDir.Metrics(), false, true, 1, nil, nil)

// TODO: remove this patch once we removed all the old series from tests in current package.
s.PatchValue(&deployer.SupportedJujuSeries,
func(time.Time, string, string) (set.Strings, error) {
return set.NewStrings(
"jammy", "focal", "jammy", "xenial", "quantal",
), nil
},
)

err := s.runDeployForState(c, charmDir.Path, "--base", "ubuntu@12.10", "--force")
c.Assert(err, jc.ErrorIsNil)
s.AssertApplication(c, "terms1", curl, 1, 0)
Expand All @@ -1479,15 +1450,10 @@ func (s *DeploySuite) setupNonESMBase(c *gc.C) (corebase.Base, string) {
supportedNotEMS := supported.Difference(set.NewStrings(corebase.ESMSupportedJujuSeries()...))
c.Assert(supportedNotEMS.Size(), jc.GreaterThan, 0)

// TODO: remove this patch once we removed all the old series from tests in current package.
s.PatchValue(&deployer.SupportedJujuSeries,
func(time.Time, string, string) (set.Strings, error) {
return set.NewStrings(
"centos7", "centos9", "genericlinux", "kubernetes",
"jammy", "focal", "jammy", "xenial",
), nil
},
)
// TODO remove this patch once we removed all the old bases from tests in current package.
s.PatchValue(&deployer.SupportedJujuBases, func(time.Time, corebase.Base, string) ([]corebase.Base, error) {
return transform.SliceOrErr([]string{"centos@7", "centos@9", "ubuntu@22.04", "ubuntu@20.04", "ubuntu@16.04"}, corebase.ParseBaseFromString)
})

nonEMSSeries := supportedNotEMS.SortedValues()[0]

Expand Down Expand Up @@ -1533,7 +1499,7 @@ func (s *DeploySuite) TestDeployLocalWithSupportedNonESMSeries(c *gc.C) {
func (s *DeploySuite) TestDeployLocalWithNotSupportedNonESMSeries(c *gc.C) {
_, loggingPath := s.setupNonESMBase(c)
err := s.runDeploy(c, loggingPath, "--base", "ubuntu@17.10")
c.Assert(err, gc.ErrorMatches, "logging is not available on the following series: artful not supported")
c.Assert(err, gc.ErrorMatches, "logging is not available on the following base: ubuntu@17.10/stable")
}

// setupConfigFile creates a configuration file for testing set
Expand Down Expand Up @@ -1907,16 +1873,6 @@ var _ = gc.Suite(&DeployUnitTestSuite{})
func (s *DeployUnitTestSuite) SetUpTest(c *gc.C) {
s.IsolationSuite.SetUpTest(c)

// TODO: remove this patch once we removed all the old series from tests in current package.
s.PatchValue(&deployer.SupportedJujuSeries,
func(time.Time, string, string) (set.Strings, error) {
return set.NewStrings(
"centos7", "centos9", "genericlinux", "kubernetes",
"jammy", "focal", "jammy", "xenial",
), nil
},
)

cookiesFile := filepath.Join(c.MkDir(), ".go-cookies")
s.PatchEnvironment("JUJU_COOKIEFILE", cookiesFile)
}
Expand Down
6 changes: 1 addition & 5 deletions cmd/juju/application/deployer/bundlehandler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -711,11 +711,7 @@ func (h *bundleHandler) addLocalCharm(chParams bundlechanges.AddCharmParams, chB
charmPath = filepath.Join(h.bundleDir, charmPath)
}

chSeries, err := corebase.GetSeriesFromBase(chBase)
if err != nil {
return errors.Annotatef(err, "cannot deploy local charm at %q", charmPath)
}
ch, curl, err := corecharm.NewCharmAtPathForceSeries(charmPath, chSeries, h.force)
ch, curl, err := corecharm.NewCharmAtPathForceBase(charmPath, chBase, h.force)
if err != nil {
return errors.Annotatef(err, "cannot deploy local charm at %q", charmPath)
}
Expand Down
Loading

0 comments on commit 7a9eb97

Please sign in to comment.