Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JUJU-1019] Bug 1969929 bundle revision only #13996

Merged
merged 9 commits into from
May 11, 2022
5 changes: 3 additions & 2 deletions api/client/charms/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
api "github.com/juju/juju/api/client/resources"
apicharm "github.com/juju/juju/api/common/charm"
commoncharms "github.com/juju/juju/api/common/charms"
apiservererrors "github.com/juju/juju/apiserver/errors"
"github.com/juju/juju/rpc/params"
)

Expand Down Expand Up @@ -77,12 +78,12 @@ func (c *Client) ResolveCharms(charms []CharmToResolve) ([]ResolvedCharm, error)
}
var result params.ResolveCharmWithChannelResults
if err := c.facade.FacadeCall("ResolveCharms", args, &result); err != nil {
return nil, errors.Trace(err)
return nil, errors.Trace(apiservererrors.RestoreError(err))
}
resolvedCharms := make([]ResolvedCharm, len(charms))
for i, r := range result.Results {
if r.Error != nil {
resolvedCharms[i] = ResolvedCharm{Error: r.Error}
resolvedCharms[i] = ResolvedCharm{Error: apiservererrors.RestoreError(r.Error)}
continue
}
curl, err := charm.ParseURL(r.URL)
Expand Down
2 changes: 1 addition & 1 deletion apiserver/apiserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ func (s *apiserverSuite) TestEmbeddedCommandInvalidUser(c *gc.C) {
User: "123@",
Commands: []string{"status --color"},
}
s.assertEmbeddedCommand(c, cmdArgs, "", &params.Error{Message: `user name "123@" not valid`})
s.assertEmbeddedCommand(c, cmdArgs, "", &params.Error{Message: `user name "123@" not valid`, Code: params.CodeNotValid})
}

func (s *apiserverSuite) TestEmbeddedCommandInvalidMacaroon(c *gc.C) {
Expand Down
7 changes: 7 additions & 0 deletions apiserver/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,8 @@ func ServerError(err error) *params.Error {
code = params.CodeNotImplemented
case errors.IsForbidden(err):
code = params.CodeForbidden
case errors.IsNotValid(err):
code = params.CodeNotValid
case IsIncompatibleSeriesError(err), stateerrors.IsIncompatibleSeriesError(err):
code = params.CodeIncompatibleSeries
case IsDischargeRequiredError(err):
Expand Down Expand Up @@ -284,6 +286,9 @@ func DestroyErr(desc string, ids []string, errs []error) error {
// RestoreError makes a best effort at converting the given error
// back into an error originally converted by ServerError().
func RestoreError(err error) error {
if err == nil {
return nil
}
err = errors.Cause(err)

if apiErr, ok := err.(*params.Error); !ok {
Expand Down Expand Up @@ -367,6 +372,8 @@ func RestoreError(err error) error {
return errors.NewNotYetAvailable(nil, msg)
case params.IsCodeTryAgain(err):
return ErrTryAgain
case params.IsCodeNotValid(err):
return errors.NewNotValid(nil, msg)
default:
return err
}
Expand Down
12 changes: 6 additions & 6 deletions apiserver/facades/agent/secretsmanager/secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,9 @@ func (s *SecretsManagerSuite) TestCreateSecrets(c *gc.C) {
Results: []params.StringResult{{
Result: URL.WithRevision(1).ShortString(),
}, {
Error: &params.Error{Message: `rotate interval "-1h0m0s" not valid`},
Error: &params.Error{Message: `rotate interval "-1h0m0s" not valid`, Code: params.CodeNotValid},
}, {
Error: &params.Error{Message: `empty secret value not valid`},
Error: &params.Error{Message: `empty secret value not valid`, Code: params.CodeNotValid},
}},
})
}
Expand Down Expand Up @@ -201,15 +201,15 @@ func (s *SecretsManagerSuite) TestUpdateSecrets(c *gc.C) {
}, {
Error: &params.Error{Message: `at least one attribute to update must be specified`},
}, {
Error: &params.Error{Message: `rotate interval -1h0m0s not valid`},
Error: &params.Error{Code: params.CodeNotValid, Message: `rotate interval -1h0m0s not valid`},
}, {
Error: &params.Error{Code: "not supported", Message: `updating a single secret attribute "password" not supported`},
}, {
Error: &params.Error{Code: "not supported", Message: `updating secret revision 2 not supported`},
}, {
Error: &params.Error{Code: "", Message: `secret URL with controller UUID "deadbeef-1bad-500d-9000-4b1d0d061111" not valid`},
Error: &params.Error{Code: params.CodeNotValid, Message: `secret URL with controller UUID "deadbeef-1bad-500d-9000-4b1d0d061111" not valid`},
}, {
Error: &params.Error{Code: "", Message: `secret URL with model UUID "deadbeef-1bad-500d-9000-4b1d0d061111" not valid`},
Error: &params.Error{Code: params.CodeNotValid, Message: `secret URL with model UUID "deadbeef-1bad-500d-9000-4b1d0d061111" not valid`},
}},
})
}
Expand Down Expand Up @@ -328,7 +328,7 @@ func (s *SecretsManagerSuite) TestSecretsRotated(c *gc.C) {
Error: &params.Error{Code: "", Message: `boom`},
},
{
Error: &params.Error{Code: "", Message: `secret URL scheme "" not valid`},
Error: &params.Error{Code: params.CodeNotValid, Message: `secret URL scheme "" not valid`},
},
},
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1307,7 +1307,7 @@ func (s *iaasProvisionerSuite) TestVolumeBlockDevices(c *gc.C) {
{Error: apiservertesting.ErrUnauthorized},
{Error: apiservertesting.ErrUnauthorized},
{Error: apiservertesting.ErrUnauthorized},
{Error: &params.Error{Message: `volume attachment host tag "application-mysql" not valid`}},
{Error: &params.Error{Code: params.CodeNotValid, Message: `volume attachment host tag "application-mysql" not valid`}},
},
})
}
Expand Down
1 change: 1 addition & 0 deletions apiserver/facades/agent/uniter/uniter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4646,6 +4646,7 @@ func (s *uniterNetworkInfoSuite) TestNetworkInfoPermissions(c *gc.C) {
Results: map[string]params.NetworkInfoResult{
"unknown": {
Error: &params.Error{
Code: params.CodeNotValid,
Message: `undefined for unit charm: endpoint "unknown" not valid`,
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func (p *instanceTypesSuite) TestInstanceTypes(c *gc.C) {
{
Error: &params.Error{Message: "Instances matching constraint not found", Code: "not found"}},
{
Error: &params.Error{Message: "asking gce cloud information to aws cloud not valid", Code: ""}}}
Error: &params.Error{Message: "asking gce cloud information to aws cloud not valid", Code: "not valid"}}}
c.Assert(r.Results, gc.DeepEquals, expected)
}

Expand Down
2 changes: 1 addition & 1 deletion apiserver/facades/client/metricsdebug/metricsdebug_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func (s *metricsDebugSuite) TestSetMeterStatus(c *gc.C) {
},
assert: func(c *gc.C, results params.ErrorResults) {
err := results.OneError()
c.Assert(err, gc.DeepEquals, &params.Error{Message: "meter status \"NOT AVAILABLE\" not valid"})
c.Assert(err, gc.DeepEquals, &params.Error{Message: "meter status \"NOT AVAILABLE\" not valid", Code: params.CodeNotValid})
},
}, {
about: "not such application",
Expand Down
2 changes: 1 addition & 1 deletion apiserver/facades/client/storage/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,7 @@ func (s *storageSuite) TestImportValidationErrors(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
c.Assert(results.Results, jc.DeepEquals, []params.ImportStorageResult{
{Error: &params.Error{Message: `storage kind "block" not supported`, Code: "not supported"}},
{Error: &params.Error{Message: `pool name "123" not valid`}},
{Error: &params.Error{Message: `pool name "123" not valid`, Code: `not valid`}},
})
}

Expand Down
4 changes: 2 additions & 2 deletions apiserver/registration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func (s *registrationSuite) TestRegisterInvalidUserTag(c *gc.C) {

func (s *registrationSuite) TestRegisterInvalidNonce(c *gc.C) {
s.testInvalidRequest(
c, `{"user": "user-bob", "nonce": ""}`, `nonce not valid`, "",
c, `{"user": "user-bob", "nonce": ""}`, `nonce not valid`, params.CodeNotValid,
http.StatusInternalServerError,
)
}
Expand All @@ -130,7 +130,7 @@ func (s *registrationSuite) TestRegisterInvalidCiphertext(c *gc.C) {
fmt.Sprintf(
`{"user": "user-bob", "nonce": "%s"}`,
base64.StdEncoding.EncodeToString(validNonce),
), `secret key not valid`, "",
), `secret key not valid`, params.CodeNotValid,
http.StatusInternalServerError,
)
}
Expand Down
13 changes: 7 additions & 6 deletions cmd/juju/application/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,14 @@ func (c *DeployCommand) SetFlags(f *gnuflag.FlagSet) {
c.flagSet = f
}

// Init validates the flags.
func (c *DeployCommand) Init(args []string) error {
// NOTE: For deploying a charm with the revision flag, a channel is
// also required. It's required to ensure that juju knows which channel
// should be used for refreshing/upgrading the charm in the future.However
// a bundle does not require a channel, today you cannot refresh/upgrade
// a bundle, only the components. These flags will be verified in the
// GetDeployer instead.
if err := c.validateStorageByModelType(); err != nil {
if !errors.IsNotFound(err) {
return errors.Trace(err)
Expand Down Expand Up @@ -715,12 +722,6 @@ func (c *DeployCommand) Init(args []string) error {
// do a late validation at Run().
c.unknownModel = true
}
if c.channelStr == "" && c.Revision != -1 {
// Tell the user they need to specify a channel
return errors.New(
`when using --revision option, you must also use --channel option`,
)
}
if c.channelStr != "" {
c.Channel, err = charm.ParseChannelNormalize(c.channelStr)
if err != nil {
Expand Down
5 changes: 1 addition & 4 deletions cmd/juju/application/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,6 @@ var initErrorTests = []struct {
}, {
args: []string{"bundle", "--map-machines", "foo"},
err: `error in --map-machines: expected "existing" or "<bundle-id>=<machine-id>", got "foo"`,
}, {
args: []string{"charm-name", "--revision", "3"},
err: `when using --revision option, you must also use --channel option`,
},
}

Expand Down Expand Up @@ -438,7 +435,7 @@ func (s *DeploySuite) TestDeployFromPath(c *gc.C) {
func (s *DeploySuite) TestDeployFromPathUnsupportedSeries(c *gc.C) {
path := testcharms.RepoWithSeries("bionic").ClonedDirPath(c.MkDir(), "multi-series")
err := s.runDeploy(c, path, "--series", "quantal")
c.Assert(err, gc.ErrorMatches, `series "quantal" not supported by charm, supported series are: precise,trusty,xenial,yakkety,bionic`)
c.Assert(err, gc.ErrorMatches, `series "quantal" not supported by charm, supported series are: precise, trusty, xenial, yakkety, bionic`)
}

func (s *DeploySuite) TestDeployFromPathUnsupportedSeriesForce(c *gc.C) {
Expand Down
10 changes: 7 additions & 3 deletions cmd/juju/application/deployer/deployer.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,15 +491,19 @@ func (d *factory) repositoryCharm() (Deployer, error) {
if err != nil {
return nil, errors.Trace(err)
}
if charm.CharmHub.Matches(userRequestedURL.Schema) && d.channel.Empty() && d.revision != -1 {
// Tell the user they need to specify a channel
return nil, errors.Errorf("specifying a revision requires a channel for future upgrades. Please use --channel")
}
// To deploy by revision, the revision number must be in the origin for a
// charmhub charm and in the url for a charmstore charm.
if charm.CharmHub.Matches(userRequestedURL.Schema) {
if userRequestedURL.Revision != -1 {
return nil, errors.Errorf("cannot specify revision in a charm or bundle name. Please use --revision.")
}
if d.revision != -1 && d.channel.Empty() {
return nil, errors.Errorf("specifying a revision requires a channel for future upgrades. Please use --channel")
}
//if d.revision != -1 && d.channel.Empty() {
// return nil, errors.Errorf("specifying a revision requires a channel for future upgrades. Please use --channel")
//}
} else if charm.CharmStore.Matches(userRequestedURL.Schema) {
if userRequestedURL.Revision != -1 && d.revision != -1 && userRequestedURL.Revision != d.revision {
return nil, errors.Errorf("two different revisions to deploy: specified %d and %d, please choose one.", userRequestedURL.Revision, d.revision)
Expand Down
1 change: 1 addition & 0 deletions cmd/juju/application/deployer/deployer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ func (s *deployerSuite) TestGetDeployerCharmStoreCharmWithRevision(c *gc.C) {
func (s *deployerSuite) TestGetDeployerCharmHubCharmWithRevision(c *gc.C) {
cfg := s.channelDeployerConfig()
cfg.Revision = 42
cfg.Channel, _ = charm.ParseChannel("stable")
ch := charm.MustParseURL("ch:test-charm")
deployer, err := s.testGetDeployerRepositoryCharmWithRevision(c, ch, cfg)
c.Assert(err, jc.ErrorIsNil)
Expand Down
4 changes: 2 additions & 2 deletions cmd/juju/application/deployer/series_selector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func (s *SeriesSelectorSuite) TestCharmSeries(c *gc.C) {
conf: defaultSeries{},
supportedJujuSeries: set.NewStrings("bionic", "cosmic"),
},
err: `series "bionic" not supported by charm, supported series are: utopic,vivid`,
err: `series "bionic" not supported by charm, supported series are: utopic, vivid`,
},
{
title: "juju deploy multiseries --series=bionic --force # unsupported forced",
Expand Down Expand Up @@ -254,7 +254,7 @@ func (s *SeriesSelectorSuite) TestCharmSeries(c *gc.C) {
supportedSeries: []string{"bionic", "utopic", "vivid"},
conf: defaultSeries{},
},
err: `series "cosmic" not supported by charm, supported series are: bionic,utopic,vivid`,
err: `series "cosmic" not supported by charm, supported series are: bionic, utopic, vivid`,
},
{
title: "juju deploy bionic/multiseries --series=cosmic # unsupported series",
Expand Down
5 changes: 4 additions & 1 deletion cmd/juju/application/store/charmadapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

apicharm "github.com/juju/juju/api/client/charms"
commoncharm "github.com/juju/juju/api/common/charm"
apiservererrors "github.com/juju/juju/apiserver/errors"
"github.com/juju/juju/charmhub"
"github.com/juju/juju/charmhub/transport"
)
Expand Down Expand Up @@ -109,7 +110,9 @@ func (c *CharmAdaptor) resolveCharmFallback(url *charm.URL, preferredOrigin comm

resultURL, channel, supportedSeries, err := charmRepo.ResolveWithPreferredChannel(url, csparams.Channel(preferredOrigin.Risk))
if err != nil {
return nil, commoncharm.Origin{}, nil, errors.Trace(err)
// Ideally the would be restored before now, however the
// callee is in the core package, let's not add dependecies there.
return nil, commoncharm.Origin{}, nil, errors.Trace(apiservererrors.RestoreError(err))
}
origin := preferredOrigin
origin.Risk = string(channel)
Expand Down
2 changes: 1 addition & 1 deletion core/charm/repository/charmhub_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ func (selectNextBaseSuite) TestSelectNextBaseWithInvalidBaseChannel(c *gc.C) {
Architecture: "amd64",
},
})
c.Assert(err, gc.ErrorMatches, `base: channel cannot be empty`)
c.Assert(err, gc.ErrorMatches, `base: empty channel not valid`)
}

func (selectNextBaseSuite) TestSelectNextBaseWithValidBases(c *gc.C) {
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ require (
github.com/im7mortal/kmutex v1.0.1
github.com/juju/ansiterm v0.0.0-20210929141451-8b71cc96ebdc
github.com/juju/blobstore/v2 v2.0.0-20220204135513-4876189e56a4
github.com/juju/charm/v8 v8.0.0-20220207013334-ec6de7e9b78e
github.com/juju/charm/v8 v8.0.0-20220509231111-ed6d505a46f4
github.com/juju/charmrepo/v6 v6.0.0-20220207014006-e6af52d614e4
github.com/juju/clock v0.0.0-20220203021603-d9deb868a28a
github.com/juju/cmd/v3 v3.0.0-20220203030511-039f3566372a
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -434,8 +434,8 @@ github.com/juju/ansiterm v0.0.0-20210929141451-8b71cc96ebdc h1:ZQrgZFsLzkw7o3CoD
github.com/juju/ansiterm v0.0.0-20210929141451-8b71cc96ebdc/go.mod h1:PyXUpnI3olx3bsPcHt98FGPX/KCFZ1Fi+hw1XLI6384=
github.com/juju/blobstore/v2 v2.0.0-20220204135513-4876189e56a4 h1:OQVAtaX0oZQX1roCwPMXM7DSnmlvDQXxxcl6tn73GiU=
github.com/juju/blobstore/v2 v2.0.0-20220204135513-4876189e56a4/go.mod h1:lFTSngYRJBy/mUOWUjAghwKcP0iXxqBYqJtuchC3cJI=
github.com/juju/charm/v8 v8.0.0-20220207013334-ec6de7e9b78e h1:fu+YIqgQbwlSl1Wird+vnZDqmLhk0LqjcDiZZvA9MQk=
github.com/juju/charm/v8 v8.0.0-20220207013334-ec6de7e9b78e/go.mod h1:tZ0JfWOdv11qu4Gm5lPD0KHBeuVUH2vbrKFyYS6JUAw=
github.com/juju/charm/v8 v8.0.0-20220509231111-ed6d505a46f4 h1:7Zqo515Y4k21nJtf5tC6vyVx3tw4jyd67HfXoSV60Z0=
github.com/juju/charm/v8 v8.0.0-20220509231111-ed6d505a46f4/go.mod h1:tZ0JfWOdv11qu4Gm5lPD0KHBeuVUH2vbrKFyYS6JUAw=
github.com/juju/charmrepo/v6 v6.0.0-20220207014006-e6af52d614e4 h1:mx+an4ZTHZ+VYufUfniTz9P2MzBqdnua57bLkIq2VXE=
github.com/juju/charmrepo/v6 v6.0.0-20220207014006-e6af52d614e4/go.mod h1:JvfFzbn5ckwtdB4UW2xIy4jG62ulPPlMf0+S1PZsO2k=
github.com/juju/clock v0.0.0-20190205081909-9c5c9712527c/go.mod h1:nD0vlnrUjcjJhqN5WuCWZyzfd5AHZAC9/ajvbSx69xA=
Expand Down
5 changes: 5 additions & 0 deletions rpc/params/apierror.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ const (
CodeDeadlineExceeded = "deadline exceeded"
CodeLeaseError = "lease error"
CodeNotYetAvailable = "not yet available; try again later"
CodeNotValid = "not valid"
)

// ErrCode returns the error code associated with
Expand Down Expand Up @@ -269,6 +270,10 @@ func IsCodeNotYetAvailable(err error) bool {
return ErrCode(err) == CodeNotYetAvailable
}

func IsCodeNotValid(err error) bool {
return ErrCode(err) == CodeNotValid
}

// IsCodeNotFoundOrCodeUnauthorized is used in API clients which,
// pre-API, used errors.IsNotFound; this is because an API client is
// not necessarily privileged to know about the existence or otherwise
Expand Down
1 change: 1 addition & 0 deletions worker/uniter/runner/context/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ func (s *InterfaceSuite) TestUnitNetworkInfo(c *gc.C) {
"unknown": {
Error: &params.Error{
Message: `undefined for unit charm: endpoint "unknown" not valid`,
Code: params.CodeNotValid,
},
},
},
Expand Down