Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/799#discussion_r135778904
  
    --- Diff: 
core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java 
---
    @@ -379,26 +388,89 @@ private synchronized void close() {
                 // eg rebind code, brooklyn.libraries list -- deferred start 
allows caller to
                 // determine whether not to start or to start all after things 
are installed
                 Runnable startRunnable = new Runnable() {
    +                private void rollbackBundle() {
    +                    if (updating) {
    +                        if (oldZipFile==null) {
    +                            throw new IllegalStateException("Did not have 
old ZIP file to install");
    +                        }
    +                        log.debug("Rolling back bundle 
"+result.getVersionedName()+" to state from "+oldZipFile);
    +                        try {
    +                            File zipFileNow = 
osgiManager.managedBundlesRecord.rollbackManagedBundleFile(result, oldZipFile);
    +                            result.bundle.update(new 
FileInputStream(Preconditions.checkNotNull(zipFileNow, "Couldn't find contents 
of old version of bundle")));
    +                        } catch (Exception e) {
    +                            Exceptions.propagateIfFatal(e);
    +                            log.error("Error rolling back following failed 
install of updated "+result.getVersionedName()+"; "
    +                                + "installation will likely be corrupted 
and correct version should be manually installed.", e);
    +                        }
    +                        
    +                        
((BasicManagedBundle)result.getMetadata()).setPersistenceNeeded(true);
    +                        
mgmt().getRebindManager().getChangeListener().onChanged(result.getMetadata());
    +                    } else {
    +                        log.debug("Uninstalling bundle 
"+result.getVersionedName()+" (roll back of failed fresh install, no previous 
version to revert to)");
    +                        
osgiManager.uninstallUploadedBundle(result.getMetadata());
    +                        
    +                        
((BasicManagedBundle)result.getMetadata()).setPersistenceNeeded(true);
    +                        
mgmt().getRebindManager().getChangeListener().onUnmanaged(result.getMetadata());
    +                    }
    +                }
                     public void run() {
                         if (start) {
                             try {
                                 log.debug("Starting bundle 
"+result.getVersionedName());
                                 result.bundle.start();
                             } catch (BundleException e) {
    +                            log.warn("Error starting bundle 
"+result.getVersionedName()+", uninstalling, restoring any old bundle, then 
re-throwing error: "+e);
    +                            rollbackBundle();
    +                            
                                 throw Exceptions.propagate(e);
                             }
                         }
             
                         if (loadCatalogBom) {
    -                        if (updating) {
    -                            osgiManager.uninstallCatalogItemsFromBundle( 
result.getVersionedName() );
    -                            // (ideally removal and addition would be 
atomic)
    -                        }
    -                        osgiManager.loadCatalogBom(result.bundle, force, 
validateTypes);
    -                        Iterable<RegisteredType> items = 
mgmt().getTypeRegistry().getMatching(RegisteredTypePredicates.containingBundle(result.getMetadata()));
    -                        log.debug("Adding items from bundle 
"+result.getVersionedName()+": "+items);
    -                        for (RegisteredType ci: items) {
    -                            result.catalogItemsInstalled.add(ci.getId());
    +                        Iterable<RegisteredType> itemsFromOldBundle = null;
    +                        Map<RegisteredType, RegisteredType> 
itemsReplacedHere = null;
    +                        try {
    +                            if (updating) {
    +                                itemsFromOldBundle = 
osgiManager.uninstallCatalogItemsFromBundle( result.getVersionedName() );
    +                                // (ideally removal and addition would be 
atomic)
    +                            }
    +                            itemsReplacedHere = MutableMap.of();
    +                            osgiManager.loadCatalogBom(result.bundle, 
force, validateTypes, itemsReplacedHere);
    +                            Iterable<RegisteredType> items = 
mgmt().getTypeRegistry().getMatching(RegisteredTypePredicates.containingBundle(result.getMetadata()));
    +                            log.debug("Adding items from bundle 
"+result.getVersionedName()+": "+items);
    +                            for (RegisteredType ci: items) {
    +                                
result.catalogItemsInstalled.add(ci.getId());
    +                            }
    +                        } catch (Exception e) {
    +                            // unable to install new items; rollback 
bundles
    +                            // and reload replaced items
    +                            log.warn("Error adding Brooklyn items from 
bundle "+result.getVersionedName()+", uninstalling, restoring any old bundle 
and items, then re-throwing error: "+Exceptions.collapseText(e));
    --- End diff --
    
    Again we'll lose the stacktrace and `caused by` of the exception, if 
rollback throws an exception.


---
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 [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to