Bundles of fun

[Alex Heneveld <al...@cloudsoftcorp.com>]

Hi folks-

The versioning changes discussed are merged [1].  Bundles seem to be 
working very well and with the OSGi mapping sorted out and OSGi coming 
to the fore I've been working on changing BOM YAML upload to be 
installed as a bundle.  The idea here is that if a user uploads a 
catalog YAML, we create a bundle to wrap it, containing just the 
`catalog.bom` supplied and a `manifest.mf` inferred from `bundle` and 
`version` in the BOM so it's a proper OSGi manifest.  This is also 
working, at [2], but it has had a sequence of surprising implications.

Big benefits of this approach are that:

* We have access to the original BOM should we want to let a user see or 
edit it
* We can approach upgrades and dependencies at the level of bundles 
instead of items
* We can resolve dependencies such that co-bundled items are preferred 
(e.g. if "my-cluster" refers to "my-node" we can bundle them together 
and make sure "my-cluster:1" referring to "my-node" without a version 
doesn't deploy a later "my-node:2")

The above aren't done, but become much easier.

However I wanted to discuss the surprising implications.


*(A) Java class scanning for @Catalog annotations - action DEPRECATE*

Previously you could in a BOM ask for another library to have its Java 
scanned; but our class scanning doesn't play nicely with OSGi, and it 
has always been the case that you can't scan the containing bundle (e.g. 
a catalog.bom in a bundle can't say to scan that bundle).  I've 
preserved the old behaviour (scanning _other_ libraries) but I'm 
thinking we should DEPRECATE JAVA CLASS SCANNING.  It isn't the OSGi 
way, and the BOM is simple enough that it doesn't feel needed.  If 
anyone feels strongly we could write a simple JJS to find annotations 
and generate a BOM - but I think we go all the way and DEPRECATE 
@CATALOG ANNOTATIONS.


*(B) If no `bundle` (or `id`) is specified in your BOM - action 
WARN+DEPRECATE*

In this case we have three choices.  We can fail, which breaks backwards 
compatibility; or we can skip bundling it, which means we don't get the 
benefits above; or we can make one up.  The last feels cleanest and I've 
gone with it.  This gives one minor change to behaviour, if you upload 
the same non-snapshot BOM twice, and it doesn't declare a bundle name, 
the second time will fail.  This is because the catalog items being 
added are changing such that they refer to a different 
containingBundle.  In any case we WARN if `bundle` isn't specified so I 
think this is fine.  (We could be a little smarter and ignore the 
difference if the bundle is a wrapper bundle, but I don't think that's 
worth it.).  Seems right that people should always supply a name for 
their bundle/BOM.


*(C) Bundles sensitive to load order - action CHANGE TO TYPE REGISTRY, 
move to DEPRECATE CATALOG CLASSES*

Say you have bundle "A" declaring entity "a".  Then you have "B" which 
declares an entity "b" that references "a".  If you install "A" then 
"B", things are fine.  However when you rebind there is no order 
guarantee, and it may try to scan the BOM of B before scanning the BOM 
of A, and of course it will fail.  This is a pre-existing problem, but 
made much more pronounced with more bundles.  After considering several 
options I think it makes the most sense to shift to using the 
BrooklynTypeRegistry.  This is a much simpler record of items (and fits 
with YOML).  We can kill the horrid old tree/nested catalog that relied 
on complex classpaths, and we can kill CatalogItems (since we have the 
BOMs we don't need additional XML records of catalog items), and we can 
clean up big chunks of BasicBrooklynCatalog.  I reckon we'll cut 
_thousands_ of lines!  We won't be able to remove this yet, but we move 
away from it for most things, and deprecate the rest, and can remove the 
code in a cycle or two.


*(D) We lose the ability to deprecate and disable individual items - 
action DEPRECATE+WARN*

Because we don't persist catalog items, we shouldn't make changes to 
them.  Deprecating and disabling will no longer work.  We could do 
something to add support, but I think better (unless anyone is relying 
on that?) to lose them for now, and then re-enable them at the level of 
_bundles_, as part of a richer upgrade path (separate discussion thread).


*(E) Templates need new support - action ADD THIS*

The TypeRegistry doesn't support templates.  However the whole idea was 
half-baked, as we assume templates are applications and we do weird 
stuff about what's ready for deployment and what isn't.  I suggest much 
cleaner is to use three separate concepts:
- type (all "templates" become "EntitySpec<? extends Application>")
- "template" tag or boolean (true for existing templates)
- "non-deployable" tag or boolean to indicate something is not 
deployable (infer for existing, true if it gives load errors)
Make sense?


I've mostly completed (A)-(D) at [2] and am working on (E) but it 
started to get so much I wanted to bring it to the ML.

Best
Alex


[1] https://github.com/apache/brooklyn-server/pull/743
[2] https://github.com/apache/brooklyn-server/pull/746