Service to application #210

Merged
merged 6 commits into from May 26, 2016

Conversation

Projects
None yet
4 participants
Owner

wallyworld commented May 25, 2016

"Service" has been renamed to "Application". A few changes:

  1. Use gopkg.in/names.v2 to pick up new application tags
  2. Use yaml.v2 - there were a few old v1 methods around
  3. Introduce unmarshalling of bundle metadata what accepts both services and applications
    (allow older bundle files to be read)

Waiting on names.v2 to be updated. There's a bot issue to get that sorted out.

bundledata.go
+// UnmarshalJSON implements the json.Unmarshaler interface.
+func (bd *BundleData) UnmarshalJSON(b []byte) error {
+ // We account for the fact that the YAML may contain a legacy entry
+ // for "services" instead of "applications".
@axw

axw May 25, 2016

Member

please move the comment in the "if" block

bundledata.go
+
+// UnmarshalYAML implements the yaml.Unmarshaler interface.
+func (bd *BundleData) UnmarshalYAML(f func(interface{}) error) error {
+ // We account for the fact that the YAML may contain a legacy entry
@@ -36,11 +36,11 @@ The OpenStack cloud deployed is completely clean; the charms don't attempt to co
Niggles
-------
-The neutron-gateway service requires a service unit with two network interfaces to provide full functionality; this part of OpenStack provides L3 routing between tenant networks and the rest of the world. Its possible todo this when testing on OpenStack by adding a second network interface to the neutron-gateway service:
+The neutron-gateway application requires an application unit with two network interfaces to provide full functionality; this part of OpenStack provides L3 routing between tenant networks and the rest of the world. Its possible todo this when testing on OpenStack by adding a second network interface to the neutron-gateway application:
@axw

axw May 25, 2016

Member

not sure that we should be changing this file, since it's taken from a real bundle...

@@ -102,66 +102,66 @@ relations:
- mysql:shared-db
- - nova-cloud-controller:amqp
- rabbitmq-server:amqp
- - - nova-cloud-controller:image-service
@axw

axw May 25, 2016

Member

pretty sure those interface names correspond to openstack terms, not juju services/applications. simple sed script? :)

@wallyworld

wallyworld May 25, 2016

Owner

FFS, missed that cleanup, fixed

Member

axw commented May 25, 2016

LGTM with a couple of small things. Are you sure nothing is relying on the yaml.v1 API being implemented?

bundledata.go
)
+type bundleDataCompat struct {
+ // LegacyServices holds application entries for older bundle files
@rogpeppe

rogpeppe May 25, 2016

Owner

How about just embedding BundleData into this struct?
Something like this is simpler, I think, and consolidates the legacy checking
into one place:

type noMethodsBundleData BundleData
type legacyBundleData struct {
    noMethodsBundleData `bson:",inline" yaml:",inline"`
    Services            map[string]*ApplicationSpec `json:"services" yaml:"services" bson:"services"`
}

func (bdc *legacyBundleData) setBundleData(bd *BundleData) error {
    if len(bdc.Applications) > 0 && len(bdc.Services) > 0 {
        return fmt.Errorf("cannot specify both applications and services")
    }
    if len(bdc.Services) > 0 {
        bdc.Applications = bdc.Services
    }
    *bd = BundleData(bdc.noMethodsBundleData)
    return nil
}

func (bd *BundleData) UnmarshalJSON(b []byte) error {
    var bdc legacyBundleData
    if err := json.Unmarshal(b, &bdc); err != nil {
        return err
    }
    return bdc.setBundleData(bd)
}

func (bd *BundleData) UnmarshalYAML(f func(interface{}) error) error {
    var bdc legacyBundleData
    if err := f(&bdc); err != nil {
        return err
    }
    return bdc.setBundleData(bd)
}

func (bd *BundleData) SetBSON(raw bson.Raw) error {
    var bdc legacyBundleData
    if err := raw.Unmarshal(&bdc); err != nil {
        return err
    }
    return bdc.setBundleData(bd)
}
@wallyworld

wallyworld May 26, 2016

Owner

Nice idea, I forgot about the inline tag

bundledata_test.go
+ Relations: [][]string{
+ {"wordpress:db", "mysql:db"},
+ },
+ },
}}
@rogpeppe

rogpeppe May 25, 2016

Owner

Let's have a test for a bundle that specifies both service and applications, please.
(I think that should be an error).

Owner

rogpeppe commented May 25, 2016

I have an implementation suggestion for the unmarshaling logic above.

I think I'd probably leave the yaml.v1 interface intact as axw suggests.

Also, it would be good to have some round-trip marshal/unmarshal tests
for BundleData as there are with Meta, and compatibility unmarshal tests
for json and bson too.

Otherwise LGTM, thanks!

Owner

wallyworld commented May 26, 2016

$$merge$$

Contributor

jujubot commented May 26, 2016

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

@jujubot jujubot merged commit 54511ff into v6-unstable May 26, 2016

@wallyworld wallyworld deleted the service-to-application branch May 26, 2016

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