Permalink
Browse files

state: reject deploying charm with invalid OS

If a charm specifies supported series, do not allow
deployment of the charm with series of operating
systems other than those of the supported series.

Fixes https://bugs.launchpad.net/juju-core/+bug/1526296
  • Loading branch information...
1 parent 31d1381 commit ff86e5c5413b2920986dc2769d57c6adadf8237f @axw axw committed Dec 16, 2015
Showing with 77 additions and 10 deletions.
  1. +44 −10 state/state.go
  2. +33 −0 state/state_test.go
View
@@ -21,6 +21,8 @@ import (
"github.com/juju/names"
jujutxn "github.com/juju/txn"
"github.com/juju/utils"
+ "github.com/juju/utils/os"
+ "github.com/juju/utils/series"
"github.com/juju/utils/set"
"gopkg.in/juju/charm.v6-unstable"
"gopkg.in/mgo.v2"
@@ -1151,13 +1153,45 @@ func (st *State) AddService(args AddServiceArgs) (service *Service, err error) {
storagePools.Add(storageParams.Pool)
}
- series := args.Series
- if series == "" {
- series = args.Charm.URL().Series
- }
- // Should not happen, but just in case.
- if series == "" {
- return nil, errors.New("series is empty")
+ if args.Series == "" {
+ // args.Series is not set, so use the series in the URL.
+ args.Series = args.Charm.URL().Series
+ if args.Series == "" {
+ // Should not happen, but just in case.
+ return nil, errors.New("series is empty")
+ }
+ } else {
+ // User has specified series. Overriding supported series is
+ // handled by the client, so args.Series is not necessarily
+ // one of the charm's supported series. We require that the
+ // specified series is of the same operating system as one of
+ // the supported series. For old-style charms with the series
+ // in the URL, that series is the one and only supported
+ // series.
+ var supportedSeries []string
+ if series := args.Charm.URL().Series; series != "" {
+ supportedSeries = []string{series}
+ } else {
+ supportedSeries = args.Charm.Meta().Series
+ }
+ seriesOS, err := series.GetOSFromSeries(args.Series)
+ if err != nil {
+ return nil, errors.Trace(err)
+ }
+ supportedOperatingSystems := make(map[os.OSType]bool)
+ for _, supportedSeries := range supportedSeries {
+ os, err := series.GetOSFromSeries(supportedSeries)
+ if err != nil {
+ return nil, errors.Trace(err)
+ }
+ supportedOperatingSystems[os] = true
+ }
+ if !supportedOperatingSystems[seriesOS] {
+ return nil, errors.NewNotSupported(errors.Errorf(
+ "series %q (OS %q) not supported by charm",
+ args.Series, seriesOS,
+ ), "")
+ }
}
for _, placement := range args.Placement {
@@ -1174,15 +1208,15 @@ func (st *State) AddService(args AddServiceArgs) (service *Service, err error) {
}
subordinate := args.Charm.Meta().Subordinate
if err := validateUnitMachineAssignment(
- m, series, subordinate, storagePools,
+ m, args.Series, subordinate, storagePools,
); err != nil {
return nil, errors.Annotatef(
err, "cannot deploy to machine %s", m,
)
}
case directivePlacement:
- if err := st.precheckInstance(series, args.Constraints, data.directive); err != nil {
+ if err := st.precheckInstance(args.Series, args.Constraints, data.directive); err != nil {
return nil, errors.Trace(err)
}
}
@@ -1195,7 +1229,7 @@ func (st *State) AddService(args AddServiceArgs) (service *Service, err error) {
DocID: serviceID,
Name: args.Name,
EnvUUID: env.UUID(),
- Series: series,
+ Series: args.Series,
Subordinate: args.Charm.Meta().Subordinate,
CharmURL: args.Charm.URL(),
RelationCount: len(peers),
View
@@ -1954,6 +1954,39 @@ func (s *StateSuite) TestAddServiceMachinePlacementInvalidSeries(c *gc.C) {
c.Assert(err, gc.ErrorMatches, "cannot add service \"wordpress\": cannot deploy to machine .*: series does not match")
}
+func (s *StateSuite) TestAddServiceIncompatibleOSWithSeriesInURL(c *gc.C) {
+ charm := s.AddTestingCharm(c, "dummy")
+ // A charm with a series in its URL is implicitly supported by that
+ // series only.
+ _, err := s.State.AddService(state.AddServiceArgs{
+ Name: "wordpress", Owner: s.Owner.String(), Charm: charm,
+ Series: "centos7",
+ })
+ c.Assert(err, gc.ErrorMatches, "cannot add service \"wordpress\": series \"centos7\" \\(OS \"CentOS\"\\) not supported by charm")
+}
+
+func (s *StateSuite) TestAddServiceCompatibleOSWithSeriesInURL(c *gc.C) {
+ charm := s.AddTestingCharm(c, "dummy")
+ // A charm with a series in its URL is implicitly supported by that
+ // series only.
+ _, err := s.State.AddService(state.AddServiceArgs{
+ Name: "wordpress", Owner: s.Owner.String(), Charm: charm,
+ Series: charm.URL().Series,
+ })
+ c.Assert(err, jc.ErrorIsNil)
+}
+
+func (s *StateSuite) TestAddServiceOSIncompatibleWithSupportedSeries(c *gc.C) {
+ charm := state.AddTestingCharmMultiSeries(c, s.State, "multi-series")
+ // A charm with supported series can only be force-deployed to series
+ // of the same operating systems as the suppoted series.
+ _, err := s.State.AddService(state.AddServiceArgs{
+ Name: "wordpress", Owner: s.Owner.String(), Charm: charm,
+ Series: "centos7",
+ })
+ c.Assert(err, gc.ErrorMatches, "cannot add service \"wordpress\": series \"centos7\" \\(OS \"CentOS\"\\) not supported by charm")
+}
+
func (s *StateSuite) TestAllServices(c *gc.C) {
charm := s.AddTestingCharm(c, "dummy")
services, err := s.State.AllServices()

0 comments on commit ff86e5c

Please sign in to comment.