diff --git a/cmd/juju/application/bundle/bundle.go b/cmd/juju/application/bundle/bundle.go index 842e56f0b871..113ab2ed9dc8 100644 --- a/cmd/juju/application/bundle/bundle.go +++ b/cmd/juju/application/bundle/bundle.go @@ -4,6 +4,7 @@ package bundle import ( + "fmt" "reflect" "sort" "strings" @@ -329,6 +330,10 @@ func verifyBundle(data *charm.BundleData, bundleDir string, verifyConstraints fu return err } + if err := verifyMixedSeriesBasesMatch(data); err != nil { + return errors.Trace(err) + } + var verifyError error if bundleDir == "" { verifyError = data.Verify(verifyConstraints, verifyStorage, verifyDevices) @@ -345,3 +350,55 @@ func verifyBundle(data *charm.BundleData, bundleDir string, verifyConstraints fu } return errors.Trace(verifyError) } + +func verifyMixedSeriesBasesMatch(data *charm.BundleData) error { + if data == nil { + return nil + } + if data.Series != "" && data.DefaultBase != "" { + b, err := corebase.ParseBaseFromString(data.DefaultBase) + if err != nil { + return errors.Trace(err) + } + s, err := corebase.GetSeriesFromBase(b) + if err != nil { + return errors.Trace(err) + } + if s != data.Series { + return errors.NewNotValid(nil, fmt.Sprintf("bundle series %q and base %q must match if supplied", data.Series, data.DefaultBase)) + } + } + + for name, m := range data.Machines { + if m != nil && m.Series != "" && m.Base != "" { + b, err := corebase.ParseBaseFromString(m.Base) + if err != nil { + return errors.Trace(err) + } + s, err := corebase.GetSeriesFromBase(b) + if err != nil { + return errors.Trace(err) + } + if s != m.Series { + return errors.NewNotValid(nil, fmt.Sprintf("machine %q series %q and base %q must match if supplied", name, m.Series, m.Base)) + } + } + } + + for name, app := range data.Applications { + if app != nil && app.Series != "" && app.Base != "" { + b, err := corebase.ParseBaseFromString(app.Base) + if err != nil { + return errors.Trace(err) + } + s, err := corebase.GetSeriesFromBase(b) + if err != nil { + return errors.Trace(err) + } + if s != app.Series { + return errors.NewNotValid(nil, fmt.Sprintf("application %q series %q and base %q must match if supplied", name, app.Series, app.Base)) + } + } + } + return nil +} diff --git a/cmd/juju/application/bundle/bundle_test.go b/cmd/juju/application/bundle/bundle_test.go index a01b4986a5b5..e42db9a575ed 100644 --- a/cmd/juju/application/bundle/bundle_test.go +++ b/cmd/juju/application/bundle/bundle_test.go @@ -303,6 +303,30 @@ func (s *composeAndVerifyRepSuite) TestComposeAndVerifyBundleOverlayUnmarshallEr c.Assert(unmarshallErrors[0], gc.Equals, expectedError) } +func (s *composeAndVerifyRepSuite) TestComposeAndVerifyBundleMixingBaseAndSeries(c *gc.C) { + defer s.setupMocks(c).Finish() + bundleData, err := charm.ReadBundleData(strings.NewReader(mixedSeriesBaseBundle)) + c.Assert(err, jc.ErrorIsNil) + s.expectParts(&charm.BundleDataPart{Data: bundleData}) + s.expectBasePath() + + obtained, _, err := ComposeAndVerifyBundle(s.bundleDataSource, nil) + c.Assert(err, jc.ErrorIsNil) + c.Assert(obtained, gc.DeepEquals, bundleData) +} + +func (s *composeAndVerifyRepSuite) TestComposeAndVerifyBundleMixingBaseAndSeriesMisMatch(c *gc.C) { + defer s.setupMocks(c).Finish() + bundleData, err := charm.ReadBundleData(strings.NewReader(mixedSeriesBaseBundleMismatch)) + c.Assert(err, jc.ErrorIsNil) + s.expectParts(&charm.BundleDataPart{Data: bundleData}) + s.expectBasePath() + + obtained, _, err := ComposeAndVerifyBundle(s.bundleDataSource, []string{s.overlayFile}) + c.Assert(err, gc.ErrorMatches, `application "wordpress" series "jammy" and base "ubuntu@20.04" must match if supplied`) + c.Assert(obtained, gc.IsNil) +} + func (s *composeAndVerifyRepSuite) setupOverlayFile(c *gc.C) { s.overlayDir = c.MkDir() s.overlayFile = filepath.Join(s.overlayDir, "config.yaml") @@ -421,13 +445,13 @@ func (m stringSliceMatcher) String() string { } const unsupportedConstraintBundle = ` -series: bionic +default-base: ubuntu@18.04 applications: mysql: charm: ch:mysql revision: 42 channel: stable - series: xenial + base: ubuntu@16.04 num_units: 1 constraints: image-id=ubuntu-bf2 to: @@ -436,28 +460,28 @@ applications: charm: ch:wordpress channel: stable revision: 47 - series: xenial + base: ubuntu@16.04 num_units: 1 to: - "1" machines: "0": - series: xenial + base: ubuntu@16.04 "1": - series: xenial + base: ubuntu@16.04 relations: - - wordpress:db - mysql:db ` const wordpressBundle = ` -series: bionic +default-base: ubuntu@18.04 applications: mysql: charm: ch:mysql revision: 42 channel: stable - series: xenial + base: ubuntu@16.04 num_units: 1 to: - "0" @@ -465,15 +489,15 @@ applications: charm: ch:wordpress channel: stable revision: 47 - series: xenial + base: ubuntu@16.04 num_units: 1 to: - "1" machines: "0": - series: xenial + base: ubuntu@16.04 "1": - series: xenial + base: ubuntu@16.04 relations: - - wordpress:db - mysql:db @@ -508,3 +532,68 @@ relations: - - wordpress:db - mysql:db ` + +const mixedSeriesBaseBundle = ` +series: bionic +default-base: ubuntu@18.04 +applications: + mysql: + charm: ch:mysql + revision: 42 + channel: stable + series: xenial + base: ubuntu@16.04 + num_units: 1 + to: + - "0" + wordpress: + charm: ch:wordpress + channel: stable + revision: 47 + series: jammy + num_units: 1 + to: + - "1" +machines: + "0": + series: xenial + base: ubuntu@16.04 + "1": + series: jammy +relations: +- - wordpress:db + - mysql:db +` + +const mixedSeriesBaseBundleMismatch = ` +series: bionic +default-base: ubuntu@18.04 +applications: + mysql: + charm: ch:mysql + revision: 42 + channel: stable + series: xenial + base: ubuntu@16.04 + num_units: 1 + to: + - "0" + wordpress: + charm: ch:wordpress + channel: stable + revision: 47 + series: jammy + base: ubuntu@20.04 + num_units: 1 + to: + - "1" +machines: + "0": + series: xenial + base: ubuntu@16.04 + "1": + series: jammy +relations: +- - wordpress:db + - mysql:db +` diff --git a/core/bundle/changes/changes_test.go b/core/bundle/changes/changes_test.go index 6f75fa3df349..e4a88e432268 100644 --- a/core/bundle/changes/changes_test.go +++ b/core/bundle/changes/changes_test.go @@ -3556,10 +3556,13 @@ func (s *changesSuite) assertParseDataWithDevices(c *gc.C, content string, expec } func (s *changesSuite) assertLocalBundleChanges(c *gc.C, charmDir, bundleContent, base string) { - b, err := corebase.ParseBaseFromString(base) - c.Assert(err, jc.ErrorIsNil) - series, err := corebase.GetSeriesFromBase(b) - c.Assert(err, jc.ErrorIsNil) + var series string + if base != "" { + b, err := corebase.ParseBaseFromString(base) + c.Assert(err, jc.ErrorIsNil) + series, err = corebase.GetSeriesFromBase(b) + c.Assert(err, jc.ErrorIsNil) + } expected := []record{{ Id: "addCharm-0", @@ -3606,10 +3609,13 @@ func (s *changesSuite) assertLocalBundleChanges(c *gc.C, charmDir, bundleContent } func (s *changesSuite) assertLocalBundleChangesWithDevices(c *gc.C, charmDir, bundleContent, base string) { - b, err := corebase.ParseBaseFromString(base) - c.Assert(err, jc.ErrorIsNil) - series, err := corebase.GetSeriesFromBase(b) - c.Assert(err, jc.ErrorIsNil) + var series string + if base != "" { + b, err := corebase.ParseBaseFromString(base) + c.Assert(err, jc.ErrorIsNil) + series, err = corebase.GetSeriesFromBase(b) + c.Assert(err, jc.ErrorIsNil) + } expected := []record{{ Id: "addCharm-0", @@ -3664,6 +3670,17 @@ func (s *changesSuite) TestLocalCharmWithExplicitBase(c *gc.C) { charm: %s base: ubuntu@16.04 `, charmDir) + charmMeta := ` +name: multi-series +summary: That's a dummy charm with multi-series. +description: A dummy charm. +series: + - jammy + - focal + - bionic +`[1:] + err := os.WriteFile(filepath.Join(charmDir, "metadata.yaml"), []byte(charmMeta), 0644) + c.Assert(err, jc.ErrorIsNil) s.assertLocalBundleChanges(c, charmDir, bundleContent, "ubuntu@16.04/stable") s.assertLocalBundleChangesWithDevices(c, charmDir, bundleContent, "ubuntu@16.04/stable") } diff --git a/core/bundle/changes/handlers.go b/core/bundle/changes/handlers.go index 53bb83844869..dd2c1cdb3935 100644 --- a/core/bundle/changes/handlers.go +++ b/core/bundle/changes/handlers.go @@ -32,9 +32,11 @@ type resolver struct { func (r *resolver) handleApplications() (map[string]string, error) { add := r.changes.add applications := r.bundle.Applications - defaultSeries := r.bundle.Series - defaultBase := r.bundle.DefaultBase existing := r.model + defaultBase, err := computeBase(r.bundle.DefaultBase, r.bundle.Series, corebase.Base{}) + if err != nil { + return nil, errors.Trace(err) + } charms := make(map[string]string, len(applications)) addedApplications := make(map[string]string, len(applications)) @@ -53,11 +55,11 @@ func (r *resolver) handleApplications() (map[string]string, error) { application.Series = corebase.LegacyKubernetesSeries() } existingApp := existing.GetApplication(name) - computedSeries, err := getSeries(application, defaultSeries) + series, err := getSeries(application) if err != nil { return nil, errors.Trace(err) } - computedBase, err := getBase(application.Base, defaultBase, computedSeries) + computedBase, err := computeBase(application.Base, series, defaultBase) if err != nil { return nil, errors.Trace(err) } @@ -410,9 +412,11 @@ func equalStringSlice(a, b []string) bool { func (r *resolver) handleMachines() (map[string]*AddMachineChange, error) { add := r.changes.add machines := r.bundle.Machines - defaultSeries := r.bundle.Series - defaultBase := r.bundle.DefaultBase existing := r.model + defaultBase, err := computeBase(r.bundle.DefaultBase, r.bundle.Series, corebase.Base{}) + if err != nil { + return nil, errors.Trace(err) + } addedMachines := make(map[string]*AddMachineChange, len(machines)) // Iterate over the map using its sorted keys so that results are @@ -430,13 +434,9 @@ func (r *resolver) handleMachines() (map[string]*AddMachineChange, error) { if machine == nil { machine = &charm.MachineSpec{} } - computedSeries := machine.Series - if computedSeries == "" { - computedSeries = defaultSeries - } - computedBase, err := getBase(machine.Base, defaultBase, computedSeries) + computedBase, err := computeBase(machine.Base, machine.Series, defaultBase) if err != nil { - return nil, err + return nil, errors.Trace(err) } var id string @@ -592,12 +592,11 @@ func (r *resolver) handleOffers(addedApplications map[string]string, deployedBun } type unitProcessor struct { - add func(Change) - existing *Model - bundle *charm.BundleData - defaultSeries string - defaultBase string - logger Logger + add func(Change) + existing *Model + bundle *charm.BundleData + defaultBase corebase.Base + logger Logger // The added applications and machines are maps from names to // change IDs. @@ -1014,11 +1013,11 @@ func (p *unitProcessor) addNewMachine(application *charm.ApplicationSpec, contai if err != nil { return unitPlacement{}, err } - computedSeries, err := getSeries(application, p.defaultSeries) + series, err := getSeries(application) if err != nil { return unitPlacement{}, err } - computedBase, err := getBase(application.Base, p.defaultBase, computedSeries) + computedBase, err := computeBase(application.Base, series, p.defaultBase) if err != nil { return unitPlacement{}, err } @@ -1110,11 +1109,11 @@ func (p *unitProcessor) addContainer(up unitPlacement, application *charm.Applic if err != nil { return unitPlacement{}, err } - computedSeries, err := getSeries(application, p.defaultSeries) + series, err := getSeries(application) if err != nil { return unitPlacement{}, err } - computedBase, err := getBase(application.Base, p.defaultBase, computedSeries) + computedBase, err := computeBase(application.Base, series, p.defaultBase) if err != nil { return unitPlacement{}, err } @@ -1155,13 +1154,16 @@ func (r *resolver) handleUnits(addedApplications map[string]string, addedMachine for _, v := range addedMachines { machChangeIDs.Add(v.Id()) } + defaultBase, err := computeBase(r.bundle.DefaultBase, r.bundle.Series, corebase.Base{}) + if err != nil { + return errors.Trace(err) + } processor := &unitProcessor{ add: r.changes.add, existing: r.model, bundle: r.bundle, - defaultSeries: r.bundle.Series, - defaultBase: r.bundle.DefaultBase, + defaultBase: defaultBase, logger: r.logger, addedApplications: addedApplications, addedMachines: addedMachines, @@ -1174,7 +1176,7 @@ func (r *resolver) handleUnits(addedApplications map[string]string, addedMachine } processor.addAllNeededUnits() - err := processor.processUnitPlacement() + err = processor.processUnitPlacement() if err != nil { return errors.Trace(err) } @@ -1270,28 +1272,18 @@ func applicationKey(charm, arch string, base corebase.Base, channel string, revi } // getSeries retrieves the series of a application from the ApplicationSpec or from the -// charm path or URL if provided, otherwise falling back on a default series. +// charm path or URL if provided. // // DEPRECATED: This should be all about bases. -func getSeries(application *charm.ApplicationSpec, defaultSeries string) (string, error) { - if application.Base != "" { - base, err := corebase.ParseBaseFromString(application.Base) - if err != nil { - return "", errors.Trace(err) - } - return corebase.GetSeriesFromBase(base) - } +func getSeries(application *charm.ApplicationSpec) (string, error) { if application.Series != "" { return application.Series, nil } - // Handle local charm paths. if charm.IsValidLocalCharmOrBundlePath(application.Charm) { - return defaultSeries, nil + return "", nil } - // The following is safe because the bundle data is assumed to be already - // verified, and therefore this must be a valid charm URL. charmURL, err := charm.ParseURL(application.Charm) if err != nil { return "", errors.Trace(err) @@ -1303,22 +1295,16 @@ func getSeries(application *charm.ApplicationSpec, defaultSeries string) (string } return charmURL.Series, nil } - return defaultSeries, nil + return "", nil } -// getBase calculates the base to use for a resource. If none is provided, we will fall back to -// the specified series and convert to a base -func getBase(base string, defaultBase string, computedSeries string) (corebase.Base, error) { +func computeBase(base string, series string, defaultBase corebase.Base) (corebase.Base, error) { if base != "" { return corebase.ParseBaseFromString(base) + } else if series != "" { + return corebase.GetBaseFromSeries(series) } - if defaultBase != "" { - return corebase.ParseBaseFromString(defaultBase) - } - if computedSeries != "" { - return corebase.GetBaseFromSeries(computedSeries) - } - return corebase.Base{}, nil + return defaultBase, nil } // parseEndpoint creates an endpoint from its string representation. diff --git a/go.mod b/go.mod index d4e67d3bcde9..9540bcab95f9 100644 --- a/go.mod +++ b/go.mod @@ -47,7 +47,7 @@ require ( github.com/im7mortal/kmutex v1.0.1 github.com/juju/ansiterm v1.0.0 github.com/juju/blobstore/v3 v3.0.2 - github.com/juju/charm/v11 v11.0.1 + github.com/juju/charm/v11 v11.0.2 github.com/juju/clock v1.0.3 github.com/juju/cmd/v3 v3.0.10 github.com/juju/collections v1.0.4 diff --git a/go.sum b/go.sum index d2a8163a6573..ca02a440e5d8 100644 --- a/go.sum +++ b/go.sum @@ -760,8 +760,8 @@ github.com/juju/ansiterm v1.0.0 h1:gmMvnZRq7JZJx6jkfSq9/+2LMrVEwGwt7UR6G+lmDEg= github.com/juju/ansiterm v1.0.0/go.mod h1:PyXUpnI3olx3bsPcHt98FGPX/KCFZ1Fi+hw1XLI6384= github.com/juju/blobstore/v3 v3.0.2 h1:roZ4YBuZYmWId6y/6ZLQSAMmNlHOclHD8PQAMOQer6E= github.com/juju/blobstore/v3 v3.0.2/go.mod h1:NXEgMhrVH5744/zLfSkzsySlDQUpCgzvcNxjJJhICko= -github.com/juju/charm/v11 v11.0.1 h1:N3J+P+xZ+02mzeLvyp5DUlRfG5P/jFoOvJULaJiukZw= -github.com/juju/charm/v11 v11.0.1/go.mod h1:Mge5Ko3pPgocmk4v1pQgmBhF8BuBLGTCFu3jq83JvHk= +github.com/juju/charm/v11 v11.0.2 h1:qVcrG9X5fTN++aBfM4QwzOQRd6h31degr3vD2fTgjxs= +github.com/juju/charm/v11 v11.0.2/go.mod h1:Mge5Ko3pPgocmk4v1pQgmBhF8BuBLGTCFu3jq83JvHk= github.com/juju/clock v0.0.0-20190205081909-9c5c9712527c/go.mod h1:nD0vlnrUjcjJhqN5WuCWZyzfd5AHZAC9/ajvbSx69xA= github.com/juju/clock v0.0.0-20220202072423-1b0f830854c4/go.mod h1:zDZCPSgCJQINeZtQwHx2/cFk4seaBC8Yiqe8V82xiP0= github.com/juju/clock v0.0.0-20220203021603-d9deb868a28a/go.mod h1:GZ/FY8Cqw3KHG6DwRVPUKbSPTAwyrU28xFi5cqZnLsc=