Skip to content

Commit

Permalink
Allow series and base in bundles if they match
Browse files Browse the repository at this point in the history 
Add a verification step into the client to ensure that series and base
match.

As such, we can treat series and bases as equivalent when they show up
precedence-wise.

Immidately combine into a base in core. As a helpful side effect, this
means we can minimise our dealing with series in this part of the code,
which will make it very easy to remove completely
  • Loading branch information
@jack-w-shaw
jack-w-shaw committed Aug 17, 2023
1 parent 6a8e3d5 commit 03e77ee
Show file tree
Hide file tree
Showing 6 changed files with 221 additions and 80 deletions.
57 changes: 57 additions & 0 deletions cmd/juju/application/bundle/bundle.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package bundle

import (
"fmt"
"reflect"
"sort"
"strings"
Expand Down Expand Up @@ -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)
Expand All @@ -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
}
109 changes: 99 additions & 10 deletions cmd/juju/application/bundle/bundle_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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:
Expand All @@ -436,44 +460,44 @@ 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"
wordpress:
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
Expand Down Expand Up @@ -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
`
33 changes: 25 additions & 8 deletions core/bundle/changes/changes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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")
}
Expand Down
Loading

0 comments on commit 03e77ee

Please sign in to comment.