Skip to content

Commit

Permalink
Merge pull request juju#17348 from jack-w-shaw/JUJU-5996_deprecate_bu…
Browse files Browse the repository at this point in the history 
…ndles_with_series

juju#17348

Show a deprecation warning when deploying a bundle with series

We are deprecating series in favour of base in general. In Juju 4.0 we will error out if a bundle provides a base. This change in in preparation for this later breaking change

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

Deploy some bundles

```
$ cat ./bundle.yaml
series: focal
applications:
 ubuntu:
 charm: ubuntu
 num_units: 1
 to:
 - "0"
machines:
 "0":

$ juju deploy ./bundle.yaml
WARNING series in being deprecated in favour of bases. For more information about the transition to bases see https://discourse.charmhub.io/t/transition-from-series-to-base-in-juju-4-0/14127
...
```

```
$ cat bundle.yaml 
applications:
 ubuntu:
 series: focal
 charm: ubuntu
 num_units: 1
 to:
 - "0"
machines:
 "0":
 base: [email protected]

$ juju deploy ./bundle.yaml
WARNING series in being deprecated in favour of bases. For more information about the transition to bases see https://discourse.charmhub.io/t/transition-from-series-to-base-in-juju-4-0/14127
...
```

```
$ cat bundle.yaml 
applications:
 ubuntu:
 base: [email protected]
 charm: ubuntu
 num_units: 1
 to:
 - "0"
machines:
 "0":
 series: focal

$ juju deploy ./bundle.yaml
WARNING series in being deprecated in favour of bases. For more information about the transition to bases see https://discourse.charmhub.io/t/transition-from-series-to-base-in-juju-4-0/14127
...
```

```
$ cat ./bundle.yaml
series: focal
default-base: [email protected]
applications:
 ubuntu:
 charm: ubuntu
 num_units: 1
 to:
 - "0"
machines:
 "0":

$ juju deploy ./bundle.yaml
WARNING series in being deprecated in favour of bases. For more information about the transition to bases see https://discourse.charmhub.io/t/transition-from-series-to-base-in-juju-4-0/14127
...
```

```
default-base: [email protected]
applications:
 ubuntu:
 charm: ubuntu
 num_units: 1
 to:
 - "0"
machines:
 "0":

$ juju deploy ./bundle.yaml
...
```

```
$ juju deploy charmed-kubernetes
Located bundle "charmed-kubernetes" in charm-hub, revision 1243
WARNING series in being deprecated in favour of bases. For more information about the transition to bases see https://discourse.charmhub.io/t/transition-from-series-to-base-in-juju-4-0/14127
...
```

## Links

**Jira card:** [JUJU-5596](https://warthogs.atlassian.net/browse/JUJU-5996)

[JUJU-5596]: https://warthogs.atlassian.net/browse/JUJU-5596?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
  • Loading branch information
@jujubot
jujubot authored May 23, 2024
2 parents 8271f5c + c9aaaad commit 8d753a3
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 10 deletions.
32 changes: 29 additions & 3 deletions cmd/juju/application/bundle/bundle.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"strings"

"github.com/juju/charm/v11"
"github.com/juju/cmd/v3"
"github.com/juju/errors"
"github.com/juju/names/v4"

Expand Down Expand Up @@ -250,7 +251,7 @@ func applicationConfigValue(key string, valueMap interface{}) (interface{}, erro
// combined bundle data. Returns a slice of errors encountered while
// processing the bundle. They are for informational purposes and do
// not require failing the bundle deployment.
func ComposeAndVerifyBundle(base BundleDataSource, pathToOverlays []string) (*charm.BundleData, []error, error) {
func ComposeAndVerifyBundle(ctx *cmd.Context, base BundleDataSource, pathToOverlays []string) (*charm.BundleData, []error, error) {
var dsList []charm.BundleDataSource
unMarshallErrors := make([]error, 0)
unMarshallErrors = append(unMarshallErrors, gatherErrors(base)...)
Expand All @@ -269,7 +270,7 @@ func ComposeAndVerifyBundle(base BundleDataSource, pathToOverlays []string) (*ch
if err != nil {
return nil, nil, errors.Trace(err)
}
if err = verifyBundle(bundleData, base.BasePath()); err != nil {
if err = verifyBundle(ctx, bundleData, base.BasePath()); err != nil {
return nil, nil, errors.Trace(err)
}

Expand All @@ -287,7 +288,7 @@ func gatherErrors(ds BundleDataSource) []error {
return returnErrors
}

func verifyBundle(data *charm.BundleData, bundleDir string) error {
func verifyBundle(ctx *cmd.Context, data *charm.BundleData, bundleDir string) error {
verifyConstraints := func(s string) error {
_, err := constraints.Parse(s)
return err
Expand All @@ -301,6 +302,8 @@ func verifyBundle(data *charm.BundleData, bundleDir string) error {
return err
}

deprecationWarningForSeries(ctx, data)

var errs []string
// This method cannot be included within data.Verify because
// to verify corresponding series and base match we need to be
Expand All @@ -326,6 +329,29 @@ func verifyBundle(data *charm.BundleData, bundleDir string) error {
return errors.Trace(verifyError)
}

func deprecationWarningForSeries(ctx *cmd.Context, data *charm.BundleData) {
includeSeries := false
if data.Series != "" {
includeSeries = true
}
for _, m := range data.Machines {
if m != nil && m.Series != "" {
includeSeries = true
break
}
}
for _, app := range data.Applications {
if app != nil && app.Series != "" {
includeSeries = true
break
}
}

if includeSeries {
ctx.Warningf("series in being deprecated in favour of bases. For more information about the transition to bases see https://discourse.charmhub.io/t/transition-from-series-to-base-in-juju-4-0/14127")
}
}

func verifyMixedSeriesBasesMatch(data *charm.BundleData) error {
if data == nil {
return nil
Expand Down
21 changes: 16 additions & 5 deletions cmd/juju/application/bundle/bundle_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"strings"

"github.com/juju/charm/v11"
"github.com/juju/cmd/v3"
"github.com/juju/errors"
jc "github.com/juju/testing/checkers"
"go.uber.org/mock/gomock"
Expand Down Expand Up @@ -215,8 +216,10 @@ func (s *composeAndVerifyRepSuite) TestComposeAndVerifyBundleNoOverlay(c *gc.C)
c.Assert(err, jc.ErrorIsNil)
s.expectParts(&charm.BundleDataPart{Data: bundleData})
s.expectBasePath()
ctx, err := cmd.DefaultContext()
c.Assert(err, jc.ErrorIsNil)

obtained, _, err := ComposeAndVerifyBundle(s.bundleDataSource, nil)
obtained, _, err := ComposeAndVerifyBundle(ctx, s.bundleDataSource, nil)
c.Assert(err, jc.ErrorIsNil)
c.Assert(obtained, gc.DeepEquals, bundleData)
}
Expand All @@ -228,13 +231,15 @@ func (s *composeAndVerifyRepSuite) TestComposeAndVerifyBundleOverlay(c *gc.C) {
s.expectParts(&charm.BundleDataPart{Data: bundleData})
s.expectBasePath()
s.setupOverlayFile(c)
ctx, err := cmd.DefaultContext()
c.Assert(err, jc.ErrorIsNil)

expected := *bundleData
expected.Applications["wordpress"].Options = map[string]interface{}{
"blog-title": "magic bundle config",
}

obtained, _, err := ComposeAndVerifyBundle(s.bundleDataSource, []string{s.overlayFile})
obtained, _, err := ComposeAndVerifyBundle(ctx, s.bundleDataSource, []string{s.overlayFile})
c.Assert(err, jc.ErrorIsNil)
c.Assert(obtained, gc.DeepEquals, &expected)
}
Expand All @@ -250,13 +255,15 @@ func (s *composeAndVerifyRepSuite) TestComposeAndVerifyBundleOverlayUnmarshallEr
})
s.expectBasePath()
s.setupOverlayFile(c)
ctx, err := cmd.DefaultContext()
c.Assert(err, jc.ErrorIsNil)

expected := *bundleData
expected.Applications["wordpress"].Options = map[string]interface{}{
"blog-title": "magic bundle config",
}

obtained, unmarshallErrors, err := ComposeAndVerifyBundle(s.bundleDataSource, []string{s.overlayFile})
obtained, unmarshallErrors, err := ComposeAndVerifyBundle(ctx, s.bundleDataSource, []string{s.overlayFile})
c.Assert(err, jc.ErrorIsNil)
c.Assert(obtained, gc.DeepEquals, &expected)
c.Assert(unmarshallErrors, gc.HasLen, 1)
Expand All @@ -269,8 +276,10 @@ func (s *composeAndVerifyRepSuite) TestComposeAndVerifyBundleMixingBaseAndSeries
c.Assert(err, jc.ErrorIsNil)
s.expectParts(&charm.BundleDataPart{Data: bundleData})
s.expectBasePath()
ctx, err := cmd.DefaultContext()
c.Assert(err, jc.ErrorIsNil)

obtained, _, err := ComposeAndVerifyBundle(s.bundleDataSource, nil)
obtained, _, err := ComposeAndVerifyBundle(ctx, s.bundleDataSource, nil)
c.Assert(err, jc.ErrorIsNil)
c.Assert(obtained, gc.DeepEquals, bundleData)
}
Expand All @@ -281,8 +290,10 @@ func (s *composeAndVerifyRepSuite) TestComposeAndVerifyBundleMixingBaseAndSeries
c.Assert(err, jc.ErrorIsNil)
s.expectParts(&charm.BundleDataPart{Data: bundleData})
s.expectBasePath()
ctx, err := cmd.DefaultContext()
c.Assert(err, jc.ErrorIsNil)

obtained, _, err := ComposeAndVerifyBundle(s.bundleDataSource, nil)
obtained, _, err := ComposeAndVerifyBundle(ctx, s.bundleDataSource, nil)
c.Assert(err, gc.ErrorMatches, `(?s)the provided bundle has the following errors:.*application "wordpress" series "jammy" and base "[email protected]" must match if both supplied.*invalid constraints.*`)
c.Assert(obtained, gc.IsNil)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/juju/application/deployer/bundle.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (d *deployBundle) deploy(

// Compose bundle to be deployed and check its validity before running
// any pre/post checks.
bundleData, unmarshalErrors, err := bundle.ComposeAndVerifyBundle(d.bundleDataSource, d.bundleOverlayFile)
bundleData, unmarshalErrors, err := bundle.ComposeAndVerifyBundle(ctx, d.bundleDataSource, d.bundleOverlayFile)
if err != nil {
return errors.Annotatef(err, "cannot deploy bundle")
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/juju/application/diffbundle.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func (c *diffBundleCommand) Run(ctx *cmd.Context) error {
return errors.Trace(err)
}

bundle, _, err := appbundle.ComposeAndVerifyBundle(baseSrc, c.bundleOverlays)
bundle, _, err := appbundle.ComposeAndVerifyBundle(ctx, baseSrc, c.bundleOverlays)
if err != nil {
return errors.Trace(err)
}
Expand Down

0 comments on commit 8d753a3

Please sign in to comment.