Skip to content

Commit

Permalink
Remove SeriesSelector
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jack-w-shaw committed Aug 23, 2023
1 parent 3113a35 commit a6b4368
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
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
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
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 a6b4368

Please sign in to comment.