You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by tbouron <gi...@git.apache.org> on 2015/11/11 15:14:05 UTC

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

GitHub user tbouron opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020

    Add TOSCA metadata support for catalog entities and templates

    This implementation adds TOSCA type's metadata as a strongly typed tag into entities and templates catalog items. This information can be retrieved from the REST API, endpoints:
    - /v1/catalog/applications
    - /v1/catalog/entities
    
    TOSCA type will default to `brooklyn.nodes.SoftwareProcess` but can be overridden by specifying:
    - a `Tosca` annotation on Java entities, [here is an example](https://github.com/tbouron/incubator-brooklyn/commit/fe169eab38d6c17afcbc2221914609eaa8e1e1f0#diff-4b98848c850b4fc0e6a3927b3d76f167R43)
    - the TOSCA type when adding a pure CAMP YAML blueprint to the catalog like this:
    ```
    brooklyn.catalog:
      id: my.app
      version: 1.0
      description: My application
      displayName: My application
      itemType: template
      derivedFrom: brooklyn.nodes.WebApplication
    
    services:
    - type: org.apache.brooklyn.entity.stock.BasicApplication
    ```

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/tbouron/incubator-brooklyn tosca-type-definition

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/1020.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1020
    
----
commit fe169eab38d6c17afcbc2221914609eaa8e1e1f0
Author: Thomas Bouron <th...@cloudsoftcorp.com>
Date:   2015-11-11T14:01:45Z

    Add TOSCA metadata support for catalog entities and templates

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020#discussion_r44743504
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemBuilder.java ---
    @@ -121,6 +121,11 @@ public CatalogItemBuilder(CatalogItemType dto) {
             return this;
         }
     
    +    public CatalogItemBuilder<CatalogItemType> tag(Object tag) {
    --- End diff --
    
    good addition but needs test cases


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/1020


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020#issuecomment-157426583
  
    These ideas sound fine to me.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020#discussion_r44743528
  
    --- Diff: usage/rest-api/src/main/java/org/apache/brooklyn/rest/domain/CatalogEntitySummary.java ---
    @@ -46,6 +56,7 @@ public CatalogEntitySummary(
                 @JsonProperty("planYaml") String planYaml,
                 @JsonProperty("description") String description,
                 @JsonProperty("iconUrl") String iconUrl,
    +            @JsonProperty("tags") Set<Object> tags,
    --- End diff --
    
    shouldn't this be on `CatalogItemSummary` ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020#discussion_r44760482
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/BrooklynToscaTags.java ---
    @@ -0,0 +1,57 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.core.catalog.internal;
    +
    +import com.google.common.base.Objects;
    +
    +/**
    + * Strongly types tag used to store TOSCA metadata such as:
    + * <ul>
    + *     <li>TOSCA type</li>
    + *     <li>Requirements</li>
    + *     <li>relationships</li>
    + * </ul>
    + */
    +public class BrooklynToscaTags {
    +
    +    private String derivedFrom = "brooklyn.nodes.SoftwareProcess";
    --- End diff --
    
    This is the default `root` node defined in `brooklyn-tosca` which has a `host` requirement. See [here](https://github.com/cloudsoft/brooklyn-tosca/blob/master/a4c-brooklyn-plugin/src/main/resources/brooklyn/brooklyn-resources/brooklyn-resources.yaml)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020#issuecomment-157421133
  
    > We could re-use/re-purpose @tbouron's Tag related changes to do this
    
    Indeed, the plan would be to keep the `tag()` utility method, then create a `InterfacesTag` that would be generated based upon all implemented interfaces for all `CatalogItem`. This will then be exposed through the REST API. 
    
    Does this looks sensible to you @ahgittin @sjcorbett @nakomis ?
    
    If yes, I'll close this PR and create another one just for the tag support within `CatalogItemSummary`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by nakomis <gi...@git.apache.org>.
Github user nakomis commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020#issuecomment-157418980
  
    Yes, we've thrown around a few ideas here, and feel that it can be generated automatically (or semi-automatically) in the downstream. It would be dependent upon Brooklyn making the interfaces an entity implements available via the REST API, which would probably be a good feature to add anyway. We could re-use/re-purpose @tbouron's Tag related changes to do this
    
    As for how this will work in the downstream project (brooklyn-tosca), I'll put something together and post it to the brooklyn-tosca Slack channel as it's not directly related to Brooklyn


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020#discussion_r44760536
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemBuilder.java ---
    @@ -121,6 +121,11 @@ public CatalogItemBuilder(CatalogItemType dto) {
             return this;
         }
     
    +    public CatalogItemBuilder<CatalogItemType> tag(Object tag) {
    --- End diff --
    
    I'm in a process of creating tests for that, I'll push ASAP


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020#issuecomment-157390067
  
    > That make sense. What about creating a tosca module similar to the camp module? Would that work for you?
    
    I think that might make sense; currently that's what the separate `brooklyn-tosca` project is, in my mind.  It would then be a question of whether that should be promoted to Brooklyn.
    
    Creating a `brooklyn-tosca` project in Brooklyn which doesn't actually do TOSCA but only supplies a bit of metadata feels very confusing.
    
    Sorry I'm even more against the direction this is going.
    
    I understand it is easier for you to do development if TOSCA metadata is specified directly in Brooklyn; but it makes Brooklyn larger and in a way that is harder for other people.  I'd really like to find a way you can do what you need to do without baking TOSCA references in to Brooklyn.  (I see you've made MySql now depend on TOSCA, to get that annotation; bear in mind since inception MySql and Tomat etc have been defined without any reference to CAMP or even YAML, and they still are.)  Any concepts TOSCA wants, it makes sense to consider adding, but in a pure form which could conceivably be useful for non-TOSCA purposes.  Otherwise I think you're best off redeclaring tosca-friendly items in the downstream, or a translation/overlay.
    
    In short I don't think we have enough pull towards TOSCA to yet bake in this metadata in Brooklyn.  At some point that might change but I can't see this merged without at minimum community discussion.
    
    @sjcorbett WDYT?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020#issuecomment-156300646
  
    i'm not sure we want to bake TOSCA (or any standard) in to the `api` project -- brooklyn thus far has been entirely neutral.  (all things camp-specific are contributed through a plugin / services mechanism.)
    
    also as we move towards entities being written in yaml i think we'd want a way to specify tosca derived-from types in yaml.
    
    i'd suggest instead we:
    * add support for tags on catalog items in brooklyn (which you've mostly done)
    * redefine the items you care about using yaml in the downstream `brooklyn-tosca` project including a *string* tag `"tosca-derived-from: xxxx"`, e.g.
    
        id: brooklyn.tosca.nodes.MySql
        tags: [ "tosca-derived-from: tosca.nodes.Database" ]
        item:
          type: o.a.b...MySqlNode



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020#issuecomment-156433483
  
    @ahgittin I addressed comments, I've also added capabilities and requirements. Tests are still missing but I will resolve that soon.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020#issuecomment-157696007
  
    Other PR created, see #1039.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020#discussion_r44743453
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/BrooklynToscaTags.java ---
    @@ -0,0 +1,57 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.core.catalog.internal;
    +
    +import com.google.common.base.Objects;
    +
    +/**
    + * Strongly types tag used to store TOSCA metadata such as:
    + * <ul>
    + *     <li>TOSCA type</li>
    + *     <li>Requirements</li>
    + *     <li>relationships</li>
    + * </ul>
    + */
    +public class BrooklynToscaTags {
    +
    +    private String derivedFrom = "brooklyn.nodes.SoftwareProcess";
    --- End diff --
    
    curious; why equal this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020#issuecomment-157451989
  
    > Probably worth some discussion on list about how to expose supertypes. Adding dozens of tags to support all interfaces is not very nice; probably it is just some which we care about. Also as it happens in #1017 I'm working on a TypeRegistry to replace the catalog, and supertypes is one of the things I'm adding to the RegisteredType which will replace CatalogItem. Maybe you can look at that and help move that ball down the field.
    
    I was more thinking of adding only one tag that will contain the list of all interfaces an entity implements rather than having a tag per interface. I'll look into #1017 and see if I can come up with something.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020#issuecomment-156368715
  
    > i'm not sure we want to bake TOSCA (or any standard) in to the api project -- brooklyn thus far has been entirely neutral. (all things camp-specific are contributed through a plugin / services mechanism.)
    
    That make sense. What about creating a `tosca` module similar to the `camp` module? Would that work for you?
    
    > also as we move towards entities being written in yaml i think we'd want a way to specify tosca derived-from types in yaml.
    
    It's already done, see the example in the PR description.
    
    > i'd suggest instead we:
    > - add support for tags on catalog items in brooklyn (which you've mostly done)
    > - redefine the items you care about using yaml in the downstream brooklyn-tosca project including a string tag "tosca-derived-from: xxxx", e.g.
    > ```
    >    id: brooklyn.tosca.nodes.MySql
    >    tags: [ "tosca-derived-from: tosca.nodes.Database" ]
    >    item:
    >      type: o.a.b...MySqlNode
    >```
    
    Regarding your first point, I added support for each TOSCA metadata rather than tags. I did that because it makes more sense to me to specify each metadata as a property of catalog items, I also find it easier to use and document rather than wrap it into a `tags` object. The underlying implementation (using the `BrooklynToscaTag`) is something behind the hood and blueprint writer should not be aware of it.
    
    Regarding your second point, we want to use existing entities defined in Brooklyn as they get imported by the A4C plugin. We also want to be able to specify TOSCA metadata on Java entities hence the `@Tosca` annotation and the change within the `MysqlNode` entity. We could use only `brooklyn-tosca` project to specify those, add them to the catalog and then import them. But the first approach makes things way easier to test and use.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020#discussion_r44760684
  
    --- Diff: usage/rest-api/src/main/java/org/apache/brooklyn/rest/domain/CatalogEntitySummary.java ---
    @@ -46,6 +56,7 @@ public CatalogEntitySummary(
                 @JsonProperty("planYaml") String planYaml,
                 @JsonProperty("description") String description,
                 @JsonProperty("iconUrl") String iconUrl,
    +            @JsonProperty("tags") Set<Object> tags,
    --- End diff --
    
    True. My first implementation didn't use tags to store TOSCA metadata, that's why I modified the `CatalogEntitySummary`. I'll modify that.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020#issuecomment-157416626
  
    +1 to not writing this directly into Brooklyn's entities.
    
    Our goal should be for brooklyn-tosca to work against any old Brooklyn and to handle entities that haven't been written yet. For that reason I do not favour redefining blueprints items in brooklyn-tosca.
    
    I'd like to see the information that's contained in this PR's `@Tosca` annotation be computed automatically by whatever is ingesting it in brooklyn-tosca. We can do this by inspecting the interfaces each entity implements. Taking the example of the MySQL node in this PR:
    ```
    @Tosca(derivedFrom = "brooklyn.nodes.Database", capabilities = {
            @Tosca.Capability(id = "database_endpoint", type = "tosca.capabilities.Endpoint.Database")
    })
    ```
    Both `derivedFrom` and the capability can be inferred by the fact that `MySqlNode` implements `HasDatastoreUrl`. 
    
    @tbouron @nakomis I believe you have some thoughts in this direction.
    
    I haven't settled on a way of doing the same thing with requirements just yet. I'm not convinced by the addition to the Tomcat8 interface in this PR:
    ```
    @Tosca(requirements = {
            @Tosca.Requirement(id = "database_endpoint", capabilityType = "tosca.capabilities.Endpoint.Database", relationshipType = "brooklyn.relationships.Configure", upperBound = 1)
    })
    ```
    I lean towards either inspecting the types of an entity's config keys or just leaving requirements to the writer of the Tosca topologies.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Add TOSCA metadata support for ca...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/1020#issuecomment-157442362
  
    Great ideas.  I like the idea of using (and introducing if needed) some general purpose interfaces like `HasDatastoreUrl` as markers to map this to a TOSCA type hierarchy.
    
    Yes, add the `tag()` methods for `CatalogItem`; this is useful.  Include some tests please.  (And probably close this PR.)
    
    Probably worth some discussion on list about how to expose supertypes.  Adding dozens of tags to support all interfaces is not very nice; probably it is just some which we care about.  Also as it happens in #1017 I'm working on a `TypeRegistry` to replace the catalog, and supertypes is one of the things I'm adding to the `RegisteredType` which will replace `CatalogItem`.  Maybe you can look at that and help move that ball down the field.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---