Cross-model relations: consume command #6774

Merged
merged 8 commits into from Jan 6, 2017

Conversation

Projects
None yet
3 participants
Member

babbageclunk commented Jan 6, 2017

Consume adds a remote application to the model without creating any relations to it.

juju consume othermodel.mysql

The remote application can also be specified by URL.

At the moment the command takes no more options than the normal model command options - in the future there will also be a way to specify the local name for the application in this model.

QA steps:

  • Bootstrap and create 2 models, a and b.
  • Deploy mysql to a.
  • Deploy wordpress to b.
  • In b run juju consume a.mysql - it will show the local name for the application.
  • Confirm that you see the remote mysql in status.
  • Connect wordpress to the remote mysql (using the local name).
  • Expose wordpress.
  • Check that it works!

Just a few nitpics.
Also, the api package changes need tests. But can be done as a followup if needed

apiserver/application/application.go
@@ -1079,6 +1079,47 @@ func (api *API) charmIcon(backend Backend, curl *charm.URL) ([]byte, error) {
return common.CharmArchiveEntry(charmPath, "icon.svg", true)
}
+// Consume adds remote applications to the model without
apiserver/application/application.go
+ return "", errors.Trace(err)
+ }
+ if url.HasEndpoint() {
+ return "", errors.New("remote application shouldn't include endpoint")
@wallyworld

wallyworld Jan 6, 2017

Owner

let's include the URL in the error message

@@ -450,5 +450,17 @@ type RemoteApplicationInfoResult struct {
// RemoteApplicationInfoResults represents the result of a RemoteApplicationInfo call.
type RemoteApplicationInfoResults struct {
- Results []RemoteApplicationInfoResult
+ Results []RemoteApplicationInfoResult `json:"results"`
@wallyworld

wallyworld Jan 6, 2017

Owner

oops, thanks!

+ $ juju consume local:/u/fred/db2
+
+See also:
+ add-relation
@wallyworld

wallyworld Jan 6, 2017

Owner

also juju offer

@babbageclunk

babbageclunk Jan 6, 2017

Member

Offer's on the next line! ;)

cmd/juju/application/consume.go
+// relating them to other applications.
+type consumeCommand struct {
+ modelcmd.ModelCommandBase
+ api serviceConsumeAPI
@wallyworld

wallyworld Jan 6, 2017

Owner

s/service/application

@babbageclunk

babbageclunk Jan 6, 2017

Member

Oh yeah. I actually didn't realise that was what that service prefix referred to - the service/application change was just when I started, so it's not as visceral for me.

Fixed.

cmd/juju/commands/main.go
@@ -355,6 +355,9 @@ func registerCommands(r commandRegistry, ctx *cmd.Context) {
r.Register(application.NewUnexposeCommand())
r.Register(application.NewServiceGetConstraintsCommand())
r.Register(application.NewServiceSetConstraintsCommand())
+ if featureflag.Enabled(feature.CrossModelRelations) {
+ r.Register(application.NewConsumeCommand())
@wallyworld

wallyworld Jan 6, 2017

Owner

can we move this next to the other cmr commands

@babbageclunk

babbageclunk Jan 6, 2017

Member

Done. Hmm - should I move it to the crossmodel package?

@wallyworld

wallyworld Jan 6, 2017

Owner

All commands are registered in main.go so it's all ok in here

core/crossmodel/applicationurl.go
@@ -59,6 +59,10 @@ func (u *ApplicationURL) String() string {
return fmt.Sprintf("%s:/%s", u.Directory, u.Path())
}
+func (u *ApplicationURL) HasEndpoint() bool {
@wallyworld

wallyworld Jan 6, 2017

Owner

needs comment

@babbageclunk

babbageclunk Jan 6, 2017

Member

Commented.

core/crossmodel/applicationurl.go
@@ -59,6 +59,10 @@ func (u *ApplicationURL) String() string {
return fmt.Sprintf("%s:/%s", u.Directory, u.Path())
}
+func (u *ApplicationURL) HasEndpoint() bool {
+ return strings.Contains(u.ApplicationName, ":")
@wallyworld

wallyworld Jan 6, 2017

Owner

yeah, we really need to add a proper Endpoint attribute

@wallyworld

wallyworld Jan 6, 2017

Owner

this can be in a followup

Member

babbageclunk commented Jan 6, 2017

$$merge$$

Contributor

jujubot commented Jan 6, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented Jan 6, 2017

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/9973

Member

babbageclunk commented Jan 6, 2017

Fixed test.

$$merge$$

Contributor

jujubot commented Jan 6, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit a2a2a71 into juju:develop Jan 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment