Hi Aled, all. Sounds good to me. Once the PR is merged, I'll push on another one to fix the other issues I found in the past 2 days.
I also agree with you on 2 and 3, I'll address that. Best. On Thu, 23 Mar 2017 at 11:02 Aled Sage <[email protected]> wrote: > Hi Thomas, all, > > I think "bundle: ..." is actually a much better name to use in the > catalog.bom than "symbolicName: ...". The latter makes sense to those > who know OSGi, but otherwise doesn't suggest anything about "bundling". > > --- > > I think we're very close to merging [1] - further improvements can > continue incrementally in subsequent PRs, rather than all of this having > to be done in [1]. > > --- > > There's a change compared to the mailing list discussion, I think [2,3]. > Alex has implemented it so that if "symbolicName" and "id" are used at > the top-level of the catalog.bom, then they *must match*. This seems > wrong to me. It's conflating the name spaces of OSGi bundles and catalog > items. I suggest we revert back to (at least how I interpretted) the > previous mailing list discussion: if "bundle: " is present, then that is > used; if "bundle" is absent, then we use "id"; if both are absent, then > we fail. > > Aled > > p.s. As an aside, we should deprecate (and soon delete) support for > "symbolicName" in a catalog.bom - one should use id/version. Nowhere in > our docs to we say symbolicName. It's just yet another example of us > supporting multiple ways of doing the same thing, where it's not at all > clear they mean the same thing, and risking confusing users. > > [1] https://github.com/apache/brooklyn-server/pull/485 > [2] > https://github.com/apache/brooklyn-server/pull/485#discussion_r107144368 > [3] > https://github.com/apache/brooklyn-server/pull/485#issuecomment-288052012 > > > On 23/03/2017 10:23, Geoff Macartney wrote: > > some further thoughts: > > > > re. 1 I agree we should discriminate between the "symbolicName" of the > > bundle and use of the term "symbolicName" in the definition of catalog > ids. > > (I'm not sure if just putting it into a "manifest" subsection will keep > it > > separate, because of the use of "getFirstAs" at > > > https://github.com/apache/brooklyn-server/blob/master/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java#L542 > > ). > > > > re. use of "CatalogPopulator" I think we shall want to factor apart > > CatalogBomScanner/CatalogPopulator into separate classes, - > > - CatalogBomScanner to be the management context service listener > > - CatalogPopulator becomes: > > -- CatalogBundleTracker to do the BundleTracker stuff > > -- CatalogBundleLoader - separate out the handling of the loading of the > > "catalog.bom" from a given bundle > > > > and then 485 can use CatalogBundleLoader. We'll still need to think > about > > the delete side of things, and handling errors, as Thomas says. > > > > re. the Swagger issue, an alternative would be that we could just drop > > "createFromYaml" and "createFromArchive" and let "createPoly" continue to > > do as it does, handling either case; maybe best renamed > "createFromUpload". > > > > Geoff > > > > On Thu, 23 Mar 2017 at 09:40 Thomas Bouron < > [email protected]> > > wrote: > > > >> Hi all. > >> > >> So I spent time to review and test [1] and here are the few issues I > bumped > >> into: > >> > >> 1. the use of `symbolicName` for the bundle clashes Brooklyn: the > >> `BasicBrooklynCatalog` watches this key to define catalog items IDs > [2] > >> 2. because of 1. the bundle that I'm trying to install fails. But > upon > >> failure, the bundle is not uninstalled so any subsequent bundle > addition > >> fail with the first error > >> 3. we now have multiple API definitions for the same endpoint (i.e. > >> /v1/catalog for yaml and ZIP/JAR) While this is perfectly valid, > Swagger > >> does not support it [3]. It means that the Swagger UI displays only > the > >> latest API definition registered, in this case, that's the one to > upload > >> ZIP/JAR. > >> > >> For 1. I can see 2 solutions: using a special section `manifest` where > we > >> put the `symbolicName` and `version`, or changing the field name to > >> `bundle` (I prefer option 2) > >> For 2. we need to add some logic to uninstall the bundle on failure into > >> the `CatalogPopulator`, as per as Geoff comment[4] > >> For 3. even the new Swagger spec (called now OpenAPI) does not support > >> multiple definitions on the same endpoint [5]. I'm not sure if we can > leave > >> with that. One solution would be to split the endpoint to create > specific > >> one, based on the type of upload. > >> > >> [1] https://github.com/apache/brooklyn-server/pull/485 > >> [2] > >> > >> > https://github.com/apache/brooklyn-server/blob/master/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java#L542 > >> [3] https://github.com/swagger-api/swagger-core/issues/935 > >> [4] > >> > https://github.com/apache/brooklyn-server/pull/485#discussion_r107482279 > >> [5] https://github.com/OAI/OpenAPI-Specification/issues/182 > >> > >> On Mon, 20 Mar 2017 at 15:16 Thomas Bouron < > >> [email protected]> > >> wrote: > >> > >> Hi Brooklyners. > >> > >> Uploading ZIPs is really something I'm excited to see coming to life so > I > >> would like to help to push this forward. > >> > >> I've re-read this email thread and it sounds like we have a consensus on > >> the "one true way" approach, i.e. always requiring the `symbolicName` > and > >> `version` when uploading ZIP/JAR (that includes Svet in his previous > >> email). > >> > >> One thing not explicitly settled is the format of bom for those 2 > fields: > >> Alex prefers to put these at the root, Aled in a `manifest` section. I > >> don't have any strong opinion on this: I was leaning toward Aled's > >> suggestion but the argument that says this applies to the whole bundle > make > >> sense so I now think putting this at the root is probably better. I can > see > >> that Alex already updated his PR[1] to this structure. What the people > >> think about this? > >> > >> I'm going to review [1] but are we happy with this consensus? One thing > >> that will need to be addressed is the persistence/rebind issues but this > >> can be tackled in following PRs. > >> > >> Best. > >> > >> [1] https://github.com/apache/brooklyn-server/pull/485 > >> > >> On Tue, 7 Mar 2017 at 16:18 Svetoslav Neykov < > >> [email protected]> wrote: > >> > >> Having quickly scanned the thread I still don't see a reason why the > >> requirement to have a consistent naming for the bundle itself which is > not > >> user visible? Could instead have behaviour similar to the machinNamer > here. > >> > >> Slight preference for optional symbolicName when it's a jar file. > Though I > >> really like Geoff's idea of "one true way". The workflow to create a jar > >> file is very different from creating a zip file (either from a folder or > >> user supplied) so don't think they conflict. > >> > >> Svet. > >> > >> > >>> On 7.03.2017 г., at 17:47, Alex Heneveld < > >> [email protected]> > >> wrote: > >>> > >>> Considering the points made I'm pretty sure it's the case that > requiring > >> a symbolic name and version in the BOM presents little if any > unnecessary > >> burden, in which case I'm persuaded of the "one true way" philosophy. I > >> really like it. :) > >>> Can anyone think of a case where it would be a burden? My thinking is > >> that: > >>> * If you're uploading a ZIP of YAML and scripts and icons, you have a > >> catalog.bom and that's a useful place to require the symname and version > >>> * If your ZIP/JAR also has a MANIFEST.MF the symname and version will > be > >> present in it also; the user is duplicating that info (this is the > "little" > >> unnecssary burden) but it's tiny and the benefit of one true way > dominates; > >> we do require that they match > >>> * If your ZIP/JAR doesn't have a BOM there's no point in uploading it > to > >> the catalog endpoint; either it's a JAR but this isn't meant as an > "install > >> arbitrary bundles" endpoint, or a non-JAR in which case what are we > >> supposed to do with it once you upload it? > >>> It keeps the contract simple, and the CLI very simple. `br push > >> /path/to/blueprint/x/` > >>> (But Geoff as a side point I think the CLI should _not_ attempt > >> validation of the input/version -- the server will do that anyway and no > >> benefit in doing it twice, but more code to maintain.) > >>> I'm further persuaded given it is in a file, we should require a > >> version. I was concerned about needing to supply the version every > time on > >> the CLI but this has gone away. > >>> I see no reason for a separate "manifest" section. Removing this > >> eliminates the prospect of Aled's pathological/"perverted" example. I > >> favour: > >>> brooklyn.catalog: > >>> symbolicName: com.example.Foo > >>> version: 1.0 > >>> items: > >>> - id: foo > >>> version: 3.0 # optional version override > >>> itemType: entity > >>> item: > >>> type: blah > >>> ... > >>> > >>> That is, the fields symbolic-name and version are _required_ under the > >> `brooklyn.catalog` when POSTing a ZIP to the catalog endpoint, and are > used > >> to define the bundle. The version (and icon, etc) are then inherited by > >> default by items defined therein. If the ZIP is a JAR which contains a > >> MANIFEST.MF, the bundle information (symbolic name and version) must > match > >> exactly. > >>> Sound right? > >>> > >>> Best > >>> Alex > >>> > >>> > >>> On 03/03/2017 09:59, Geoff Macartney wrote: > >>>> It also means clients ('br' at least) can easily validate the upload > >> even > >>>> before execution by checking that all the manifest details are > present. > >>>> > >>>> On Fri, 3 Mar 2017 at 09:57 Geoff Macartney < > >>>> [email protected]> wrote: > >>>> > >>>>> Just on the last point of "We could also support the example below", > >> I'd > >>>>> say let's not even be that flexible - my feeling is that the more > >> flexible > >>>>> we are, the more confusing it is, and the harder to get right. If > >> we're > >>>>> going to add this capability let's just have one way to do it; if you > >> are > >>>>> going to do a ZIP upload, you MUST have a catalog.bom, which MUST > >> contain a > >>>>> manifest section, which MUST have both a symbolic name and a version. > >> That > >>>>> way everything's explicit and very clear, every time. > >>>>> > >>>>> > >>>>> > >>>>> On Thu, 2 Mar 2017 at 11:45 Aled Sage <[email protected]> wrote: > >>>>> > >>>>> Geoff, all, > >>>>> > >>>>> I was imagining the manifest version (in the catalog.bom) to be > >> separate > >>>>> from the item version. The reason is that we support multiple items > in > >>>>> the bom that can be independently versioned. > >>>>> > >>>>> Somone perverted could write: > >>>>> > >>>>> brooklyn.catalog: > >>>>> version: 1.0 > >>>>> manifest: > >>>>> symbolic_name: com.example.Foo > >>>>> version: 2.0 > >>>>> items: > >>>>> - id: foo > >>>>> version: 3.0 > >>>>> itemType: entity > >>>>> item: > >>>>> type: blah > >>>>> - id: bar > >>>>> version: 4.0 > >>>>> itemType: entity > >>>>> item: > >>>>> type: blah.blah > >>>>> > >>>>> Here, the "1.0" version is not used by anything; the auto-generated > >> OSGi > >>>>> bundle would be version 2.0; catalog item foo would be 3.0; and > catalog > >>>>> item bar would be 4.0. > >>>>> > >>>>> If we were starting from scratch, I'd be tempted to lock things down > a > >>>>> lot more for what we actually support. For now, it feels like asking > >> for > >>>>> an explicit version is a reasonable thing to do. > >>>>> > >>>>> --- > >>>>> We could also support the example below, where "1.0" is used by both > >> the > >>>>> manifest and the item: > >>>>> > >>>>> brooklyn.catalog: > >>>>> version: 1.0 > >>>>> manifest: > >>>>> symbolic_name: com.example.Foo > >>>>> items: > >>>>> - id: foo > >>>>> itemType: entity > >>>>> item: > >>>>> type: blah > >>>>> > >>>>> The general rule would be: if there is a version in the manifest > >>>>> section, that will be used; if not then a top-level version number > will > >>>>> be used. If that is also missing, then fail. > >>>>> > >>>>> i.e. we would not accept: > >>>>> > >>>>> # Fails - manifest has no version. > >>>>> brooklyn.catalog: > >>>>> manifest: > >>>>> symbolic_name: com.example.Foo > >>>>> items: > >>>>> - id: foo > >>>>> version: 1.0 > >>>>> itemType: entity > >>>>> item: > >>>>> type: blah > >>>>> > >>>>> Thoughts? > >>>>> > >>>>> Aled > >>>>> > >>>>> > >>>>> On 02/03/2017 10:02, Geoff Macartney wrote: > >>>>>> I take Alex's point about the manifest being Java specific, and I > >> agree > >>>>>> therefore we shouldn't insist on it. > >>>>>> > >>>>>> +1 also to preferring explicit name/version in the catalog.bom > rather > >>>>> than > >>>>>> as API params, I agree we do > >>>>>> need to keep the version in source control. > >>>>>> > >>>>>> Question on your straw man, does the 'version' below > >>>>>> > >>>>>> brooklyn.catalog: > >>>>>> manifest: > >>>>>> symbolic_name: com.example.Foo > >>>>>> version: 1.0.0-SNAPSHOT > >>>>>> items: > >>>>>> > >>>>>> _replace_ the 'version' in existing catalogs > >>>>>> > >>>>>> brooklyn.catalog: > >>>>>> version: "0.1.0-SNAPSHOT" > >>>>>> > >>>>>> or is it an independently variable value, i.e. the version of the > >> bundle, > >>>>>> separate from the version of catalog items that it happens to > contain? > >>>>>> (Sounds a bit disquieting, but I guess it's possible). > >>>>>> > >>>>>> e.g. could you have > >>>>>> > >>>>>> brooklyn.catalog: > >>>>>> version: 0.11.0-SNAPSHOT > >>>>>> manifest: > >>>>>> symbolic_name: com.example.Foo > >>>>>> version: 1.0.0-SNAPSHOT > >>>>>> items: > >>>>>> - id: foo > >>>>>> type: xyz > >>>>>> > >>>>>> which would be bundle com.example.Foo:1.0.0-SNAPSHOT, which happens > to > >>>>>> contain item foo:0.11.0-SNAPSHOT? > >>>>>> > >>>>>> If so, what would happen if you left out the second line above? What > >>>>>> version of 'foo' would the catalog contain? > >>>>>> > >>>>>> Or does the version within the manifest _mean_ that it is not only > the > >>>>>> bundle version you are specifying, but the catalog item versions > too? > >> (I > >>>>>> guess unless the item explicitly supplies its own version.) > >>>>>> > >>>>>> Geoff > >>>>>> > >>>>>> > >>>>>> > >>>>>> > >>>>>> On Wed, 1 Mar 2017 at 17:26 Aled Sage <[email protected]> wrote: > >>>>>> > >>>>>> Hi all, > >>>>>> > >>>>>> Discussion of the REST api kicked off again in > >>>>>> > >> > https://github.com/apache/brooklyn-server/pull/485#issuecomment-283280366: > >>>>>> Alex wrote: > >>>>>> > >>>>>> Requiring a MANIFEST.MF makes the input strongly java-centric; > >> I'd > >>>>>> like to appeal to people who write YAML blueprints with > >> co-bundled > >>>>>> scripts and images. They'd wonder why they can't simply make > and > >>>>>> upload a ZIP. They could probably be persuaded to supply a > name > >> and > >>>>>> optional version on a CLI or UI, and understand why it is > needed > >>>>>> (hence supporting those args) but it would not be idiomatic to > >>>>>> anyone but a java programmer to create a META-INF/ dir with a > >>>>>> MANIFEST.MF and its syntax. Meanwhile it is very easy for us > to > >>>>>> convert the ZIP to a JAR on the server. Feels like > >> uncontroversial > >>>>>> good UX. > >>>>>> > >>>>>> OTOH for a java programmer a MANIFEST.MF is natural, and > they'd > >> want > >>>>>> to drop the name/version args if they are in that file, and I > see > >> no > >>>>>> reason to forbid that pattern. > >>>>>> > >>>>>> I agree with Alex, that we should not require a > META-INF/MANIFEST.MF. > >> As > >>>>>> for Geoff's suggestion that we could auto-generate a manifest in the > >>>>>> `br` CLI, I'd prefer a more general solution that works for users of > >> the > >>>>>> REST api as well (i.e. doing it server-side). > >>>>>> > >>>>>> --- > >>>>>> Svet suggested that the catalog.bom could give the symbolic name + > >>>>>> version, via additional metadata in that file. > >>>>>> > >>>>>> What I really like about that is it's in version control, becuase > it's > >>>>>> in the file/repo. If alternatively the name/version are just passed > as > >>>>>> REST api parameters, then it's not in version control (and is more > >>>>>> susceptible to typos etc). > >>>>>> > >>>>>> I'm not sure what we'd want this to look like. As a straw man: > >>>>>> > >>>>>> brooklyn.catalog: > >>>>>> manifest: > >>>>>> symbolic_name: com.example.Foo > >>>>>> version: 1.0.0-SNAPSHOT > >>>>>> items: > >>>>>> - ... > >>>>>> > >>>>>> If the user built their own real OSGi bundle, then they wouldn't > need > >> to > >>>>>> include this "manifest" section. If they did include that and it > >>>>>> contradicted the OSGi bundle's manifest, then we'd fail-fast. > >>>>>> > >>>>>> (Note this reminds me of the (unrelated) metadata described in > >>>>>> https://github.com/brooklyncentral/blueprint-repository- there is a > >>>>>> "publish" block that can be added to the catalog.bom.) > >>>>>> > >>>>>> --- > >>>>>> With Svet's suggestion, if there was no manifest section/file, then > >>>>>> should we auto-generate something from the item(s) in the > catalog.bom? > >>>>>> > >>>>>> I can see how that could easily work for a .bom file that has a > single > >>>>>> item (e.g. we use the catalog item's id + version, possibly with a > >>>>>> special prefix in the symbolic name to avoid conflicts). > >>>>>> > >>>>>> However, if there are multiple items then it would get trickier. > >>>>>> > >>>>>> I'm inclined to say that for a minimal viable product (MVP) we can > >>>>>> insist on the "manifest" section in the catalog.bom. > >>>>>> > >>>>>> Aled > >>>>>> > >>>>>> > >>>>>> On 20/12/2016 16:34, Svetoslav Neykov wrote: > >>>>>>>> Svet, if instead we tried to infer it from the catalog.bom, would > we > >>>>>> require some additional metadata within the .bom file? Or would we > use > >>>>> the > >>>>>> catalog item's id + version? I'm not convinced by the latter - it > >> would > >>>>>> mean some .bom files would work and others wouldn't (e.g. if the > .bom > >> had > >>>>>> multiple items with different versions). Better to support the > >> explicit > >>>>>> approach IMO. > >>>>>>> I imagine it would be additional metadata. On the other hand I > don't > >> see > >>>>>> a technical reason why we need an explicit symbolicName and version > - > >>>>> they > >>>>>> can be auto-generated. > >>>>>>> Svet. > >>>>>>> > >>>>>>> > >>>>>>>> On 20.12.2016 г., at 17:50, Aled Sage <[email protected]> > wrote: > >>>>>>>> > >>>>>>>> Hi all, > >>>>>>>> > >>>>>>>> +1 > >>>>>>>> > >>>>>>>> (D) sounds good. What version are you imagining the bundle would > be, > >> if > >>>>>> one runs `br catalog add ~/my/project/ --name > com.example.myproject`? > >>>>>>>> --- > >>>>>>>> I like the idea of uploading a plain zip (rather than only > >> supporting > >>>>>> OSGi bundles) - that makes it simpler for non-java folk. The use of > >> OSGi > >>>>>> becomes a (hidden) implementation detail to many users. > >>>>>>>> --- > >>>>>>>> If auto-generating the manifest, I think we need the user to be > >>>>> explicit > >>>>>> about symbolic name and version. Having these supplied in the REST > api > >>>>> call > >>>>>> (as Alex suggests) would achieve that. > >>>>>>>> Svet, if instead we tried to infer it from the catalog.bom, would > we > >>>>>> require some additional metadata within the .bom file? Or would we > use > >>>>> the > >>>>>> catalog item's id + version? I'm not convinced by the latter - it > >> would > >>>>>> mean some .bom files would work and others wouldn't (e.g. if the > .bom > >> had > >>>>>> multiple items with different versions). Better to support the > >> explicit > >>>>>> approach IMO. > >>>>>>>> --- > >>>>>>>> For E ("have a mechanism whereby deployed entities based on an > >> affected > >>>>>> blueprint are optionally migrated to the new code"), that feels > like a > >>>>>> separate discussion. It could equally apply to a pure YAML .bom file > >> that > >>>>>> has been added to the catalog. > >>>>>>>> I suggest we discuss that in a separate email thread. > >>>>>>>> > >>>>>>>> --- > >>>>>>>> For (G), it's an interesting suggestion from Svet to make use of > >> Karaf > >>>>>> Cellar for HA nodes. I'm hesitant (e.g. if restarting a standalone > >>>>> Brooklyn > >>>>>> node whose VM has died, then it adds big additional requirements for > >> what > >>>>>> constitutes the "persisted state"). On the other hand, it's good to > >> use > >>>>>> well-established technologies rather than re-inventing things! > >>>>>>>> An alternative ("pure brooklyn") approach could be to write the > >> bundle > >>>>>> to persisted state; on rebind, we'd install + activate those > bundles. > >>>>>>>> --- > >>>>>>>> For "catalogGroupId", I agree with Svet that in the initial > use-case > >>>>>> this can be an implementation detail. > >>>>>>>> It could be set as the bundle's symbolic name + version: > everything > >>>>> from > >>>>>> the bundle should be deleted at once, along with the bundle. > >>>>>>>> Longer term, I can see how exposing "catalogGroupId" to the user > >> could > >>>>>> support more use-cases (e.g. for several catalog items from > different > >>>>>> bundles to work together). I don't think we should try to support > that > >>>>> yet. > >>>>>>>> Aled > >>>>>>>> > >>>>>>>> > >>>>>>>> On 19/12/2016 17:19, Geoff Macartney wrote: > >>>>>>>>> hi Alex, > >>>>>>>>> > >>>>>>>>> this looks like a good feature to have, I shall look at the PR as > >> soon > >>>>>> as I > >>>>>>>>> can. > >>>>>>>>> > >>>>>>>>> The catalog.bom scanner feature was initially enabled by default, > >> but > >>>>> we > >>>>>>>>> had to > >>>>>>>>> disable it because it turned out not to work properly with > rebind. > >> I > >>>>>> don't > >>>>>>>>> think > >>>>>>>>> it should be a lot of work to fix that but it hasn't been > something > >>>>>> we've > >>>>>>>>> got round > >>>>>>>>> to yet. This would be a great opportunity to look back at that. > >>>>>>>>> > >>>>>>>>> Some random thoughts: > >>>>>>>>> > >>>>>>>>> re (C), if we are going to treat the zips as bundles, my gut feel > >> is > >>>>>> that > >>>>>>>>> we > >>>>>>>>> should insist on a manifest and get the metadata from it. It > >> doesn't > >>>>>> feel > >>>>>>>>> to me > >>>>>>>>> like it makes much sense to allow a zip file without a > MANIFEST.MF > >> but > >>>>>>>>> convey > >>>>>>>>> the intended bundle metadata to Brooklyn via HTTP headers. And > >> rather > >>>>>> than > >>>>>>>>> infer bundle metadata I think it's better to ask users to be > >> explicit > >>>>>> about > >>>>>>>>> what > >>>>>>>>> their intentions are. To make users lives easier, we could > >>>>>>>>> add a command to br to generate the manifest (locally) with > correct > >>>>>> syntax, > >>>>>>>>> so that the manifest is in the right place, rather than have br > add > >>>>> the > >>>>>> data > >>>>>>>>> to the "upload" request headers. > >>>>>>>>> > >>>>>>>>> re. (D) will be glad to have a look at it > >>>>>>>>> > >>>>>>>>> re. (E) it would certainly need to be optional - maybe keep it as > >> an > >>>>>>>>> explicit > >>>>>>>>> separate command ('upgrade'?) > >>>>>>>>> > >>>>>>>>> (F) it does seem like a lot of work but might be nice for users > who > >>>>> are > >>>>>> not > >>>>>>>>> keen on command lines. > >>>>>>>>> > >>>>>>>>> G - I: we'll definitely need to pay close attention to > persistence > >>>>> and > >>>>>>>>> rebind; > >>>>>>>>> I wonder also about HA operation, are there any additional > >>>>> implications? > >>>>>>>>> (J) I think it would be good to treat all the files from a jar, > >> sorry > >>>>>>>>> bundle, > >>>>>>>>> as an atomic group - cleaner that way perhaps than allowing > >>>>>> delete/update > >>>>>>>>> of > >>>>>>>>> individual entries from a bundle on a piecemeal basis. Rest > >> support > >>>>> on > >>>>>>>>> delete > >>>>>>>>> catalog could warn about related catalog entries being deleted > and > >> ask > >>>>>> for > >>>>>>>>> a "--force" param to confirm. > >>>>>>>>> > >>>>>>>>> Geoff > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> On Fri, 16 Dec 2016 at 15:24 Svetoslav Neykov < > >>>>>>>>> [email protected]> wrote: > >>>>>>>>> > >>>>>>>>>> +1 > >>>>>>>>>> > >>>>>>>>>> Some thoughts: > >>>>>>>>>> * (A) add a utility class BundleMaker > >>>>>>>>>> Sounds very similar to > >>>>>>>>>> https://ops4j1.jira.com/wiki/display/ops4j/Tinybundles < > >>>>>>>>>> https://ops4j1.jira.com/wiki/display/ops4j/Tinybundles> > >>>>>>>>>> Looking at the code it's much more focused on zip files > so I > >>>>> guess > >>>>>>>>>> there's no much overlap, but worth keeping in mind > >>>>>>>>>> * (C) accept bundle symbolic name and version > >>>>>>>>>> Why require them at all? Could infer them from the > >> catalog.bom > >>>>> in > >>>>>> some > >>>>>>>>>> way - maybe require those properties to be in there. If not > >> present > >>>>> are > >>>>>>>>>> they really needed? > >>>>>>>>>> * (G) Bundles installed via this mechanism are not > persisted > >>>>>> currently & > >>>>>>>>>> (I) We persist the individual catalog items as YAML, so we end > up > >>>>> with > >>>>>> two > >>>>>>>>>> records > >>>>>>>>>> Suggest marking the catalog items coming from bundles as > >>>>>>>>>> non-persistable. Then try to share the bundles between HA nodes. > >>>>> (Karaf > >>>>>>>>>> Cellar?) > >>>>>>>>>> * (J) Introduce a catalogGroupId field on catalog items; > >>>>>>>>>> Agree this could be useful and I like the idea of > deleting > >> the > >>>>>> bundle > >>>>>>>>>> altogether with the catalog items. From user's perspective I > don't > >>>>> see > >>>>>> the > >>>>>>>>>> need for an extra field (i.e. it's an implementation detail). > >>>>>>>>>> > >>>>>>>>>> Svet. > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>>> On 16.12.2016 г., at 12:50, Alex Heneveld < > >>>>>>>>>> [email protected]> wrote: > >>>>>>>>>>> Hi Brooklyners- > >>>>>>>>>>> > >>>>>>>>>>> In the code we currently have two routes for users to install > new > >>>>>>>>>>> blueprints: > >>>>>>>>>>> > >>>>>>>>>>> (1) upload a catalog YAML file to /v1/catalog > >>>>>>>>>>> > >>>>>>>>>>> (2) install a bundle with catalog.bom in the root > >>>>>>>>>>> > >>>>>>>>>>> The feature (2) is disabled by default, but I'd like to move > >> towards > >>>>>>>>>>> enabling it. This will make it easier to create nicely > >> structured > >>>>> BOM > >>>>>>>>>>> files because scripts etc can be taken out of the BOM, stored > as > >>>>>> files in > >>>>>>>>>>> the same bundle. (Because URLs of the form > >>>>>>>>>>> `classpath://scripts/install.sh` use the bundle's classpath to > >>>>>> resolve.) > >>>>>>>>>>> As a first step in #485 [1] I do a few things: > >>>>>>>>>>> > >>>>>>>>>>> (A) add a utility class BundleMaker that lets us create and > >> modify > >>>>>>>>>>> bundles/zips, to make it easier to do things we might want to > >> with > >>>>>>>>>> bundles, > >>>>>>>>>>> especially for testing > >>>>>>>>>>> > >>>>>>>>>>> (B) add an endpoint to the REST API which allows uploading a > >> bundle > >>>>>> ZIP > >>>>>>>>>>> (C) accept bundle symbolic name and version in that REST API to > >>>>>>>>>> facilitate > >>>>>>>>>>> uploading non-bundle ZIPs where the OSGi MANIFEST.MF is > >>>>> automatically > >>>>>>>>>>> generated > >>>>>>>>>>> > >>>>>>>>>>> With this PR, if you have a directory on your local file system > >> with > >>>>>>>>>>> scripts and config files, and a BOM which refers to them, you > can > >>>>> just > >>>>>>>>>> ZIP > >>>>>>>>>>> that up an upload it, specifying the bundle name so that a YAML > >>>>>> blueprint > >>>>>>>>>>> author never needs to touch any java-isms. > >>>>>>>>>>> > >>>>>>>>>>> Where I see this going is a development workflow where a user > can > >>>>> edit > >>>>>>>>>>> files locally and upload the ZIP to have that installed, and if > >> they > >>>>>> make > >>>>>>>>>>> changes locally they can POST it again to have catalog items > >> updated > >>>>>>>>>>> (because default version is a SNAPSHOT). We could also: > >>>>>>>>>>> > >>>>>>>>>>> (D) have `br catalog add ~/my/project/ --name my.project` > create > >> a > >>>>> ZIP > >>>>>>>>>> and > >>>>>>>>>>> POST it, with bundle name metadata, so essentially the user's > >>>>> process > >>>>>> is > >>>>>>>>>>> just to run that whenever they make a change > >>>>>>>>>>> > >>>>>>>>>>> (E) have a mechanism whereby deployed entities based on an > >> affected > >>>>>>>>>>> blueprint are optionally migrated to the new code, so if you've > >>>>>> changed > >>>>>>>>>> an > >>>>>>>>>>> enricher the changes are picked up, or if say a launch.sh > script > >> has > >>>>>>>>>>> changed, a restart will run the new code > >>>>>>>>>>> > >>>>>>>>>>> The above are fairly straightforward programmatically (although > >> good > >>>>>> user > >>>>>>>>>>> interaction with (E) needs some thought). So I think we can > >> pretty > >>>>>>>>>> quickly > >>>>>>>>>>> get to a much smoother dev workflow. > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> That's the highlight of this message. You can jump to the end, > >>>>> unless > >>>>>>>>>>> you're interested in some important but low level details... > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> I'm also tempted by: > >>>>>>>>>>> > >>>>>>>>>>> (F) Integration with web-based IDE and/or Brooklyn reading and > >>>>> writing > >>>>>>>>>>> straight from GitHub -- but this seems like a lot of work and > I'm > >>>>> not > >>>>>>>>>>> convinced it's much better than (D) workflow-wise > >>>>>>>>>>> > >>>>>>>>>>> Before we can change (2) to be the default, or start widely > using > >>>>> the > >>>>>>>>>> POST > >>>>>>>>>>> a ZIP feature, we need to sort out some issues to do with > >>>>> persistence > >>>>>> and > >>>>>>>>>>> reloading: > >>>>>>>>>>> > >>>>>>>>>>> (G) Bundles installed via this mechanism are not persisted > >>>>> currently, > >>>>>> so > >>>>>>>>>> if > >>>>>>>>>>> you move to a different Brooklyn using the same backing store, > >>>>> you'll > >>>>>>>>>> lose > >>>>>>>>>>> those bundles > >>>>>>>>>>> > >>>>>>>>>>> (H) On rebind, bundles aren't always activated when needed, > >> meaning > >>>>>> items > >>>>>>>>>>> can't be loaded > >>>>>>>>>>> > >>>>>>>>>>> (I) We persist the individual catalog items as YAML, so we end > up > >>>>> with > >>>>>>>>>> two > >>>>>>>>>>> records — the YAML from the catalog.bom in the bundle, and the > >> YAML > >>>>>>>>>>> persisted for the item. This isn't a problem per se, but > >> something > >>>>> to > >>>>>>>>>>> think about, and some sometimes surprising behaviour. In > >> particular > >>>>>> if > >>>>>>>>>> you > >>>>>>>>>>> delete the persisted YAML, the bundle is still there so the > item > >> is > >>>>> no > >>>>>>>>>>> longer deleted after a full rebind. > >>>>>>>>>>> > >>>>>>>>>>> One idea which might be useful is: > >>>>>>>>>>> > >>>>>>>>>>> (J) Introduce a catalogGroupId field on catalog items; this > will > >> do > >>>>>> two > >>>>>>>>>>> things: if you try to delete an item with such a record, > you'll > >> be > >>>>>>>>>>> encouraged to delete all such items (maybe disallowed to delete > >> an > >>>>>>>>>>> individual one), with the effect of deleting the bundle if it > >> comes > >>>>>> from > >>>>>>>>>> a > >>>>>>>>>>> bundle; and when resolving types we search first for items with > >> the > >>>>>> same > >>>>>>>>>>> catalogGroupId (so that e.g. if I install MyCluster:1.0 and > >>>>>> MyNode:1.0 in > >>>>>>>>>>> the same group, the former can refer simply to "MyNode" but if > I > >>>>>> install > >>>>>>>>>> a > >>>>>>>>>>> 2.0 version of that group, the 1.0 cluster still loads the 1.0 > >> node > >>>>> -- > >>>>>>>>>> this > >>>>>>>>>>> has bitten people i the past) > >>>>>>>>>>> > >>>>>>>>>>> There is a related Brooklyn upgrade problem worth mentioning, > >> which > >>>>>> the > >>>>>>>>>>> above might help with, where: > >>>>>>>>>>> > >>>>>>>>>>> (K) If I migrate from Brooklyn 10 to 11 when it comes out, I'll > >> no > >>>>>> longer > >>>>>>>>>>> have certain entities that were at v10, since we don't include > >>>>> those; > >>>>>> an > >>>>>>>>>>> upgrade could include rules that certain groupIds need to be > >>>>> updated, > >>>>>> or > >>>>>>>>>> it > >>>>>>>>>>> can search and attempt to automatically apply the updates > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> Quite a lot here and we don't need to solve it but I wanted to: > >>>>>>>>>>> > >>>>>>>>>>> * Share the current thinking > >>>>>>>>>>> > >>>>>>>>>>> * Get opinions on the general dev workflow suggested by (D) > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> Thanks for feedback -- and if we like it help with (D) would be > >>>>>>>>>> appreciated! > >>>>>>>>>>> Best > >>>>>>>>>>> Alex > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> [1] . https://github.com/apache/brooklyn-server/pull/485 > >> -- > >> > >> Thomas Bouron • Software Engineer @ Cloudsoft Corporation • > >> http://www.cloudsoftcorp.com/ > >> Github: https://github.com/tbouron > >> Twitter: https://twitter.com/eltibouron > >> > >> -- > >> > >> Thomas Bouron • Software Engineer @ Cloudsoft Corporation • > >> http://www.cloudsoftcorp.com/ > >> Github: https://github.com/tbouron > >> Twitter: https://twitter.com/eltibouron > >> > > -- Thomas Bouron • Software Engineer @ Cloudsoft Corporation • http://www.cloudsoftcorp.com/ Github: https://github.com/tbouron Twitter: https://twitter.com/eltibouron
